From c02173db7a97e4ac1b2b7d03fabf0e1566ec2427 Mon Sep 17 00:00:00 2001 From: dualfroz Date: Sat, 10 Oct 2026 11:50:51 +0200 Subject: [PATCH] fix: return NextMarker in ListObjects only when a delimiter is set Fixes #999 --- s3api/controllers/bucket-get.go | 6 ++ s3api/controllers/bucket-get_test.go | 42 +++++++++++++ tests/integration/ListObjects.go | 88 +++++++++++++++++++++++----- tests/integration/group-tests.go | 2 + tests/test_rest_list_objects.sh | 3 - 5 files changed, 122 insertions(+), 19 deletions(-) diff --git a/s3api/controllers/bucket-get.go b/s3api/controllers/bucket-get.go index 4340a75b..9cbb93ac 100644 --- a/s3api/controllers/bucket-get.go +++ b/s3api/controllers/bucket-get.go @@ -656,6 +656,12 @@ func (c S3ApiController) ListObjects(ctx fiber.Ctx) (*Response, error) { }, err } + // S3 returns NextMarker only when the request has a delimiter. Without + // one, clients use the last key in the response as the next marker. + if delimiter == "" { + res.NextMarker = nil + } + // S3 echoes the request marker back in the response, and includes the // element even when the request carried no marker. Backends return a nil // pointer for an empty marker, which encoding/xml drops entirely, so diff --git a/s3api/controllers/bucket-get_test.go b/s3api/controllers/bucket-get_test.go index ac290891..d9547cad 100644 --- a/s3api/controllers/bucket-get_test.go +++ b/s3api/controllers/bucket-get_test.go @@ -1176,6 +1176,9 @@ func TestS3ApiController_ListObjects(t *testing.T) { emptyMarker := "" listResultEchoed := listResult listResultEchoed.Marker = &emptyMarker + // S3 returns NextMarker only when the request has a delimiter + listResultNextMarker := listResultEchoed + listResultNextMarker.NextMarker = utils.GetStringPtr("my-key") tests := []struct { name string input testInput @@ -1246,6 +1249,45 @@ func TestS3ApiController_ListObjects(t *testing.T) { }, }, }, + { + name: "next marker omitted without delimiter", + input: testInput{ + locals: defaultLocals, + beRes: listResultNextMarker, + }, + output: testOutput{ + response: &Response{ + Data: listResultEchoed, + Headers: map[string]*string{ + "x-amz-bucket-region": utils.GetStringPtr(defaultRegion), + }, + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + }, + }, + { + name: "next marker returned with delimiter", + input: testInput{ + locals: defaultLocals, + beRes: listResultNextMarker, + queries: map[string]string{ + "delimiter": "delim", + }, + }, + output: testOutput{ + response: &Response{ + Data: listResultNextMarker, + Headers: map[string]*string{ + "x-amz-bucket-region": utils.GetStringPtr(defaultRegion), + }, + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/tests/integration/ListObjects.go b/tests/integration/ListObjects.go index 31dfead9..fc2aa3a4 100644 --- a/tests/integration/ListObjects.go +++ b/tests/integration/ListObjects.go @@ -132,11 +132,9 @@ func ListObjects_truncated(s *S3Conf) error { maxKeys, out1.MaxKeys) } - if out1.NextMarker == nil { - return fmt.Errorf("expected non nil next marker") - } - if *out1.NextMarker != "baz" { - return fmt.Errorf("expected next-marker to be baz, instead got %v", + // S3 returns NextMarker only when the request has a delimiter + if out1.NextMarker != nil { + return fmt.Errorf("expected nil next marker, instead got %v", *out1.NextMarker) } @@ -145,10 +143,13 @@ func ListObjects_truncated(s *S3Conf) error { contents[:2], out1.Contents) } + // without a NextMarker the last key is the marker for the next page + marker := out1.Contents[len(out1.Contents)-1].Key + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) out2, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ Bucket: &bucket, - Marker: out1.NextMarker, + Marker: marker, }) cancel() if err != nil { @@ -162,9 +163,9 @@ func ListObjects_truncated(s *S3Conf) error { return fmt.Errorf("expected output not to be truncated") } - if getString(out2.Marker) != getString(out1.NextMarker) { + if getString(out2.Marker) != getString(marker) { return fmt.Errorf("expected marker to be %v, instead got %v", - getString(out1.NextMarker), getString(out2.Marker)) + getString(marker), getString(out2.Marker)) } if !compareObjects(contents[2:], out2.Contents) { @@ -175,6 +176,64 @@ func ListObjects_truncated(s *S3Conf) error { }) } +func ListObjects_truncated_with_delimiter(s *S3Conf) error { + testName := "ListObjects_truncated_with_delimiter" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + contents, err := putObjects(s3client, []string{"foo", "bar", "baz"}, bucket) + if err != nil { + return err + } + + delim, maxKeys := "/", int32(2) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out1, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ + Bucket: &bucket, + Delimiter: &delim, + MaxKeys: &maxKeys, + }) + cancel() + if err != nil { + return err + } + + if out1.IsTruncated == nil || !*out1.IsTruncated { + return fmt.Errorf("expected output to be truncated") + } + if out1.NextMarker == nil { + return fmt.Errorf("expected non nil next marker") + } + if *out1.NextMarker != "baz" { + return fmt.Errorf("expected next-marker to be baz, instead got %v", + *out1.NextMarker) + } + if !compareObjects(contents[:2], out1.Contents) { + return fmt.Errorf("expected the output to be %v, instead got %v", + contents[:2], out1.Contents) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out2, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ + Bucket: &bucket, + Delimiter: &delim, + Marker: out1.NextMarker, + }) + cancel() + if err != nil { + return err + } + + if out2.IsTruncated == nil || *out2.IsTruncated { + return fmt.Errorf("expected output not to be truncated") + } + if !compareObjects(contents[2:], out2.Contents) { + return fmt.Errorf("expected the output to be %v, instead got %v", + contents[2:], out2.Contents) + } + return nil + }) +} + func ListObjects_invalid_max_keys(s *S3Conf) error { testName := "ListObjects_invalid_max_keys" maxKeys := int32(-5) @@ -448,10 +507,10 @@ func ListObjects_list_all_objs(s *S3Conf) error { allObjects = append(allObjects, out.Contents...) - if out.NextMarker == nil || !*out.IsTruncated { + if !*out.IsTruncated { break } - marker = out.NextMarker + marker = out.Contents[len(out.Contents)-1].Key } if !compareObjects(contents, allObjects) { @@ -719,17 +778,14 @@ func ListObjects_mp_masking_truncation(s *S3Conf) error { return fmt.Errorf("expected first page objects %v, instead got %v", contents[:2], out1.Contents) } - if out1.NextMarker == nil || *out1.NextMarker == "" { - return fmt.Errorf("expected non-empty NextMarker") - } - if *out1.NextMarker != "obj-b" { - return fmt.Errorf("expected NextMarker to be obj-b, instead got %v", *out1.NextMarker) + if out1.NextMarker != nil { + return fmt.Errorf("expected nil NextMarker, instead got %v", *out1.NextMarker) } ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) out2, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ Bucket: &bucket, - Marker: out1.NextMarker, + Marker: out1.Contents[len(out1.Contents)-1].Key, }) cancel() if err != nil { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index c0f82614..97467aa0 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -312,6 +312,7 @@ func TestListObjects(ts *TestState) { ts.Run(ListObjects_non_existing_bucket) ts.Run(ListObjects_with_prefix) ts.Run(ListObjects_truncated) + ts.Run(ListObjects_truncated_with_delimiter) ts.Run(ListObjects_paginated) ts.Run(ListObjects_invalid_max_keys) ts.Run(ListObjects_max_keys_0) @@ -3151,6 +3152,7 @@ func GetIntTests() IntTests { "ListObjects_non_existing_bucket": ListObjects_non_existing_bucket, "ListObjects_with_prefix": ListObjects_with_prefix, "ListObjects_truncated": ListObjects_truncated, + "ListObjects_truncated_with_delimiter": ListObjects_truncated_with_delimiter, "ListObjects_paginated": ListObjects_paginated, "ListObjects_invalid_max_keys": ListObjects_invalid_max_keys, "ListObjects_max_keys_0": ListObjects_max_keys_0, diff --git a/tests/test_rest_list_objects.sh b/tests/test_rest_list_objects.sh index 8f30c9be..341c6205 100755 --- a/tests/test_rest_list_objects.sh +++ b/tests/test_rest_list_objects.sh @@ -122,9 +122,6 @@ source ./tests/drivers/objects_and_versions.sh # tags: curl,ListObjects,minimal-request @test "REST - list objects v1 - no NextMarker without delimiter" { - if [ "$DIRECT" != "true" ]; then - skip "https://github.com/versity/versitygw/issues/999" - fi run get_bucket_name "$BUCKET_ONE_NAME" assert_success bucket_name="$output"