Skip to content

Commit 3c9e027

Browse files
committed
Fix for Default MD5 check getting skipped even when http checksum algorithm is not set
1 parent 196077d commit 3c9e027

File tree

3 files changed

+214
-2
lines changed

3 files changed

+214
-2
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Bug fix for Default Md5 checksum was getting skipped even when new http checksum was not set to any checksum algorithm."
6+
}

services/s3/src/main/java/software/amazon/awssdk/services/s3/checksums/ChecksumsEnabledValidator.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@
2222
import static software.amazon.awssdk.services.s3.model.ServerSideEncryption.AWS_KMS;
2323

2424
import java.util.Arrays;
25+
import java.util.Optional;
2526
import software.amazon.awssdk.annotations.SdkInternalApi;
2627
import software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute;
2728
import software.amazon.awssdk.core.ClientType;
2829
import software.amazon.awssdk.core.SdkRequest;
30+
import software.amazon.awssdk.core.checksums.ChecksumSpecs;
2931
import software.amazon.awssdk.core.checksums.SdkChecksum;
3032
import software.amazon.awssdk.core.exception.RetryableException;
3133
import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
@@ -108,14 +110,24 @@ public static boolean shouldRecordChecksum(SdkRequest sdkRequest,
108110
return false;
109111
}
110112

111-
//Checksum validation will already be done as a part of HttpChecksum validations if RESOLVED_CHECKSUM_SPECS is present.
112-
if (executionAttributes.getOptionalAttribute(SdkExecutionAttribute.RESOLVED_CHECKSUM_SPECS).isPresent()) {
113+
//Checksum validation is done at Service side when HTTP Checksum algorithm attribute is set.
114+
if (isHttpCheckSumValidationEnabled(executionAttributes)) {
113115
return false;
114116
}
115117

116118
return checksumEnabledPerConfig(executionAttributes);
117119
}
118120

121+
private static boolean isHttpCheckSumValidationEnabled(ExecutionAttributes executionAttributes) {
122+
Optional<ChecksumSpecs> resolvedChecksum =
123+
executionAttributes.getOptionalAttribute(SdkExecutionAttribute.RESOLVED_CHECKSUM_SPECS);
124+
if (resolvedChecksum.isPresent()) {
125+
ChecksumSpecs checksumSpecs = resolvedChecksum.get();
126+
return checksumSpecs.algorithm() != null;
127+
}
128+
return false;
129+
}
130+
119131
public static boolean responseChecksumIsValid(SdkHttpResponse httpResponse) {
120132
return !hasServerSideEncryptionHeader(httpResponse);
121133
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.services.s3.functionaltests;
17+
18+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
19+
import static com.github.tomakehurst.wiremock.client.WireMock.any;
20+
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
21+
import static com.github.tomakehurst.wiremock.client.WireMock.containing;
22+
import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
23+
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
24+
import static com.github.tomakehurst.wiremock.client.WireMock.verify;
25+
import static org.assertj.core.api.Assertions.assertThat;
26+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
27+
28+
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
29+
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
30+
import java.io.BufferedReader;
31+
import java.io.InputStream;
32+
import java.io.InputStreamReader;
33+
import java.net.URI;
34+
import java.nio.charset.StandardCharsets;
35+
import java.util.function.Function;
36+
import java.util.stream.Collectors;
37+
import org.junit.jupiter.api.AfterEach;
38+
import org.junit.jupiter.api.Test;
39+
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
40+
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
41+
import software.amazon.awssdk.core.async.AsyncRequestBody;
42+
import software.amazon.awssdk.core.exception.RetryableException;
43+
import software.amazon.awssdk.core.sync.RequestBody;
44+
import software.amazon.awssdk.regions.Region;
45+
import software.amazon.awssdk.services.s3.S3AsyncClient;
46+
import software.amazon.awssdk.services.s3.S3AsyncClientBuilder;
47+
import software.amazon.awssdk.services.s3.S3Client;
48+
import software.amazon.awssdk.services.s3.S3ClientBuilder;
49+
import software.amazon.awssdk.services.s3.model.ChecksumAlgorithm;
50+
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
51+
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
52+
import software.amazon.awssdk.services.s3.utils.CaptureChecksumValidationInterceptor;
53+
54+
@WireMockTest
55+
public class PutObjectWithChecksumTest {
56+
57+
String CONTENT = "Hello, World!";
58+
String INCORRECT_ETAG = "65A8E27D8879283831B664BD8B7F0AD5";
59+
String ETAG = "65A8E27D8879283831B664BD8B7F0AD4";
60+
public static final Function<InputStream, String> stringFromStream = inputStream ->
61+
new BufferedReader(
62+
new InputStreamReader(inputStream, StandardCharsets.UTF_8)).lines().collect(Collectors.joining("\n"));
63+
private static final String BUCKET = "Example-Bucket";
64+
private static final String EXAMPLE_RESPONSE_BODY = "Hello world";
65+
private final CaptureChecksumValidationInterceptor captureChecksumValidationInterceptor =
66+
new CaptureChecksumValidationInterceptor();
67+
68+
private S3ClientBuilder getSyncClientBuilder(WireMockRuntimeInfo wm) {
69+
return S3Client.builder()
70+
.region(Region.US_EAST_1)
71+
.endpointOverride(URI.create(wm.getHttpBaseUrl()))
72+
.credentialsProvider(
73+
StaticCredentialsProvider.create(AwsBasicCredentials.create("key", "secret")));
74+
}
75+
76+
@AfterEach
77+
public void reset() {
78+
captureChecksumValidationInterceptor.reset();
79+
}
80+
81+
private S3AsyncClientBuilder getAsyncClientBuilder(WireMockRuntimeInfo wm) {
82+
return S3AsyncClient.builder()
83+
.region(Region.US_EAST_1)
84+
.endpointOverride(URI.create(wm.getHttpBaseUrl()))
85+
.credentialsProvider(
86+
StaticCredentialsProvider.create(AwsBasicCredentials.create("key", "secret")));
87+
}
88+
89+
@Test
90+
public void sync_putObject_default_MD5_validation_withIncorrectChecksum(WireMockRuntimeInfo wm) {
91+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
92+
.withHeader("x-amz-checksum-crc32", "i9aeUg==")
93+
.withHeader("etag", INCORRECT_ETAG)));
94+
S3Client s3Client =
95+
getSyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
96+
97+
assertThatExceptionOfType(RetryableException.class)
98+
.isThrownBy(() -> s3Client.putObject(b -> b.bucket(BUCKET).key("KEY"),
99+
RequestBody.fromBytes(CONTENT.getBytes())))
100+
.withMessage("Data read has a different checksum than expected. Was 0x" + ETAG.toLowerCase() + ", but expected 0x"
101+
+ INCORRECT_ETAG.toLowerCase() + ". This commonly means that the data was corrupted between the client "
102+
+ "and service. Note: Despite this error, the upload still completed and was persisted in S3.");
103+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
104+
}
105+
106+
@Test
107+
public void sync_putObject_default_MD5_validation_withCorrectChecksum(WireMockRuntimeInfo wm) {
108+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
109+
.withHeader("x-amz-checksum-crc32", "WRONG_CHECKSUM")
110+
.withHeader("etag", ETAG)));
111+
S3Client s3Client =
112+
getSyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor))
113+
.build();
114+
PutObjectResponse putObjectResponse = s3Client.putObject(b -> b.bucket(BUCKET)
115+
.key("KEY"),
116+
RequestBody.fromBytes(CONTENT.getBytes()));
117+
assertThat(putObjectResponse.eTag()).isEqualTo(ETAG);
118+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
119+
120+
}
121+
122+
@Test
123+
public void async_putObject_default_MD5_validation_withIncorrectChecksum(WireMockRuntimeInfo wm) {
124+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
125+
.withHeader("x-amz-checksum-crc32", "i9aeUg==")
126+
.withHeader("etag", INCORRECT_ETAG)));
127+
S3Client s3Client =
128+
getSyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
129+
130+
assertThatExceptionOfType(RetryableException.class)
131+
.isThrownBy(() -> s3Client.putObject(b -> b.bucket(BUCKET).key("KEY"),
132+
RequestBody.fromBytes(CONTENT.getBytes())))
133+
.withMessage("Data read has a different checksum than expected. Was 0x" + ETAG.toLowerCase() + ", but expected 0x"
134+
+ INCORRECT_ETAG.toLowerCase() + ". This commonly means that the data was corrupted between the client "
135+
+ "and service. Note: Despite this error, the upload still completed and was persisted in S3.");
136+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
137+
138+
}
139+
140+
@Test
141+
public void async_putObject_default_MD5_validation_withCorrectChecksum(WireMockRuntimeInfo wm) {
142+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
143+
.withHeader("x-amz-checksum-crc32", "WRONG_CHECKSUM")
144+
.withHeader("etag", ETAG)));
145+
S3AsyncClient s3Async =
146+
getAsyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
147+
PutObjectResponse putObjectResponse = s3Async.putObject(PutObjectRequest.builder()
148+
.bucket(BUCKET)
149+
.key("KEY")
150+
.build(), AsyncRequestBody.fromString(CONTENT)).join();
151+
152+
assertThat(putObjectResponse.eTag()).isEqualTo(ETAG);
153+
assertThat(captureChecksumValidationInterceptor.requestChecksumInTrailer()).isNull();
154+
}
155+
156+
@Test
157+
public void sync_putObject_httpChecksumValidation_withIncorrectChecksum(WireMockRuntimeInfo wm) {
158+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
159+
.withHeader("x-amz-checksum-crc32", "7ErD0A==")
160+
.withHeader("etag", INCORRECT_ETAG)));
161+
S3Client s3Client =
162+
getSyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
163+
164+
s3Client.putObject(b -> b.bucket(BUCKET).key("KEY").checksumAlgorithm(ChecksumAlgorithm.CRC32),
165+
RequestBody.fromBytes(CONTENT.getBytes()));
166+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
167+
assertThat(captureChecksumValidationInterceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
168+
169+
verify(putRequestedFor(anyUrl()).withRequestBody(containing("Hello, World!"))
170+
.withRequestBody(containing("x-amz-checksum-crc32:7ErD0A=="))
171+
.withRequestBody(containing("0;")));
172+
}
173+
174+
@Test
175+
public void async_putObject_httpChecksumValidation_withIncorrectChecksum(WireMockRuntimeInfo wm) {
176+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
177+
.withHeader("x-amz-checksum-crc32", "7ErD0A==")
178+
.withHeader("etag", INCORRECT_ETAG)));
179+
S3AsyncClient s3Async =
180+
getAsyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
181+
182+
PutObjectResponse putObjectResponse = s3Async.putObject(PutObjectRequest.builder()
183+
.bucket(BUCKET)
184+
.checksumAlgorithm(ChecksumAlgorithm.CRC32)
185+
.key("KEY")
186+
.build(), AsyncRequestBody.fromString(CONTENT)).join();
187+
188+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
189+
assertThat(captureChecksumValidationInterceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
190+
191+
verify(putRequestedFor(anyUrl()).withRequestBody(containing("x-amz-checksum-crc32:7ErD0A=="))
192+
.withRequestBody(containing("Hello, World!")));
193+
}
194+
}

0 commit comments

Comments
 (0)