From 7c5258e6e93d96f0ebf27a2fe747b8d99d4f16fd Mon Sep 17 00:00:00 2001 From: niksis02 Date: Tue, 17 Dec 2024 18:50:57 +0400 Subject: [PATCH] fix: Adds a check to ensure that the CompleteMultipartUpload parts are not empty. --- s3api/controllers/base.go | 14 ++++++++++++++ s3api/controllers/base_test.go | 24 +++++++++++++++++++++++- s3err/s3err.go | 6 ++++++ tests/integration/group-tests.go | 2 ++ tests/integration/tests.go | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 1 deletion(-) diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 00e5ec5..d9662fc 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -3189,6 +3189,20 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { }) } + if len(data.Parts) == 0 { + if c.debug { + log.Println("empty parts provided for complete multipart upload") + } + return SendXMLResponse(ctx, nil, + s3err.GetAPIError(s3err.ErrEmptyParts), + &MetaOpts{ + Logger: c.logger, + MetricsMng: c.mm, + Action: metrics.ActionCompleteMultipartUpload, + BucketOwner: parsedAcl.Owner, + }) + } + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ Readonly: c.readonly, diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index 635af42..598056e 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -1713,6 +1713,19 @@ func TestS3ApiController_CreateActions(t *testing.T) { ` + completMpBody := ` + + + etag + 1 + + + ` + + completMpEmptyBody := ` + + ` + app.Use(func(ctx *fiber.Ctx) error { ctx.Locals("account", auth.Account{Access: "valid access"}) ctx.Locals("isRoot", true) @@ -1765,11 +1778,20 @@ func TestS3ApiController_CreateActions(t *testing.T) { wantErr: false, statusCode: 400, }, + { + name: "Complete-multipart-upload-empty-parts", + app: app, + args: args{ + req: httptest.NewRequest(http.MethodPost, "/my-bucket/my-key?uploadId=23423", strings.NewReader(completMpEmptyBody)), + }, + wantErr: false, + statusCode: 400, + }, { name: "Complete-multipart-upload-success", app: app, args: args{ - req: httptest.NewRequest(http.MethodPost, "/my-bucket/my-key?uploadId=23423", strings.NewReader(`body`)), + req: httptest.NewRequest(http.MethodPost, "/my-bucket/my-key?uploadId=23423", strings.NewReader(completMpBody)), }, wantErr: false, statusCode: 200, diff --git a/s3err/s3err.go b/s3err/s3err.go index 4d0d971..3db2763 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -72,6 +72,7 @@ const ( ErrInvalidPartNumberMarker ErrInvalidObjectAttributes ErrInvalidPart + ErrEmptyParts ErrInvalidPartNumber ErrInternalError ErrInvalidCopyDest @@ -253,6 +254,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "One or more of the specified parts could not be found. The part may not have been uploaded, or the specified entity tag may not match the part's entity tag.", HTTPStatusCode: http.StatusBadRequest, }, + ErrEmptyParts: { + Code: "InvalidRequest", + Description: "You must specify at least one part", + HTTPStatusCode: http.StatusBadRequest, + }, ErrInvalidPartNumber: { Code: "InvalidArgument", Description: "Part number must be an integer between 1 and 10000, inclusive.", diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index bdbc545..6c7b9c3 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -320,6 +320,7 @@ func TestCompleteMultipartUpload(s *S3Conf) { CompletedMultipartUpload_non_existing_bucket(s) CompleteMultipartUpload_invalid_part_number(s) CompleteMultipartUpload_invalid_ETag(s) + CompleteMultipartUpload_empty_parts(s) CompleteMultipartUpload_success(s) if !s.azureTests { CompleteMultipartUpload_racey_success(s) @@ -849,6 +850,7 @@ func GetIntTests() IntTests { "CompletedMultipartUpload_non_existing_bucket": CompletedMultipartUpload_non_existing_bucket, "CompleteMultipartUpload_invalid_part_number": CompleteMultipartUpload_invalid_part_number, "CompleteMultipartUpload_invalid_ETag": CompleteMultipartUpload_invalid_ETag, + "CompleteMultipartUpload_empty_parts": CompleteMultipartUpload_empty_parts, "CompleteMultipartUpload_success": CompleteMultipartUpload_success, "CompleteMultipartUpload_racey_success": CompleteMultipartUpload_racey_success, "PutBucketAcl_non_existing_bucket": PutBucketAcl_non_existing_bucket, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 2ad465f..78ec389 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -7327,6 +7327,38 @@ func CompleteMultipartUpload_invalid_ETag(s *S3Conf) error { }) } +func CompleteMultipartUpload_empty_parts(s *S3Conf) error { + testName := "CompleteMultipartUpload_empty_parts" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + mp, err := createMp(s3client, bucket, obj) + if err != nil { + return err + } + + _, _, err = uploadParts(s3client, 5*1024*1024, 1, bucket, obj, *mp.UploadId) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{ + Bucket: &bucket, + Key: &obj, + UploadId: mp.UploadId, + MultipartUpload: &types.CompletedMultipartUpload{ + Parts: []types.CompletedPart{}, // empty parts list + }, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrEmptyParts)); err != nil { + return err + } + + return nil + }) +} + func CompleteMultipartUpload_success(s *S3Conf) error { testName := "CompleteMultipartUpload_success" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {