From 888fd5c1ad03c794ac231d5fb12e194d9feb7045 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Mon, 21 Sep 2026 17:30:35 +0400 Subject: [PATCH] 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 +}