From 3b903f60443523c69635320f834e463cafcd0723 Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Tue, 22 Oct 2024 14:28:50 -0400 Subject: [PATCH] fix: Fixes max-parts, max-keys, max-uploads validation defaulting to 1000 --- s3api/controllers/base.go | 56 +++++++++++++++--------------- s3api/utils/utils.go | 2 +- tests/integration/group-tests.go | 4 +++ tests/integration/tests.go | 58 ++++++++++++++++++++++++++++++-- 4 files changed, 90 insertions(+), 30 deletions(-) diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 097cc025..89139cc8 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -83,7 +83,6 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { key := ctx.Params("key") keyEnd := ctx.Params("*1") uploadId := ctx.Query("uploadId") - maxParts := int32(ctx.QueryInt("max-parts", -1)) partNumberMarker := ctx.Query("part-number-marker") acceptRange := ctx.Get("Range") acct := ctx.Locals("account").(auth.Account) @@ -221,16 +220,6 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { } if uploadId != "" { - if maxParts < 0 && ctx.Request().URI().QueryArgs().Has("max-parts") { - return SendResponse(ctx, - s3err.GetAPIError(s3err.ErrInvalidMaxParts), - &MetaOpts{ - Logger: c.logger, - MetricsMng: c.mm, - Action: metrics.ActionListParts, - BucketOwner: parsedAcl.Owner, - }) - } if partNumberMarker != "" { n, err := strconv.Atoi(partNumberMarker) if err != nil || n < 0 { @@ -248,8 +237,24 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { }) } } + mxParts := ctx.Query("max-parts") + maxParts, err := utils.ParseUint(mxParts) + if err != nil { + if c.debug { + log.Printf("error parsing max parts %q: %v", + mxParts, err) + } + return SendResponse(ctx, + s3err.GetAPIError(s3err.ErrInvalidMaxParts), + &MetaOpts{ + Logger: c.logger, + MetricsMng: c.mm, + Action: metrics.ActionListParts, + BucketOwner: parsedAcl.Owner, + }) + } - err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ Readonly: c.readonly, Acl: parsedAcl, AclPermission: types.PermissionRead, @@ -268,17 +273,13 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { BucketOwner: parsedAcl.Owner, }) } - var mxParts *int32 - if ctx.Request().URI().QueryArgs().Has("max-parts") { - mxParts = &maxParts - } res, err := c.be.ListParts(ctx.Context(), &s3.ListPartsInput{ Bucket: &bucket, Key: &key, UploadId: &uploadId, PartNumberMarker: &partNumberMarker, - MaxParts: mxParts, + MaxParts: &maxParts, }) return SendXMLResponse(ctx, res, err, &MetaOpts{ @@ -346,7 +347,7 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { partNumberMarker := ctx.Get("X-Amz-Part-Number-Marker") maxPartsParsed, err := utils.ParseUint(maxParts) if err != nil { - return SendXMLResponse(ctx, nil, err, + return SendXMLResponse(ctx, nil, s3err.GetAPIError(s3err.ErrInvalidMaxParts), &MetaOpts{ Logger: c.logger, MetricsMng: c.mm, @@ -736,7 +737,7 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { log.Printf("error parsing max keys %q: %v", maxkeysStr, err) } - return SendXMLResponse(ctx, nil, err, + return SendXMLResponse(ctx, nil, s3err.GetAPIError(s3err.ErrInvalidMaxKeys), &MetaOpts{ Logger: c.logger, MetricsMng: c.mm, @@ -869,12 +870,13 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { log.Printf("error parsing max uploads %q: %v", maxUploadsStr, err) } - return SendXMLResponse(ctx, nil, err, &MetaOpts{ - Logger: c.logger, - MetricsMng: c.mm, - Action: metrics.ActionListMultipartUploads, - BucketOwner: parsedAcl.Owner, - }) + return SendXMLResponse(ctx, nil, s3err.GetAPIError(s3err.ErrInvalidMaxUploads), + &MetaOpts{ + Logger: c.logger, + MetricsMng: c.mm, + Action: metrics.ActionListMultipartUploads, + BucketOwner: parsedAcl.Owner, + }) } res, err := c.be.ListMultipartUploads(ctx.Context(), &s3.ListMultipartUploadsInput{ @@ -919,7 +921,7 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { log.Printf("error parsing max keys %q: %v", maxkeysStr, err) } - return SendXMLResponse(ctx, nil, err, + return SendXMLResponse(ctx, nil, s3err.GetAPIError(s3err.ErrInvalidMaxKeys), &MetaOpts{ Logger: c.logger, MetricsMng: c.mm, @@ -970,7 +972,7 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { log.Printf("error parsing max keys %q: %v", maxkeysStr, err) } - return SendXMLResponse(ctx, nil, err, + return SendXMLResponse(ctx, nil, s3err.GetAPIError(s3err.ErrInvalidMaxKeys), &MetaOpts{ Logger: c.logger, MetricsMng: c.mm, diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index c872f781..1a6a5f4a 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -180,7 +180,7 @@ func ParseUint(str string) (int32, error) { } num, err := strconv.ParseUint(str, 10, 16) if err != nil { - return 1000, s3err.GetAPIError(s3err.ErrInvalidMaxKeys) + return 1000, fmt.Errorf("invalid uint: %w", err) } return int32(num), nil } diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 54d7b07d..8c5a083a 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -280,6 +280,8 @@ func TestUploadPartCopy(s *S3Conf) { func TestListParts(s *S3Conf) { ListParts_incorrect_uploadId(s) ListParts_incorrect_object_key(s) + ListParts_invalid_max_parts(s) + ListParts_default_max_parts(s) ListParts_truncated(s) ListParts_success(s) } @@ -796,6 +798,8 @@ func GetIntTests() IntTests { "UploadPartCopy_by_range_success": UploadPartCopy_by_range_success, "ListParts_incorrect_uploadId": ListParts_incorrect_uploadId, "ListParts_incorrect_object_key": ListParts_incorrect_object_key, + "ListParts_invalid_max_parts": ListParts_invalid_max_parts, + "ListParts_default_max_parts": ListParts_default_max_parts, "ListParts_truncated": ListParts_truncated, "ListParts_success": ListParts_success, "ListMultipartUploads_non_existing_bucket": ListMultipartUploads_non_existing_bucket, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 834753a2..9fbd2ec0 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -6269,6 +6269,60 @@ func ListParts_incorrect_object_key(s *S3Conf) error { }) } +func ListParts_invalid_max_parts(s *S3Conf) error { + testName := "ListParts_invalid_max_parts" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + out, err := createMp(s3client, bucket, obj) + if err != nil { + return err + } + + invMaxParts := int32(-3) + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.ListParts(ctx, &s3.ListPartsInput{ + Bucket: &bucket, + Key: &obj, + UploadId: out.UploadId, + MaxParts: &invMaxParts, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidMaxParts)); err != nil { + return err + } + + return nil + }) +} + +func ListParts_default_max_parts(s *S3Conf) error { + testName := "ListParts_default_max_parts" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + out, err := createMp(s3client, bucket, obj) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.ListParts(ctx, &s3.ListPartsInput{ + Bucket: &bucket, + Key: &obj, + UploadId: out.UploadId, + }) + cancel() + if err != nil { + return err + } + + if *res.MaxParts != 1000 { + return fmt.Errorf("expected max parts to be 1000, instead got %v", *res.MaxParts) + } + + return nil + }) +} + func ListParts_truncated(s *S3Conf) error { testName := "ListParts_truncated" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -6407,15 +6461,15 @@ func ListMultipartUploads_empty_result(s *S3Conf) error { func ListMultipartUploads_invalid_max_uploads(s *S3Conf) error { testName := "ListMultipartUploads_invalid_max_uploads" - maxUploads := int32(-3) return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + maxUploads := int32(-3) ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{ Bucket: &bucket, MaxUploads: &maxUploads, }) cancel() - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidMaxKeys)); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidMaxUploads)); err != nil { return err }