diff --git a/s3api/controllers/bucket-get.go b/s3api/controllers/bucket-get.go index 9cbb93a..d1393f5 100644 --- a/s3api/controllers/bucket-get.go +++ b/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 diff --git a/s3api/controllers/bucket-get_test.go b/s3api/controllers/bucket-get_test.go index d9547ca..8770150 100644 --- a/s3api/controllers/bucket-get_test.go +++ b/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) { diff --git a/tests/integration/ListObjects.go b/tests/integration/ListObjects.go index fc2aa3a..59fdfe4 100644 --- a/tests/integration/ListObjects.go +++ b/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 { - return fmt.Errorf("expected nil next marker, instead got %v", + 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) } @@ -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 { - return fmt.Errorf("expected nil NextMarker, instead got %v", *out1.NextMarker) + 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) } 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 {