From 370d9093728689b34937fbbbfef23cc02f02a9d7 Mon Sep 17 00:00:00 2001 From: Barry Loong <20846761+loongyh@users.noreply.github.com> Date: Tue, 15 Sep 2026 12:27:54 +0800 Subject: [PATCH 1/4] fix: strip `aws-chunked` from stored `Content-Encoding` --- s3api/controllers/object-post.go | 2 +- s3api/controllers/object-post_test.go | 30 +++++++++++++++ s3api/controllers/object-put.go | 4 +- s3api/controllers/object-put_test.go | 55 +++++++++++++++++++++++++++ s3api/utils/utils.go | 25 ++++++++++++ s3api/utils/utils_test.go | 27 +++++++++++++ 6 files changed, 140 insertions(+), 3 deletions(-) diff --git a/s3api/controllers/object-post.go b/s3api/controllers/object-post.go index 407e8e16..421f3abd 100644 --- a/s3api/controllers/object-post.go +++ b/s3api/controllers/object-post.go @@ -147,7 +147,7 @@ func (c S3ApiController) CreateMultipartUpload(ctx fiber.Ctx) (*Response, error) contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") - contentEncoding := ctx.Get("Content-Encoding") + contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding")) tagging := ctx.Get("X-Amz-Tagging") expires := ctx.Get("Expires") websiteRedirectLocation := ctx.Get("X-Amz-Website-Redirect-Location") diff --git a/s3api/controllers/object-post_test.go b/s3api/controllers/object-post_test.go index dd5253d8..c6766ccb 100644 --- a/s3api/controllers/object-post_test.go +++ b/s3api/controllers/object-post_test.go @@ -328,6 +328,28 @@ func TestS3ApiController_CreateMultipartUpload(t *testing.T) { }, }, }, + { + name: "strips aws-chunked content encoding", + input: testInput{ + locals: defaultLocals, + beRes: s3response.InitiateMultipartUploadResult{}, + headers: map[string]string{ + "Content-Encoding": "aws-chunked,gzip", + }, + }, + output: testOutput{ + response: &Response{ + Data: s3response.InitiateMultipartUploadResult{}, + Headers: map[string]*string{ + "x-amz-checksum-algorithm": nil, + "x-amz-checksum-type": nil, + }, + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -336,6 +358,14 @@ func TestS3ApiController_CreateMultipartUpload(t *testing.T) { if tt.name == "successful response" && createMultipartUploadInput.StorageClass != types.StorageClassGlacier { t.Fatalf("expected storage class %q, got %q", types.StorageClassGlacier, createMultipartUploadInput.StorageClass) } + if tt.name == "strips aws-chunked content encoding" { + if createMultipartUploadInput.ContentEncoding == nil { + t.Fatal("expected content encoding to be set") + } + if *createMultipartUploadInput.ContentEncoding != "gzip" { + t.Fatalf("expected content encoding %q, got %q", "gzip", *createMultipartUploadInput.ContentEncoding) + } + } return tt.input.beRes.(s3response.InitiateMultipartUploadResult), tt.input.beErr }, GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) { diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index c07dd072..a1317921 100644 --- a/s3api/controllers/object-put.go +++ b/s3api/controllers/object-put.go @@ -516,7 +516,7 @@ func (c S3ApiController) CopyObject(ctx fiber.Ctx) (*Response, error) { metaDirective := types.MetadataDirective(ctx.Get("X-Amz-Metadata-Directive", string(types.MetadataDirectiveCopy))) taggingDirective := types.TaggingDirective(ctx.Get("X-Amz-Tagging-Directive", string(types.TaggingDirectiveCopy))) contentType := ctx.Get("Content-Type", defaultContentType) - contentEncoding := ctx.Get("Content-Encoding") + contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding")) contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") @@ -698,7 +698,7 @@ func (c S3ApiController) PutObject(ctx fiber.Ctx) (*Response, error) { bucket := ctx.Params("bucket") key := strings.TrimPrefix(ctx.Path(), fmt.Sprintf("/%s/", bucket)) contentType := ctx.Get("Content-Type", defaultContentType) - contentEncoding := ctx.Get("Content-Encoding") + contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding")) contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") diff --git a/s3api/controllers/object-put_test.go b/s3api/controllers/object-put_test.go index 14611a2d..0f72fa9d 100644 --- a/s3api/controllers/object-put_test.go +++ b/s3api/controllers/object-put_test.go @@ -1331,6 +1331,61 @@ func TestS3ApiController_PutObject(t *testing.T) { }) }) + t.Run("strips aws-chunked content encoding", func(t *testing.T) { + be := &BackendMock{ + PutObjectFunc: func(_ context.Context, input s3response.PutObjectInput) (s3response.PutObjectOutput, error) { + if input.ContentEncoding == nil { + t.Fatal("expected content encoding to be set") + } + if *input.ContentEncoding != "gzip" { + t.Fatalf("expected content encoding %q, got %q", "gzip", *input.ContentEncoding) + } + return s3response.PutObjectOutput{ETag: "etag", VersionID: "version-id"}, nil + }, + GetBucketPolicyFunc: func(_ context.Context, _ string) ([]byte, error) { + return nil, s3err.GetAPIError(s3err.ErrAccessDenied) + }, + GetObjectLockConfigurationFunc: func(_ context.Context, _ string) ([]byte, error) { + return nil, s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound) + }, + GetBucketVersioningFunc: func(_ context.Context, _ string) (s3response.GetBucketVersioningOutput, error) { + return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrNotImplemented) + }, + } + + ctrl := S3ApiController{be: be} + testController(t, ctrl.PutObject, &Response{ + Headers: map[string]*string{ + "ETag": utils.GetStringPtr("etag"), + "x-amz-checksum-crc32": nil, + "x-amz-checksum-crc32c": nil, + "x-amz-checksum-crc64nvme": nil, + "x-amz-checksum-sha1": nil, + "x-amz-checksum-sha256": nil, + "x-amz-checksum-sha512": nil, + "x-amz-checksum-md5": nil, + "x-amz-checksum-xxhash64": nil, + "x-amz-checksum-xxhash3": nil, + "x-amz-checksum-xxhash128": nil, + "x-amz-checksum-type": nil, + "x-amz-version-id": utils.GetStringPtr("version-id"), + "x-amz-object-size": nil, + }, + MetaOpts: &MetaOptions{ + BucketOwner: "root", + ObjectETag: utils.GetStringPtr("etag"), + ContentLength: 0, + ObjectSize: 0, + EventName: s3event.EventObjectCreatedPut, + }, + }, nil, ctxInputs{ + locals: defaultLocals, + headers: map[string]string{ + "Content-Encoding": "aws-chunked,gzip", + }, + }) + }) + tests := []struct { name string input testInput diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index f0b42bdc..3ad79622 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -1112,3 +1112,28 @@ func ValidateLocationConstraint(constraint *string, region string) error { return nil } + +// The coding announced when a body is framed in aws-chunked, as the SDKs do to +// carry a trailing checksum. +const awsChunkedEncoding = "aws-chunked" + +// StripAwsChunkedEncoding drops the aws-chunked token, which frames the request +// rather than the object, from a Content-Encoding value. +func StripAwsChunkedEncoding(contentEncoding string) string { + if contentEncoding == "" { + return "" + } + + codings := strings.Split(contentEncoding, ",") + kept := make([]string, 0, len(codings)) + for _, coding := range codings { + trimmed := strings.TrimSpace(coding) + if trimmed == "" || strings.EqualFold(trimmed, awsChunkedEncoding) { + continue + } + + kept = append(kept, trimmed) + } + + return strings.Join(kept, ",") +} diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index 10390de7..02a17ddf 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -1515,3 +1515,30 @@ func TestValidateCopySource(t *testing.T) { }) } } + +func TestStripAwsChunkedEncoding(t *testing.T) { + tests := []struct { + name string + contentEncoding string + want string + }{ + {"empty", "", ""}, + {"only aws-chunked", "aws-chunked", ""}, + {"only aws-chunked, uppercase", "AWS-CHUNKED", ""}, + {"only aws-chunked, padded", " aws-chunked ", ""}, + {"no aws-chunked", "gzip", "gzip"}, + {"other codings kept in order", "deflate,gzip", "deflate,gzip"}, + {"aws-chunked first", "aws-chunked,gzip", "gzip"}, + {"aws-chunked last", "gzip,aws-chunked", "gzip"}, + {"aws-chunked in the middle", "deflate,aws-chunked,gzip", "deflate,gzip"}, + {"spaces around codings", "aws-chunked, gzip", "gzip"}, + {"repeated aws-chunked", "aws-chunked,aws-chunked", ""}, + {"empty coding dropped", "gzip,,aws-chunked", "gzip"}, + {"coding containing the token is kept", "aws-chunked-custom", "aws-chunked-custom"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, StripAwsChunkedEncoding(tt.contentEncoding)) + }) + } +} From 25f9790c0a8ef1ce7b1e336500930a1ce08e5b0b Mon Sep 17 00:00:00 2001 From: Barry Loong <20846761+loongyh@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:57:34 +0800 Subject: [PATCH 2/4] fix(s3api): strip `aws-chunked` only for streaming uploads - Replace `StripAwsChunkedEncoding` with `ParseContentEncoding`, which drops the token only when `x-amz-content-sha256` names a streaming payload type, so a client that sends `aws-chunked` on a hex-payload request keeps it - Revert the `CopyObject` and `CreateMultipartUpload` call sites: both routes are registered with `streamBody` false, so a streaming payload type is rejected before the controller and the token can only be a stored value - Reject `aws-chunked` combined with `UNSIGNED-PAYLOAD` in the authentication middleware, beside the existing payload-type validation, with a new `InvalidArgAwsChunkedUnsignedPayload` - Add REST tests for the three cases: stripped from a chunked upload, kept on a hex-payload request, and rejected with `UNSIGNED-PAYLOAD` - Add `CONTENT_ENCODING` to the PutObject REST script and a `check_content_encoding` driver for HeadObject --- s3api/controllers/object-post.go | 2 +- s3api/controllers/object-post_test.go | 30 -------- s3api/controllers/object-put.go | 4 +- s3api/controllers/object-put_test.go | 61 ++++++++++++++++- s3api/middlewares/authentication.go | 5 ++ s3api/utils/utils.go | 29 ++++++-- s3api/utils/utils_test.go | 68 ++++++++++++++----- s3err/invalid-argument.go | 5 ++ tests/drivers/head_object/head_object_rest.sh | 23 +++++++ tests/drivers/put_object/put_object_rest.sh | 26 +++++++ tests/rest_scripts/put_object.sh | 5 ++ tests/test_rest_chunked.sh | 24 +++++++ tests/test_rest_put_object.sh | 29 ++++++++ 13 files changed, 253 insertions(+), 58 deletions(-) diff --git a/s3api/controllers/object-post.go b/s3api/controllers/object-post.go index 421f3abd..407e8e16 100644 --- a/s3api/controllers/object-post.go +++ b/s3api/controllers/object-post.go @@ -147,7 +147,7 @@ func (c S3ApiController) CreateMultipartUpload(ctx fiber.Ctx) (*Response, error) contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") - contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding")) + contentEncoding := ctx.Get("Content-Encoding") tagging := ctx.Get("X-Amz-Tagging") expires := ctx.Get("Expires") websiteRedirectLocation := ctx.Get("X-Amz-Website-Redirect-Location") diff --git a/s3api/controllers/object-post_test.go b/s3api/controllers/object-post_test.go index c6766ccb..dd5253d8 100644 --- a/s3api/controllers/object-post_test.go +++ b/s3api/controllers/object-post_test.go @@ -328,28 +328,6 @@ func TestS3ApiController_CreateMultipartUpload(t *testing.T) { }, }, }, - { - name: "strips aws-chunked content encoding", - input: testInput{ - locals: defaultLocals, - beRes: s3response.InitiateMultipartUploadResult{}, - headers: map[string]string{ - "Content-Encoding": "aws-chunked,gzip", - }, - }, - output: testOutput{ - response: &Response{ - Data: s3response.InitiateMultipartUploadResult{}, - Headers: map[string]*string{ - "x-amz-checksum-algorithm": nil, - "x-amz-checksum-type": nil, - }, - MetaOpts: &MetaOptions{ - BucketOwner: "root", - }, - }, - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -358,14 +336,6 @@ func TestS3ApiController_CreateMultipartUpload(t *testing.T) { if tt.name == "successful response" && createMultipartUploadInput.StorageClass != types.StorageClassGlacier { t.Fatalf("expected storage class %q, got %q", types.StorageClassGlacier, createMultipartUploadInput.StorageClass) } - if tt.name == "strips aws-chunked content encoding" { - if createMultipartUploadInput.ContentEncoding == nil { - t.Fatal("expected content encoding to be set") - } - if *createMultipartUploadInput.ContentEncoding != "gzip" { - t.Fatalf("expected content encoding %q, got %q", "gzip", *createMultipartUploadInput.ContentEncoding) - } - } return tt.input.beRes.(s3response.InitiateMultipartUploadResult), tt.input.beErr }, GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) { diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index a1317921..71f9135b 100644 --- a/s3api/controllers/object-put.go +++ b/s3api/controllers/object-put.go @@ -516,7 +516,7 @@ func (c S3ApiController) CopyObject(ctx fiber.Ctx) (*Response, error) { metaDirective := types.MetadataDirective(ctx.Get("X-Amz-Metadata-Directive", string(types.MetadataDirectiveCopy))) taggingDirective := types.TaggingDirective(ctx.Get("X-Amz-Tagging-Directive", string(types.TaggingDirectiveCopy))) contentType := ctx.Get("Content-Type", defaultContentType) - contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding")) + contentEncoding := ctx.Get("Content-Encoding") contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") @@ -698,7 +698,7 @@ func (c S3ApiController) PutObject(ctx fiber.Ctx) (*Response, error) { bucket := ctx.Params("bucket") key := strings.TrimPrefix(ctx.Path(), fmt.Sprintf("/%s/", bucket)) contentType := ctx.Get("Content-Type", defaultContentType) - contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding")) + contentEncoding := utils.ParseContentEncoding(ctx) contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") diff --git a/s3api/controllers/object-put_test.go b/s3api/controllers/object-put_test.go index 0f72fa9d..71497b82 100644 --- a/s3api/controllers/object-put_test.go +++ b/s3api/controllers/object-put_test.go @@ -1331,7 +1331,7 @@ func TestS3ApiController_PutObject(t *testing.T) { }) }) - t.Run("strips aws-chunked content encoding", func(t *testing.T) { + t.Run("strips aws-chunked from a streaming upload", func(t *testing.T) { be := &BackendMock{ PutObjectFunc: func(_ context.Context, input s3response.PutObjectInput) (s3response.PutObjectOutput, error) { if input.ContentEncoding == nil { @@ -1381,7 +1381,64 @@ func TestS3ApiController_PutObject(t *testing.T) { }, nil, ctxInputs{ locals: defaultLocals, headers: map[string]string{ - "Content-Encoding": "aws-chunked,gzip", + "Content-Encoding": "aws-chunked,gzip", + "X-Amz-Content-Sha256": "STREAMING-UNSIGNED-PAYLOAD-TRAILER", + }, + }) + }) + + t.Run("keeps aws-chunked when the payload is not streamed", func(t *testing.T) { + be := &BackendMock{ + PutObjectFunc: func(_ context.Context, input s3response.PutObjectInput) (s3response.PutObjectOutput, error) { + if input.ContentEncoding == nil { + t.Fatal("expected content encoding to be set") + } + if *input.ContentEncoding != "aws-chunked" { + t.Fatalf("expected content encoding %q, got %q", "aws-chunked", *input.ContentEncoding) + } + return s3response.PutObjectOutput{ETag: "etag", VersionID: "version-id"}, nil + }, + GetBucketPolicyFunc: func(_ context.Context, _ string) ([]byte, error) { + return nil, s3err.GetAPIError(s3err.ErrAccessDenied) + }, + GetObjectLockConfigurationFunc: func(_ context.Context, _ string) ([]byte, error) { + return nil, s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound) + }, + GetBucketVersioningFunc: func(_ context.Context, _ string) (s3response.GetBucketVersioningOutput, error) { + return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrNotImplemented) + }, + } + + ctrl := S3ApiController{be: be} + testController(t, ctrl.PutObject, &Response{ + Headers: map[string]*string{ + "ETag": utils.GetStringPtr("etag"), + "x-amz-checksum-crc32": nil, + "x-amz-checksum-crc32c": nil, + "x-amz-checksum-crc64nvme": nil, + "x-amz-checksum-sha1": nil, + "x-amz-checksum-sha256": nil, + "x-amz-checksum-sha512": nil, + "x-amz-checksum-md5": nil, + "x-amz-checksum-xxhash64": nil, + "x-amz-checksum-xxhash3": nil, + "x-amz-checksum-xxhash128": nil, + "x-amz-checksum-type": nil, + "x-amz-version-id": utils.GetStringPtr("version-id"), + "x-amz-object-size": nil, + }, + MetaOpts: &MetaOptions{ + BucketOwner: "root", + ObjectETag: utils.GetStringPtr("etag"), + ContentLength: 0, + ObjectSize: 0, + EventName: s3event.EventObjectCreatedPut, + }, + }, nil, ctxInputs{ + locals: defaultLocals, + headers: map[string]string{ + "Content-Encoding": "aws-chunked", + "X-Amz-Content-Sha256": "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", }, }) }) diff --git a/s3api/middlewares/authentication.go b/s3api/middlewares/authentication.go index 7d8ab356..079d688b 100644 --- a/s3api/middlewares/authentication.go +++ b/s3api/middlewares/authentication.go @@ -132,6 +132,11 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string, if !streamBody && utils.IsStreamingPayload(hashPayload) { return s3err.GetAPIError(s3err.ErrInvalidSHA256PayloadUsage) } + // aws-chunked frames the body, so it contradicts an unsigned payload, + // which declares the body is sent as-is + if utils.IsUnsignedPaylod(hashPayload) && utils.HasAwsChunkedEncoding(ctx.Get("Content-Encoding")) { + return s3err.GetInvalidArgumentErr(s3err.InvalidArgAwsChunkedUnsignedPayload, hashPayload) + } canonicalString, err := utils.CheckValidSignature(ctx, authData, derivedKey, hashPayload, tdate, contentLength) if err != nil { diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index 3ad79622..07aa2e72 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -1113,15 +1113,30 @@ func ValidateLocationConstraint(constraint *string, region string) error { return nil } -// The coding announced when a body is framed in aws-chunked, as the SDKs do to -// carry a trailing checksum. +// The coding a client announces when it frames a body in aws-chunked, as the +// SDKs do to carry a trailing checksum. const awsChunkedEncoding = "aws-chunked" -// StripAwsChunkedEncoding drops the aws-chunked token, which frames the request -// rather than the object, from a Content-Encoding value. -func StripAwsChunkedEncoding(contentEncoding string) string { - if contentEncoding == "" { - return "" +// HasAwsChunkedEncoding reports whether a Content-Encoding value carries the +// aws-chunked token. +func HasAwsChunkedEncoding(contentEncoding string) bool { + for _, coding := range strings.Split(contentEncoding, ",") { + if strings.EqualFold(strings.TrimSpace(coding), awsChunkedEncoding) { + return true + } + } + + return false +} + +// ParseContentEncoding returns the Content-Encoding to store for a request, +// dropping the aws-chunked token when the payload type says the body was framed +// in it. S3 strips it only for streaming uploads: on any other request the +// token is a value the client chose and is stored as sent. +func ParseContentEncoding(ctx fiber.Ctx) string { + contentEncoding := ctx.Get("Content-Encoding") + if !IsStreamingPayload(ctx.Get("X-Amz-Content-Sha256")) { + return contentEncoding } codings := strings.Split(contentEncoding, ",") diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index 02a17ddf..52147920 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -20,6 +20,7 @@ import ( "encoding/xml" "errors" "math/rand" + "net/http" "net/url" "reflect" "strings" @@ -1516,29 +1517,64 @@ func TestValidateCopySource(t *testing.T) { } } -func TestStripAwsChunkedEncoding(t *testing.T) { +func TestHasAwsChunkedEncoding(t *testing.T) { tests := []struct { name string contentEncoding string - want string + want bool }{ - {"empty", "", ""}, - {"only aws-chunked", "aws-chunked", ""}, - {"only aws-chunked, uppercase", "AWS-CHUNKED", ""}, - {"only aws-chunked, padded", " aws-chunked ", ""}, - {"no aws-chunked", "gzip", "gzip"}, - {"other codings kept in order", "deflate,gzip", "deflate,gzip"}, - {"aws-chunked first", "aws-chunked,gzip", "gzip"}, - {"aws-chunked last", "gzip,aws-chunked", "gzip"}, - {"aws-chunked in the middle", "deflate,aws-chunked,gzip", "deflate,gzip"}, - {"spaces around codings", "aws-chunked, gzip", "gzip"}, - {"repeated aws-chunked", "aws-chunked,aws-chunked", ""}, - {"empty coding dropped", "gzip,,aws-chunked", "gzip"}, - {"coding containing the token is kept", "aws-chunked-custom", "aws-chunked-custom"}, + {"empty", "", false}, + {"only aws-chunked", "aws-chunked", true}, + {"uppercase", "AWS-CHUNKED", true}, + {"padded", " aws-chunked ", true}, + {"alongside another coding", "aws-chunked, gzip", true}, + {"other coding only", "gzip", false}, + {"token is a prefix of another coding", "aws-chunked-custom", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, StripAwsChunkedEncoding(tt.contentEncoding)) + assert.Equal(t, tt.want, HasAwsChunkedEncoding(tt.contentEncoding)) + }) + } +} + +func TestParseContentEncoding(t *testing.T) { + tests := []struct { + name string + contentSha256 string + contentEncoding string + want string + }{ + // streaming: the token frames the request, so S3 drops it + {"streaming, only aws-chunked", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked", ""}, + {"streaming, aws-chunked first", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked,gzip", "gzip"}, + {"streaming, aws-chunked last", "STREAMING-AWS4-HMAC-SHA256-PAYLOAD", "gzip,aws-chunked", "gzip"}, + {"streaming, aws-chunked in the middle", "STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER", "deflate,aws-chunked,gzip", "deflate,gzip"}, + {"streaming, spaces around codings", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked, gzip", "gzip"}, + {"streaming, uppercase", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "AWS-CHUNKED,gzip", "gzip"}, + {"streaming, repeated", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked,aws-chunked", ""}, + {"streaming, empty coding dropped", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "gzip,,aws-chunked", "gzip"}, + {"streaming, token is a prefix of another coding", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked-custom", "aws-chunked-custom"}, + {"streaming, no aws-chunked", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "gzip", "gzip"}, + {"streaming, no content encoding", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "", ""}, + // not streaming: the token is a value the client chose, and S3 keeps it + {"hex payload keeps aws-chunked", "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", "aws-chunked", "aws-chunked"}, + {"hex payload keeps other codings", "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", "aws-chunked,gzip", "aws-chunked,gzip"}, + {"ecdsa streaming is not chunk decoded, so the value is kept", "STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD", "aws-chunked", "aws-chunked"}, + {"no payload header", "", "gzip", "gzip"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + headers := http.Header{} + if tt.contentSha256 != "" { + headers.Set("X-Amz-Content-Sha256", tt.contentSha256) + } + if tt.contentEncoding != "" { + headers.Set("Content-Encoding", tt.contentEncoding) + } + + ctx := fiberCtxFromURL(t, http.MethodPut, "http://localhost/bucket/object", headers) + assert.Equal(t, tt.want, ParseContentEncoding(ctx)) }) } } diff --git a/s3err/invalid-argument.go b/s3err/invalid-argument.go index 17a0e587..edeb84a4 100644 --- a/s3err/invalid-argument.go +++ b/s3err/invalid-argument.go @@ -62,6 +62,7 @@ const ( InvalidArgIndexDocumentSuffix InvalidArgMissingIndexDocumentSuffix InvalidArgErrorDocumentKey + InvalidArgAwsChunkedUnsignedPayload ) var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{ @@ -121,6 +122,10 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{ Description: "x-amz-content-sha256 must be UNSIGNED-PAYLOAD, STREAMING-UNSIGNED-PAYLOAD-TRAILER, STREAMING-AWS4-HMAC-SHA256-PAYLOAD, STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER, STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD, STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD-TRAILER or a valid sha256 value.", ArgumentName: "x-amz-content-sha256", }, + InvalidArgAwsChunkedUnsignedPayload: { + Description: "aws-chunked encoding is not supported when x-amz-content-sha256 UNSIGNED-PAYLOAD is supplied", + ArgumentName: "x-amz-content-sha256", + }, InvalidArgCopySource: { Description: "You can only specify a copy source header for copy requests.", ArgumentName: "x-amz-copy-source", diff --git a/tests/drivers/head_object/head_object_rest.sh b/tests/drivers/head_object/head_object_rest.sh index 26ff1229..69391bbf 100644 --- a/tests/drivers/head_object/head_object_rest.sh +++ b/tests/drivers/head_object/head_object_rest.sh @@ -95,6 +95,29 @@ verify_checksum_doesnt_exist() { fi } +parse_content_encoding() { + if ! check_param_count_v2 "file" 1 $#; then + return 1 + fi + content_encoding=$(grep -i "^content-encoding:" "$1" | cut -d' ' -f2- | sed 's/\r$//') + echo "$content_encoding" +} + +check_content_encoding() { + if ! check_param_count_v2 "bucket, key, expected content encoding" 3 $#; then + return 1 + fi + if ! content_encoding=$(head_object_rest_expect_success_callback "$1" "$2" "" "parse_content_encoding" 2>&1); then + log 2 "error calling HeadObject command: $content_encoding" + return 1 + fi + if [ "$content_encoding" != "$3" ]; then + log 2 "content encoding mismatch (expected: '$3', actual: '$content_encoding')" + return 1 + fi + return 0 +} + parse_content_length() { if ! check_param_count_v2 "file" 1 $#; then return 1 diff --git a/tests/drivers/put_object/put_object_rest.sh b/tests/drivers/put_object/put_object_rest.sh index d28ac772..62ca6ce4 100644 --- a/tests/drivers/put_object/put_object_rest.sh +++ b/tests/drivers/put_object/put_object_rest.sh @@ -224,6 +224,32 @@ attempt_chunked_upload_with_bad_first_signature() { return 0 } +put_object_rest_with_content_encoding() { + if ! check_param_count_v2 "data file, bucket name, key, content encoding" 4 $#; then + return 1 + fi + if ! result=$(COMMAND_LOG="$COMMAND_LOG" DATA_FILE="$1" BUCKET_NAME="$2" OBJECT_KEY="$3" CONTENT_ENCODING="$4" OUTPUT_FILE="$TEST_FILE_FOLDER/result.txt" ./tests/rest_scripts/put_object.sh 2>&1); then + log 2 "error: $result" + return 1 + fi + if [ "$result" != "200" ]; then + log 2 "expected response code of '200', was '$result' ($(cat "$TEST_FILE_FOLDER/result.txt"))" + return 1 + fi + return 0 +} + +put_object_rest_unsigned_payload_with_aws_chunked() { + if ! check_param_count_v2 "data file, bucket name, key" 3 $#; then + return 1 + fi + if ! put_object_rest_expect_error "$1" "$2" "$3" "PAYLOAD=UNSIGNED-PAYLOAD CONTENT_ENCODING=aws-chunked" "400" "InvalidArgument" "aws-chunked encoding is not supported"; then + log 2 "expected aws-chunked with UNSIGNED-PAYLOAD to be rejected" + return 1 + fi + return 0 +} + chunked_upload_success() { if ! check_param_count_v2 "data file, bucket name, key" 3 $#; then return 1 diff --git a/tests/rest_scripts/put_object.sh b/tests/rest_scripts/put_object.sh index a4c1038d..6b01f75f 100755 --- a/tests/rest_scripts/put_object.sh +++ b/tests/rest_scripts/put_object.sh @@ -30,6 +30,8 @@ checksum_type="$CHECKSUM_TYPE" payload="$PAYLOAD" # shellcheck disable=SC2153 expires="$EXPIRES" +# shellcheck disable=SC2153 +content_encoding="$CONTENT_ENCODING" # use this parameter to check incorrect checksums # shellcheck disable=SC2153,SC2154 checksum_hash="$CHECKSUM" @@ -47,6 +49,9 @@ else fi cr_data=("PUT" "/$bucket_name/$key" "") +if [ -n "$content_encoding" ]; then + cr_data+=("content-encoding:$content_encoding") +fi if [ -n "$expires" ]; then cr_data+=("expires:$expires") fi diff --git a/tests/test_rest_chunked.sh b/tests/test_rest_chunked.sh index eccf947c..e149aae2 100755 --- a/tests/test_rest_chunked.sh +++ b/tests/test_rest_chunked.sh @@ -21,6 +21,7 @@ source ./tests/logger.sh source ./tests/setup.sh source ./tests/drivers/file.sh source ./tests/drivers/create_bucket/create_bucket_rest.sh +source ./tests/drivers/head_object/head_object_rest.sh source ./tests/drivers/get_object_lock_config/get_object_lock_config_rest.sh source ./tests/drivers/put_bucket_ownership_controls/put_bucket_ownership_controls_rest.sh @@ -104,6 +105,29 @@ source ./tests/drivers/put_bucket_ownership_controls/put_bucket_ownership_contro assert_success } +# tags: openssl,chunked,PutObject,content-encoding +@test "REST - chunked upload, aws-chunked not stored as Content-Encoding" { + run get_bucket_name "$BUCKET_ONE_NAME" + assert_success + bucket_name="$output" + + run setup_bucket_v2 "$bucket_name" + assert_success + + run get_file_name + assert_success + test_file="$output" + + run create_file_single_char "$test_file" 8192 'a' + assert_success + + run chunked_upload_success "$TEST_FILE_FOLDER/$test_file" "$bucket_name" "$test_file" + assert_success + + run check_content_encoding "$bucket_name" "$test_file" "" + assert_success +} + # tags: openssl,chunked,PutObject @test "REST - chunked upload, success (null bytes)" { run get_bucket_name "$BUCKET_ONE_NAME" diff --git a/tests/test_rest_put_object.sh b/tests/test_rest_put_object.sh index 37cf8a86..4a6b6d77 100755 --- a/tests/test_rest_put_object.sh +++ b/tests/test_rest_put_object.sh @@ -43,6 +43,35 @@ export RUN_USERS=true assert_success } +# tags: curl, PutObject, content-encoding +@test "REST - PutObject - aws-chunked kept when the payload is not chunked" { + run get_bucket_name "$BUCKET_ONE_NAME" + assert_success + bucket_name="$output" + + run setup_bucket_and_file_v2 "$bucket_name" "$test_file" + assert_success + + run put_object_rest_with_content_encoding "$TEST_FILE_FOLDER/$test_file" "$bucket_name" "$test_file" "aws-chunked" + assert_success + + run check_content_encoding "$bucket_name" "$test_file" "aws-chunked" + assert_success +} + +# tags: curl, PutObject, content-encoding, x-amz-content-sha256, invalid-header +@test "REST - PutObject - aws-chunked with UNSIGNED-PAYLOAD rejected" { + run get_bucket_name "$BUCKET_ONE_NAME" + assert_success + bucket_name="$output" + + run setup_bucket_and_file_v2 "$bucket_name" "$test_file" + assert_success + + run put_object_rest_unsigned_payload_with_aws_chunked "$TEST_FILE_FOLDER/$test_file" "$bucket_name" "$test_file" + assert_success +} + # tags: curl, PutObject, Expires, invalid-header @test "REST - PutObject - invalid 'Expires' parameter" { run get_bucket_name "$BUCKET_ONE_NAME" From 888fd5c1ad03c794ac231d5fb12e194d9feb7045 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Mon, 21 Sep 2026 17:30:35 +0400 Subject: [PATCH 3/4] fix: match S3's `Content-Encoding` error details for aws-chunked with `UNSIGNED-PAYLOAD` The rejection now reports `Content-Encoding` as the `ArgumentName` and the bare `aws-chunked` token as the `ArgumentValue`, rather than `x-amz-content-sha256` and the payload type, so a request sending `gzip,aws-chunked` gets back just the offending coding, and the message carries S3's trailing period. Adds integration tests for the three cases: `UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding` for a framed upload where only `aws-chunked` is dropped and the remaining codings keep their order, `PutObject_plain_stores_aws_chunked_content_encoding` for a hex-payload PUT that stores the token as sent, and `PutObject_unsigned_payload_with_aws_chunked_content_encoding` for the full error shape across four header spellings. --- s3api/middlewares/authentication.go | 2 +- s3api/utils/utils.go | 10 +-- s3err/invalid-argument.go | 4 +- tests/integration/PutObject.go | 89 +++++++++++++++++++ tests/integration/group-tests.go | 6 ++ .../unsigned_streaming_payload_trailer.go | 47 ++++++++++ tests/integration/utils.go | 19 ++++ 7 files changed, 169 insertions(+), 8 deletions(-) diff --git a/s3api/middlewares/authentication.go b/s3api/middlewares/authentication.go index 079d688b..c3a304e4 100644 --- a/s3api/middlewares/authentication.go +++ b/s3api/middlewares/authentication.go @@ -135,7 +135,7 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string, // aws-chunked frames the body, so it contradicts an unsigned payload, // which declares the body is sent as-is if utils.IsUnsignedPaylod(hashPayload) && utils.HasAwsChunkedEncoding(ctx.Get("Content-Encoding")) { - return s3err.GetInvalidArgumentErr(s3err.InvalidArgAwsChunkedUnsignedPayload, hashPayload) + return s3err.GetInvalidArgumentErr(s3err.InvalidArgAwsChunkedUnsignedPayload, utils.AwsChunkedEncoding) } canonicalString, err := utils.CheckValidSignature(ctx, authData, derivedKey, hashPayload, tdate, contentLength) diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index 07aa2e72..a8ce1c11 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -1113,15 +1113,15 @@ func ValidateLocationConstraint(constraint *string, region string) error { return nil } -// The coding a client announces when it frames a body in aws-chunked, as the -// SDKs do to carry a trailing checksum. -const awsChunkedEncoding = "aws-chunked" +// AwsChunkedEncoding is the coding a client announces when it frames a body in +// aws-chunked, as the SDKs do to carry a trailing checksum. +const AwsChunkedEncoding = "aws-chunked" // HasAwsChunkedEncoding reports whether a Content-Encoding value carries the // aws-chunked token. func HasAwsChunkedEncoding(contentEncoding string) bool { for _, coding := range strings.Split(contentEncoding, ",") { - if strings.EqualFold(strings.TrimSpace(coding), awsChunkedEncoding) { + if strings.EqualFold(strings.TrimSpace(coding), AwsChunkedEncoding) { return true } } @@ -1143,7 +1143,7 @@ func ParseContentEncoding(ctx fiber.Ctx) string { kept := make([]string, 0, len(codings)) for _, coding := range codings { trimmed := strings.TrimSpace(coding) - if trimmed == "" || strings.EqualFold(trimmed, awsChunkedEncoding) { + if trimmed == "" || strings.EqualFold(trimmed, AwsChunkedEncoding) { continue } diff --git a/s3err/invalid-argument.go b/s3err/invalid-argument.go index edeb84a4..aad893f7 100644 --- a/s3err/invalid-argument.go +++ b/s3err/invalid-argument.go @@ -123,8 +123,8 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{ ArgumentName: "x-amz-content-sha256", }, InvalidArgAwsChunkedUnsignedPayload: { - Description: "aws-chunked encoding is not supported when x-amz-content-sha256 UNSIGNED-PAYLOAD is supplied", - ArgumentName: "x-amz-content-sha256", + Description: "aws-chunked encoding is not supported when x-amz-content-sha256 UNSIGNED-PAYLOAD is supplied.", + ArgumentName: "Content-Encoding", }, InvalidArgCopySource: { Description: "You can only specify a copy source header for copy requests.", diff --git a/tests/integration/PutObject.go b/tests/integration/PutObject.go index 50f837e0..4ff6be5b 100644 --- a/tests/integration/PutObject.go +++ b/tests/integration/PutObject.go @@ -1468,3 +1468,92 @@ func PutObject_plain_body_with_decoded_length(s *S3Conf) error { return nil }) } + +// PutObject_plain_stores_aws_chunked_content_encoding checks that aws-chunked +// is stored as sent when the request wasn't framed in it. +// +// The token is transport only where x-amz-content-sha256 names a streaming +// payload type. On a plain PUT it is a coding the client chose, and S3 keeps it +// like any other. +func PutObject_plain_stores_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PutObject_plain_stores_aws_chunked_content_encoding" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + for i, contentEncoding := range []string{"aws-chunked", "gzip,aws-chunked"} { + object := fmt.Sprintf("plain-obj-%v", i) + + req, err := createSignedReq(http.MethodPut, s.endpoint, fmt.Sprintf("%s/%s", bucket, object), + s.awsID, s.awsSecret, "s3", s.awsRegion, "", []byte("hello world"), time.Now(), + map[string]string{"Content-Encoding": contentEncoding}) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("test %v: expected the response status code to be %v, instead got %v", + i+1, http.StatusOK, resp.StatusCode) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != contentEncoding { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, contentEncoding, stored) + } + } + + return nil + }) +} + +func PutObject_unsigned_payload_with_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PutObject_unsigned_payload_with_aws_chunked_content_encoding" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + object := "my-obj" + for i, contentEncoding := range []string{ + "aws-chunked", + "aws-chunked,gzip", + "gzip, aws-chunked", + "AWS-Chunked", + } { + req, err := createSignedReq(http.MethodPut, s.endpoint, fmt.Sprintf("%s/%s", bucket, object), + s.awsID, s.awsSecret, "s3", s.awsRegion, "UNSIGNED-PAYLOAD", []byte("hello world"), time.Now(), + map[string]string{"Content-Encoding": contentEncoding}) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + + // the error names the offending header and reports the token + // alone, not the whole header value + err = checkHTTPResponseApiErr(resp, + s3err.GetInvalidArgumentErr(s3err.InvalidArgAwsChunkedUnsignedPayload, "aws-chunked")) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &object, + }) + cancel() + if err == nil { + return fmt.Errorf("expected the rejected uploads to leave no object, but %v exists", object) + } + + return nil + }) +} diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 01542058..4531d65f 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -170,6 +170,8 @@ func TestPutObject(ts *TestState) { ts.Run(PutObject_special_chars) ts.Run(PutObject_aborted_plain_body) ts.Run(PutObject_plain_body_with_decoded_length) + ts.Run(PutObject_plain_stores_aws_chunked_content_encoding) + ts.Run(PutObject_unsigned_payload_with_aws_chunked_content_encoding) ts.Run(PutObject_tagging) ts.Run(PutObject_missing_object_lock_retention_config) ts.Run(PutObject_with_object_lock) @@ -2161,6 +2163,7 @@ func TestUnsignedStreaminPayloadTrailer(ts *TestState) { ts.Run(UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme) ts.Run(UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers) ts.Run(UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer) + ts.Run(UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding) ts.Run(UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum) ts.Run(UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_full_object) ts.Run(UnsignedStreamingPayloadTrailer_UploadPart_trailer_and_mp_algo_mismatch) @@ -2905,6 +2908,8 @@ func GetIntTests() IntTests { "PutObject_aborted_plain_body": PutObject_aborted_plain_body, "PutObject_plain_body_with_decoded_length": PutObject_plain_body_with_decoded_length, "UploadPart_plain_body_with_decoded_length": UploadPart_plain_body_with_decoded_length, + "PutObject_plain_stores_aws_chunked_content_encoding": PutObject_plain_stores_aws_chunked_content_encoding, + "PutObject_unsigned_payload_with_aws_chunked_content_encoding": PutObject_unsigned_payload_with_aws_chunked_content_encoding, "PutObject_tagging": PutObject_tagging, "PutObject_success": PutObject_success, "PutObject_default_content_type": PutObject_default_content_type, @@ -3633,6 +3638,7 @@ func GetIntTests() IntTests { "UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme": UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme, "UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers": UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers, "UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer": UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer, + "UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding": UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding, "UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum": UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum, "UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_full_object": UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_full_object, "UnsignedStreamingPayloadTrailer_UploadPart_trailer_and_mp_algo_mismatch": UnsignedStreamingPayloadTrailer_UploadPart_trailer_and_mp_algo_mismatch, diff --git a/tests/integration/unsigned_streaming_payload_trailer.go b/tests/integration/unsigned_streaming_payload_trailer.go index ca93953e..fe1feac0 100644 --- a/tests/integration/unsigned_streaming_payload_trailer.go +++ b/tests/integration/unsigned_streaming_payload_trailer.go @@ -437,6 +437,53 @@ func UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer(s *S3Conf }) } +// UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding checks that the +// aws-chunked token is dropped from the stored Content-Encoding when the body +// really was framed in it, and that every other coding survives in order. +func UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding(s *S3Conf) error { + testName := "UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + for i, test := range []struct { + contentEncoding string + stored string + }{ + // nothing is left, so no Content-Encoding is stored at all + {"aws-chunked", ""}, + {"aws-chunked,gzip", "gzip"}, + // the remaining codings keep their order + {"gzip,aws-chunked,br", "gzip,br"}, + // a framed body doesn't make every coding transport + {"gzip", "gzip"}, + } { + object := fmt.Sprintf("streaming-obj-%v", i) + reqHeaders := map[string]string{ + "x-amz-decoded-content-length": "11", + "Content-Encoding": test.contentEncoding, + } + body := []byte("B\r\nhello world\r\n0\r\n\r\n") + + _, apiErr, err := testUnsignedStreamingPayloadTrailerObjectPut(s, bucket, object, body, reqHeaders) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if apiErr != nil { + return fmt.Errorf("test %v failed: (%s) %s", i+1, apiErr.Code, apiErr.Message) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != test.stored { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, test.stored, stored) + } + } + + return nil + }) +} + func UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum(s *S3Conf) error { testName := "UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index f849a966..2b4e995f 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -4178,3 +4178,22 @@ func checkAndAbortUpload(client *s3.Client, bucket, key, uploadId string) error cancel() return err } + +// getStoredContentEncoding returns the Content-Encoding stored for an object, +// reporting an absent header as the empty string. +func getStoredContentEncoding(s3client *s3.Client, bucket, object string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &object, + }) + cancel() + if err != nil { + return "", err + } + if out.ContentEncoding == nil { + return "", nil + } + + return *out.ContentEncoding, nil +} From bbe398af72847a167dc2f11661899c2d9d064eae Mon Sep 17 00:00:00 2001 From: niksis02 Date: Tue, 22 Sep 2026 02:02:45 +0400 Subject: [PATCH 4/4] fix: strip `aws-chunked` when the request declares no payload type `ParseContentEncoding` dropped the `aws-chunked` token only for streaming payload types, so a presigned request kept it: such a request is signed as `UNSIGNED-PAYLOAD` and sends no `x-amz-content-sha256` header, and S3 strips the token there. Keep it only when the request declares a non streaming payload type. S3 applies this to the `Content-Encoding` header before any API sees it, so `CopyObject` and `CreateMultipartUpload` follow the same rule even though neither carries a body to frame. `POSTObject` takes its value from a form field rather than the header and is left alone. --- s3api/controllers/object-post.go | 2 +- s3api/controllers/object-put.go | 2 +- s3api/utils/utils.go | 21 ++- s3api/utils/utils_test.go | 13 +- tests/integration/group-tests.go | 6 + tests/integration/presigned_urls.go | 224 ++++++++++++++++++++++++++++ tests/integration/utils.go | 34 +++++ 7 files changed, 294 insertions(+), 8 deletions(-) diff --git a/s3api/controllers/object-post.go b/s3api/controllers/object-post.go index 407e8e16..61216d30 100644 --- a/s3api/controllers/object-post.go +++ b/s3api/controllers/object-post.go @@ -147,7 +147,7 @@ func (c S3ApiController) CreateMultipartUpload(ctx fiber.Ctx) (*Response, error) contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") - contentEncoding := ctx.Get("Content-Encoding") + contentEncoding := utils.ParseContentEncoding(ctx) tagging := ctx.Get("X-Amz-Tagging") expires := ctx.Get("Expires") websiteRedirectLocation := ctx.Get("X-Amz-Website-Redirect-Location") diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index 71f9135b..12487ef3 100644 --- a/s3api/controllers/object-put.go +++ b/s3api/controllers/object-put.go @@ -516,7 +516,7 @@ func (c S3ApiController) CopyObject(ctx fiber.Ctx) (*Response, error) { metaDirective := types.MetadataDirective(ctx.Get("X-Amz-Metadata-Directive", string(types.MetadataDirectiveCopy))) taggingDirective := types.TaggingDirective(ctx.Get("X-Amz-Tagging-Directive", string(types.TaggingDirectiveCopy))) contentType := ctx.Get("Content-Type", defaultContentType) - contentEncoding := ctx.Get("Content-Encoding") + contentEncoding := utils.ParseContentEncoding(ctx) contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index a8ce1c11..841d6e94 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -1130,12 +1130,25 @@ func HasAwsChunkedEncoding(contentEncoding string) bool { } // ParseContentEncoding returns the Content-Encoding to store for a request, -// dropping the aws-chunked token when the payload type says the body was framed -// in it. S3 strips it only for streaming uploads: on any other request the -// token is a value the client chose and is stored as sent. +// dropping the aws-chunked token unless the request declares a non streaming +// payload type in x-amz-content-sha256. +// +// S3 keys this on the declaration alone. A streaming payload type frames the +// body in aws-chunked, and a request that declares no payload type - a +// presigned URL, signed as UNSIGNED-PAYLOAD and carrying no header - may frame +// it too, so both drop the token. A declared hex digest or UNSIGNED-PAYLOAD +// says the body is sent as-is, so there the token is a coding the client chose +// and is stored as sent. +// +// S3 applies this to the Content-Encoding header itself, before any API sees +// it, so every controller that reads the header inherits it - including +// CopyObject and CreateMultipartUpload, which carry no body to frame. POSTObject +// takes its value from a form field and ignores the header, so it stores the +// token as sent and must not call this. func ParseContentEncoding(ctx fiber.Ctx) string { contentEncoding := ctx.Get("Content-Encoding") - if !IsStreamingPayload(ctx.Get("X-Amz-Content-Sha256")) { + payloadType := ctx.Get("X-Amz-Content-Sha256") + if payloadType != "" && !IsStreamingPayload(payloadType) { return contentEncoding } diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index 52147920..b7b5bf47 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -1557,11 +1557,20 @@ func TestParseContentEncoding(t *testing.T) { {"streaming, token is a prefix of another coding", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked-custom", "aws-chunked-custom"}, {"streaming, no aws-chunked", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "gzip", "gzip"}, {"streaming, no content encoding", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "", ""}, - // not streaming: the token is a value the client chose, and S3 keeps it + // no payload type: a presigned request is signed as UNSIGNED-PAYLOAD and + // may still frame its body, so S3 drops the token + {"no payload type, only aws-chunked", "", "aws-chunked", ""}, + {"no payload type, aws-chunked first", "", "aws-chunked,gzip", "gzip"}, + {"no payload type, aws-chunked last", "", "gzip,aws-chunked", "gzip"}, + {"no payload type, uppercase", "", "AWS-CHUNKED", ""}, + {"no payload type, token is a prefix of another coding", "", "aws-chunked-custom", "aws-chunked-custom"}, + {"no payload type, no aws-chunked", "", "gzip", "gzip"}, + // a declared non streaming payload type: the token is a value the client + // chose, and S3 keeps it {"hex payload keeps aws-chunked", "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", "aws-chunked", "aws-chunked"}, {"hex payload keeps other codings", "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", "aws-chunked,gzip", "aws-chunked,gzip"}, + {"unsigned payload keeps aws-chunked", "UNSIGNED-PAYLOAD", "aws-chunked", "aws-chunked"}, {"ecdsa streaming is not chunk decoded, so the value is kept", "STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD", "aws-chunked", "aws-chunked"}, - {"no payload header", "", "gzip", "gzip"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 4531d65f..cf3493c6 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -68,6 +68,9 @@ func TestPresignedAuthentication(ts *TestState) { ts.Run(PresignedAuth_incorrect_secret_key) ts.Run(PresignedAuth_sigv2_not_supported) ts.Run(PresignedAuth_PutObject_success) + ts.Run(PresignedAuth_PutObject_strips_aws_chunked_content_encoding) + ts.Run(PresignedAuth_CopyObject_strips_aws_chunked_content_encoding) + ts.Run(PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding) ts.Run(PresignedAuth_Put_GetObject_with_data) if !ts.conf.azureTests { ts.Run(PresignedAuth_Put_GetObject_with_UTF8_chars) @@ -2818,6 +2821,9 @@ func GetIntTests() IntTests { "PresignedAuth_incorrect_secret_key": PresignedAuth_incorrect_secret_key, "PresignedAuth_sigv2_not_supported": PresignedAuth_sigv2_not_supported, "PresignedAuth_PutObject_success": PresignedAuth_PutObject_success, + "PresignedAuth_PutObject_strips_aws_chunked_content_encoding": PresignedAuth_PutObject_strips_aws_chunked_content_encoding, + "PresignedAuth_CopyObject_strips_aws_chunked_content_encoding": PresignedAuth_CopyObject_strips_aws_chunked_content_encoding, + "PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding": PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding, "PutObject_missing_object_lock_retention_config": PutObject_missing_object_lock_retention_config, "PutObject_name_too_long": PutObject_name_too_long, "PutObject_with_object_lock": PutObject_with_object_lock, diff --git a/tests/integration/presigned_urls.go b/tests/integration/presigned_urls.go index ae59a005..c4c29003 100644 --- a/tests/integration/presigned_urls.go +++ b/tests/integration/presigned_urls.go @@ -16,6 +16,7 @@ package integration import ( "context" + "encoding/xml" "fmt" "io" "net/http" @@ -25,7 +26,9 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/versity/versitygw/s3err" + "github.com/versity/versitygw/s3response" ) func PresignedAuth_security_token_with_permanent_credentials(s *S3Conf) error { @@ -966,3 +969,224 @@ func PresignedAuth_UploadPart(s *S3Conf) error { return nil }) } + +// PresignedAuth_PutObject_strips_aws_chunked_content_encoding checks that +// aws-chunked is dropped from the stored Content-Encoding on a presigned PUT. +func PresignedAuth_PutObject_strips_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PresignedAuth_PutObject_strips_aws_chunked_content_encoding" + return presignedAuthHandler(s, testName, func(client *s3.PresignClient, bucket string) error { + s3client := s.GetClient() + for i, test := range []struct { + contentEncoding string + stored string + }{ + // nothing is left, so no Content-Encoding is stored at all + {"aws-chunked", ""}, + {"aws-chunked,gzip", "gzip"}, + // the remaining codings keep their order + {"gzip,aws-chunked,br", "gzip,br"}, + {"AWS-Chunked", ""}, + // the token has to match in full + {"aws-chunked-custom", "aws-chunked-custom"}, + {"gzip", "gzip"}, + } { + object := fmt.Sprintf("presigned-obj-%v", i) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + v4req, err := client.PresignPutObject(ctx, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &object, + }) + cancel() + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + req, err := http.NewRequest(v4req.Method, v4req.URL, strings.NewReader("hello world")) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + req.Header.Set("Content-Encoding", test.contentEncoding) + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("test %v: expected the response status code to be %v, instead got %v", + i+1, http.StatusOK, resp.StatusCode) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != test.stored { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, test.stored, stored) + } + } + + return nil + }) +} + +// PresignedAuth_CopyObject_strips_aws_chunked_content_encoding checks that +// aws-chunked is dropped from the stored Content-Encoding on a presigned copy. +func PresignedAuth_CopyObject_strips_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PresignedAuth_CopyObject_strips_aws_chunked_content_encoding" + return presignedAuthHandler(s, testName, func(_ *s3.PresignClient, bucket string) error { + s3client := s.GetClient() + srcObj := "copy-source" + + // the source carries a coding of its own, so a stored value can only + // have come from the copy request + _, err := putObjectWithData(int64(len("hello world")), &s3.PutObjectInput{ + Bucket: &bucket, + Key: &srcObj, + ContentEncoding: getPtr("br"), + }, s3client) + if err != nil { + return err + } + + for i, test := range []struct { + contentEncoding string + stored string + }{ + {"aws-chunked", ""}, + {"aws-chunked,gzip", "gzip"}, + {"gzip,aws-chunked,br", "gzip,br"}, + {"AWS-Chunked", ""}, + // the token has to match in full + {"aws-chunked-custom", "aws-chunked-custom"}, + {"gzip", "gzip"}, + } { + object := fmt.Sprintf("copy-dst-%v", i) + + req, err := createPresignedReq(http.MethodPut, s.endpoint, + fmt.Sprintf("%s/%s", bucket, object), s.awsID, s.awsSecret, s.awsRegion, time.Now(), + map[string]string{ + "X-Amz-Copy-Source": fmt.Sprintf("/%s/%s", bucket, srcObj), + "X-Amz-Metadata-Directive": string(types.MetadataDirectiveReplace), + "Content-Encoding": test.contentEncoding, + }) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("test %v: expected the response status code to be %v, instead got %v", + i+1, http.StatusOK, resp.StatusCode) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != test.stored { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, test.stored, stored) + } + } + + return nil + }) +} + +// PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding checks +// that aws-chunked is dropped from the Content-Encoding a presigned +// CreateMultipartUpload stores for the completed object. +func PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding" + return presignedAuthHandler(s, testName, func(_ *s3.PresignClient, bucket string) error { + s3client := s.GetClient() + for i, test := range []struct { + contentEncoding string + stored string + }{ + {"aws-chunked", ""}, + {"aws-chunked,gzip", "gzip"}, + {"gzip,aws-chunked,br", "gzip,br"}, + {"AWS-Chunked", ""}, + // the token has to match in full + {"aws-chunked-custom", "aws-chunked-custom"}, + {"gzip", "gzip"}, + } { + object := fmt.Sprintf("mp-obj-%v", i) + + req, err := createPresignedReq(http.MethodPost, s.endpoint, + fmt.Sprintf("%s/%s?uploads=", bucket, object), + s.awsID, s.awsSecret, s.awsRegion, time.Now(), + map[string]string{"Content-Encoding": test.contentEncoding}) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + body, err := io.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return fmt.Errorf("test %v failed to read the response body: %w", i+1, err) + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("test %v: expected the response status code to be %v, instead got %v", + i+1, http.StatusOK, resp.StatusCode) + } + + var out s3response.InitiateMultipartUploadResult + if err := xml.Unmarshal(body, &out); err != nil { + return fmt.Errorf("test %v failed to parse the response body: %w", i+1, err) + } + + // the parts carry no Content-Encoding: the completed object can + // only inherit what the create named + parts, _, err := uploadParts(s3client, 5*1024*1024, 1, bucket, object, out.UploadId) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + compParts := []types.CompletedPart{} + for _, el := range parts { + compParts = append(compParts, types.CompletedPart{ + ETag: el.ETag, + PartNumber: el.PartNumber, + }) + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{ + Bucket: &bucket, + Key: &object, + UploadId: &out.UploadId, + MultipartUpload: &types.CompletedMultipartUpload{Parts: compParts}, + }) + cancel() + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != test.stored { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, test.stored, stored) + } + } + + return nil + }) +} diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 2b4e995f..e2a9650c 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -412,6 +412,40 @@ func createSignedReq(method, endpoint, path, access, secret, service, region, ov return req, nil } +// createPresignedReq presigns an S3 request for an operation the SDK's +// PresignClient doesn't cover. Every header passed is signed, and the returned +// request carries back exactly what the signer decided to sign. +func createPresignedReq(method, endpoint, path, access, secret, region string, date time.Time, headers map[string]string) (*http.Request, error) { + req, err := http.NewRequest(method, fmt.Sprintf("%v/%v", endpoint, path), nil) + if err != nil { + return nil, fmt.Errorf("failed to create the request: %w", err) + } + // the signer doesn't add the expiration - the SDK's PresignClient puts it in + // the query before signing - and S3 rejects a presigned URL without it + query := req.URL.Query() + query.Set("X-Amz-Expires", "900") + req.URL.RawQuery = query.Encode() + + for key, val := range headers { + req.Header.Set(key, val) + } + + uri, signedHeaders, err := v4.NewSigner().PresignHTTP(req.Context(), + aws.Credentials{AccessKeyID: access, SecretAccessKey: secret}, + req, "UNSIGNED-PAYLOAD", "s3", region, date) + if err != nil { + return nil, fmt.Errorf("failed to presign the request: %w", err) + } + + presigned, err := http.NewRequest(method, uri, nil) + if err != nil { + return nil, fmt.Errorf("failed to create the presigned request: %w", err) + } + presigned.Header = signedHeaders + + return presigned, nil +} + type APIErrorResponse struct { XMLName xml.Name `xml:"Error"` Code string