Skip to content

Commit 4fc647e

Browse files
millemsmillems
committed
Do not raise a "checksum mismatch" when a PutObject is retried with an async HTTP client.
1 parent d7ae0b0 commit 4fc647e

File tree

9 files changed

+161
-9
lines changed

9 files changed

+161
-9
lines changed

‎.changes/next-release/bugfix-AmazonS3-b8193be.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "Amazon S3",
3+
"type": "bugfix",
4+
"description": "Fixed an issue where a 'checksum mismatch' error is raised whenever a PutObject request is retried while using an async client."
5+
}

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/checksums/ChecksumCalculatingAsyncRequestBody.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public Optional<Long> contentLength() {
4242

4343
@Override
4444
public void subscribe(Subscriber<? super ByteBuffer> s) {
45+
sdkChecksum.reset();
4546
wrapped.subscribe(new ChecksumCalculatingSubscriber(s, sdkChecksum));
4647
}
4748

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ public static void validatePutObjectChecksum(PutObjectResponse response, Executi
131131
byte[] ssHash = Base16Lower.decode(response.eTag().replace("\"", ""));
132132

133133
if (!Arrays.equals(digest, ssHash)) {
134-
throw SdkClientException.create("Data read has a different checksum than expected.");
134+
throw SdkClientException.create(
135+
String.format("Data read has a different checksum than expected. Was 0x%s, but expected 0x%s",
136+
BinaryUtils.toHex(digest), BinaryUtils.toHex(ssHash)));
135137
}
136138
}
137139
}

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/AsyncChecksumValidationInterceptor.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ public final class AsyncChecksumValidationInterceptor implements ExecutionInterc
4242
@Override
4343
public Optional<AsyncRequestBody> modifyAsyncHttpContent(Context.ModifyHttpRequest context,
4444
ExecutionAttributes executionAttributes) {
45-
4645
boolean putObjectTrailingChecksumsEnabled =
4746
putObjectChecksumEnabled(context.request(), ASYNC, executionAttributes, context.httpRequest());
4847

@@ -58,7 +57,6 @@ public Optional<AsyncRequestBody> modifyAsyncHttpContent(Context.ModifyHttpReque
5857
@Override
5958
public Optional<Publisher<ByteBuffer>> modifyAsyncHttpResponseContent(Context.ModifyHttpResponse context,
6059
ExecutionAttributes executionAttributes) {
61-
6260
if (getObjectChecksumEnabledPerResponse(context.request(), context.httpResponse())
6361
&& context.responsePublisher().isPresent()) {
6462
long contentLength = context.httpResponse()
@@ -78,7 +76,6 @@ public Optional<Publisher<ByteBuffer>> modifyAsyncHttpResponseContent(Context.Mo
7876

7977
@Override
8078
public void afterUnmarshalling(Context.AfterUnmarshalling context, ExecutionAttributes executionAttributes) {
81-
8279
boolean putObjectChecksumsEnabled =
8380
putObjectChecksumEnabled(context.request(), ASYNC, executionAttributes, context.httpRequest());
8481

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/EnableTrailingChecksumInterceptor.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import software.amazon.awssdk.http.SdkHttpResponse;
3131
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
3232
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
33+
import software.amazon.awssdk.utils.Validate;
3334

3435
@SdkInternalApi
3536
public final class EnableTrailingChecksumInterceptor implements ExecutionInterceptor {
@@ -60,7 +61,11 @@ public SdkResponse modifyResponse(Context.ModifyResponse context, ExecutionAttri
6061

6162
if (getObjectChecksumEnabledPerResponse(context.request(), httpResponse)) {
6263
GetObjectResponse getResponse = (GetObjectResponse) response;
63-
return getResponse.toBuilder().contentLength(getResponse.contentLength() - S3_MD5_CHECKSUM_LENGTH).build();
64+
Long contentLength = getResponse.contentLength();
65+
Validate.notNull(contentLength, "Service returned null 'Content-Length'.");
66+
return getResponse.toBuilder()
67+
.contentLength(contentLength - S3_MD5_CHECKSUM_LENGTH)
68+
.build();
6469
}
6570

6671
return response;

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/SyncChecksumValidationInterceptor.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ public final class SyncChecksumValidationInterceptor implements ExecutionInterce
4343
@Override
4444
public Optional<RequestBody> modifyHttpContent(Context.ModifyHttpRequest context,
4545
ExecutionAttributes executionAttributes) {
46-
4746
if (putObjectChecksumEnabled(context.request(), SYNC, executionAttributes, context.httpRequest())
4847
&& context.requestBody().isPresent()) {
4948
SdkChecksum checksum = new Md5Checksum();
@@ -65,7 +64,6 @@ public Optional<RequestBody> modifyHttpContent(Context.ModifyHttpRequest context
6564
@Override
6665
public Optional<InputStream> modifyHttpResponseContent(Context.ModifyHttpResponse context,
6766
ExecutionAttributes executionAttributes) {
68-
6967
if (getObjectChecksumEnabledPerResponse(context.request(), context.httpResponse())
7068
&& context.responseBody().isPresent()) {
7169

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/ChecksumResetsOnRetryTest.java

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
/*
2+
* Copyright 2010-2019 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.checksums;
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.stubFor;
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
import static software.amazon.awssdk.core.async.AsyncResponseTransformer.toBytes;
24+
25+
import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
26+
import com.github.tomakehurst.wiremock.client.WireMock;
27+
import com.github.tomakehurst.wiremock.junit.WireMockRule;
28+
import com.github.tomakehurst.wiremock.stubbing.Scenario;
29+
import java.net.URI;
30+
import java.nio.charset.StandardCharsets;
31+
import java.util.function.Consumer;
32+
import org.apache.commons.lang3.ArrayUtils;
33+
import org.junit.Before;
34+
import org.junit.Rule;
35+
import org.junit.Test;
36+
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
37+
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
38+
import software.amazon.awssdk.core.ResponseBytes;
39+
import software.amazon.awssdk.core.async.AsyncRequestBody;
40+
import software.amazon.awssdk.core.sync.RequestBody;
41+
import software.amazon.awssdk.regions.Region;
42+
import software.amazon.awssdk.services.s3.S3AsyncClient;
43+
import software.amazon.awssdk.services.s3.S3Client;
44+
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
45+
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
46+
import software.amazon.awssdk.utils.BinaryUtils;
47+
48+
/**
49+
* Verifies that the checksum validators are reset on an HTTP retry.
50+
*/
51+
public class ChecksumResetsOnRetryTest {
52+
@Rule
53+
public WireMockRule mockServer = new WireMockRule(0);
54+
55+
private S3Client s3Client;
56+
57+
private S3AsyncClient s3AsyncClient;
58+
59+
private byte[] body;
60+
61+
private byte[] bodyWithTrailingChecksum;
62+
63+
private String bodyEtag;
64+
65+
@Before
66+
public void setup() {
67+
StaticCredentialsProvider credentials = StaticCredentialsProvider.create(AwsBasicCredentials.create("akid", "skid"));
68+
s3Client = S3Client.builder()
69+
.credentialsProvider(credentials)
70+
.region(Region.US_WEST_2)
71+
.endpointOverride(URI.create("http://localhost:" + mockServer.port()))
72+
.build();
73+
74+
s3AsyncClient = S3AsyncClient.builder()
75+
.credentialsProvider(credentials)
76+
.region(Region.US_WEST_2)
77+
.endpointOverride(URI.create("http://localhost:" + mockServer.port()))
78+
.build();
79+
80+
body = "foo".getBytes(StandardCharsets.UTF_8);
81+
String checksumAsHexString = "acbd18db4cc2f85cedef654fccc4a4d8";
82+
bodyEtag = "\"" + checksumAsHexString + "\"";
83+
bodyWithTrailingChecksum = ArrayUtils.addAll(body, BinaryUtils.fromHex(checksumAsHexString));
84+
}
85+
86+
@Test
87+
public void syncPutObject_resetsChecksumOnRetry() {
88+
stubSuccessAfterOneRetry(r -> r.withHeader("ETag", bodyEtag));
89+
90+
PutObjectResponse response = s3Client.putObject(r -> r.bucket("foo").key("bar"), RequestBody.fromBytes(body));
91+
assertThat(response.eTag()).isEqualTo(bodyEtag);
92+
}
93+
94+
@Test
95+
public void asyncPutObject_resetsChecksumOnRetry() {
96+
stubSuccessAfterOneRetry(r -> r.withHeader("ETag", bodyEtag));
97+
98+
PutObjectResponse response = s3AsyncClient.putObject(r -> r.bucket("foo").key("bar"), AsyncRequestBody.fromBytes(body)).join();
99+
assertThat(response.eTag()).isEqualTo(bodyEtag);
100+
}
101+
102+
@Test
103+
public void syncGetObject_resetsChecksumOnRetry() {
104+
stubSuccessAfterOneRetry(r -> r.withHeader("ETag", bodyEtag)
105+
.withHeader("x-amz-transfer-encoding", "append-md5")
106+
.withHeader("content-length", Integer.toString(bodyWithTrailingChecksum.length))
107+
.withBody(bodyWithTrailingChecksum));
108+
109+
ResponseBytes<GetObjectResponse> response = s3Client.getObjectAsBytes(r -> r.bucket("foo").key("bar"));
110+
assertThat(response.response().eTag()).isEqualTo(bodyEtag);
111+
assertThat(response.asByteArray()).isEqualTo(body);
112+
}
113+
114+
@Test
115+
public void asyncGetObject_resetsChecksumOnRetry() {
116+
stubSuccessAfterOneRetry(r -> r.withHeader("ETag", bodyEtag)
117+
.withHeader("x-amz-transfer-encoding", "append-md5")
118+
.withHeader("content-length", Integer.toString(bodyWithTrailingChecksum.length))
119+
.withBody(bodyWithTrailingChecksum));
120+
121+
ResponseBytes<GetObjectResponse> response = s3AsyncClient.getObject(r -> r.bucket("foo").key("bar"), toBytes()).join();
122+
assertThat(response.response().eTag()).isEqualTo(bodyEtag);
123+
assertThat(response.asByteArray()).isEqualTo(body);
124+
}
125+
126+
private void stubSuccessAfterOneRetry(Consumer<ResponseDefinitionBuilder> successfulResponseModifier) {
127+
WireMock.reset();
128+
129+
String scenario = "stubSuccessAfterOneRetry";
130+
stubFor(any(anyUrl())
131+
.willReturn(aResponse().withStatus(500).withBody("<xml></xml>"))
132+
.inScenario(scenario)
133+
.whenScenarioStateIs(Scenario.STARTED)
134+
.willSetStateTo("200"));
135+
136+
ResponseDefinitionBuilder successfulResponse = aResponse().withStatus(200).withBody("<xml></xml>");
137+
successfulResponseModifier.accept(successfulResponse);
138+
stubFor(any(anyUrl())
139+
.willReturn(successfulResponse)
140+
.inScenario(scenario)
141+
.whenScenarioStateIs("200"));
142+
}
143+
}
144+

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/AsyncChecksumValidationInterceptorTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public void afterUnmarshalling_putObjectRequest_shouldValidateChecksum_throwExce
172172
InterceptorTestUtils.afterUnmarshallingContext(putObjectRequest, sdkHttpRequest, response, sdkHttpResponse);
173173

174174
assertThatThrownBy(() -> interceptor.afterUnmarshalling(afterUnmarshallingContext, getExecutionAttributesWithChecksum()))
175-
.hasMessage("Data read has a different checksum than expected.");
175+
.hasMessageContaining("Data read has a different checksum than expected.");
176176
}
177177

178178
@Test

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/SyncChecksumValidationInterceptorTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ public void afterUnmarshalling_putObjectRequest_shouldValidateChecksum_throwExce
204204
InterceptorTestUtils.afterUnmarshallingContext(putObjectRequest, sdkHttpRequest, response, sdkHttpResponse);
205205

206206
assertThatThrownBy(() -> interceptor.afterUnmarshalling(afterUnmarshallingContext, getExecutionAttributesWithChecksum()))
207-
.hasMessage("Data read has a different checksum than expected.");
207+
.hasMessageContaining("Data read has a different checksum than expected.");
208208
}
209209

210210
@Test

0 commit comments

Comments
 (0)