Skip to content

Commit

Permalink
Merge pull request #989 from niksis02/fix/complete-mp-empty-parts
Browse files Browse the repository at this point in the history
fix: Adds a check to ensure that the CompleteMultipartUpload parts ar…
  • Loading branch information
benmcclelland authored Dec 20, 2024
2 parents 47cf2a6 + 7c5258e commit b0c310a
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 b0c310a

Please sign in to comment.