Skip to content

Commit 232a84f

Browse files
joviegasjoviegas
authored
Fix for Default MD5 check getting skipped even when http checksum algorithm is not set (#3748)
* Fix for Default MD5 check getting skipped even when http checksum algorithm is not set * updated category as S3
1 parent a1c168a commit 232a84f

File tree

3 files changed

+219
-2
lines changed

3 files changed

+219
-2
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-a15138d.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "Amazon S3",
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
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/PutObjectWithChecksumTest.java

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
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+
// Exception is thrown means the default Md5 validation has taken place.
90+
@Test
91+
void sync_putObject_default_MD5_validation_withIncorrectChecksum(WireMockRuntimeInfo wm) {
92+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
93+
.withHeader("x-amz-checksum-crc32", "i9aeUg==")
94+
.withHeader("etag", INCORRECT_ETAG)));
95+
S3Client s3Client =
96+
getSyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
97+
98+
PutObjectRequest putObjectRequest = PutObjectRequest.builder().bucket(BUCKET).key("KEY").build();
99+
RequestBody requestBody = RequestBody.fromBytes(CONTENT.getBytes());
100+
101+
assertThatExceptionOfType(RetryableException.class)
102+
.isThrownBy(() -> s3Client.putObject(putObjectRequest, requestBody))
103+
.withMessage("Data read has a different checksum than expected. Was 0x" + ETAG.toLowerCase() + ", but expected 0x"
104+
+ INCORRECT_ETAG.toLowerCase() + ". This commonly means that the data was corrupted between the client "
105+
+ "and service. Note: Despite this error, the upload still completed and was persisted in S3.");
106+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
107+
}
108+
109+
@Test
110+
void sync_putObject_default_MD5_validation_withCorrectChecksum(WireMockRuntimeInfo wm) {
111+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
112+
.withHeader("x-amz-checksum-crc32", "WRONG_CHECKSUM")
113+
.withHeader("etag", ETAG)));
114+
S3Client s3Client =
115+
getSyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor))
116+
.build();
117+
PutObjectResponse putObjectResponse = s3Client.putObject(b -> b.bucket(BUCKET)
118+
.key("KEY"),
119+
RequestBody.fromBytes(CONTENT.getBytes()));
120+
assertThat(putObjectResponse.eTag()).isEqualTo(ETAG);
121+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
122+
123+
}
124+
125+
// Exception is thrown means the default Md5 validation has taken place.
126+
@Test
127+
void async_putObject_default_MD5_validation_withIncorrectChecksum(WireMockRuntimeInfo wm) {
128+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
129+
.withHeader("x-amz-checksum-crc32", "i9aeUg==")
130+
.withHeader("etag", INCORRECT_ETAG)));
131+
S3Client s3Client =
132+
getSyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
133+
134+
PutObjectRequest putObjectRequest = PutObjectRequest.builder().bucket(BUCKET).key("KEY").build();
135+
RequestBody requestBody = RequestBody.fromBytes(CONTENT.getBytes());
136+
137+
assertThatExceptionOfType(RetryableException.class)
138+
.isThrownBy(() -> s3Client.putObject(putObjectRequest, requestBody))
139+
.withMessage("Data read has a different checksum than expected. Was 0x" + ETAG.toLowerCase() + ", but expected 0x"
140+
+ INCORRECT_ETAG.toLowerCase() + ". This commonly means that the data was corrupted between the client "
141+
+ "and service. Note: Despite this error, the upload still completed and was persisted in S3.");
142+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
143+
144+
}
145+
146+
@Test
147+
void async_putObject_default_MD5_validation_withCorrectChecksum(WireMockRuntimeInfo wm) {
148+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
149+
.withHeader("x-amz-checksum-crc32", "WRONG_CHECKSUM")
150+
.withHeader("etag", ETAG)));
151+
S3AsyncClient s3Async =
152+
getAsyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
153+
PutObjectResponse putObjectResponse = s3Async.putObject(PutObjectRequest.builder()
154+
.bucket(BUCKET)
155+
.key("KEY")
156+
.build(), AsyncRequestBody.fromString(CONTENT)).join();
157+
158+
assertThat(putObjectResponse.eTag()).isEqualTo(ETAG);
159+
assertThat(captureChecksumValidationInterceptor.requestChecksumInTrailer()).isNull();
160+
}
161+
162+
// Even with incorrect Etag, exception is not thrown because default check is skipped when checksumAlgorithm is set
163+
@Test
164+
void sync_putObject_httpChecksumValidation_withIncorrectChecksum(WireMockRuntimeInfo wm) {
165+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
166+
.withHeader("x-amz-checksum-crc32", "7ErD0A==")
167+
.withHeader("etag", INCORRECT_ETAG)));
168+
S3Client s3Client =
169+
getSyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
170+
171+
s3Client.putObject(b -> b.bucket(BUCKET).key("KEY").checksumAlgorithm(ChecksumAlgorithm.CRC32),
172+
RequestBody.fromBytes(CONTENT.getBytes()));
173+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
174+
assertThat(captureChecksumValidationInterceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
175+
176+
verify(putRequestedFor(anyUrl()).withRequestBody(containing("Hello, World!"))
177+
.withRequestBody(containing("x-amz-checksum-crc32:7ErD0A=="))
178+
.withRequestBody(containing("0;")));
179+
}
180+
181+
// Even with incorrect Etag, exception is not thrown because default check is skipped when checksumAlgorithm is set
182+
@Test
183+
void async_putObject_httpChecksumValidation_withIncorrectChecksum(WireMockRuntimeInfo wm) {
184+
stubFor(any(anyUrl()).willReturn(aResponse().withStatus(200)
185+
.withHeader("x-amz-checksum-crc32", "7ErD0A==")
186+
.withHeader("etag", INCORRECT_ETAG)));
187+
S3AsyncClient s3Async =
188+
getAsyncClientBuilder(wm).overrideConfiguration(o -> o.addExecutionInterceptor(captureChecksumValidationInterceptor)).build();
189+
190+
s3Async.putObject(PutObjectRequest.builder().bucket(BUCKET).checksumAlgorithm(ChecksumAlgorithm.CRC32).key("KEY").build(),
191+
AsyncRequestBody.fromString(CONTENT)).join();
192+
193+
assertThat(captureChecksumValidationInterceptor.validationAlgorithm()).isNull();
194+
assertThat(captureChecksumValidationInterceptor.requestChecksumInTrailer()).isEqualTo("x-amz-checksum-crc32");
195+
196+
verify(putRequestedFor(anyUrl()).withRequestBody(containing("x-amz-checksum-crc32:7ErD0A=="))
197+
.withRequestBody(containing("Hello, World!")));
198+
}
199+
}

0 commit comments

Comments
 (0)