fix: return NextMarker in ListObjects only when a delimiter is set

Fixes #999

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

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) {

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 {

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,

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"