Review comments

Author: dagnir

core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/AwsChunkedV4PayloadSigner.java

@@ -25,7 +25,7 @@
 import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.X_AMZ_CONTENT_SHA256;
 import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.X_AMZ_DECODED_CONTENT_LENGTH;
 import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.X_AMZ_TRAILER;
-import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerUtils.computeAndMoveContentLength;
+import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerUtils.moveContentLength;
 
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
@@ -173,7 +173,7 @@ public void beforeSigning(SdkHttpRequest.Builder request, ContentStreamProvider
     @Override
     public CompletableFuture<Pair<SdkHttpRequest.Builder, Optional<Publisher<ByteBuffer>>>> beforeSigningAsync(
         SdkHttpRequest.Builder request, Publisher<ByteBuffer> payload) {
-        return computeAndMoveContentLength(request, payload)
+        return moveContentLength(request, payload)
             .thenApply(p -> {
                 SdkHttpRequest.Builder requestBuilder = p.left();
                 setupPreExistingTrailers(requestBuilder);

core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/util/SignerUtils.java

@@ -225,10 +225,13 @@ public static long computeAndMoveContentLength(SdkHttpRequest.Builder request, C
     }
 
     /**
-     * Move `Content-Length` to `x-amz-decoded-content-length` if not already present. If `Content-Length` is not present, then
-     * the payload is read in its entirety to calculate the length.
+     * Move Content-Length` to `x-amz-decoded-content-length` if not already present. If `Content-Length` is not present, the
+     * future is completed exceptionally. Note: this behavior differs from the sync version
+     * {@link #computeAndMoveContentLength(SdkHttpRequest.Builder, ContentStreamProvider)} as the sync version reads the entire
+     * stream to compute the length if the header is not present. The async version was introduced after the sync version; moving
+     * forward, requests that have an unknown content length should be done through chunked transfer encoding.
      */
-    public static CompletableFuture<Pair<SdkHttpRequest.Builder, Optional<Publisher<ByteBuffer>>>> computeAndMoveContentLength(
+    public static CompletableFuture<Pair<SdkHttpRequest.Builder, Optional<Publisher<ByteBuffer>>>> moveContentLength(
         SdkHttpRequest.Builder request, Publisher<ByteBuffer> contentPublisher) {
         Optional<String> decodedContentLength = request.firstMatchingHeader(X_AMZ_DECODED_CONTENT_LENGTH);
 

core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/internal/signer/util/SignerUtilsTest.java

@@ -81,42 +81,42 @@ void computeAndMoveContentLength_contentLengthNotPresent_shouldInvokeNewStream(I
     }
 
     @Test
-    void computeAndMoveContentLength_async_decodedContentLengthPresent_shouldNotSubscribeToPublisher() {
+    void moveContentLength_async_decodedContentLengthPresent_shouldNotSubscribeToPublisher() {
 
         SdkHttpRequest.Builder request = SdkHttpRequest.builder()
                                                        .appendHeader(X_AMZ_DECODED_CONTENT_LENGTH, "10")
                                                        .appendHeader(CONTENT_LENGTH, "10");
 
         Publisher<ByteBuffer> contentPublisher = Mockito.spy(Flowable.empty());
 
-        SignerUtils.computeAndMoveContentLength(request, contentPublisher).join();
+        SignerUtils.moveContentLength(request, contentPublisher).join();
         Mockito.verify(contentPublisher, Mockito.never()).subscribe(Mockito.any(Subscriber.class));
 
         assertThat(request.firstMatchingHeader(CONTENT_LENGTH)).isEmpty();
         assertThat(request.firstMatchingHeader(X_AMZ_DECODED_CONTENT_LENGTH)).contains("10");
     }
 
     @Test
-    void computeAndMoveContentLength_async_contentLengthPresent_shouldNotSubscribeToPublisher() {
+    void moveContentLength_async_contentLengthPresent_shouldNotSubscribeToPublisher() {
         SdkHttpRequest.Builder request = SdkHttpRequest.builder()
                                                        .appendHeader(CONTENT_LENGTH, "10");
 
         Publisher<ByteBuffer> contentPublisher = Mockito.spy(Flowable.empty());
 
-        SignerUtils.computeAndMoveContentLength(request, contentPublisher).join();
+        SignerUtils.moveContentLength(request, contentPublisher).join();
         Mockito.verify(contentPublisher, Mockito.never()).subscribe(Mockito.any(Subscriber.class));
 
         assertThat(request.firstMatchingHeader(CONTENT_LENGTH)).isEmpty();
         assertThat(request.firstMatchingHeader(X_AMZ_DECODED_CONTENT_LENGTH)).contains("10");
     }
 
     @Test
-    void computeAndMoveContentLength_contentLengthNotPresent_throws() {
+    void moveContentLength_contentLengthNotPresent_throws() {
         SdkHttpRequest.Builder request = SdkHttpRequest.builder();
 
         Publisher<ByteBuffer> contentPublisher = Flowable.just(ByteBuffer.wrap("content".getBytes(StandardCharsets.UTF_8)));
 
-        assertThatThrownBy(() -> SignerUtils.computeAndMoveContentLength(request, contentPublisher).join())
+        assertThatThrownBy(() -> SignerUtils.moveContentLength(request, contentPublisher).join())
             .isInstanceOf(UnsupportedOperationException.class)
             .hasMessage("Content-Length header must be specified");