-
Notifications
You must be signed in to change notification settings - Fork 932
Support trailing checksum in DefaultAwsV4HttpSigner #6296
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -31,6 +31,8 @@ | |||||||||
import java.util.ArrayList; | ||||||||||
import java.util.Collections; | ||||||||||
import java.util.List; | ||||||||||
import java.util.concurrent.CompletableFuture; | ||||||||||
import java.util.stream.Collectors; | ||||||||||
import org.reactivestreams.Publisher; | ||||||||||
import software.amazon.awssdk.annotations.SdkInternalApi; | ||||||||||
import software.amazon.awssdk.checksums.SdkChecksum; | ||||||||||
|
@@ -40,11 +42,13 @@ | |||||||||
import software.amazon.awssdk.http.SdkHttpRequest; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.chunkedencoding.ChecksumTrailerProvider; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.chunkedencoding.ChunkedEncodedInputStream; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.chunkedencoding.ChunkedEncodedPublisher; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.chunkedencoding.SigV4ChunkExtensionProvider; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.chunkedencoding.SigV4TrailerProvider; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.chunkedencoding.TrailerProvider; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.io.ChecksumInputStream; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.io.ResettableContentStreamProvider; | ||||||||||
import software.amazon.awssdk.http.auth.aws.internal.signer.io.UnbufferedChecksumSubscriber; | ||||||||||
import software.amazon.awssdk.utils.BinaryUtils; | ||||||||||
import software.amazon.awssdk.utils.Pair; | ||||||||||
import software.amazon.awssdk.utils.Validate; | ||||||||||
|
@@ -116,8 +120,44 @@ public ContentStreamProvider sign(ContentStreamProvider payload, V4RequestSignin | |||||||||
|
||||||||||
@Override | ||||||||||
public Publisher<ByteBuffer> signAsync(Publisher<ByteBuffer> payload, V4RequestSigningResult requestSigningResult) { | ||||||||||
// TODO(sra-identity-and-auth): implement this first and remove addFlexibleChecksumInTrailer logic in HttpChecksumStage | ||||||||||
throw new UnsupportedOperationException(); | ||||||||||
ChunkedEncodedPublisher.Builder chunkedStreamBuilder = ChunkedEncodedPublisher.builder() | ||||||||||
.publisher(payload) | ||||||||||
.chunkSize(chunkSize) | ||||||||||
.addEmptyTrailingChunk(true); | ||||||||||
|
||||||||||
preExistingTrailers.forEach(t -> chunkedStreamBuilder.addTrailer(() -> t)); | ||||||||||
|
||||||||||
SdkHttpRequest.Builder request = requestSigningResult.getSignedRequest(); | ||||||||||
|
||||||||||
String checksum = request.firstMatchingHeader(X_AMZ_CONTENT_SHA256).orElseThrow( | ||||||||||
() -> new IllegalArgumentException(X_AMZ_CONTENT_SHA256 + " must be set!") | ||||||||||
); | ||||||||||
|
||||||||||
switch (checksum) { | ||||||||||
case STREAMING_SIGNED_PAYLOAD: { | ||||||||||
RollingSigner rollingSigner = new RollingSigner(requestSigningResult.getSigningKey(), | ||||||||||
requestSigningResult.getSignature()); | ||||||||||
chunkedStreamBuilder.addExtension(new SigV4ChunkExtensionProvider(rollingSigner, credentialScope)); | ||||||||||
break; | ||||||||||
} | ||||||||||
case STREAMING_UNSIGNED_PAYLOAD_TRAILER: | ||||||||||
setupChecksumTrailerIfNeeded(chunkedStreamBuilder); | ||||||||||
break; | ||||||||||
case STREAMING_SIGNED_PAYLOAD_TRAILER: { | ||||||||||
setupChecksumTrailerIfNeeded(chunkedStreamBuilder); | ||||||||||
RollingSigner rollingSigner = new RollingSigner(requestSigningResult.getSigningKey(), | ||||||||||
requestSigningResult.getSignature()); | ||||||||||
chunkedStreamBuilder.addExtension(new SigV4ChunkExtensionProvider(rollingSigner, credentialScope)); | ||||||||||
chunkedStreamBuilder.addTrailer( | ||||||||||
new SigV4TrailerProvider(chunkedStreamBuilder.trailers(), rollingSigner, credentialScope) | ||||||||||
); | ||||||||||
break; | ||||||||||
} | ||||||||||
default: | ||||||||||
throw new UnsupportedOperationException(); | ||||||||||
} | ||||||||||
|
||||||||||
return chunkedStreamBuilder.build(); | ||||||||||
} | ||||||||||
|
||||||||||
@Override | ||||||||||
|
@@ -127,27 +167,66 @@ public void beforeSigning(SdkHttpRequest.Builder request, ContentStreamProvider | |||||||||
setupPreExistingTrailers(request); | ||||||||||
|
||||||||||
// pre-existing trailers | ||||||||||
encodedContentLength = calculateEncodedContentLength(request, contentLength); | ||||||||||
|
||||||||||
if (checksumAlgorithm != null) { | ||||||||||
String checksumHeaderName = checksumHeaderName(checksumAlgorithm); | ||||||||||
request.appendHeader(X_AMZ_TRAILER, checksumHeaderName); | ||||||||||
} | ||||||||||
request.putHeader(Header.CONTENT_LENGTH, Long.toString(encodedContentLength)); | ||||||||||
request.appendHeader(CONTENT_ENCODING, AWS_CHUNKED); | ||||||||||
} | ||||||||||
|
||||||||||
@Override | ||||||||||
public CompletableFuture<Pair<SdkHttpRequest.Builder, Publisher<ByteBuffer>>> beforeSigningAsync( | ||||||||||
SdkHttpRequest.Builder request, Publisher<ByteBuffer> payload) { | ||||||||||
return moveContentLength(request, payload) | ||||||||||
.thenApply(p -> { | ||||||||||
SdkHttpRequest.Builder requestBuilder = p.left(); | ||||||||||
setupPreExistingTrailers(requestBuilder); | ||||||||||
|
||||||||||
long decodedContentLength = requestBuilder.firstMatchingHeader("x-amz-decoded-content-length") | ||||||||||
.map(Long::parseLong) | ||||||||||
// should not happen, this header is added by moveContentLength | ||||||||||
.orElseThrow(() -> new RuntimeException("x-amz-decoded-content-length " | ||||||||||
+ "header not present")); | ||||||||||
|
||||||||||
long encodedContentLength = calculateEncodedContentLength(request, decodedContentLength); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable 'request' is being passed to calculateEncodedContentLength, but it should be 'requestBuilder' which is the actual SdkHttpRequest.Builder instance being used in this context.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||
|
||||||||||
if (checksumAlgorithm != null) { | ||||||||||
String checksumHeaderName = checksumHeaderName(checksumAlgorithm); | ||||||||||
request.appendHeader(X_AMZ_TRAILER, checksumHeaderName); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable 'request' should be 'requestBuilder' to maintain consistency with the rest of the method and ensure the header is added to the correct request builder instance.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||
} | ||||||||||
request.putHeader(Header.CONTENT_LENGTH, Long.toString(encodedContentLength)); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable 'request' should be 'requestBuilder' to maintain consistency and ensure the header is set on the correct request builder instance.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||
request.appendHeader(CONTENT_ENCODING, AWS_CHUNKED); | ||||||||||
Comment on lines
+200
to
+201
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable 'request' should be 'requestBuilder' to maintain consistency and ensure the header is appended to the correct request builder instance.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||
return Pair.of(requestBuilder, p.right()); | ||||||||||
}); | ||||||||||
} | ||||||||||
|
||||||||||
private long calculateEncodedContentLength(SdkHttpRequest.Builder requestBuilder, long decodedContentLength) { | ||||||||||
long encodedContentLength = 0; | ||||||||||
|
||||||||||
encodedContentLength += calculateExistingTrailersLength(); | ||||||||||
|
||||||||||
String checksum = request.firstMatchingHeader(X_AMZ_CONTENT_SHA256).orElseThrow( | ||||||||||
String checksum = requestBuilder.firstMatchingHeader(X_AMZ_CONTENT_SHA256).orElseThrow( | ||||||||||
() -> new IllegalArgumentException(X_AMZ_CONTENT_SHA256 + " must be set!") | ||||||||||
); | ||||||||||
|
||||||||||
switch (checksum) { | ||||||||||
case STREAMING_SIGNED_PAYLOAD: { | ||||||||||
long extensionsLength = 81; // ;chunk-signature:<sigv4 hex signature, 64 bytes> | ||||||||||
encodedContentLength += calculateChunksLength(contentLength, extensionsLength); | ||||||||||
encodedContentLength += calculateChunksLength(decodedContentLength, extensionsLength); | ||||||||||
break; | ||||||||||
} | ||||||||||
case STREAMING_UNSIGNED_PAYLOAD_TRAILER: | ||||||||||
if (checksumAlgorithm != null) { | ||||||||||
encodedContentLength += calculateChecksumTrailerLength(checksumHeaderName(checksumAlgorithm)); | ||||||||||
} | ||||||||||
encodedContentLength += calculateChunksLength(contentLength, 0); | ||||||||||
encodedContentLength += calculateChunksLength(decodedContentLength, 0); | ||||||||||
break; | ||||||||||
case STREAMING_SIGNED_PAYLOAD_TRAILER: { | ||||||||||
long extensionsLength = 81; // ;chunk-signature:<sigv4 hex signature, 64 bytes> | ||||||||||
encodedContentLength += calculateChunksLength(contentLength, extensionsLength); | ||||||||||
encodedContentLength += calculateChunksLength(decodedContentLength, extensionsLength); | ||||||||||
if (checksumAlgorithm != null) { | ||||||||||
encodedContentLength += calculateChecksumTrailerLength(checksumHeaderName(checksumAlgorithm)); | ||||||||||
} | ||||||||||
|
@@ -161,12 +240,7 @@ public void beforeSigning(SdkHttpRequest.Builder request, ContentStreamProvider | |||||||||
// terminating \r\n | ||||||||||
encodedContentLength += 2; | ||||||||||
|
||||||||||
if (checksumAlgorithm != null) { | ||||||||||
String checksumHeaderName = checksumHeaderName(checksumAlgorithm); | ||||||||||
request.appendHeader(X_AMZ_TRAILER, checksumHeaderName); | ||||||||||
} | ||||||||||
request.putHeader(Header.CONTENT_LENGTH, Long.toString(encodedContentLength)); | ||||||||||
request.appendHeader(CONTENT_ENCODING, AWS_CHUNKED); | ||||||||||
return encodedContentLength; | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
|
@@ -271,6 +345,24 @@ private void setupChecksumTrailerIfNeeded(ChunkedEncodedInputStream.Builder buil | |||||||||
builder.inputStream(checksumInputStream).addTrailer(checksumTrailer); | ||||||||||
} | ||||||||||
|
||||||||||
private void setupChecksumTrailerIfNeeded(ChunkedEncodedPublisher.Builder builder) { | ||||||||||
if (checksumAlgorithm != null) { | ||||||||||
String checksumHeaderName = checksumHeaderName(checksumAlgorithm); | ||||||||||
SdkChecksum sdkChecksum = fromChecksumAlgorithm(checksumAlgorithm); | ||||||||||
Publisher<ByteBuffer> checksummedPayload = computeChecksum(builder.publisher(), sdkChecksum); | ||||||||||
|
||||||||||
builder.publisher(checksummedPayload); | ||||||||||
|
||||||||||
TrailerProvider checksumTrailer = new ChecksumTrailerProvider(sdkChecksum, checksumHeaderName); | ||||||||||
builder.addTrailer(checksumTrailer); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
private Publisher<ByteBuffer> computeChecksum(Publisher<ByteBuffer> publisher, SdkChecksum checksum) { | ||||||||||
return subscriber -> publisher.subscribe( | ||||||||||
new UnbufferedChecksumSubscriber(Collections.singletonList(checksum), subscriber)); | ||||||||||
} | ||||||||||
|
||||||||||
static class Builder { | ||||||||||
private CredentialScope credentialScope; | ||||||||||
private Integer chunkSize; | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should be more descriptive and use a more specific exception type. Consider using IllegalStateException instead of RuntimeException and improve the message clarity.
Copilot uses AI. Check for mistakes.