From bbe398af72847a167dc2f11661899c2d9d064eae Mon Sep 17 00:00:00 2001 From: niksis02 Date: Tue, 22 Sep 2026 02:02:45 +0400 Subject: [PATCH] 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