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") +}