From e7a678fa72c195671da30e4a9e3cb8a77d0c3235 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 30 Jul 2026 12:57:31 -0700 Subject: [PATCH] s3: keep the list marker exclusive for versioned objects (#10496) * s3: keep the list marker exclusive for versioned objects A versioned object lives in a ".versions" directory, so the entry name never matched the marker and start-after/marker returned the marker key itself. * s3: match the list marker against the raw entry name too A backend that echoes the marker it was given returns the ".versions" directory name, which no longer matched once the comparison used the object name alone. Cover both, and unit test each half. --- .../s3_versioning_list_marker_test.go | 62 +++++++++++++++++++ weed/s3api/s3api_object_handlers_list.go | 8 ++- ...api_object_handlers_list_versioned_test.go | 49 +++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 test/s3/versioning/s3_versioning_list_marker_test.go diff --git a/test/s3/versioning/s3_versioning_list_marker_test.go b/test/s3/versioning/s3_versioning_list_marker_test.go new file mode 100644 index 000000000..a9a1811a8 --- /dev/null +++ b/test/s3/versioning/s3_versioning_list_marker_test.go @@ -0,0 +1,62 @@ +package s3api + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestVersionedListMarkerIsExclusive covers start-after and marker on a versioned bucket, +// where an object is stored in a ".versions" directory and so never matches the +// marker by entry name. +func TestVersionedListMarkerIsExclusive(t *testing.T) { + client := getS3Client(t) + bucketName := getNewBucketName() + + createBucket(t, client, bucketName) + defer deleteBucket(t, client, bucketName) + enableVersioning(t, client, bucketName) + + for _, key := range []string{"file-0", "file-1", "file-2", "logs/file-0", "logs/file-1", "logs/file-2"} { + putObject(t, client, bucketName, key, "content") + } + + listV2 := func(prefix, startAfter string) []string { + t.Helper() + resp, err := client.ListObjectsV2(context.TODO(), &s3.ListObjectsV2Input{ + Bucket: aws.String(bucketName), + Prefix: aws.String(prefix), + StartAfter: aws.String(startAfter), + }) + require.NoError(t, err) + return contentKeys(resp.Contents) + } + listV1 := func(prefix, marker string) []string { + t.Helper() + resp, err := client.ListObjects(context.TODO(), &s3.ListObjectsInput{ + Bucket: aws.String(bucketName), + Prefix: aws.String(prefix), + Marker: aws.String(marker), + }) + require.NoError(t, err) + return contentKeys(resp.Contents) + } + + assert.Equal(t, []string{"file-2"}, listV2("file-", "file-1")) + assert.Equal(t, []string{"file-2"}, listV1("file-", "file-1")) + assert.Equal(t, []string{"logs/file-2"}, listV2("logs/", "logs/file-1")) + assert.Equal(t, []string{"logs/file-2"}, listV1("logs/", "logs/file-1")) +} + +func contentKeys(contents []types.Object) []string { + keys := make([]string, 0, len(contents)) + for _, c := range contents { + keys = append(keys, *c.Key) + } + return keys +} diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index f6fa358ce..47975c7a2 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/weed/s3api/s3api_object_handlers_list.go @@ -678,7 +678,13 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb. // listFilerEntries always calls doListFilerEntries with inclusiveStartFrom=false // (S3 marker semantics are exclusive), but keep the guard explicit to preserve // behavior if inclusive callers are introduced in the future. - if !inclusiveStartFrom && marker != "" && entry.Name == marker { + // A versioned object lives in a ".versions" directory, so the marker also + // has to be matched against the object name that directory stands for. + markerName := entry.Name + if entry.IsDirectory { + markerName = strings.TrimSuffix(markerName, s3_constants.VersionsFolder) + } + if !inclusiveStartFrom && marker != "" && (entry.Name == marker || markerName == marker) { continue } diff --git a/weed/s3api/s3api_object_handlers_list_versioned_test.go b/weed/s3api/s3api_object_handlers_list_versioned_test.go index 2bcbfa269..43ec65f27 100644 --- a/weed/s3api/s3api_object_handlers_list_versioned_test.go +++ b/weed/s3api/s3api_object_handlers_list_versioned_test.go @@ -684,3 +684,52 @@ func TestProcessDirectorySkipsBeforeMarker(t *testing.T) { }) } } + +// TestVersionedListSkipsMarkerObject covers the exclusive marker on a versioned bucket, +// where the marker names an object but the entry is its ".versions" directory. +func TestVersionedListSkipsMarkerObject(t *testing.T) { + s3a := &S3ApiServer{option: &S3ApiServerOption{BucketsPath: "/buckets"}} + client := &testFilerClient{ + entriesByDir: map[string][]*filer_pb.Entry{ + "/buckets/test-bucket": { + liveVersionsDir("file-1"), + liveVersionsDir("file-2"), + }, + }, + } + + cursor := &ListingCursor{maxKeys: 1000} + var seen []string + _, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{dir: "/buckets/test-bucket", marker: "file-1", bucket: "test-bucket"}, cursor, func(dir string, entry *filer_pb.Entry) { + seen = append(seen, entry.Name) + cursor.maxKeys-- + }) + + assert.NoError(t, err) + assert.Equal(t, []string{"file-2"}, seen, "the marker object should not be returned") +} + +// TestVersionedListSkipsEchoedVersionsMarker covers a backend that echoes the marker it +// was given, when that marker is a ".versions" directory name. +func TestVersionedListSkipsEchoedVersionsMarker(t *testing.T) { + s3a := &S3ApiServer{option: &S3ApiServerOption{BucketsPath: "/buckets"}} + client := &markerEchoFilerClient{ + entriesByDir: map[string][]*filer_pb.Entry{ + "/buckets/test-bucket": { + liveVersionsDir("file-1"), + liveVersionsDir("file-2"), + }, + }, + returnFollowing: true, + } + + cursor := &ListingCursor{maxKeys: 1000} + var seen []string + _, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{dir: "/buckets/test-bucket", marker: "file-1" + s3_constants.VersionsFolder, bucket: "test-bucket"}, cursor, func(dir string, entry *filer_pb.Entry) { + seen = append(seen, entry.Name) + cursor.maxKeys-- + }) + + assert.NoError(t, err) + assert.Equal(t, []string{"file-2"}, seen, "the echoed marker entry should not be returned") +}