Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-16b5911.json
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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");
}
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand Down Expand Up @@ -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==");
Expand All @@ -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==");
}

Expand Down Expand Up @@ -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");
Expand All @@ -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 =
Expand Down Expand Up @@ -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 =
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,11 @@
"location":"header",
"locationName":"x-amz-checksum-mode"
},
"ContentEncoding":{
"shape":"String",
"location":"header",
"locationName":"Content-Encoding"
},
"ChecksumAlgorithm":{
"shape":"ChecksumAlgorithm",
"location":"header",
Expand All @@ -925,6 +930,11 @@
"location":"header",
"locationName":"x-amz-checksum-mode"
},
"ContentEncoding":{
"shape":"String",
"location":"header",
"locationName":"Content-Encoding"
},
"ChecksumAlgorithm":{
"shape":"ChecksumAlgorithm",
"location":"header",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,11 @@
"documentation":"<p>Object data.</p>",
"streaming":true
},
"ContentEncoding":{
"shape":"String",
"location":"header",
"locationName":"Content-Encoding"
},
"ChecksumMode":{
"shape":"ChecksumMode",
"location":"header",
Expand All @@ -788,6 +793,11 @@
"location":"header",
"locationName":"x-amz-checksum-mode"
},
"ContentEncoding":{
"shape":"String",
"location":"header",
"locationName":"Content-Encoding"
},
"ChecksumAlgorithm":{
"shape":"ChecksumAlgorithm",
"location":"header",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down