Skip to content

Commit

Permalink
fix: Adds a check to ensure that the CompleteMultipartUpload parts ar…
Browse files Browse the repository at this point in the history
…e not empty.
  • Loading branch information
niksis02 committed Dec 17, 2024
1 parent c17db86 commit 7c5258e
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 1 deletion.
14 changes: 14 additions & 0 deletions s3api/controllers/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
24 changes: 23 additions & 1 deletion s3api/controllers/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1713,6 +1713,19 @@ func TestS3ApiController_CreateActions(t *testing.T) {
</SelectObjectContentRequest>
`

completMpBody := `
<CompleteMultipartUpload xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
<Part>
<ETag>etag</ETag>
<PartNumber>1</PartNumber>
</Part>
</CompleteMultipartUpload>
`

completMpEmptyBody := `
<CompleteMultipartUpload xmlns="http://s3.amazonaws.com/doc/2006-03-01/"></CompleteMultipartUpload>
`

app.Use(func(ctx *fiber.Ctx) error {
ctx.Locals("account", auth.Account{Access: "valid access"})
ctx.Locals("isRoot", true)
Expand Down Expand Up @@ -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(`<root><key>body</key></root>`)),
req: httptest.NewRequest(http.MethodPost, "/my-bucket/my-key?uploadId=23423", strings.NewReader(completMpBody)),
},
wantErr: false,
statusCode: 200,
Expand Down
6 changes: 6 additions & 0 deletions s3err/s3err.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ const (
ErrInvalidPartNumberMarker
ErrInvalidObjectAttributes
ErrInvalidPart
ErrEmptyParts
ErrInvalidPartNumber
ErrInternalError
ErrInvalidCopyDest
Expand Down Expand Up @@ -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.",
Expand Down
2 changes: 2 additions & 0 deletions tests/integration/group-tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
32 changes: 32 additions & 0 deletions tests/integration/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 7c5258e

Please sign in to comment.