Repository: boto/botocore
Pull request #2859 (2023-01-30 19:18:29)

Append `aws-chunked` to `Content-Encoding` header if present
Primarily in various S3 operations, users may provide a `ContentEncoding` parameter. This tells the service how the content that is being sent/requested is encoded. An example of this would be `gzip`. However, for streaming AWS operations (i.e. ones that have trailers) where a `ChecksumAlgorithm` parameter is provided, the SDK is currently overriding any user input to `aws-chunked`. Instead this special content encoding should be appended to the user input.

codecov-commenter: # [Codecov](https://codecov.io/gh/boto/botocore/pull/2859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) Report
Base: **93.54**% // Head: **93.54**% // Increases project coverage by **`+0.00%`** :tada:
> Coverage data is based on head [(`979a329`)](https://codecov.io/gh/boto/botocore/pull/2859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) compared to base [(`db243c8`)](https://codecov.io/gh/boto/botocore/commit/db243c8969796461996a1ffe2da725148bcc42f2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).
> Patch coverage: 100.00% of modified lines in pull request are covered.

> :exclamation: Current head 979a329 differs from pull request most recent head c7fa316. Consider uploading reports for the commit c7fa316 to get more accurate results

:mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto)

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff            @@
##           develop    #2859   +/-   ##
========================================
  Coverage    93.54%   93.54%           
========================================
  Files           63       63           
  Lines        13398    13400    +2     
========================================
+ Hits         12533    12535    +2     
  Misses         865      865           
```


| [Impacted Files](https://codecov.io/gh/boto/botocore/pull/2859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) | Coverage Δ | |
|---|---|---|
| [botocore/httpchecksum.py](https://codecov.io/gh/boto/botocore/pull/2859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto#diff-Ym90b2NvcmUvaHR0cGNoZWNrc3VtLnB5) | `94.16% <100.00%> (+0.04%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/boto/botocore/pull/2859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).

dlm6693: @nateprewitt I'll wait until #2860 is merged, then I'll update the functional test to rely on the defaults in the client constructor.

Patch:
diff --git a/.changes/next-release/bugfix-s3-10151.json b/.changes/next-release/bugfix-s3-10151.json
new file mode 100644
index 000000000..daac1dcc6
--- /dev/null
+++ b/.changes/next-release/bugfix-s3-10151.json
@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "``s3``",
+  "description": "boto3 no longer overwrites user supplied `Content-Encoding` with `aws-chunked` when user also supplies `ChecksumAlgorithm`."
+}
diff --git a/botocore/httpchecksum.py b/botocore/httpchecksum.py
index 7a6c4ac2a..db78057e5 100644
--- a/botocore/httpchecksum.py
+++ b/botocore/httpchecksum.py
@@ -331,7 +331,12 @@ def _apply_request_trailer_checksum(request):
         return
 
     headers["Transfer-Encoding"] = "chunked"
-    headers["Content-Encoding"] = "aws-chunked"
+    if "Content-Encoding" in headers:
+        # We need to preserve the existing content encoding and add
+        # aws-chunked as a new content encoding.
+        headers["Content-Encoding"] += ",aws-chunked"
+    else:
+        headers["Content-Encoding"] = "aws-chunked"
     headers["X-Amz-Trailer"] = location_name
 
     content_length = determine_content_length(body)


Test patch:
diff --git a/tests/functional/test_s3.py b/tests/functional/test_s3.py
index 3b245fbfa..3e4e60ab7 100644
--- a/tests/functional/test_s3.py
+++ b/tests/functional/test_s3.py
@@ -2141,6 +2141,27 @@ def test_checksums_included_in_expected_operations(
         assert "Content-MD5" in stub.requests[-1].headers
 
 
+@pytest.mark.parametrize(
+    "content_encoding, expected_header",
+    [("foo", b"foo,aws-chunked"), (None, b"aws-chunked")],
+)
+def test_checksum_content_encoding(content_encoding, expected_header):
+    op_kwargs = {
+        "Bucket": "mybucket",
+        "Key": "mykey",
+        "Body": b"foo",
+        "ChecksumAlgorithm": "sha256",
+    }
+    if content_encoding is not None:
+        op_kwargs["ContentEncoding"] = content_encoding
+    s3 = _create_s3_client()
+    with ClientHTTPStubber(s3) as http_stubber:
+        http_stubber.add_response()
+        s3.put_object(**op_kwargs)
+        request_headers = http_stubber.requests[-1].headers
+        assert request_headers["Content-Encoding"] == expected_header
+
+
 def _s3_addressing_test_cases():
     # The default behavior for sigv2. DNS compatible buckets
     yield dict(
diff --git a/tests/unit/test_httpchecksum.py b/tests/unit/test_httpchecksum.py
index e421546ef..f5550e354 100644
--- a/tests/unit/test_httpchecksum.py
+++ b/tests/unit/test_httpchecksum.py
@@ -301,6 +301,34 @@ class TestHttpChecksumHandlers(unittest.TestCase):
         # The body should not have been wrapped
         self.assertIsInstance(request["body"], bytes)
 
+    def test_apply_request_checksum_content_encoding_preset(self):
+        request = self._build_request(b"")
+        request["context"]["checksum"] = {
+            "request_algorithm": {
+                "in": "trailer",
+                "algorithm": "crc32",
+                "name": "x-amz-checksum-crc32",
+            }
+        }
+        request["headers"]["Content-Encoding"] = "foo"
+        apply_request_checksum(request)
+        # The content encoding should only have been appended
+        self.assertEqual(
+            request["headers"]["Content-Encoding"], "foo,aws-chunked"
+        )
+
+    def test_apply_request_checksum_content_encoding_default(self):
+        request = self._build_request(b"")
+        request["context"]["checksum"] = {
+            "request_algorithm": {
+                "in": "trailer",
+                "algorithm": "crc32",
+                "name": "x-amz-checksum-crc32",
+            }
+        }
+        apply_request_checksum(request)
+        self.assertEqual(request["headers"]["Content-Encoding"], "aws-chunked")
+
     def test_response_checksum_algorithm_no_model(self):
         request = self._build_request(b"")
         operation_model = self._make_operation_model()