Fix for Issue #3569 where the user set Content-Encoding was over writ…

…ten to aws-chunked with Checksum algorithm enabled (#3720)
aws · Jan 26, 2023 · 303a375 · 303a375
1 parent ee51283
commit 303a375
Show file tree

Hide file tree

Showing 13 changed files with 115 additions and 14 deletions.
diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-16b5911.json b/.changes/next-release/bugfix-AWSSDKforJavav2-16b5911.json
@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "Append Content-encoding header instead of over writing the header when Checksum algorithm is selected along with user set Content-encoding"
+}
diff --git a/...rc/main/java/software/amazon/awssdk/authcrt/signer/internal/DefaultAwsCrtS3V4aSigner.java b/...rc/main/java/software/amazon/awssdk/authcrt/signer/internal/DefaultAwsCrtS3V4aSigner.java
@@ -224,6 +224,6 @@ private static long getChecksumTrailerLength(SignerChecksumParams signerParams,
     private static void updateRequestWithTrailer(SignerChecksumParams signerChecksumParams,
                                                  SdkHttpFullRequest.Builder mutableRequest) {
         mutableRequest.putHeader("x-amz-trailer", signerChecksumParams.checksumHeaderName());
-        mutableRequest.putHeader("Content-Encoding", "aws-chunked");
+        mutableRequest.appendHeader("Content-Encoding", "aws-chunked");
     }
 }
diff --git a/...auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java b/...auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java
@@ -236,7 +236,7 @@ protected String calculateContentHash(SdkHttpFullRequest.Builder mutableRequest,
                             ChecksumSpecs.builder().headerName(signerParams.checksumParams().checksumHeaderName()).build())) {
                         isTrailingChecksum = true;
                         mutableRequest.putHeader("x-amz-trailer", headerForTrailerChecksumLocation);
-                        mutableRequest.putHeader("Content-Encoding", "aws-chunked");
+                        mutableRequest.appendHeader("Content-Encoding", "aws-chunked");
                     }
                 }
                 // Make sure "Content-Length" header is not empty so that HttpClient

diff --git a/...azon/awssdk/core/internal/interceptor/AsyncRequestBodyHttpChecksumTrailerInterceptor.java b/...azon/awssdk/core/internal/interceptor/AsyncRequestBodyHttpChecksumTrailerInterceptor.java
@@ -105,7 +105,7 @@ private static SdkHttpRequest updateHeadersForTrailerChecksum(Context.ModifyHttp
 
         return context.httpRequest().copy(r ->
                 r.putHeader(HttpChecksumConstant.HEADER_FOR_TRAILER_REFERENCE, checksum.headerName())
-                        .putHeader("Content-encoding", HttpChecksumConstant.AWS_CHUNKED_HEADER)
+                        .appendHeader("Content-encoding", HttpChecksumConstant.AWS_CHUNKED_HEADER)
                         .putHeader("x-amz-content-sha256", HttpChecksumConstant.CONTENT_SHA_256_FOR_UNSIGNED_TRAILER)
                         .putHeader("x-amz-decoded-content-length", Long.toString(originalContentLength))
                         .putHeader(Header.CONTENT_LENGTH,

diff --git a/...oftware/amazon/awssdk/core/internal/interceptor/SyncHttpChecksumInTrailerInterceptor.java b/...oftware/amazon/awssdk/core/internal/interceptor/SyncHttpChecksumInTrailerInterceptor.java
@@ -108,7 +108,7 @@ public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, Execu
         long originalContentLength = context.requestBody().get().optionalContentLength().orElse(0L);
         return context.httpRequest().copy(
             r -> r.putHeader(HttpChecksumConstant.HEADER_FOR_TRAILER_REFERENCE, checksum.headerName())
-                  .putHeader("Content-encoding", AWS_CHUNKED_HEADER)
+                  .appendHeader("Content-encoding", AWS_CHUNKED_HEADER)
                   .putHeader("x-amz-content-sha256", CONTENT_SHA_256_FOR_UNSIGNED_TRAILER)
                   .putHeader("x-amz-decoded-content-length", Long.toString(originalContentLength))
                   .putHeader(CONTENT_LENGTH,

diff --git a/...it/java/software/amazon/awssdk/services/s3/checksum/AsyncHttpChecksumIntegrationTest.java b/...it/java/software/amazon/awssdk/services/s3/checksum/AsyncHttpChecksumIntegrationTest.java
@@ -49,6 +49,7 @@
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import software.amazon.awssdk.services.s3.model.PutObjectResponse;
 import software.amazon.awssdk.services.s3.utils.CaptureChecksumValidationInterceptor;
+import software.amazon.awssdk.services.s3.utils.ChecksumUtils;
 import software.amazon.awssdk.testutils.RandomTempFile;
 
 public class AsyncHttpChecksumIntegrationTest extends S3IntegrationTestBase {
@@ -115,6 +116,7 @@ void asyncHttpsValidUnsignedTrailerChecksumCalculatedBySdkClient_withSmallReques
                                                             .key(KEY).checksumMode(ChecksumMode.ENABLED)
                                                             .build(), AsyncResponseTransformer.toBytes()).join().asUtf8String();
         assertThat(interceptor.validationAlgorithm()).isEqualTo(Algorithm.CRC32);
+        assertThat(interceptor.contentEncoding()).isEqualTo("aws-chunked");
         assertThat(interceptor.responseValidation()).isEqualTo(ChecksumValidation.VALIDATED);
         assertThat(response).isEqualTo("Hello world");
     }
@@ -126,10 +128,12 @@ void asyncHttpsValidUnsignedTrailerChecksumCalculatedBySdkClient_withHugeRequest
         s3Async.putObject(PutObjectRequest.builder()
                                           .bucket(BUCKET)
                                           .key(KEY)
+                                          .contentEncoding("gzip")
                                           .checksumAlgorithm(ChecksumAlgorithm.CRC32)
                                           .build(), AsyncRequestBody.fromString(createDataOfSize(64 * KB, 'a'))).join();
         assertThat(interceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
         assertThat(interceptor.requestChecksumInHeader()).isNull();
+        assertThat(interceptor.contentEncoding()).isEqualTo("gzip,aws-chunked");
 
         String response = s3Async.getObject(GetObjectRequest.builder().bucket(BUCKET)
                                                             .key(KEY).checksumMode(ChecksumMode.ENABLED)

diff --git a/.../src/it/java/software/amazon/awssdk/services/s3/checksum/HttpChecksumIntegrationTest.java b/.../src/it/java/software/amazon/awssdk/services/s3/checksum/HttpChecksumIntegrationTest.java
@@ -34,6 +34,7 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import software.amazon.awssdk.authcrt.signer.internal.DefaultAwsCrtS3V4aSigner;
+import software.amazon.awssdk.core.HttpChecksumConstant;
 import software.amazon.awssdk.core.ResponseInputStream;
 import software.amazon.awssdk.core.checksums.Algorithm;
 import software.amazon.awssdk.core.checksums.ChecksumValidation;
@@ -45,13 +46,12 @@
 import software.amazon.awssdk.services.s3.model.ChecksumMode;
 import software.amazon.awssdk.services.s3.model.GetObjectRequest;
 import software.amazon.awssdk.services.s3.model.GetObjectResponse;
-import software.amazon.awssdk.services.s3.model.NoSuchBucketException;
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import software.amazon.awssdk.services.s3.model.PutObjectResponse;
 import software.amazon.awssdk.services.s3.model.S3Exception;
 import software.amazon.awssdk.services.s3.utils.CaptureChecksumValidationInterceptor;
+import software.amazon.awssdk.services.s3.utils.ChecksumUtils;
 import software.amazon.awssdk.testutils.RandomTempFile;
-import software.amazon.awssdk.testutils.Waiter;
 
 public class HttpChecksumIntegrationTest extends S3IntegrationTestBase {
 
@@ -102,6 +102,7 @@ public void validHeaderChecksumCalculatedBySdkClient() {
                                                                                 .key(KEY)
                                                                                 .build(), RequestBody.fromString("Hello world"));
         assertThat(interceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
+        assertThat(interceptor.contentEncoding()).isEqualTo(HttpChecksumConstant.AWS_CHUNKED_HEADER);
         assertThat(interceptor.requestChecksumInHeader()).isNull();
         assertThat(putObjectResponse.sdkHttpResponse().firstMatchingHeader("x-amz-checksum-crc32"))
             .hasValue("i9aeUg==");
@@ -112,11 +113,13 @@ public void validHeaderChecksumSentDirectlyInTheField() {
         PutObjectResponse putObjectResponse = s3Https.putObject(PutObjectRequest.builder()
                                                                                 .bucket(BUCKET)
                                                                                 .checksumAlgorithm(ChecksumAlgorithm.CRC32)
+                                                                                .contentEncoding("gzip")
                                                                                 .checksumCRC32("i9aeUg==")
                                                                                 .key(KEY)
                                                                                 .build(), RequestBody.fromString("Hello world"));
         assertThat(interceptor.requestChecksumInHeader()).isEqualTo("i9aeUg==");
         assertThat(interceptor.requestChecksumInTrailer()).isNull();
+        assertThat(interceptor.contentEncoding()).isEqualTo("gzip");
         assertThat(putObjectResponse.sdkHttpResponse().firstMatchingHeader("x-amz-checksum-crc32")).hasValue("i9aeUg==");
     }
 
@@ -191,6 +194,7 @@ public void syncValidSignedTrailerChecksumCalculatedBySdkClient() {
             new InputStreamReader(s3HttpsObject, StandardCharsets.UTF_8))
             .lines()
             .collect(Collectors.joining("\n"));
+        assertThat(interceptor.contentEncoding()).isEmpty();
         assertThat(interceptor.validationAlgorithm()).isEqualTo(Algorithm.CRC32);
         assertThat(interceptor.responseValidation()).isEqualTo(ChecksumValidation.VALIDATED);
         assertThat(text).isEqualTo("Hello world");
@@ -204,10 +208,12 @@ public void syncValidSignedTrailerChecksumCalculatedBySdkClient_Empty_String() {
         s3.putObject(PutObjectRequest.builder()
                                      .bucket(BUCKET)
                                      .key(KEY)
+                                     .contentEncoding("gzip")
                                      .checksumAlgorithm(ChecksumAlgorithm.CRC32)
                                      .build(), RequestBody.fromString(""));
 
         assertThat(interceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
+        assertThat(interceptor.contentEncoding()).isEqualTo("gzip,aws-chunked");
         assertThat(interceptor.requestChecksumInHeader()).isNull();
 
         ResponseInputStream<GetObjectResponse> s3HttpsObject =
@@ -247,16 +253,18 @@ public void syncValidSignedTrailerChecksumCalculatedBySdkClientWithSigv4a() {
     }
 
     @Test
-    public void syncValidUnsignedTrailerChecksumCalculatedBySdkClientWithSigv4a() {
+    public void syncValidSignedTrailerChecksumCalculatedBySdkClientWithSigv4a_withContentEncoding() {
 
         s3.putObject(PutObjectRequest.builder()
                                      .bucket(BUCKET)
                                      .key(KEY)
                                      .checksumAlgorithm(ChecksumAlgorithm.CRC32)
+                                     .contentEncoding("gzip")
                                      .overrideConfiguration(o -> o.signer(DefaultAwsCrtS3V4aSigner.create()))
                                      .build(), RequestBody.fromString("Hello world"));
 
         assertThat(interceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
+        assertThat(interceptor.contentEncoding()).isEqualTo("gzip,aws-chunked");
         assertThat(interceptor.requestChecksumInHeader()).isNull();
 
         ResponseInputStream<GetObjectResponse> s3HttpsObject =
@@ -276,9 +284,11 @@ public void syncUnsignedPayloadForHugeMessage() throws InterruptedException {
                                           .bucket(BUCKET)
                                           .key(KEY)
                                           .checksumAlgorithm(ChecksumAlgorithm.CRC32)
+                                          .contentEncoding("gzip")
                                           .build(), RequestBody.fromString(createDataOfSize(HUGE_MSG_SIZE, 'a')));
 
         assertThat(interceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
+        assertThat(interceptor.contentEncoding()).isEqualTo("gzip,aws-chunked");
         assertThat(interceptor.requestChecksumInHeader()).isNull();
 
         Thread.sleep(1000);

diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/S3SignerTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/S3SignerTest.java
@@ -102,6 +102,28 @@ public void payloadSigningWithChecksum() {
                                         .withRequestBody(containing("0;")));
     }
 
+    @Test
+    public void payloadSigningWithChecksumWithContentEncodingSuppliedByUser() {
+
+        S3Client s3Client = getS3Client(true, true, URI.create(getEndpoint()));
+        stubFor(any(urlMatching(".*"))
+                    .willReturn(response()));
+        s3Client.putObject(PutObjectRequest.builder()
+                                           .checksumAlgorithm(ChecksumAlgorithm.CRC32).contentEncoding("deflate")
+                                           .bucket("test").key("test").build(), RequestBody.fromBytes("abc".getBytes()));
+        verify(putRequestedFor(anyUrl()).withHeader(CONTENT_TYPE, equalTo(Mimetype.MIMETYPE_OCTET_STREAM)));
+        verify(putRequestedFor(anyUrl()).withHeader(CONTENT_LENGTH, equalTo("296")));
+        verify(putRequestedFor(anyUrl()).withHeader("x-amz-trailer", equalTo(CRC32_TRAILER.headerName())));
+        verify(putRequestedFor(anyUrl()).withHeader("x-amz-decoded-content-length", equalTo("3")));
+        verify(putRequestedFor(anyUrl()).withHeader("x-amz-content-sha256", equalTo("STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER"
+        )));
+        verify(putRequestedFor(anyUrl()).withHeader("Content-Encoding", equalTo("aws-chunked")));
+        verify(putRequestedFor(anyUrl()).withHeader("Content-Encoding", equalTo("deflate")));
+        verify(putRequestedFor(anyUrl()).withRequestBody(containing("x-amz-checksum-crc32:NSRBwg=="))
+                                        .withRequestBody(containing("x-amz-trailer-signature:"))
+                                        .withRequestBody(containing("0;")));
+    }
+
     @Test
     public void payloadSigningWithNoChecksum() {
 

diff --git a/...t/java/software/amazon/awssdk/services/s3/utils/CaptureChecksumValidationInterceptor.java b/...t/java/software/amazon/awssdk/services/s3/utils/CaptureChecksumValidationInterceptor.java
@@ -27,6 +27,11 @@ public class CaptureChecksumValidationInterceptor implements ExecutionIntercepto
     private  ChecksumValidation responseValidation;
     private  String requestChecksumInTrailer;
     private  String requestChecksumInHeader;
+    private  String contentEncoding;
+
+    public String contentEncoding() {
+        return contentEncoding;
+    }
 
     public Algorithm validationAlgorithm() {
         return validationAlgorithm;
@@ -64,6 +69,7 @@ public void afterExecution(Context.AfterExecution context, ExecutionAttributes e
             executionAttributes.getOptionalAttribute(SdkExecutionAttribute.HTTP_CHECKSUM_VALIDATION_ALGORITHM).orElse(null);
         responseValidation =
             executionAttributes.getOptionalAttribute(SdkExecutionAttribute.HTTP_RESPONSE_CHECKSUM_VALIDATION).orElse(null);
+        contentEncoding = String.join(",", context.httpRequest().matchingHeaders("content-encoding"));
     }
 
     @Override

diff --git a/...d-classes-test/src/main/resources/codegen-resources/customresponsemetadata/service-2.json b/...d-classes-test/src/main/resources/codegen-resources/customresponsemetadata/service-2.json
@@ -907,6 +907,11 @@
           "location":"header",
           "locationName":"x-amz-checksum-mode"
         },
+        "ContentEncoding":{
+          "shape":"String",
+          "location":"header",
+          "locationName":"Content-Encoding"
+        },
         "ChecksumAlgorithm":{
           "shape":"ChecksumAlgorithm",
           "location":"header",
@@ -925,6 +930,11 @@
           "location":"header",
           "locationName":"x-amz-checksum-mode"
         },
+        "ContentEncoding":{
+          "shape":"String",
+          "location":"header",
+          "locationName":"Content-Encoding"
+        },
         "ChecksumAlgorithm":{
           "shape":"ChecksumAlgorithm",
           "location":"header",

diff --git a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/xml/service-2.json b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/xml/service-2.json
@@ -765,6 +765,11 @@
           "documentation":"<p>Object data.</p>",
           "streaming":true
         },
+        "ContentEncoding":{
+          "shape":"String",
+          "location":"header",
+          "locationName":"Content-Encoding"
+        },
         "ChecksumMode":{
           "shape":"ChecksumMode",
           "location":"header",
@@ -788,6 +793,11 @@
           "location":"header",
           "locationName":"x-amz-checksum-mode"
         },
+        "ContentEncoding":{
+          "shape":"String",
+          "location":"header",
+          "locationName":"Content-Encoding"
+        },
         "ChecksumAlgorithm":{
           "shape":"ChecksumAlgorithm",
           "location":"header",

diff --git a/...t/java/software/amazon/awssdk/services/AsyncRequestBodyFlexibleChecksumInTrailerTest.java b/...t/java/software/amazon/awssdk/services/AsyncRequestBodyFlexibleChecksumInTrailerTest.java
@@ -106,6 +106,25 @@ public void asyncStreaming_NoSigner_shouldContainChecksum_fromInterceptors() {
                                              AsyncResponseTransformer.toBytes()).join();
         //payload would in json form as  "{"StringMember":"foo"}x-amz-checksum-crc32:tcUDMQ==[\r][\n]"
         verifyHeadersForPutRequest("44", "3", "x-amz-checksum-crc32");
+        verify(putRequestedFor(anyUrl()).withHeader("Content-Encoding", equalTo("aws-chunked")));
+
+        verify(putRequestedFor(anyUrl()).withRequestBody(
+            containing(
+                "3" + CRLF + "abc" + CRLF
+                + "0" + CRLF
+                + "x-amz-checksum-crc32:NSRBwg==" + CRLF + CRLF)));
+    }
+    @Test
+    public void asyncStreaming_NoSigner_shouldContainChecksum_fromInterceptors_withContentEncoding() {
+        stubResponseWithHeaders();
+        asyncClient.putOperationWithChecksum(b -> b.checksumAlgorithm(ChecksumAlgorithm.CRC32).contentEncoding("deflate"),
+                                             AsyncRequestBody.fromString(
+                                                 "abc"),
+                                             AsyncResponseTransformer.toBytes()).join();
+        //payload would in json form as  "{"StringMember":"foo"}x-amz-checksum-crc32:tcUDMQ==[\r][\n]"
+        verifyHeadersForPutRequest("44", "3", "x-amz-checksum-crc32");
+        verify(putRequestedFor(anyUrl()).withHeader("Content-Encoding", equalTo("aws-chunked")));
+        verify(putRequestedFor(anyUrl()).withHeader("Content-Encoding", equalTo("deflate")));
 
         verify(putRequestedFor(anyUrl()).withRequestBody(
             containing(
@@ -114,6 +133,7 @@ public void asyncStreaming_NoSigner_shouldContainChecksum_fromInterceptors() {
                 + "x-amz-checksum-crc32:NSRBwg==" + CRLF + CRLF)));
     }
 
+
     @Test
     public void asyncStreaming_withRetry_NoSigner_shouldContainChecksum_fromInterceptors() {
         stubForFailureThenSuccess(500, "500");

diff --git a/...ses-test/src/test/java/software/amazon/awssdk/services/SyncHttpChecksumInTrailerTest.java b/...ses-test/src/test/java/software/amazon/awssdk/services/SyncHttpChecksumInTrailerTest.java
@@ -34,21 +34,14 @@
 import com.github.tomakehurst.wiremock.stubbing.Scenario;
 import com.github.tomakehurst.wiremock.verification.LoggedRequest;
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.net.URI;
-import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
-import java.util.Base64;
 import java.util.List;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider;
-import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
-import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
 import software.amazon.awssdk.core.HttpChecksumConstant;
-import software.amazon.awssdk.core.checksums.Algorithm;
-import software.amazon.awssdk.core.checksums.SdkChecksum;
 import software.amazon.awssdk.core.sync.RequestBody;
 import software.amazon.awssdk.core.sync.ResponseTransformer;
 import software.amazon.awssdk.http.ContentStreamProvider;
@@ -94,6 +87,26 @@ public void sync_streaming_NoSigner_appends_trailer_checksum() {
                 + "x-amz-checksum-crc32:i9aeUg==" + CRLF + CRLF)));
     }
 
+    @Test
+    public void sync_streaming_NoSigner_appends_trailer_checksum_withContentEncodingSetByUser() {
+        stubResponseWithHeaders();
+
+
+        client.putOperationWithChecksum(r ->
+            r.checksumAlgorithm(ChecksumAlgorithm.CRC32)
+                                            .contentEncoding("deflate"),
+                                        RequestBody.fromString("Hello world"),
+                                        ResponseTransformer.toBytes());
+        verify(putRequestedFor(anyUrl()).withHeader("Content-Encoding", equalTo("aws-chunked")));
+        verify(putRequestedFor(anyUrl()).withHeader("Content-Encoding", equalTo("deflate")));
+        //b is hex value of 11.
+        verify(putRequestedFor(anyUrl()).withRequestBody(
+            containing(
+                "b" + CRLF + "Hello world" + CRLF
+                + "0" + CRLF
+                + "x-amz-checksum-crc32:i9aeUg==" + CRLF + CRLF)));
+    }
+
     @Test
     public void sync_streaming_specifiedLengthIsLess_NoSigner_appends_trailer_checksum() {
         stubResponseWithHeaders();