fix: fixes the invalid x-amz-mp-object-size header error in CompleteMultipartUpload.

Fixes #1398

The `x-amz-mp-object-size` request header can have two erroneous states: an invalid value or a negative integer. AWS returns different error descriptions for each case. This PR fixes the error description for the invalid header value case.

The invalid case can't be integration tested as SDK expects `int64` as the header value.

s3api/controllers/base.go

@@ -42,10 +42,15 @@ type S3ApiController struct {
 }
 
 const (
+	// time constants
 	iso8601Format             = "20060102T150405Z"
 	iso8601TimeFormatExtended = "Mon Jan _2 15:04:05 2006"
 	timefmt                   = "Mon, 02 Jan 2006 15:04:05 GMT"
-	maxXMLBodyLen             = 4 * 1024 * 1024
+
+	maxXMLBodyLen     = 4 * 1024 * 1024
+	minPartNumber     = 1
+	maxPartNumber     = 10000
+	defaultMaxBuckets = int32(10000)
 )
 
 var (

s3api/controllers/bucket-delete.go

@@ -153,6 +153,7 @@ func (c S3ApiController) DeleteBucketCors(ctx *fiber.Ctx) (*Response, error) {
 	return &Response{
 		MetaOpts: &MetaOptions{
 			BucketOwner: parsedAcl.Owner,
+			Status:      http.StatusNoContent,
 		},
 	}, err
 }

s3api/controllers/bucket-delete_test.go

@@ -286,6 +286,7 @@ func TestS3ApiController_DeleteBucketCors(t *testing.T) {
 				response: &Response{
 					MetaOpts: &MetaOptions{
 						BucketOwner: "root",
+						Status:      http.StatusNoContent,
 					},
 				},
 				err: s3err.GetAPIError(s3err.ErrAdminMethodNotSupported),
@@ -300,6 +301,7 @@ func TestS3ApiController_DeleteBucketCors(t *testing.T) {
 				response: &Response{
 					MetaOpts: &MetaOptions{
 						BucketOwner: "root",
+						Status:      http.StatusNoContent,
 					},
 				},
 			},

s3api/controllers/bucket-get.go

@@ -61,7 +61,9 @@ func (c S3ApiController) GetBucketTagging(ctx *fiber.Ctx) (*Response, error) {
 		}, err
 	}
 	resp := s3response.Tagging{
-		TagSet: s3response.TagSet{Tags: []s3response.Tag{}},
+		TagSet: s3response.TagSet{
+			Tags: make([]s3response.Tag, 0, len(tags)),
+		},
 	}
 
 	for key, val := range tags {

s3api/controllers/bucket-list.go

@@ -31,10 +31,10 @@ func (c S3ApiController) ListBuckets(ctx *fiber.Ctx) (*Response, error) {
 	maxBucketsStr := ctx.Query("max-buckets")
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
 
-	var maxBuckets int32 = 10000
+	maxBuckets := defaultMaxBuckets
 	if maxBucketsStr != "" {
 		maxBucketsParsed, err := strconv.ParseInt(maxBucketsStr, 10, 32)
-		if err != nil || maxBucketsParsed < 0 || maxBucketsParsed > 10000 {
+		if err != nil || maxBucketsParsed < 0 || maxBucketsParsed > int64(defaultMaxBuckets) {
 			debuglogger.Logf("error parsing max-buckets %q: %v", maxBucketsStr, err)
 			return &Response{
 				MetaOpts: &MetaOptions{},
@@ -47,7 +47,7 @@ func (c S3ApiController) ListBuckets(ctx *fiber.Ctx) (*Response, error) {
 		s3response.ListBucketsInput{
 			Owner:             acct.Access,
 			IsAdmin:           acct.Role == auth.RoleAdmin,
-			MaxBuckets:        int32(maxBuckets),
+			MaxBuckets:        maxBuckets,
 			ContinuationToken: cToken,
 			Prefix:            prefix,
 		})

s3api/controllers/bucket-put.go

@@ -301,14 +301,14 @@ func (c S3ApiController) PutBucketAcl(ctx *fiber.Ctx) (*Response, error) {
 	grantFullControl := ctx.Get("X-Amz-Grant-Full-Control")
 	grantRead := ctx.Get("X-Amz-Grant-Read")
 	grantReadACP := ctx.Get("X-Amz-Grant-Read-Acp")
-	granWrite := ctx.Get("X-Amz-Grant-Write")
+	grantWrite := ctx.Get("X-Amz-Grant-Write")
 	grantWriteACP := ctx.Get("X-Amz-Grant-Write-Acp")
 	// context locals
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
 	isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool)
 
-	grants := grantFullControl + grantRead + grantReadACP + granWrite + grantWriteACP
+	grants := grantFullControl + grantRead + grantReadACP + grantWrite + grantWriteACP
 	var input *auth.PutBucketAclInput
 
 	err := auth.VerifyAccess(ctx.Context(), c.be,
@@ -424,7 +424,7 @@ func (c S3ApiController) PutBucketAcl(ctx *fiber.Ctx) (*Response, error) {
 			GrantFullControl: &grantFullControl,
 			GrantRead:        &grantRead,
 			GrantReadACP:     &grantReadACP,
-			GrantWrite:       &granWrite,
+			GrantWrite:       &grantWrite,
 			GrantWriteACP:    &grantWriteACP,
 		}
 	} else {
@@ -459,11 +459,11 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) {
 	grantFullControl := ctx.Get("X-Amz-Grant-Full-Control")
 	grantRead := ctx.Get("X-Amz-Grant-Read")
 	grantReadACP := ctx.Get("X-Amz-Grant-Read-Acp")
-	granWrite := ctx.Get("X-Amz-Grant-Write")
+	grantWrite := ctx.Get("X-Amz-Grant-Write")
 	grantWriteACP := ctx.Get("X-Amz-Grant-Write-Acp")
 	lockEnabled := strings.EqualFold(ctx.Get("X-Amz-Bucket-Object-Lock-Enabled"), "true")
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
-	grants := grantFullControl + grantRead + grantReadACP + granWrite + grantWriteACP
+	grants := grantFullControl + grantRead + grantReadACP + grantWrite + grantWriteACP
 	objectOwnership := types.ObjectOwnership(
 		ctx.Get("X-Amz-Object-Ownership", string(types.ObjectOwnershipBucketOwnerEnforced)),
 	)
@@ -518,7 +518,7 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) {
 		GrantFullControl: &grantFullControl,
 		GrantRead:        &grantRead,
 		GrantReadACP:     &grantReadACP,
-		GrantWrite:       &granWrite,
+		GrantWrite:       &grantWrite,
 		GrantWriteACP:    &grantWriteACP,
 		AccessControlPolicy: &auth.AccessControlPolicy{
 			Owner: &types.Owner{

s3api/controllers/object-delete.go

@@ -32,7 +32,7 @@ func (c S3ApiController) DeleteObjectTagging(ctx *fiber.Ctx) (*Response, error)
 	key := strings.TrimPrefix(ctx.Path(), fmt.Sprintf("/%s/", bucket))
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
 	isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool)
-	IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
+	isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 
 	err := auth.VerifyAccess(ctx.Context(), c.be,
@@ -45,7 +45,7 @@ func (c S3ApiController) DeleteObjectTagging(ctx *fiber.Ctx) (*Response, error)
 			Bucket:         bucket,
 			Object:         key,
 			Action:         auth.DeleteObjectTaggingAction,
-			IsBucketPublic: IsBucketPublic,
+			IsBucketPublic: isBucketPublic,
 		})
 	if err != nil {
 		return &Response{
@@ -71,7 +71,7 @@ func (c S3ApiController) AbortMultipartUpload(ctx *fiber.Ctx) (*Response, error)
 	uploadId := ctx.Query("uploadId")
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
 	isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool)
-	IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
+	isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 
 	err := auth.VerifyAccess(ctx.Context(), c.be,
@@ -84,7 +84,7 @@ func (c S3ApiController) AbortMultipartUpload(ctx *fiber.Ctx) (*Response, error)
 			Bucket:         bucket,
 			Object:         key,
 			Action:         auth.AbortMultipartUploadAction,
-			IsBucketPublic: IsBucketPublic,
+			IsBucketPublic: isBucketPublic,
 		})
 	if err != nil {
 		return &Response{
@@ -116,7 +116,7 @@ func (c S3ApiController) DeleteObject(ctx *fiber.Ctx) (*Response, error) {
 	// context locals
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
 	isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool)
-	IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
+	isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 
 	//TODO: check s3:DeleteObjectVersion policy in case a use tries to delete a version of an object
@@ -131,7 +131,7 @@ func (c S3ApiController) DeleteObject(ctx *fiber.Ctx) (*Response, error) {
 			Bucket:         bucket,
 			Object:         key,
 			Action:         auth.DeleteObjectAction,
-			IsBucketPublic: IsBucketPublic,
+			IsBucketPublic: isBucketPublic,
 		})
 	if err != nil {
 		return &Response{
@@ -152,7 +152,7 @@ func (c S3ApiController) DeleteObject(ctx *fiber.Ctx) (*Response, error) {
 			},
 		},
 		bypass,
-		IsBucketPublic,
+		isBucketPublic,
 		c.be,
 	)
 	if err != nil {

s3api/controllers/object-head.go

@@ -63,7 +63,7 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) (*Response, error) {
 
 	var partNumber *int32
 	if ctx.Request().URI().QueryArgs().Has("partNumber") {
-		if partNumberQuery < 1 || partNumberQuery > 10000 {
+		if partNumberQuery < minPartNumber || partNumberQuery > maxPartNumber {
 			debuglogger.Logf("invalid part number: %d", partNumberQuery)
 			return &Response{
 				MetaOpts: &MetaOptions{

s3api/controllers/object-post.go

@@ -36,7 +36,7 @@ func (c S3ApiController) RestoreObject(ctx *fiber.Ctx) (*Response, error) {
 	key := strings.TrimPrefix(ctx.Path(), fmt.Sprintf("/%s/", bucket))
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
 	isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool)
-	IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
+	isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 
 	err := auth.VerifyAccess(ctx.Context(), c.be,
@@ -49,7 +49,7 @@ func (c S3ApiController) RestoreObject(ctx *fiber.Ctx) (*Response, error) {
 			Bucket:         bucket,
 			Object:         key,
 			Action:         auth.RestoreObjectAction,
-			IsBucketPublic: IsBucketPublic,
+			IsBucketPublic: isBucketPublic,
 		})
 	if err != nil {
 		return &Response{
@@ -87,7 +87,7 @@ func (c S3ApiController) SelectObjectContent(ctx *fiber.Ctx) (*Response, error)
 	key := strings.TrimPrefix(ctx.Path(), fmt.Sprintf("/%s/", bucket))
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
 	isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool)
-	IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
+	isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 
 	err := auth.VerifyAccess(ctx.Context(), c.be,
@@ -100,7 +100,7 @@ func (c S3ApiController) SelectObjectContent(ctx *fiber.Ctx) (*Response, error)
 			Bucket:         bucket,
 			Object:         key,
 			Action:         auth.GetObjectAction,
-			IsBucketPublic: IsBucketPublic,
+			IsBucketPublic: isBucketPublic,
 		})
 	if err != nil {
 		return &Response{
@@ -237,7 +237,7 @@ func (c S3ApiController) CompleteMultipartUpload(ctx *fiber.Ctx) (*Response, err
 	// context locals
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
 	isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool)
-	IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
+	isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
 	parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
 
 	err := auth.VerifyAccess(ctx.Context(), c.be,
@@ -250,7 +250,7 @@ func (c S3ApiController) CompleteMultipartUpload(ctx *fiber.Ctx) (*Response, err
 			Bucket:         bucket,
 			Object:         key,
 			Action:         auth.PutObjectAction,
-			IsBucketPublic: IsBucketPublic,
+			IsBucketPublic: isBucketPublic,
 		})
 	if err != nil {
 		return &Response{
@@ -283,23 +283,22 @@ func (c S3ApiController) CompleteMultipartUpload(ctx *fiber.Ctx) (*Response, err
 	var mpuObjectSize *int64
 	if mpuObjSizeHdr != "" {
 		val, err := strconv.ParseInt(mpuObjSizeHdr, 10, 64)
-		//TODO: Not sure if invalid request should be returned
 		if err != nil {
-			debuglogger.Logf("invalid value for 'x-amz-mp-objects-size' header: %v", err)
+			debuglogger.Logf("invalid value for 'x-amz-mp-object-size' header: %v", err)
 			return &Response{
 				MetaOpts: &MetaOptions{
 					BucketOwner: parsedAcl.Owner,
 				},
-			}, s3err.GetAPIError(s3err.ErrInvalidRequest)
+			}, s3err.GetInvalidMpObjectSizeErr(mpuObjSizeHdr)
 		}
 
 		if val < 0 {
-			debuglogger.Logf("value for 'x-amz-mp-objects-size' header is less than 0: %v", val)
+			debuglogger.Logf("value for 'x-amz-mp-object-size' header is less than 0: %v", val)
 			return &Response{
 				MetaOpts: &MetaOptions{
 					BucketOwner: parsedAcl.Owner,
 				},
-			}, s3err.GetInvalidMpObjectSizeErr(val)
+			}, s3err.GetNegatvieMpObjectSizeErr(val)
 		}
 
 		mpuObjectSize = &val

s3api/controllers/object-post_test.go

@@ -420,7 +420,7 @@ func TestS3ApiController_CompleteMultipartUpload(t *testing.T) {
 						BucketOwner: "root",
 					},
 				},
-				err: s3err.GetAPIError(s3err.ErrInvalidRequest),
+				err: s3err.GetInvalidMpObjectSizeErr("invalid_mp_object_size"),
 			},
 		},
 		{
@@ -438,7 +438,7 @@ func TestS3ApiController_CompleteMultipartUpload(t *testing.T) {
 						BucketOwner: "root",
 					},
 				},
-				err: s3err.GetInvalidMpObjectSizeErr(-4),
+				err: s3err.GetNegatvieMpObjectSizeErr(-4),
 			},
 		},
 		{

s3api/controllers/object-put.go

@@ -51,7 +51,7 @@ func (c S3ApiController) PutObjectTagging(ctx *fiber.Ctx) (*Response, error) {
 		Acc:            acct,
 		Bucket:         bucket,
 		Object:         key,
-		Action:         auth.PutBucketTaggingAction,
+		Action:         auth.PutObjectTaggingAction,
 		IsBucketPublic: IsBucketPublic,
 	})
 	if err != nil {
@@ -233,7 +233,7 @@ func (c S3ApiController) UploadPart(ctx *fiber.Ctx) (*Response, error) {
 		}, err
 	}
 
-	if partNumber < 1 || partNumber > 10000 {
+	if partNumber < minPartNumber || partNumber > maxPartNumber {
 		debuglogger.Logf("invalid part number: %d", partNumber)
 		return &Response{
 			MetaOpts: &MetaOptions{
@@ -349,7 +349,7 @@ func (c S3ApiController) UploadPartCopy(ctx *fiber.Ctx) (*Response, error) {
 		}, err
 	}
 
-	if partNumber < 1 || partNumber > 10000 {
+	if partNumber < minPartNumber || partNumber > maxPartNumber {
 		debuglogger.Logf("invalid part number: %d", partNumber)
 		return &Response{
 			MetaOpts: &MetaOptions{
@@ -389,7 +389,7 @@ func (c S3ApiController) PutObjectAcl(ctx *fiber.Ctx) (*Response, error) {
 	grantFullControl := ctx.Get("X-Amz-Grant-Full-Control")
 	grantRead := ctx.Get("X-Amz-Grant-Read")
 	grantReadACP := ctx.Get("X-Amz-Grant-Read-Acp")
-	granWrite := ctx.Get("X-Amz-Grant-Write")
+	grantWrite := ctx.Get("X-Amz-Grant-Write")
 	grantWriteACP := ctx.Get("X-Amz-Grant-Write-Acp")
 	// context locals
 	acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
@@ -420,7 +420,7 @@ func (c S3ApiController) PutObjectAcl(ctx *fiber.Ctx) (*Response, error) {
 		Key:              &key,
 		GrantFullControl: &grantFullControl,
 		GrantRead:        &grantRead,
-		GrantWrite:       &granWrite,
+		GrantWrite:       &grantWrite,
 		ACL:              types.ObjectCannedACL(acl),
 		GrantReadACP:     &grantReadACP,
 		GrantWriteACP:    &grantWriteACP,
@@ -524,7 +524,7 @@ func (c S3ApiController) CopyObject(ctx *fiber.Ctx) (*Response, error) {
 	}
 
 	if taggingDirective != "" && taggingDirective != types.TaggingDirectiveCopy && taggingDirective != types.TaggingDirectiveReplace {
-		debuglogger.Logf("invalid tagging direcrive: %v", taggingDirective)
+		debuglogger.Logf("invalid tagging directive: %v", taggingDirective)
 		return &Response{
 			MetaOpts: &MetaOptions{
 				BucketOwner: parsedAcl.Owner,

s3err/s3err.go

@@ -894,7 +894,7 @@ func GetIncorrectMpObjectSizeErr(expected, actual int64) APIError {
 	}
 }
 
-func GetInvalidMpObjectSizeErr(val int64) APIError {
+func GetNegatvieMpObjectSizeErr(val int64) APIError {
 	return APIError{
 		Code:           "InvalidRequest",
 		Description:    fmt.Sprintf("Value for x-amz-mp-object-size header is less than zero: '%v'", val),
@@ -902,6 +902,14 @@ func GetInvalidMpObjectSizeErr(val int64) APIError {
 	}
 }
 
+func GetInvalidMpObjectSizeErr(val string) APIError {
+	return APIError{
+		Code:           "InvalidRequest",
+		Description:    fmt.Sprintf("Value for x-amz-mp-object-size header is invalid: '%s'", val),
+		HTTPStatusCode: http.StatusBadRequest,
+	}
+}
+
 func CreateExceedingRangeErr(objSize int64) APIError {
 	return APIError{
 		Code:           "InvalidArgument",

tests/integration/tests.go

@@ -11260,7 +11260,7 @@ func CompleteMultipartUpload_mpu_object_size(s *S3Conf) error {
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		_, err = s3client.CompleteMultipartUpload(ctx, input)
 		cancel()
-		if err := checkApiErr(err, s3err.GetInvalidMpObjectSizeErr(invMpuSize)); err != nil {
+		if err := checkApiErr(err, s3err.GetNegatvieMpObjectSizeErr(invMpuSize)); err != nil {
 			return err
 		}