From 6772a19b6db8bb5362e82553bb0d3368b9b19f5e Mon Sep 17 00:00:00 2001 From: chrislu Date: Mon, 21 Jul 2025 09:02:03 -0700 Subject: [PATCH] fix next marker --- .../s3_directory_versioning_test.go | 81 +++++++++++++++++++ weed/s3api/s3api_object_handlers_list.go | 22 ++++- 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/test/s3/versioning/s3_directory_versioning_test.go b/test/s3/versioning/s3_directory_versioning_test.go index bc1928423..5a6652832 100644 --- a/test/s3/versioning/s3_directory_versioning_test.go +++ b/test/s3/versioning/s3_directory_versioning_test.go @@ -796,6 +796,87 @@ func TestPrefixFilteringLogic(t *testing.T) { t.Logf("✅ Prefix filtering logic correctly handles edge cases") } +// TestNextMarkerDelimiterFix tests the NextMarker behavior with delimiters for directory prefixes +// This addresses the test failures where NextMarker should include trailing slash for CommonPrefixes +func TestNextMarkerDelimiterFix(t *testing.T) { + s3Client := setupS3Client(t) + bucketName := "test-bucket-" + fmt.Sprintf("%d", time.Now().UnixNano()) + + // Create bucket + _, err := s3Client.CreateBucket(context.Background(), &s3.CreateBucketInput{ + Bucket: aws.String(bucketName), + }) + require.NoError(t, err) + defer cleanupBucket(t, s3Client, bucketName) + + // Create test objects that match the failing test pattern + testObjects := []string{ + "asdf", + "boo/bar", + "boo/baz/xyzzy", + "cquux/thud", + "cquux/bla", + } + + for _, objectKey := range testObjects { + _, err = s3Client.PutObject(context.Background(), &s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + Body: strings.NewReader("test content for " + objectKey), + }) + require.NoError(t, err) + } + + // Test the specific failing case: list with delimiter "/" and maxKeys=1 + // This should return objects/prefixes one at a time with correct NextMarker + + // First call: should return "asdf" with NextMarker="asdf" + listResponse, err := s3Client.ListObjects(context.Background(), &s3.ListObjectsInput{ + Bucket: aws.String(bucketName), + Delimiter: aws.String("/"), + MaxKeys: aws.Int32(1), + }) + require.NoError(t, err) + + assert.True(t, *listResponse.IsTruncated, "First response should be truncated") + assert.Len(t, listResponse.Contents, 1, "Should return exactly 1 object") + assert.Equal(t, "asdf", aws.ToString(listResponse.Contents[0].Key), "First object should be 'asdf'") + assert.Equal(t, "asdf", aws.ToString(listResponse.NextMarker), "NextMarker should be 'asdf'") + + // Second call: continue from "asdf", should return CommonPrefix "boo/" with NextMarker="boo/" + listResponse, err = s3Client.ListObjects(context.Background(), &s3.ListObjectsInput{ + Bucket: aws.String(bucketName), + Delimiter: aws.String("/"), + Marker: listResponse.NextMarker, + MaxKeys: aws.Int32(1), + }) + require.NoError(t, err) + + assert.True(t, *listResponse.IsTruncated, "Second response should be truncated") + assert.Len(t, listResponse.Contents, 0, "Should return no direct objects") + assert.Len(t, listResponse.CommonPrefixes, 1, "Should return exactly 1 CommonPrefix") + assert.Equal(t, "boo/", aws.ToString(listResponse.CommonPrefixes[0].Prefix), "CommonPrefix should be 'boo/'") + + // This is the critical assertion that was failing: + assert.Equal(t, "boo/", aws.ToString(listResponse.NextMarker), "NextMarker should be 'boo/' (with trailing slash)") + + // Third call: continue from "boo/", should return CommonPrefix "cquux/" + listResponse, err = s3Client.ListObjects(context.Background(), &s3.ListObjectsInput{ + Bucket: aws.String(bucketName), + Delimiter: aws.String("/"), + Marker: listResponse.NextMarker, + MaxKeys: aws.Int32(1), + }) + require.NoError(t, err) + + assert.False(t, *listResponse.IsTruncated, "Third response should not be truncated") + assert.Len(t, listResponse.Contents, 0, "Should return no direct objects") + assert.Len(t, listResponse.CommonPrefixes, 1, "Should return exactly 1 CommonPrefix") + assert.Equal(t, "cquux/", aws.ToString(listResponse.CommonPrefixes[0].Prefix), "CommonPrefix should be 'cquux/'") + + t.Logf("✅ NextMarker correctly includes trailing slash for CommonPrefixes") +} + // Helper function to setup S3 client func setupS3Client(t *testing.T) *s3.Client { // S3TestConfig holds configuration for S3 tests diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index 8a55db854..23aacd638 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/weed/s3api/s3api_object_handlers_list.go @@ -152,6 +152,9 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m err = s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { for { empty := true + var lastEntryWasCommonPrefix bool + var lastCommonPrefixName string + nextMarker, doErr = s3a.doListFilerEntries(client, reqDir, prefix, cursor, marker, delimiter, false, func(dir string, entry *filer_pb.Entry) { empty = false dirName, entryName, prefixName := entryUrlEncode(dir, entry.Name, encodingTypeUrl) @@ -159,6 +162,7 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m if entry.IsDirectoryKeyObject() { contents = append(contents, newListEntry(entry, "", dirName, entryName, bucketPrefix, fetchOwner, true, false)) cursor.maxKeys-- + lastEntryWasCommonPrefix = false // https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html } else if delimiter == "/" { // A response can contain CommonPrefixes only if you specify a delimiter. commonPrefixes = append(commonPrefixes, PrefixEntry{ @@ -166,6 +170,8 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m }) //All of the keys (up to 1,000) rolled up into a common prefix count as a single return when calculating the number of returns. cursor.maxKeys-- + lastEntryWasCommonPrefix = true + lastCommonPrefixName = entry.Name } } else { var delimiterFound bool @@ -196,12 +202,15 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m }) cursor.maxKeys-- delimiterFound = true + lastEntryWasCommonPrefix = true + lastCommonPrefixName = delimitedPath[0] } } } if !delimiterFound { contents = append(contents, newListEntry(entry, "", dirName, entryName, bucketPrefix, fetchOwner, false, false)) cursor.maxKeys-- + lastEntryWasCommonPrefix = false } } }) @@ -209,10 +218,21 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m return doErr } - if cursor.isTruncated { + // Adjust nextMarker for CommonPrefixes to include trailing slash (AWS S3 compliance) + if cursor.isTruncated && lastEntryWasCommonPrefix && lastCommonPrefixName != "" { + // For CommonPrefixes, NextMarker should include the trailing slash + if requestDir != "" { + nextMarker = requestDir + "/" + lastCommonPrefixName + "/" + } else { + nextMarker = lastCommonPrefixName + "/" + } + } else if cursor.isTruncated { if requestDir != "" { nextMarker = requestDir + "/" + nextMarker } + } + + if cursor.isTruncated { break } else if empty || strings.HasSuffix(originalPrefix, "/") { nextMarker = ""