diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index cc46677f..eff9cb3d 100644 --- a/s3api/controllers/base.go +++ b/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 ( diff --git a/s3api/controllers/bucket-delete.go b/s3api/controllers/bucket-delete.go index 00a169dd..22ef9286 100644 --- a/s3api/controllers/bucket-delete.go +++ b/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 } diff --git a/s3api/controllers/bucket-delete_test.go b/s3api/controllers/bucket-delete_test.go index 19eb95da..50ae11d9 100644 --- a/s3api/controllers/bucket-delete_test.go +++ b/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, }, }, }, diff --git a/s3api/controllers/bucket-get.go b/s3api/controllers/bucket-get.go index cc60d267..f4da5c34 100644 --- a/s3api/controllers/bucket-get.go +++ b/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 { diff --git a/s3api/controllers/bucket-list.go b/s3api/controllers/bucket-list.go index f990dd1e..5cae8939 100644 --- a/s3api/controllers/bucket-list.go +++ b/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, }) diff --git a/s3api/controllers/bucket-put.go b/s3api/controllers/bucket-put.go index 8a0caf43..7fbd0ea3 100644 --- a/s3api/controllers/bucket-put.go +++ b/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{ diff --git a/s3api/controllers/object-delete.go b/s3api/controllers/object-delete.go index 93721315..76dfb167 100644 --- a/s3api/controllers/object-delete.go +++ b/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 { diff --git a/s3api/controllers/object-head.go b/s3api/controllers/object-head.go index 5b0aa304..57c749b0 100644 --- a/s3api/controllers/object-head.go +++ b/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{ diff --git a/s3api/controllers/object-post.go b/s3api/controllers/object-post.go index 658320a4..35fece82 100644 --- a/s3api/controllers/object-post.go +++ b/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 diff --git a/s3api/controllers/object-post_test.go b/s3api/controllers/object-post_test.go index baca9f3e..3d96d7de 100644 --- a/s3api/controllers/object-post_test.go +++ b/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), }, }, { diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index 28444872..77397d95 100644 --- a/s3api/controllers/object-put.go +++ b/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, diff --git a/s3err/s3err.go b/s3err/s3err.go index 4dd1a374..c2c3cc22 100644 --- a/s3err/s3err.go +++ b/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", diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 65ac11f9..f2e1e023 100644 --- a/tests/integration/tests.go +++ b/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 }