Skip to content

Commit 9d5af05

Browse files
zoewanggzoewangg
authored
Avoid calling ContentStreamProvider#newStream when content length is known (#5866)
1 parent 24b398d commit 9d5af05

File tree

5 files changed

+108
-16
lines changed

5 files changed

+108
-16
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-535d16a.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Remove unnecessary invocation of `ContentStreamProvider#newStream` when content-length is known for requests that use AWS chunked encoding."
6+
}

‎core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/AwsChunkedV4aPayloadSigner.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public ContentStreamProvider sign(ContentStreamProvider payload, V4aRequestSigni
108108
@Override
109109
public void beforeSigning(SdkHttpRequest.Builder request, ContentStreamProvider payload, String checksum) {
110110
long encodedContentLength = 0;
111-
long contentLength = moveContentLength(request, payload != null ? payload.newStream() : new StringInputStream(""));
111+
long contentLength = moveContentLength(request, payload);
112112
setupPreExistingTrailers(request);
113113

114114
// pre-existing trailers

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
import software.amazon.awssdk.http.auth.aws.internal.signer.io.ResettableContentStreamProvider;
4848
import software.amazon.awssdk.utils.BinaryUtils;
4949
import software.amazon.awssdk.utils.Pair;
50-
import software.amazon.awssdk.utils.StringInputStream;
5150
import software.amazon.awssdk.utils.Validate;
5251

5352
/**
@@ -124,7 +123,7 @@ public Publisher<ByteBuffer> signAsync(Publisher<ByteBuffer> payload, V4RequestS
124123
@Override
125124
public void beforeSigning(SdkHttpRequest.Builder request, ContentStreamProvider payload) {
126125
long encodedContentLength = 0;
127-
long contentLength = moveContentLength(request, payload != null ? payload.newStream() : new StringInputStream(""));
126+
long contentLength = moveContentLength(request, payload);
128127
setupPreExistingTrailers(request);
129128

130129
// pre-existing trailers

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

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -198,22 +198,27 @@ public static void addDateHeader(SdkHttpRequest.Builder requestBuilder, String d
198198
* Move `Content-Length` to `x-amz-decoded-content-length` if not already present. If `Content-Length` is not present, then
199199
* the payload is read in its entirety to calculate the length.
200200
*/
201-
public static long moveContentLength(SdkHttpRequest.Builder request, InputStream payload) {
201+
public static long moveContentLength(SdkHttpRequest.Builder request, ContentStreamProvider contentStreamProvider) {
202202
Optional<String> decodedContentLength = request.firstMatchingHeader(X_AMZ_DECODED_CONTENT_LENGTH);
203-
if (!decodedContentLength.isPresent()) {
204-
// if the decoded length isn't present, content-length must be there
205-
String contentLength = request.firstMatchingHeader(Header.CONTENT_LENGTH).orElseGet(
206-
() -> String.valueOf(readAll(payload))
207-
);
208-
209-
request.putHeader(X_AMZ_DECODED_CONTENT_LENGTH, contentLength)
210-
.removeHeader(Header.CONTENT_LENGTH);
211-
return Long.parseLong(contentLength);
203+
204+
if (decodedContentLength.isPresent()) {
205+
request.removeHeader(Header.CONTENT_LENGTH);
206+
return Long.parseLong(decodedContentLength.get());
207+
}
208+
209+
long contentLength;
210+
Optional<String> contentLengthFromHeader =
211+
request.firstMatchingHeader(Header.CONTENT_LENGTH);
212+
if (contentLengthFromHeader.isPresent()) {
213+
contentLength = Long.parseLong(contentLengthFromHeader.get());
214+
} else {
215+
InputStream inputStream = contentStreamProvider.newStream();
216+
contentLength = inputStream == null ? 0 : readAll(inputStream);
212217
}
213218

214-
// decoded header is already there, so remove content-length just to be sure it's gone
215-
request.removeHeader(Header.CONTENT_LENGTH);
216-
return Long.parseLong(decodedContentLength.get());
219+
request.putHeader(X_AMZ_DECODED_CONTENT_LENGTH, String.valueOf(contentLength))
220+
.removeHeader(Header.CONTENT_LENGTH);
221+
return contentLength;
217222
}
218223

219224
public static InputStream getBinaryRequestPayloadStream(ContentStreamProvider streamProvider) {

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

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.http.auth.aws.internal.signer.util;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static software.amazon.awssdk.http.Header.CONTENT_LENGTH;
20+
import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.X_AMZ_DECODED_CONTENT_LENGTH;
21+
22+
import java.io.ByteArrayInputStream;
23+
import java.io.InputStream;
24+
import java.util.stream.Stream;
25+
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.params.ParameterizedTest;
27+
import org.junit.jupiter.params.provider.Arguments;
28+
import org.junit.jupiter.params.provider.MethodSource;
29+
import org.mockito.Mockito;
30+
import software.amazon.awssdk.http.ContentStreamProvider;
31+
import software.amazon.awssdk.http.SdkHttpRequest;
32+
33+
public class SignerUtilsTest {
34+
35+
@Test
36+
void moveContentLength_decodedContentLengthPresent_shouldNotInvokeNewStream() {
37+
SdkHttpRequest.Builder request = SdkHttpRequest.builder()
38+
.appendHeader(X_AMZ_DECODED_CONTENT_LENGTH, "10")
39+
.appendHeader(CONTENT_LENGTH, "10");
40+
41+
ContentStreamProvider streamProvider = Mockito.mock(ContentStreamProvider.class);
42+
long contentLength = SignerUtils.moveContentLength(request, streamProvider);
43+
Mockito.verify(streamProvider, Mockito.never()).newStream();
44+
assertThat(contentLength).isEqualTo(10L);
45+
assertThat(request.firstMatchingHeader(CONTENT_LENGTH)).isEmpty();
46+
assertThat(request.firstMatchingHeader(X_AMZ_DECODED_CONTENT_LENGTH)).contains("10");
47+
}
48+
49+
@Test
50+
void moveContentLength_contentLengthPresent_shouldNotInvokeNewStream() {
51+
SdkHttpRequest.Builder request = SdkHttpRequest.builder()
52+
.appendHeader(CONTENT_LENGTH, "10");
53+
54+
ContentStreamProvider streamProvider = Mockito.mock(ContentStreamProvider.class);
55+
long contentLength = SignerUtils.moveContentLength(request, streamProvider);
56+
Mockito.verify(streamProvider, Mockito.never()).newStream();
57+
assertThat(contentLength).isEqualTo(10L);
58+
assertThat(request.firstMatchingHeader(CONTENT_LENGTH)).isEmpty();
59+
assertThat(request.firstMatchingHeader(X_AMZ_DECODED_CONTENT_LENGTH)).contains("10");
60+
}
61+
62+
public static Stream<Arguments> streams() {
63+
return Stream.of(Arguments.of(new ByteArrayInputStream("hello".getBytes()), 5),
64+
Arguments.of(null, 0));
65+
}
66+
67+
68+
@ParameterizedTest
69+
@MethodSource("streams")
70+
void moveContentLength_contentLengthNotPresent_shouldInvokeNewStream(InputStream inputStream, long expectedLength) {
71+
SdkHttpRequest.Builder request = SdkHttpRequest.builder();
72+
73+
ContentStreamProvider streamProvider = Mockito.mock(ContentStreamProvider.class);
74+
Mockito.when(streamProvider.newStream()).thenReturn(inputStream);
75+
76+
long contentLength = SignerUtils.moveContentLength(request, streamProvider);
77+
Mockito.verify(streamProvider, Mockito.times(1)).newStream();
78+
assertThat(contentLength).isEqualTo(expectedLength);
79+
assertThat(request.firstMatchingHeader(CONTENT_LENGTH)).isEmpty();
80+
assertThat(request.firstMatchingHeader(X_AMZ_DECODED_CONTENT_LENGTH)).contains(String.valueOf(expectedLength));
81+
}
82+
}

0 commit comments

Comments
 (0)