diff --git a/integration/tests.go b/integration/tests.go index c344bfc8..60b749ba 100644 --- a/integration/tests.go +++ b/integration/tests.go @@ -339,6 +339,14 @@ func TestPutDirObject(s *S3Conf) { return } + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: &bucket, MaxKeys: -4}) + cancel() + if err == nil { + failF("%v: expected invalid argument error", testname) + return + } + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) out, err := s3client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: &bucket}) cancel() diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 5867e09b..5888b346 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -221,7 +221,7 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { cToken := ctx.Query("continuation-token") marker := ctx.Query("marker") delimiter := ctx.Query("delimiter") - maxkeys := ctx.QueryInt("max-keys") + maxkeysStr := ctx.Query("max-keys") access := ctx.Locals("access").(string) isRoot := ctx.Locals("isRoot").(bool) parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) @@ -252,12 +252,20 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { if err := auth.VerifyACL(parsedAcl, bucket, access, "READ", isRoot); err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{Logger: c.logger, Action: "ListObjectsV2", BucketOwner: parsedAcl.Owner}) } + maxkeys, err := utils.ParseMaxKeys(maxkeysStr) + if err != nil { + return SendXMLResponse(ctx, nil, err, &MetaOpts{ + Logger: c.logger, + Action: "ListObjectsV2", + BucketOwner: parsedAcl.Owner, + }) + } res, err := c.be.ListObjectsV2(ctx.Context(), &s3.ListObjectsV2Input{ Bucket: &bucket, Prefix: &prefix, ContinuationToken: &cToken, Delimiter: &delimiter, - MaxKeys: int32(maxkeys), + MaxKeys: maxkeys, }) return SendXMLResponse(ctx, res, err, &MetaOpts{Logger: c.logger, Action: "ListObjectsV2", BucketOwner: parsedAcl.Owner}) } @@ -266,12 +274,21 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { return SendXMLResponse(ctx, nil, err, &MetaOpts{Logger: c.logger, Action: "ListObjects", BucketOwner: parsedAcl.Owner}) } + maxkeys, err := utils.ParseMaxKeys(maxkeysStr) + if err != nil { + return SendXMLResponse(ctx, nil, err, &MetaOpts{ + Logger: c.logger, + Action: "ListObjects", + BucketOwner: parsedAcl.Owner, + }) + } + res, err := c.be.ListObjects(ctx.Context(), &s3.ListObjectsInput{ Bucket: &bucket, Prefix: &prefix, Marker: &marker, Delimiter: &delimiter, - MaxKeys: int32(maxkeys), + MaxKeys: maxkeys, }) return SendXMLResponse(ctx, res, err, &MetaOpts{Logger: c.logger, Action: "ListObjects", BucketOwner: parsedAcl.Owner}) } diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index a864e871..4103d726 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -20,10 +20,12 @@ import ( "fmt" "net/http" "regexp" + "strconv" "strings" "github.com/gofiber/fiber/v2" "github.com/valyala/fasthttp" + "github.com/versity/versitygw/s3err" ) var ( @@ -78,6 +80,17 @@ func SetMetaHeaders(ctx *fiber.Ctx, meta map[string]string) { } } +func ParseMaxKeys(str string) (int32, error) { + if str == "" { + return -1, nil + } + num, err := strconv.ParseUint(str, 10, 16) + if err != nil { + return -1, s3err.GetAPIError(s3err.ErrInvalidMaxKeys) + } + return int32(num), nil +} + type CustomHeader struct { Key string Value string diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index 655180eb..4fbaa6f5 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -221,3 +221,52 @@ func TestIsValidBucketName(t *testing.T) { }) } } + +func TestParseMaxKeys(t *testing.T) { + type args struct { + str string + } + tests := []struct { + name string + args args + want int32 + wantErr bool + }{ + { + name: "Parse-max-keys-empty-string", + args: args{ + str: "", + }, + want: -1, + wantErr: false, + }, + { + name: "Parse-max-keys-invalid-number-string", + args: args{ + str: "bla", + }, + want: -1, + wantErr: true, + }, + { + name: "Parse-max-keys-invalid-negative-number", + args: args{ + str: "-5", + }, + want: -1, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseMaxKeys(tt.args.str) + if (err != nil) != tt.wantErr { + t.Errorf("ParseMaxKeys() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("ParseMaxKeys() = %v, want %v", got, tt.want) + } + }) + } +}