Upstream versity/versitygw state of the touched files before aed4617372

s3api/controllers/bucket-get.go

@@ -22,6 +22,7 @@ import (
 	"github.com/gofiber/fiber/v3"
 	"github.com/versity/versitygw/auth"
 	"github.com/versity/versitygw/s3api/utils"
+	"github.com/versity/versitygw/s3err"
 	"github.com/versity/versitygw/s3response"
 )
 
@@ -614,6 +615,15 @@ func (c S3ApiController) ListObjects(ctx fiber.Ctx) (*Response, error) {
 		region = defaultRegion
 	}
 
+	// start-after is a ListObjectsV2 argument, S3 rejects it on a V1 listing
+	if ctx.Query("start-after") != "" {
+		return &Response{
+			MetaOpts: &MetaOptions{
+				BucketOwner: parsedAcl.Owner,
+			},
+		}, s3err.GetInvalidArgumentErr(s3err.InvalidArgStartAfter, "")
+	}
+
 	err := c.verifyAccess(ctx, auth.AccessOptions{
 		Acl:             parsedAcl,
 		AclPermission:   auth.PermissionRead,
@@ -656,12 +666,6 @@ 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,9 +1176,6 @@ 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
@@ -1198,6 +1195,24 @@ func TestS3ApiController_ListObjects(t *testing.T) {
 				err: s3err.GetAPIError(s3err.ErrAccessDenied),
 			},
 		},
+		{
+			name: "start-after is not supported",
+			input: testInput{
+				locals: defaultLocals,
+				beRes:  listResult,
+				queries: map[string]string{
+					"start-after": "my-key",
+				},
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{
+						BucketOwner: "root",
+					},
+				},
+				err: s3err.GetInvalidArgumentErr(s3err.InvalidArgStartAfter, ""),
+			},
+		},
 		{
 			name: "invalid max keys",
 			input: testInput{
@@ -1249,45 +1264,6 @@ 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

@@ -17,6 +17,8 @@ package integration
 import (
 	"context"
 	"fmt"
+	"net/http"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
@@ -132,9 +134,11 @@ func ListObjects_truncated(s *S3Conf) error {
 				maxKeys, out1.MaxKeys)
 		}
 
-		// S3 returns NextMarker only when the request has a delimiter
+		if out1.NextMarker == nil {
-		if out1.NextMarker != nil {
+			return fmt.Errorf("expected non nil next marker")
-			return fmt.Errorf("expected nil next marker, instead got %v",
+		}
+		if *out1.NextMarker != "baz" {
+			return fmt.Errorf("expected next-marker to be baz, instead got %v",
 				*out1.NextMarker)
 		}
 
@@ -143,13 +147,10 @@ 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: marker,
+			Marker: out1.NextMarker,
 		})
 		cancel()
 		if err != nil {
@@ -163,9 +164,9 @@ func ListObjects_truncated(s *S3Conf) error {
 			return fmt.Errorf("expected output not to be truncated")
 		}
 
-		if getString(out2.Marker) != getString(marker) {
+		if getString(out2.Marker) != getString(out1.NextMarker) {
 			return fmt.Errorf("expected marker to be %v, instead got %v",
-				getString(marker), getString(out2.Marker))
+				getString(out1.NextMarker), getString(out2.Marker))
 		}
 
 		if !compareObjects(contents[2:], out2.Contents) {
@@ -176,64 +177,6 @@ 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)
@@ -252,6 +195,33 @@ func ListObjects_invalid_max_keys(s *S3Conf) error {
 	})
 }
 
+// start-after belongs to ListObjectsV2, so S3 rejects it on a V1 listing
+// instead of ignoring it. The SDK ListObjects input has no StartAfter field,
+// so the request is signed and sent by hand.
+func ListObjects_start_after(s *S3Conf) error {
+	testName := "ListObjects_start_after"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		_, err := putObjects(s3client, []string{"bar", "baz", "foo"}, bucket)
+		if err != nil {
+			return err
+		}
+
+		req, err := createSignedReq(http.MethodGet, s.endpoint,
+			fmt.Sprintf("%v?start-after=bar", bucket),
+			s.awsID, s.awsSecret, "s3", s.awsRegion, "", nil, time.Now(), nil)
+		if err != nil {
+			return err
+		}
+
+		resp, err := s.httpClient.Do(req)
+		if err != nil {
+			return err
+		}
+
+		return checkHTTPResponseApiErr(resp, s3err.GetInvalidArgumentErr(s3err.InvalidArgStartAfter, ""))
+	})
+}
+
 func ListObjects_max_keys_0(s *S3Conf) error {
 	testName := "ListObjects_max_keys_0"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -507,10 +477,10 @@ func ListObjects_list_all_objs(s *S3Conf) error {
 
 			allObjects = append(allObjects, out.Contents...)
 
-			if !*out.IsTruncated {
+			if out.NextMarker == nil || !*out.IsTruncated {
 				break
 			}
-			marker = out.Contents[len(out.Contents)-1].Key
+			marker = out.NextMarker
 		}
 
 		if !compareObjects(contents, allObjects) {
@@ -778,14 +748,17 @@ 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 {
+		if out1.NextMarker == nil || *out1.NextMarker == "" {
-			return fmt.Errorf("expected nil NextMarker, instead got %v", *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)
 		}
 
 		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
 		out2, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{
 			Bucket: &bucket,
-			Marker: out1.Contents[len(out1.Contents)-1].Key,
+			Marker: out1.NextMarker,
 		})
 		cancel()
 		if err != nil {