Skip to content

Commit ae48b93

Browse files
davidh44davidh44
authored
Do not send Md5 checksum header if ChecksumMode is enabled (#4731)
* Do not send x-amz-te:append-md5 if ChecksumMode is enabled * Update test interceptor * Move checksums to internal * Address comments * Address comments
1 parent 018582b commit ae48b93

21 files changed

+398
-119
lines changed

‎.changes/next-release/bugfix-AmazonS3-16d0bb1.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 double checksum validation for GetObject. Now when ChecksumMode is enabled, x-amz-te:append-md5 will not be sent, and only the flexible checksum will be validated. If ChecksumMode is enabled and no ChecksumAlgorithm was returned, no validation will be performed."
6+
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/checksum/AsyncHttpChecksumIntegrationTest.java

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,15 @@
2626
import java.nio.ByteBuffer;
2727
import java.nio.file.Files;
2828
import java.util.concurrent.CompletableFuture;
29+
import java.util.stream.Stream;
2930
import org.junit.jupiter.api.AfterAll;
3031
import org.junit.jupiter.api.AfterEach;
3132
import org.junit.jupiter.api.BeforeAll;
3233
import org.junit.jupiter.api.Disabled;
3334
import org.junit.jupiter.api.Test;
3435
import org.junit.jupiter.params.ParameterizedTest;
36+
import org.junit.jupiter.params.provider.Arguments;
37+
import org.junit.jupiter.params.provider.MethodSource;
3538
import org.junit.jupiter.params.provider.ValueSource;
3639
import software.amazon.awssdk.auth.signer.S3SignerExecutionAttribute;
3740
import software.amazon.awssdk.authcrt.signer.internal.DefaultAwsCrtS3V4aSigner;
@@ -42,6 +45,7 @@
4245
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
4346
import software.amazon.awssdk.core.internal.async.FileAsyncRequestBody;
4447
import software.amazon.awssdk.services.s3.S3AsyncClient;
48+
import software.amazon.awssdk.services.s3.S3Configuration;
4549
import software.amazon.awssdk.services.s3.S3IntegrationTestBase;
4650
import software.amazon.awssdk.services.s3.model.ChecksumAlgorithm;
4751
import software.amazon.awssdk.services.s3.model.ChecksumMode;
@@ -291,4 +295,98 @@ public void putObject_with_bufferCreatedFromZeroCapacityByteBuffer() {
291295
assertThat(interceptor.responseValidation()).isEqualTo(ChecksumValidation.VALIDATED);
292296
assertThat(response).isEqualTo("");
293297
}
298+
299+
private static Stream<Arguments> getObjectChecksumValidationParams() {
300+
return Stream.of(Arguments.of(true, ChecksumAlgorithm.CRC32, ChecksumMode.ENABLED),
301+
Arguments.of(true, null, ChecksumMode.ENABLED),
302+
Arguments.of(true, ChecksumAlgorithm.CRC32, null),
303+
Arguments.of(true, null, null),
304+
Arguments.of(false, ChecksumAlgorithm.CRC32, ChecksumMode.ENABLED),
305+
Arguments.of(false, null, ChecksumMode.ENABLED),
306+
Arguments.of(false, ChecksumAlgorithm.CRC32, null),
307+
Arguments.of(false, null, null));
308+
}
309+
310+
@ParameterizedTest
311+
@MethodSource("getObjectChecksumValidationParams")
312+
public void testGetObjectChecksumValidation(boolean checksumValidationEnabled, ChecksumAlgorithm checksumAlgorithm,
313+
ChecksumMode checksumMode) {
314+
S3AsyncClient s3Async = s3AsyncClientBuilder().overrideConfiguration(o -> o.addExecutionInterceptor(interceptor))
315+
.serviceConfiguration(S3Configuration.builder()
316+
.checksumValidationEnabled(checksumValidationEnabled)
317+
.build())
318+
.build();
319+
320+
s3Async.putObject(PutObjectRequest.builder()
321+
.bucket(BUCKET)
322+
.key(KEY)
323+
.checksumAlgorithm(checksumAlgorithm)
324+
.build(), AsyncRequestBody.fromString("Hello world")).join();
325+
326+
s3Async.getObject(GetObjectRequest.builder()
327+
.bucket(BUCKET)
328+
.key(KEY)
329+
.checksumMode(checksumMode)
330+
.build(), AsyncResponseTransformer.toBytes()).join();
331+
332+
validateChecksumValidation(checksumValidationEnabled, checksumAlgorithm, checksumMode);
333+
interceptor.reset();
334+
}
335+
336+
private void validateChecksumValidation(boolean checksumValidationEnabled, ChecksumAlgorithm checksumAlgorithm,
337+
ChecksumMode checksumMode) {
338+
if (checksumValidationEnabled) {
339+
if (checksumMode == ChecksumMode.ENABLED) {
340+
assertChecksumModeEnabledWithChecksumValidationEnabled(checksumAlgorithm);
341+
} else {
342+
assertChecksumModeNotEnabledWithChecksumValidationEnabled();
343+
}
344+
} else {
345+
if (checksumMode == ChecksumMode.ENABLED) {
346+
assertChecksumModeEnabledWithChecksumValidationDisabled(checksumAlgorithm);
347+
} else {
348+
assertChecksumModeNotEnabledWithChecksumValidationDisabled();
349+
}
350+
}
351+
}
352+
353+
private void assertChecksumModeEnabledWithChecksumValidationEnabled(ChecksumAlgorithm checksumAlgorithm) {
354+
if (checksumAlgorithm == null) {
355+
assertRequestAndResponseDoNotContainMd5Header();
356+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNull();
357+
} else {
358+
assertRequestAndResponseDoNotContainMd5Header();
359+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNotNull();
360+
}
361+
}
362+
363+
private void assertChecksumModeNotEnabledWithChecksumValidationEnabled() {
364+
assertRequestAndResponseContainMd5Header();
365+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNull();
366+
}
367+
368+
private void assertChecksumModeEnabledWithChecksumValidationDisabled(ChecksumAlgorithm checksumAlgorithm) {
369+
if (checksumAlgorithm == null) {
370+
assertRequestAndResponseDoNotContainMd5Header();
371+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNull();
372+
} else {
373+
assertRequestAndResponseDoNotContainMd5Header();
374+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNotNull();
375+
}
376+
}
377+
378+
private void assertChecksumModeNotEnabledWithChecksumValidationDisabled() {
379+
assertRequestAndResponseDoNotContainMd5Header();
380+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNull();
381+
}
382+
383+
private void assertRequestAndResponseContainMd5Header() {
384+
assertThat(interceptor.requestTransferEncodingHeader()).isEqualTo("append-md5");
385+
assertThat(interceptor.responseTransferEncodingHeader()).isEqualTo("append-md5");
386+
}
387+
388+
private void assertRequestAndResponseDoNotContainMd5Header() {
389+
assertThat(interceptor.requestTransferEncodingHeader()).isNull();
390+
assertThat(interceptor.responseTransferEncodingHeader()).isNull();
391+
}
294392
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/checksum/HttpChecksumIntegrationTest.java

Lines changed: 101 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,14 @@
2929
import java.nio.charset.StandardCharsets;
3030
import java.nio.file.Files;
3131
import java.util.stream.Collectors;
32+
import java.util.stream.Stream;
3233
import org.junit.jupiter.api.AfterAll;
3334
import org.junit.jupiter.api.AfterEach;
3435
import org.junit.jupiter.api.BeforeAll;
3536
import org.junit.jupiter.api.Test;
37+
import org.junit.jupiter.params.ParameterizedTest;
38+
import org.junit.jupiter.params.provider.Arguments;
39+
import org.junit.jupiter.params.provider.MethodSource;
3640
import software.amazon.awssdk.authcrt.signer.internal.DefaultAwsCrtS3V4aSigner;
3741
import software.amazon.awssdk.core.HttpChecksumConstant;
3842
import software.amazon.awssdk.core.ResponseInputStream;
@@ -41,6 +45,7 @@
4145
import software.amazon.awssdk.core.sync.RequestBody;
4246
import software.amazon.awssdk.services.s3.S3AsyncClient;
4347
import software.amazon.awssdk.services.s3.S3Client;
48+
import software.amazon.awssdk.services.s3.S3Configuration;
4449
import software.amazon.awssdk.services.s3.S3IntegrationTestBase;
4550
import software.amazon.awssdk.services.s3.model.ChecksumAlgorithm;
4651
import software.amazon.awssdk.services.s3.model.ChecksumMode;
@@ -50,7 +55,6 @@
5055
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
5156
import software.amazon.awssdk.services.s3.model.S3Exception;
5257
import software.amazon.awssdk.services.s3.utils.CaptureChecksumValidationInterceptor;
53-
import software.amazon.awssdk.services.s3.utils.ChecksumUtils;
5458
import software.amazon.awssdk.testutils.RandomTempFile;
5559

5660
public class HttpChecksumIntegrationTest extends S3IntegrationTestBase {
@@ -149,7 +153,7 @@ public void invalidHeaderChecksumCalculatedByUserNotOverWrittenBySdkClient() {
149153
}
150154

151155
@Test
152-
public void syncValidUnsignedTrailerChecksumCalculatedBySdkClient() throws InterruptedException {
156+
public void syncValidUnsignedTrailerChecksumCalculatedBySdkClient() {
153157
s3Https.putObject(PutObjectRequest.builder()
154158
.bucket(BUCKET)
155159
.key(KEY)
@@ -173,8 +177,6 @@ public void syncValidUnsignedTrailerChecksumCalculatedBySdkClient() throws Inter
173177

174178
@Test
175179
public void syncValidSignedTrailerChecksumCalculatedBySdkClient() {
176-
177-
178180
s3.putObject(PutObjectRequest.builder()
179181
.bucket(BUCKET)
180182
.key(KEY)
@@ -198,13 +200,10 @@ public void syncValidSignedTrailerChecksumCalculatedBySdkClient() {
198200
assertThat(interceptor.validationAlgorithm()).isEqualTo(Algorithm.CRC32);
199201
assertThat(interceptor.responseValidation()).isEqualTo(ChecksumValidation.VALIDATED);
200202
assertThat(text).isEqualTo("Hello world");
201-
202203
}
203204

204205
@Test
205206
public void syncValidSignedTrailerChecksumCalculatedBySdkClient_Empty_String() {
206-
207-
208207
s3.putObject(PutObjectRequest.builder()
209208
.bucket(BUCKET)
210209
.key(KEY)
@@ -230,7 +229,6 @@ public void syncValidSignedTrailerChecksumCalculatedBySdkClient_Empty_String() {
230229

231230
@Test
232231
public void syncValidSignedTrailerChecksumCalculatedBySdkClientWithSigv4a() {
233-
234232
s3.putObject(PutObjectRequest.builder()
235233
.bucket(BUCKET)
236234
.key(KEY)
@@ -254,7 +252,6 @@ public void syncValidSignedTrailerChecksumCalculatedBySdkClientWithSigv4a() {
254252

255253
@Test
256254
public void syncValidSignedTrailerChecksumCalculatedBySdkClientWithSigv4a_withContentEncoding() {
257-
258255
s3.putObject(PutObjectRequest.builder()
259256
.bucket(BUCKET)
260257
.key(KEY)
@@ -334,7 +331,7 @@ public void syncSignedPayloadForHugeMessage() {
334331
}
335332

336333
@Test
337-
public void syncUnsignedPayloadMultiPartForHugeMessage() throws InterruptedException {
334+
public void syncUnsignedPayloadMultiPartForHugeMessage() {
338335
s3Https.putObject(PutObjectRequest.builder()
339336
.bucket(BUCKET)
340337
.key(KEY)
@@ -360,8 +357,7 @@ public void syncUnsignedPayloadMultiPartForHugeMessage() throws InterruptedExcep
360357

361358

362359
@Test
363-
public void syncValidUnsignedTrailerChecksumCalculatedBySdkClient_withSmallFileRequestBody() throws InterruptedException,
364-
IOException {
360+
public void syncValidUnsignedTrailerChecksumCalculatedBySdkClient_withSmallFileRequestBody() throws IOException {
365361
File randomFileOfFixedLength = new RandomTempFile(10 * KB);
366362

367363
s3Https.putObject(PutObjectRequest.builder()
@@ -410,4 +406,97 @@ public void syncValidUnsignedTrailerChecksumCalculatedBySdkClient_withHugeFileRe
410406
assertThat(text).isEqualTo(new String(bytes));
411407
}
412408

409+
private static Stream<Arguments> getObjectChecksumValidationParams() {
410+
return Stream.of(Arguments.of(true, ChecksumAlgorithm.CRC32, ChecksumMode.ENABLED),
411+
Arguments.of(true, null, ChecksumMode.ENABLED),
412+
Arguments.of(true, ChecksumAlgorithm.CRC32, null),
413+
Arguments.of(true, null, null),
414+
Arguments.of(false, ChecksumAlgorithm.CRC32, ChecksumMode.ENABLED),
415+
Arguments.of(false, null, ChecksumMode.ENABLED),
416+
Arguments.of(false, ChecksumAlgorithm.CRC32, null),
417+
Arguments.of(false, null, null));
418+
}
419+
420+
@ParameterizedTest
421+
@MethodSource("getObjectChecksumValidationParams")
422+
public void testGetObjectChecksumValidation(boolean checksumValidationEnabled, ChecksumAlgorithm checksumAlgorithm,
423+
ChecksumMode checksumMode) {
424+
S3Client s3 = s3ClientBuilder().overrideConfiguration(o -> o.addExecutionInterceptor(interceptor))
425+
.serviceConfiguration(S3Configuration.builder()
426+
.checksumValidationEnabled(checksumValidationEnabled)
427+
.build())
428+
.build();
429+
430+
s3.putObject(PutObjectRequest.builder()
431+
.bucket(BUCKET)
432+
.key(KEY)
433+
.checksumAlgorithm(checksumAlgorithm)
434+
.build(), RequestBody.fromString("Hello world"));
435+
436+
s3.getObject(GetObjectRequest.builder()
437+
.bucket(BUCKET)
438+
.key(KEY)
439+
.checksumMode(checksumMode)
440+
.build());
441+
442+
validateChecksumValidation(checksumValidationEnabled, checksumAlgorithm, checksumMode);
443+
interceptor.reset();
444+
}
445+
446+
private void validateChecksumValidation(boolean checksumValidationEnabled, ChecksumAlgorithm checksumAlgorithm,
447+
ChecksumMode checksumMode) {
448+
if (checksumValidationEnabled) {
449+
if (checksumMode == ChecksumMode.ENABLED) {
450+
assertChecksumModeEnabledWithChecksumValidationEnabled(checksumAlgorithm);
451+
} else {
452+
assertChecksumModeNotEnabledWithChecksumValidationEnabled();
453+
}
454+
} else {
455+
if (checksumMode == ChecksumMode.ENABLED) {
456+
assertChecksumModeEnabledWithChecksumValidationDisabled(checksumAlgorithm);
457+
} else {
458+
assertChecksumModeNotEnabledWithChecksumValidationDisabled();
459+
}
460+
}
461+
}
462+
463+
private void assertChecksumModeEnabledWithChecksumValidationEnabled(ChecksumAlgorithm checksumAlgorithm) {
464+
if (checksumAlgorithm == null) {
465+
assertRequestAndResponseDoNotContainMd5Header();
466+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNull();
467+
} else {
468+
assertRequestAndResponseDoNotContainMd5Header();
469+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNotNull();
470+
}
471+
}
472+
473+
private void assertChecksumModeNotEnabledWithChecksumValidationEnabled() {
474+
assertRequestAndResponseContainMd5Header();
475+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNull();
476+
}
477+
478+
private void assertChecksumModeEnabledWithChecksumValidationDisabled(ChecksumAlgorithm checksumAlgorithm) {
479+
if (checksumAlgorithm == null) {
480+
assertRequestAndResponseDoNotContainMd5Header();
481+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNull();
482+
} else {
483+
assertRequestAndResponseDoNotContainMd5Header();
484+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNotNull();
485+
}
486+
}
487+
488+
private void assertChecksumModeNotEnabledWithChecksumValidationDisabled() {
489+
assertRequestAndResponseDoNotContainMd5Header();
490+
assertThat(interceptor.responseFlexibleChecksumHeader()).isNull();
491+
}
492+
493+
private void assertRequestAndResponseContainMd5Header() {
494+
assertThat(interceptor.requestTransferEncodingHeader()).isEqualTo("append-md5");
495+
assertThat(interceptor.responseTransferEncodingHeader()).isEqualTo("append-md5");
496+
}
497+
498+
private void assertRequestAndResponseDoNotContainMd5Header() {
499+
assertThat(interceptor.requestTransferEncodingHeader()).isNull();
500+
assertThat(interceptor.responseTransferEncodingHeader()).isNull();
501+
}
413502
}

0 commit comments

Comments
 (0)