Skip to content

Commit 2cf35c7

Browse files
davidh44davidh44
authored
Move checksum calculation from afterMarshalling to modifyHttpRequest (#4108)
* Update HttpChecksumRequiredInterceptor * Update HttpChecksumInHeaderInterceptor * Update tests and remove constant * Add back constant to resolve japicmp * Add back javadocs
1 parent 9f53186 commit 2cf35c7

File tree

3 files changed

+31
-111
lines changed

3 files changed

+31
-111
lines changed

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/interceptor/HttpChecksumInHeaderInterceptor.java

Lines changed: 12 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,13 @@
1515

1616
package software.amazon.awssdk.core.internal.interceptor;
1717

18-
import static software.amazon.awssdk.core.HttpChecksumConstant.HTTP_CHECKSUM_VALUE;
1918
import static software.amazon.awssdk.core.HttpChecksumConstant.SIGNING_METHOD;
2019
import static software.amazon.awssdk.core.internal.util.HttpChecksumResolver.getResolvedChecksumSpecs;
2120

2221
import java.io.IOException;
2322
import java.io.UncheckedIOException;
2423
import java.util.Optional;
2524
import software.amazon.awssdk.annotations.SdkInternalApi;
26-
import software.amazon.awssdk.core.checksums.Algorithm;
2725
import software.amazon.awssdk.core.checksums.ChecksumSpecs;
2826
import software.amazon.awssdk.core.interceptor.Context;
2927
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
@@ -47,49 +45,27 @@
4745
@SdkInternalApi
4846
public class HttpChecksumInHeaderInterceptor implements ExecutionInterceptor {
4947

50-
@Override
51-
public void afterMarshalling(Context.AfterMarshalling context, ExecutionAttributes executionAttributes) {
52-
ChecksumSpecs headerChecksumSpecs = HttpChecksumUtils.checksumSpecWithRequestAlgorithm(executionAttributes).orElse(null);
53-
54-
if (shouldSkipHttpChecksumInHeader(context, executionAttributes, headerChecksumSpecs)) {
55-
return;
56-
}
57-
Optional<RequestBody> syncContent = context.requestBody();
58-
syncContent.ifPresent(
59-
requestBody -> saveContentChecksum(requestBody, executionAttributes, headerChecksumSpecs.algorithm()));
60-
}
61-
62-
@Override
63-
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) {
64-
ChecksumSpecs checksumSpecs = getResolvedChecksumSpecs(executionAttributes);
65-
66-
if (shouldSkipHttpChecksumInHeader(context, executionAttributes, checksumSpecs)) {
67-
return context.httpRequest();
68-
}
69-
70-
String httpChecksumValue = executionAttributes.getAttribute(HTTP_CHECKSUM_VALUE);
71-
if (httpChecksumValue != null) {
72-
return context.httpRequest().copy(r -> r.putHeader(checksumSpecs.headerName(), httpChecksumValue));
73-
}
74-
return context.httpRequest();
75-
76-
}
77-
7848
/**
79-
* Calculates the checksumSpecs of the provided request (and base64 encodes it), storing the result in
80-
* executionAttribute "HttpChecksumValue".
49+
* Calculates the checksum of the provided request (and base64 encodes it), and adds the header to the request.
8150
*
8251
* <p>Note: This assumes that the content stream provider can create multiple new streams. If it only supports one (e.g. with
8352
* an input stream that doesn't support mark/reset), we could consider buffering the content in memory here and updating the
8453
* request body to use that buffered content. We obviously don't want to do that for giant streams, so we haven't opted to do
8554
* that yet.
8655
*/
87-
private static void saveContentChecksum(RequestBody requestBody, ExecutionAttributes executionAttributes,
88-
Algorithm algorithm) {
56+
@Override
57+
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) {
58+
ChecksumSpecs checksumSpecs = getResolvedChecksumSpecs(executionAttributes);
59+
Optional<RequestBody> syncContent = context.requestBody();
60+
61+
if (shouldSkipHttpChecksumInHeader(context, executionAttributes, checksumSpecs) || !syncContent.isPresent()) {
62+
return context.httpRequest();
63+
}
64+
8965
try {
9066
String payloadChecksum = BinaryUtils.toBase64(HttpChecksumUtils.computeChecksum(
91-
requestBody.contentStreamProvider().newStream(), algorithm));
92-
executionAttributes.putAttribute(HTTP_CHECKSUM_VALUE, payloadChecksum);
67+
syncContent.get().contentStreamProvider().newStream(), checksumSpecs.algorithm()));
68+
return context.httpRequest().copy(r -> r.putHeader(checksumSpecs.headerName(), payloadChecksum));
9369
} catch (IOException e) {
9470
throw new UncheckedIOException(e);
9571
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/interceptor/HttpChecksumRequiredInterceptor.java

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import software.amazon.awssdk.annotations.SdkInternalApi;
2222
import software.amazon.awssdk.core.async.AsyncRequestBody;
2323
import software.amazon.awssdk.core.interceptor.Context;
24-
import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
2524
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
2625
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
2726
import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute;
@@ -41,33 +40,39 @@
4140
*/
4241
@SdkInternalApi
4342
public class HttpChecksumRequiredInterceptor implements ExecutionInterceptor {
44-
private static final ExecutionAttribute<String> CONTENT_MD5_VALUE = new ExecutionAttribute<>("ContentMd5");
4543

44+
/**
45+
* Calculates the MD5 checksum of the provided request (and base64 encodes it), and adds the header to the request.
46+
*
47+
* <p>Note: This assumes that the content stream provider can create multiple new streams. If it only supports one (e.g. with
48+
* an input stream that doesn't support mark/reset), we could consider buffering the content in memory here and updating the
49+
* request body to use that buffered content. We obviously don't want to do that for giant streams, so we haven't opted to do
50+
* that yet.
51+
*/
4652
@Override
47-
public void afterMarshalling(Context.AfterMarshalling context, ExecutionAttributes executionAttributes) {
53+
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) {
4854
boolean isHttpChecksumRequired = isHttpChecksumRequired(executionAttributes);
4955
boolean requestAlreadyHasMd5 = context.httpRequest().firstMatchingHeader(Header.CONTENT_MD5).isPresent();
5056

5157
Optional<RequestBody> syncContent = context.requestBody();
5258
Optional<AsyncRequestBody> asyncContent = context.asyncRequestBody();
5359

5460
if (!isHttpChecksumRequired || requestAlreadyHasMd5) {
55-
return;
61+
return context.httpRequest();
5662
}
5763

5864
if (asyncContent.isPresent()) {
5965
throw new IllegalArgumentException("This operation requires a content-MD5 checksum, but one cannot be calculated "
6066
+ "for non-blocking content.");
6167
}
6268

63-
syncContent.ifPresent(requestBody -> saveContentMd5(requestBody, executionAttributes));
64-
}
65-
66-
@Override
67-
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) {
68-
String contentMd5 = executionAttributes.getAttribute(CONTENT_MD5_VALUE);
69-
if (contentMd5 != null) {
70-
return context.httpRequest().copy(r -> r.putHeader(Header.CONTENT_MD5, contentMd5));
69+
if (syncContent.isPresent()) {
70+
try {
71+
String payloadMd5 = Md5Utils.md5AsBase64(syncContent.get().contentStreamProvider().newStream());
72+
return context.httpRequest().copy(r -> r.putHeader(Header.CONTENT_MD5, payloadMd5));
73+
} catch (IOException e) {
74+
throw new UncheckedIOException(e);
75+
}
7176
}
7277
return context.httpRequest();
7378
}
@@ -76,22 +81,4 @@ private boolean isHttpChecksumRequired(ExecutionAttributes executionAttributes)
7681
return executionAttributes.getAttribute(SdkInternalExecutionAttribute.HTTP_CHECKSUM_REQUIRED) != null
7782
|| HttpChecksumUtils.isMd5ChecksumRequired(executionAttributes);
7883
}
79-
80-
/**
81-
* Calculates the MD5 checksum of the provided request (and base64 encodes it), storing the result in
82-
* {@link #CONTENT_MD5_VALUE}.
83-
*
84-
* <p>Note: This assumes that the content stream provider can create multiple new streams. If it only supports one (e.g. with
85-
* an input stream that doesn't support mark/reset), we could consider buffering the content in memory here and updating the
86-
* request body to use that buffered content. We obviously don't want to do that for giant streams, so we haven't opted to do
87-
* that yet.
88-
*/
89-
private void saveContentMd5(RequestBody requestBody, ExecutionAttributes executionAttributes) {
90-
try {
91-
String payloadMd5 = Md5Utils.md5AsBase64(requestBody.contentStreamProvider().newStream());
92-
executionAttributes.putAttribute(CONTENT_MD5_VALUE, payloadMd5);
93-
} catch (IOException e) {
94-
throw new UncheckedIOException(e);
95-
}
96-
}
9784
}

‎test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/HttpChecksumInHeaderTest.java

Lines changed: 2 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
1919
import static org.mockito.ArgumentMatchers.any;
20-
import static software.amazon.awssdk.core.HttpChecksumConstant.HTTP_CHECKSUM_VALUE;
2120

2221
import io.reactivex.Flowable;
2322
import java.io.IOException;
@@ -28,7 +27,6 @@
2827
import java.util.Set;
2928
import java.util.concurrent.CompletableFuture;
3029
import java.util.stream.Collectors;
31-
import org.junit.After;
3230
import org.junit.Before;
3331
import org.junit.Test;
3432
import org.mockito.ArgumentCaptor;
@@ -38,9 +36,6 @@
3836
import software.amazon.awssdk.awscore.client.builder.AwsClientBuilder;
3937
import software.amazon.awssdk.awscore.client.builder.AwsSyncClientBuilder;
4038
import software.amazon.awssdk.core.checksums.Algorithm;
41-
import software.amazon.awssdk.core.interceptor.Context;
42-
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
43-
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
4439
import software.amazon.awssdk.http.ExecutableHttpRequest;
4540
import software.amazon.awssdk.http.HttpExecuteRequest;
4641
import software.amazon.awssdk.http.HttpExecuteResponse;
@@ -103,11 +98,6 @@ public void setup() throws IOException {
10398
});
10499
}
105100

106-
@After
107-
public void clear() {
108-
CaptureChecksumValueInterceptor.reset();
109-
}
110-
111101
@Test
112102
public void sync_json_nonStreaming_unsignedPayload_with_Sha1_in_header() {
113103
// jsonClient.flexibleCheckSumOperationWithShaChecksum(r -> r.stringMember("Hello world"));
@@ -118,9 +108,6 @@ public void sync_json_nonStreaming_unsignedPayload_with_Sha1_in_header() {
118108
assertThat(getSyncRequest().firstMatchingHeader("x-amz-checksum-sha1")).hasValue("M68rRwFal7o7B3KEMt3m0w39TaA=");
119109
// Assertion to make sure signer was not executed
120110
assertThat(getSyncRequest().firstMatchingHeader("x-amz-content-sha256")).isNotPresent();
121-
122-
assertThat(CaptureChecksumValueInterceptor.interceptorComputedChecksum).isEqualTo("M68rRwFal7o7B3KEMt3m0w39TaA=");
123-
124111
}
125112

126113
@Test
@@ -133,9 +120,6 @@ public void aync_json_nonStreaming_unsignedPayload_with_Sha1_in_header() {
133120
assertThat(getAsyncRequest().firstMatchingHeader("x-amz-checksum-sha1")).hasValue("M68rRwFal7o7B3KEMt3m0w39TaA=");
134121
// Assertion to make sure signer was not executed
135122
assertThat(getAsyncRequest().firstMatchingHeader("x-amz-content-sha256")).isNotPresent();
136-
assertThat(CaptureChecksumValueInterceptor.interceptorComputedChecksum).isEqualTo("M68rRwFal7o7B3KEMt3m0w39TaA=");
137-
138-
139123
}
140124

141125
@Test
@@ -148,9 +132,6 @@ public void sync_xml_nonStreaming_unsignedPayload_with_Sha1_in_header() {
148132
assertThat(getSyncRequest().firstMatchingHeader("x-amz-checksum-sha1")).hasValue("FB/utBbwFLbIIt5ul3Ojuy5dKgU=");
149133
// Assertion to make sure signer was not executed
150134
assertThat(getSyncRequest().firstMatchingHeader("x-amz-content-sha256")).isNotPresent();
151-
152-
assertThat(CaptureChecksumValueInterceptor.interceptorComputedChecksum).isEqualTo("FB/utBbwFLbIIt5ul3Ojuy5dKgU=");
153-
154135
}
155136

156137
@Test
@@ -169,9 +150,6 @@ public void sync_xml_nonStreaming_unsignedEmptyPayload_with_Sha1_in_header() {
169150

170151
// Assertion to make sure signer was not executed
171152
assertThat(getSyncRequest().firstMatchingHeader("x-amz-content-sha256")).isNotPresent();
172-
173-
assertThat(CaptureChecksumValueInterceptor.interceptorComputedChecksum).isNull();
174-
175153
}
176154

177155
@Test
@@ -185,8 +163,6 @@ public void aync_xml_nonStreaming_unsignedPayload_with_Sha1_in_header() {
185163
assertThat(getAsyncRequest().firstMatchingHeader("x-amz-checksum-sha1")).hasValue("FB/utBbwFLbIIt5ul3Ojuy5dKgU=");
186164
// Assertion to make sure signer was not executed
187165
assertThat(getAsyncRequest().firstMatchingHeader("x-amz-content-sha256")).isNotPresent();
188-
assertThat(CaptureChecksumValueInterceptor.interceptorComputedChecksum).isEqualTo("FB/utBbwFLbIIt5ul3Ojuy5dKgU=");
189-
190166
}
191167

192168
@Test
@@ -206,8 +182,6 @@ public void aync_xml_nonStreaming_unsignedEmptyPayload_with_Sha1_in_header() {
206182
assertThat(getAsyncRequest().firstMatchingHeader("x-amz-checksum-sha1")).isNotPresent();
207183
// Assertion to make sure signer was not executed
208184
assertThat(getAsyncRequest().firstMatchingHeader("x-amz-content-sha256")).isNotPresent();
209-
assertThat(CaptureChecksumValueInterceptor.interceptorComputedChecksum).isNull();
210-
211185
}
212186

213187
private SdkHttpRequest getSyncRequest() {
@@ -224,32 +198,15 @@ private SdkHttpRequest getAsyncRequest() {
224198

225199

226200
private <T extends AwsSyncClientBuilder<T, ?> & AwsClientBuilder<T, ?>> T initializeSync(T syncClientBuilder) {
227-
return initialize(syncClientBuilder.httpClient(httpClient)
228-
.overrideConfiguration(o -> o.addExecutionInterceptor(new CaptureChecksumValueInterceptor())));
201+
return initialize(syncClientBuilder.httpClient(httpClient));
229202
}
230203

231204
private <T extends AwsAsyncClientBuilder<T, ?> & AwsClientBuilder<T, ?>> T initializeAsync(T asyncClientBuilder) {
232-
return initialize(asyncClientBuilder.httpClient(httpAsyncClient)
233-
.overrideConfiguration(o -> o.addExecutionInterceptor(new CaptureChecksumValueInterceptor())));
205+
return initialize(asyncClientBuilder.httpClient(httpAsyncClient));
234206
}
235207

236208
private <T extends AwsClientBuilder<T, ?>> T initialize(T clientBuilder) {
237209
return clientBuilder.credentialsProvider(AnonymousCredentialsProvider.create())
238210
.region(Region.US_WEST_2);
239211
}
240-
241-
242-
private static class CaptureChecksumValueInterceptor implements ExecutionInterceptor {
243-
private static String interceptorComputedChecksum;
244-
245-
private static void reset() {
246-
interceptorComputedChecksum = null;
247-
}
248-
249-
@Override
250-
public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttributes executionAttributes) {
251-
interceptorComputedChecksum = executionAttributes.getAttribute(HTTP_CHECKSUM_VALUE);
252-
253-
}
254-
}
255212
}

0 commit comments

Comments
 (0)