Skip to content

Commit e01b0dc

Browse files
davidh44akidambisrinivasan
davidh44
authored and
akidambisrinivasan
committed
Do not validate MD5 if checksum value supplied (aws#5278)
* Do not validate Md5 if checksum value supplied * Do not validate Md5 if checksum value supplied * Add changelog * Add unit tests * Add back deleted test * Add back deleted test
1 parent 21d7bf2 commit e01b0dc

File tree

6 files changed

+123
-15
lines changed

6 files changed

+123
-15
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "Amazon S3",
3+
"contributor": "",
4+
"type": "bugfix",
5+
"description": "Fixes bug where Md5 validation is performed for S3 PutObject even if checksum value is supplied"
6+
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/PutObjectIntegrationTest.java

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package software.amazon.awssdk.services.s3;
1818

1919
import static org.assertj.core.api.Assertions.assertThat;
20+
import static software.amazon.awssdk.services.s3.internal.checksums.ChecksumsEnabledValidator.CHECKSUM;
2021
import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
2122
import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
2223

@@ -26,13 +27,19 @@
2627
import java.io.InputStream;
2728
import java.net.URI;
2829
import java.nio.charset.StandardCharsets;
30+
import java.security.MessageDigest;
31+
import java.security.NoSuchAlgorithmException;
2932
import java.util.ArrayList;
33+
import java.util.Base64;
3034
import java.util.List;
3135
import java.util.concurrent.CompletableFuture;
3236
import org.junit.AfterClass;
3337
import org.junit.BeforeClass;
3438
import org.junit.Test;
3539
import software.amazon.awssdk.core.async.BlockingInputStreamAsyncRequestBody;
40+
import software.amazon.awssdk.core.interceptor.Context;
41+
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
42+
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
3643
import software.amazon.awssdk.core.sync.RequestBody;
3744
import software.amazon.awssdk.http.ContentStreamProvider;
3845
import software.amazon.awssdk.services.s3.model.HeadObjectResponse;
@@ -46,9 +53,8 @@ public class PutObjectIntegrationTest extends S3IntegrationTestBase {
4653
private static final String BUCKET = temporaryBucketName(PutObjectIntegrationTest.class);
4754
private static final String ASYNC_KEY = "async-key";
4855
private static final String SYNC_KEY = "sync-key";
49-
50-
private static final byte[] CONTENT = "Hello".getBytes(StandardCharsets.UTF_8);
5156
private static final String TEXT_CONTENT_TYPE = "text/plain";
57+
private static final byte[] CONTENT = "Hello".getBytes(StandardCharsets.UTF_8);
5258

5359
@BeforeClass
5460
public static void setUp() throws Exception {
@@ -61,6 +67,28 @@ public static void tearDown() {
6167
deleteBucketAndAllContents(BUCKET);
6268
}
6369

70+
@Test
71+
public void putObject_withUserCalculatedChecksum_doesNotPerformMd5Validation() throws NoSuchAlgorithmException {
72+
CapturingInterceptor capturingInterceptor = new CapturingInterceptor();
73+
S3Client s3Client = s3ClientBuilder()
74+
.overrideConfiguration(o -> o.addExecutionInterceptor(capturingInterceptor))
75+
.build();
76+
77+
MessageDigest md = MessageDigest.getInstance("SHA-1");
78+
md.update(CONTENT);
79+
byte[] checksum = md.digest();
80+
String checksumVal = Base64.getEncoder().encodeToString(checksum);
81+
82+
PutObjectRequest request = PutObjectRequest.builder()
83+
.bucket(BUCKET)
84+
.key(SYNC_KEY)
85+
.checksumSHA1(checksumVal)
86+
.build();
87+
88+
s3Client.putObject(request, RequestBody.fromString("Hello"));
89+
assertThat(capturingInterceptor.isMd5Enabled).isFalse();
90+
}
91+
6492
@Test
6593
public void objectInputStreamsAreClosed() {
6694
TestContentProvider provider = new TestContentProvider(CONTENT);
@@ -148,4 +176,17 @@ boolean isClosed() {
148176
return isClosed;
149177
}
150178
}
179+
180+
private static class CapturingInterceptor implements ExecutionInterceptor {
181+
private boolean isMd5Enabled;
182+
183+
@Override
184+
public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttributes executionAttributes) {
185+
isMd5Enabled = executionAttributes.getAttribute(CHECKSUM) != null;
186+
}
187+
188+
public boolean isMd5Enabled() {
189+
return isMd5Enabled;
190+
}
191+
}
151192
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/s3express/S3ExpressIntegrationTest.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import static org.assertj.core.api.Assertions.assertThat;
1919
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2020
import static org.assertj.core.api.Assertions.fail;
21-
import static org.junit.jupiter.api.Assertions.assertThrows;
21+
import static software.amazon.awssdk.services.s3.internal.checksums.ChecksumsEnabledValidator.CHECKSUM;
2222
import static software.amazon.awssdk.testutils.service.AwsTestBase.CREDENTIALS_PROVIDER_CHAIN;
2323
import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
2424

@@ -207,7 +207,7 @@ public void s3Express_nonObjectTransferApis_Async(AsyncTestCase tc) {
207207
}
208208

209209
@Test
210-
public void putObject_withUserCalculatedChecksum_doesNotAddMultipleHeaders() throws NoSuchAlgorithmException {
210+
public void putObject_withUserCalculatedChecksum_doesNotAddMultipleHeadersOrPerformMd5Validation() throws NoSuchAlgorithmException {
211211
MessageDigest md = MessageDigest.getInstance("SHA-1");
212212
byte[] data = CONTENTS.getBytes(StandardCharsets.UTF_8);
213213

@@ -221,11 +221,9 @@ public void putObject_withUserCalculatedChecksum_doesNotAddMultipleHeaders() thr
221221
.checksumSHA1(checksumVal)
222222
.build();
223223

224-
// TODO (s3Express) - checksum calculation incorrect, but original bug is fixed
225-
RetryableException exception = assertThrows(RetryableException.class, () ->
226-
s3.putObject(request, RequestBody.fromString(CONTENTS)));
227-
assertThat(exception.getMessage()).doesNotContain("Expecting a single x-amz-checksum- header");
228-
assertThat(exception.getMessage()).contains("Data read has a different checksum than expected");
224+
s3.putObject(request, RequestBody.fromString(CONTENTS));
225+
assertThat(capturingInterceptor.capturedRequests()).hasSize(1);
226+
assertThat(capturingInterceptor.isMd5Enabled).isFalse();
229227
}
230228

231229
@Test
@@ -471,10 +469,12 @@ protected static void runAndVerify(AsyncTestCase testCase) {
471469

472470
private static class CapturingInterceptor implements ExecutionInterceptor {
473471
private final List<SdkHttpRequest> capturedRequests = new ArrayList<>();
472+
private boolean isMd5Enabled;
474473

475474
@Override
476475
public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttributes executionAttributes) {
477476
capturedRequests.add(context.httpRequest());
477+
isMd5Enabled = executionAttributes.getAttribute(CHECKSUM) != null;
478478
}
479479

480480
public void reset() {
@@ -484,5 +484,9 @@ public void reset() {
484484
public List<SdkHttpRequest> capturedRequests() {
485485
return Collections.unmodifiableList(capturedRequests);
486486
}
487+
488+
public boolean isMd5Enabled() {
489+
return isMd5Enabled;
490+
}
487491
}
488492
}

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import java.util.Arrays;
2121
import java.util.Optional;
22+
import java.util.stream.Stream;
2223
import software.amazon.awssdk.annotations.SdkInternalApi;
2324
import software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute;
2425
import software.amazon.awssdk.core.ClientType;
@@ -110,14 +111,18 @@ public static boolean shouldRecordChecksum(SdkRequest sdkRequest,
110111
}
111112

112113
//Checksum validation is done at Service side when HTTP Checksum algorithm attribute is set.
113-
if (isHttpCheckSumValidationEnabled(executionAttributes)) {
114+
if (isHttpCheckSumValidationEnabled(executionAttributes, sdkRequest)) {
114115
return false;
115116
}
116117

117118
return checksumEnabledPerConfig(executionAttributes);
118119
}
119120

120-
private static boolean isHttpCheckSumValidationEnabled(ExecutionAttributes executionAttributes) {
121+
private static boolean isHttpCheckSumValidationEnabled(ExecutionAttributes executionAttributes, SdkRequest request) {
122+
if (isChecksumValueSpecified(request)) {
123+
return true;
124+
}
125+
121126
Optional<ChecksumSpecs> resolvedChecksum =
122127
executionAttributes.getOptionalAttribute(SdkExecutionAttribute.RESOLVED_CHECKSUM_SPECS);
123128
if (resolvedChecksum.isPresent()) {
@@ -127,6 +132,11 @@ private static boolean isHttpCheckSumValidationEnabled(ExecutionAttributes execu
127132
return false;
128133
}
129134

135+
private static boolean isChecksumValueSpecified(SdkRequest request) {
136+
return Stream.of("ChecksumCRC32", "ChecksumCRC32C", "ChecksumSHA1", "ChecksumSHA256")
137+
.anyMatch(s -> request.getValueForField(s, String.class).isPresent());
138+
}
139+
130140
public static boolean responseChecksumIsValid(SdkHttpResponse httpResponse) {
131141
return !hasServerSideEncryptionHeader(httpResponse);
132142
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Map;
2222
import java.util.Objects;
2323
import java.util.Optional;
24+
import java.util.stream.Stream;
2425
import software.amazon.awssdk.annotations.SdkInternalApi;
2526
import software.amazon.awssdk.checksums.DefaultChecksumAlgorithm;
2627
import software.amazon.awssdk.checksums.spi.ChecksumAlgorithm;
@@ -97,10 +98,8 @@ public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttribut
9798
}
9899

99100
private boolean requestContainsUserCalculatedChecksum(SdkRequest request) {
100-
return request.getValueForField("ChecksumCRC32", String.class).isPresent()
101-
|| request.getValueForField("ChecksumCRC32C", String.class).isPresent()
102-
|| request.getValueForField("ChecksumSHA1", String.class).isPresent()
103-
|| request.getValueForField("ChecksumSHA256", String.class).isPresent();
101+
return Stream.of("ChecksumCRC32", "ChecksumCRC32C", "ChecksumSHA1", "ChecksumSHA256")
102+
.anyMatch(s -> request.getValueForField(s, String.class).isPresent());
104103
}
105104

106105
private boolean shouldAlwaysAddChecksum(ChecksumSpecs checksumSpecs, SdkRequest request) {

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,54 @@ public void putObjectChecksumEnabled_serverSideEncryption_false() {
154154
response)).isFalse();
155155
}
156156

157+
@Test
158+
public void putObject_crc32ValueSupplied_shouldNotValidateMd5() {
159+
ExecutionAttributes executionAttributes = getSyncExecutionAttributes();
160+
161+
assertThat(shouldRecordChecksum(PutObjectRequest.builder()
162+
.checksumCRC32("checksumVal")
163+
.build(),
164+
ClientType.SYNC,
165+
executionAttributes,
166+
emptyHttpRequest().build())).isFalse();
167+
}
168+
169+
@Test
170+
public void putObject_crc32cValueSupplied_shouldNotValidateMd5() {
171+
ExecutionAttributes executionAttributes = getSyncExecutionAttributes();
172+
173+
assertThat(shouldRecordChecksum(PutObjectRequest.builder()
174+
.checksumCRC32C("checksumVal")
175+
.build(),
176+
ClientType.SYNC,
177+
executionAttributes,
178+
emptyHttpRequest().build())).isFalse();
179+
}
180+
181+
@Test
182+
public void putObject_sha1ValueSupplied_shouldNotValidateMd5() {
183+
ExecutionAttributes executionAttributes = getSyncExecutionAttributes();
184+
185+
assertThat(shouldRecordChecksum(PutObjectRequest.builder()
186+
.checksumSHA1("checksumVal")
187+
.build(),
188+
ClientType.SYNC,
189+
executionAttributes,
190+
emptyHttpRequest().build())).isFalse();
191+
}
192+
193+
@Test
194+
public void putObject_sha256ValueSupplied_shouldNotValidateMd5() {
195+
ExecutionAttributes executionAttributes = getSyncExecutionAttributes();
196+
197+
assertThat(shouldRecordChecksum(PutObjectRequest.builder()
198+
.checksumSHA256("checksumVal")
199+
.build(),
200+
ClientType.SYNC,
201+
executionAttributes,
202+
emptyHttpRequest().build())).isFalse();
203+
}
204+
157205
@Test
158206
public void responseChecksumIsValid_defaultTrue() {
159207
assertThat(responseChecksumIsValid(SdkHttpResponse.builder().build())).isTrue();

0 commit comments

Comments
 (0)