Skip to content

Add check for SRA codepath to HttpChecksumStage #4487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
haydenbaker wants to merge 2 commits into from
Closed

Add check for SRA codepath to HttpChecksumStage #4487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d0fbe9
Add check for SRA codepath to HttpChecksumStage
haydenbaker Sep 26, 2023
beff03b
Add test for sra path
haydenbaker Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ...ain/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static software.amazon.awssdk.core.HttpChecksumConstant.CONTENT_SHA_256_FOR_UNSIGNED_TRAILER;
import static software.amazon.awssdk.core.HttpChecksumConstant.DEFAULT_ASYNC_CHUNK_SIZE;
import static software.amazon.awssdk.core.HttpChecksumConstant.SIGNING_METHOD;
import static software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute.AUTH_SCHEMES;
import static software.amazon.awssdk.core.internal.io.AwsChunkedEncodingInputStream.DEFAULT_CHUNK_SIZE;
import static software.amazon.awssdk.core.internal.util.ChunkContentUtils.calculateChecksumTrailerLength;
import static software.amazon.awssdk.core.internal.util.ChunkContentUtils.calculateStreamContentLength;
Expand Down Expand Up @@ -67,6 +68,11 @@ public SdkHttpFullRequest.Builder execute(SdkHttpFullRequest.Builder request, Re
return request;
}

// Check for SRA code-path
Copy link
Contributor

@zoewangg zoewangg Sep 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we'll still need to keep this class to add md5 checksum header (after SRA is fully released)?

Copy link
Contributor Author

@haydenbaker haydenbaker Sep 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. It appears this behavior was originally in an interceptor: https://github.com/aws/aws-sdk-java-v2/blob/78d2897f73734113071915a56bbdba78add4d5a6/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/interceptor/HttpChecksumRequiredInterceptor.java

So we either add that interceptor back or keep this stage.

if (context.executionAttributes().getAttribute(AUTH_SCHEMES) != null) {
return request;
}

ChecksumSpecs resolvedChecksumSpecs = getResolvedChecksumSpecs(context.executionAttributes());

if (flexibleChecksumInTrailerRequired(context, resolvedChecksumSpecs)) {
Expand Down
39 changes: 35 additions & 4 deletions ...java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static software.amazon.awssdk.core.HttpChecksumConstant.HEADER_FOR_TRAILER_REFERENCE;
import static software.amazon.awssdk.core.HttpChecksumConstant.SIGNING_METHOD;
import static software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute.AUTH_SCHEMES;
import static software.amazon.awssdk.core.internal.signer.SigningMethod.UNSIGNED_PAYLOAD;
import static software.amazon.awssdk.http.Header.CONTENT_LENGTH;
import static software.amazon.awssdk.http.Header.CONTENT_MD5;

import java.util.Collections;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;
Expand Down Expand Up @@ -87,14 +89,12 @@ public void async_nonStreaming_md5Required_addsMd5Checksum_doesNotAddFlexibleChe
}

@Test
public void async_streaming_md5Required_throws_IllegalArgumentException() throws Exception {
public void async_streaming_md5Required_throws_IllegalArgumentException() {
SdkHttpFullRequest.Builder requestBuilder = createHttpRequestBuilder();
boolean isAsyncStreaming = true;
RequestExecutionContext ctx = md5RequiredRequestContext(isAsyncStreaming);

Exception exception = assertThrows(IllegalArgumentException.class, () -> {
asyncStage.execute(requestBuilder, ctx);
});
Exception exception = assertThrows(IllegalArgumentException.class, () -> asyncStage.execute(requestBuilder, ctx));

assertThat(exception.getMessage()).isEqualTo("This operation requires a content-MD5 checksum, but one cannot be "
+ "calculated for non-blocking content.");
Expand Down Expand Up @@ -172,6 +172,22 @@ public void async_flexibleChecksumInHeaderRequired_addsFlexibleChecksumInHeader_
assertThat(requestBuilder.firstMatchingHeader(CONTENT_MD5)).isEmpty();
}

@Test
public void execute_whenSraCodepath_doesNotAddChecksumOrChunkEncode() throws Exception {
SdkHttpFullRequest.Builder requestBuilder = createHttpRequestBuilder();
RequestExecutionContext ctx = sraRequestContext();

asyncStage.execute(requestBuilder, ctx);

assertThat(requestBuilder.firstMatchingHeader(CHECKSUM_SPECS_HEADER)).isEmpty();
assertThat(requestBuilder.firstMatchingHeader(HEADER_FOR_TRAILER_REFERENCE)).isEmpty();
assertThat(requestBuilder.firstMatchingHeader("Content-encoding")).isEmpty();
assertThat(requestBuilder.firstMatchingHeader("x-amz-content-sha256")).isEmpty();
assertThat(requestBuilder.firstMatchingHeader("x-amz-decoded-content-length")).isEmpty();
assertThat(requestBuilder.firstMatchingHeader(CONTENT_LENGTH)).isEmpty();
assertThat(requestBuilder.firstMatchingHeader(CONTENT_MD5)).isEmpty();
}

private SdkHttpFullRequest.Builder createHttpRequestBuilder() {
return SdkHttpFullRequest.builder().contentStreamProvider(REQUEST_BODY.contentStreamProvider());
}
Expand Down Expand Up @@ -245,6 +261,21 @@ private RequestExecutionContext asyncFlexibleChecksumRequiredRequestContext(bool
return createRequestExecutionContext(executionAttributes, interceptorContextBuilder.build(), isStreaming);
}

private RequestExecutionContext sraRequestContext() {
ExecutionAttributes executionAttributes =
ExecutionAttributes.builder()
.put(AUTH_SCHEMES, Collections.emptyMap())
.build();

InterceptorContext.Builder interceptorContextBuilder =
InterceptorContext.builder()
.request(NoopTestRequest.builder().build())
.httpRequest(ValidSdkObjects.sdkHttpFullRequest().build())
.requestBody(REQUEST_BODY);

return createRequestExecutionContext(executionAttributes, interceptorContextBuilder.build(), false);
}

private RequestExecutionContext createRequestExecutionContext(ExecutionAttributes executionAttributes,
InterceptorContext interceptorContext,
boolean isAsyncStreaming) {
Expand Down