Skip to content

Commit e3cbe0f

Browse files
millemsmillems
committed
Treat zero-byte responses from HTTP clients as not having content when it comes to creating an SDK HTTP full request.
This fixes issues like #1216 where HEAD request responses have a content-length but no payload, confusing the SDK.
1 parent e49ac56 commit e3cbe0f

File tree

7 files changed

+122
-65
lines changed

7 files changed

+122
-65
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-757a115.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": "Treat zero-byte responses from async HTTP clients as not having a payload, regardless of the response content-length. This fixes an issue that could cause HEAD responses (e.g. s3's headObject responses) with a content-length specified from being treated as having a payload. This fixes issues like [#1216](https://github.com/aws/aws-sdk-java-v2/issues/1216) where the SDK attempts to read data from the response based on the content-length, not based on whether there was actually a payload."
6+
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/async/AsyncResponseHandler.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,12 @@ public void onError(Throwable err) {
8080
public CompletableFuture<T> prepare() {
8181
streamFuture = new CompletableFuture<>();
8282
return streamFuture.thenCompose(baos -> {
83-
ByteArrayInputStream content = new ByteArrayInputStream(baos.toByteArray());
84-
// Ignore aborts - we already have all of the content.
85-
AbortableInputStream abortableContent = AbortableInputStream.create(content);
86-
httpResponse.content(abortableContent);
83+
byte[] responseBytes = baos.toByteArray();
84+
if (responseBytes.length > 0) {
85+
// Ignore aborts - we already have all of the content.
86+
httpResponse.content(AbortableInputStream.create(new ByteArrayInputStream(responseBytes)));
87+
}
88+
8789
try {
8890
return CompletableFuture.completedFuture(responseHandler.handle(crc32Validator.apply(httpResponse.build()),
8991
executionAttributes));

‎http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public void teardown() throws InterruptedException {
9090
@Test
9191
public void serviceReturn500_newRequestShouldUseNewConnection() {
9292
server.return500OnFirstRequest = true;
93-
CompletableFuture<Void> firstRequest = sendGetRequest(server.port(), netty);
93+
CompletableFuture<?> firstRequest = sendGetRequest(server.port(), netty);
9494
firstRequest.join();
9595

9696
sendGetRequest(server.port(), netty).join();
@@ -100,7 +100,7 @@ public void serviceReturn500_newRequestShouldUseNewConnection() {
100100
@Test
101101
public void serviceReturn200_newRequestShouldReuseNewConnection() {
102102
server.return500OnFirstRequest = false;
103-
CompletableFuture<Void> firstRequest = sendGetRequest(server.port(), netty);
103+
CompletableFuture<?> firstRequest = sendGetRequest(server.port(), netty);
104104
firstRequest.join();
105105

106106
sendGetRequest(server.port(), netty).join();

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/HeadObjectIntegrationTest.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
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.services.s3;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
20+
21+
import java.io.ByteArrayOutputStream;
22+
import java.io.IOException;
23+
import java.nio.charset.StandardCharsets;
24+
import java.util.zip.GZIPOutputStream;
25+
import org.junit.BeforeClass;
26+
import org.junit.Test;
27+
import software.amazon.awssdk.core.sync.RequestBody;
28+
import software.amazon.awssdk.services.s3.model.HeadObjectRequest;
29+
import software.amazon.awssdk.services.s3.model.HeadObjectResponse;
30+
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
31+
32+
public class HeadObjectIntegrationTest extends S3IntegrationTestBase {
33+
private static final String BUCKET = temporaryBucketName(HeadObjectIntegrationTest.class);
34+
35+
private static final String GZIPPED_KEY = "some-key";
36+
37+
@BeforeClass
38+
public static void setupFixture() throws IOException {
39+
createBucket(BUCKET);
40+
41+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
42+
GZIPOutputStream gzos = new GZIPOutputStream(baos);
43+
gzos.write("Test".getBytes(StandardCharsets.UTF_8));
44+
45+
s3.putObject(PutObjectRequest.builder()
46+
.bucket(BUCKET)
47+
.key(GZIPPED_KEY)
48+
.contentEncoding("gzip")
49+
.build(),
50+
RequestBody.fromBytes(baos.toByteArray()));
51+
}
52+
53+
@Test
54+
public void asyncClientSupportsGzippedObjects() {
55+
HeadObjectResponse response = s3Async.headObject(r -> r.bucket(BUCKET).key(GZIPPED_KEY)).join();
56+
assertThat(response.contentEncoding()).isEqualTo("gzip");
57+
}
58+
59+
@Test
60+
public void syncClientSupportsGzippedObjects() {
61+
HeadObjectResponse response = s3.headObject(r -> r.bucket(BUCKET).key(GZIPPED_KEY));
62+
assertThat(response.contentEncoding()).isEqualTo("gzip");
63+
}
64+
}

‎services/s3/src/it/resources/log4j2.xml

Lines changed: 0 additions & 31 deletions
This file was deleted.

‎test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpTestUtils.java

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import com.github.tomakehurst.wiremock.WireMockServer;
2323
import io.reactivex.Flowable;
24+
import java.io.ByteArrayOutputStream;
2425
import java.io.InputStream;
2526
import java.net.URL;
2627
import java.nio.ByteBuffer;
@@ -38,6 +39,7 @@
3839
import software.amazon.awssdk.http.async.SdkAsyncHttpClient;
3940
import software.amazon.awssdk.http.async.SdkAsyncHttpResponseHandler;
4041
import software.amazon.awssdk.http.async.SdkHttpContentPublisher;
42+
import software.amazon.awssdk.utils.BinaryUtils;
4143

4244
public class HttpTestUtils {
4345
private HttpTestUtils() {
@@ -64,36 +66,44 @@ public static KeyStore getSelfSignedKeyStore() throws Exception {
6466
return keyStore;
6567
}
6668

67-
public static CompletableFuture<Void> sendGetRequest(int serverPort, SdkAsyncHttpClient client) {
68-
AsyncExecuteRequest req = AsyncExecuteRequest.builder()
69-
.responseHandler(new SdkAsyncHttpResponseHandler() {
70-
private SdkHttpResponse headers;
71-
72-
@Override
73-
public void onHeaders(SdkHttpResponse headers) {
74-
this.headers = headers;
75-
}
69+
public static CompletableFuture<byte[]> sendGetRequest(int serverPort, SdkAsyncHttpClient client) {
70+
return sendRequest(serverPort, client, SdkHttpMethod.GET);
71+
}
7672

77-
@Override
78-
public void onStream(Publisher<ByteBuffer> stream) {
79-
Flowable.fromPublisher(stream).forEach(b -> {
80-
});
81-
}
73+
public static CompletableFuture<byte[]> sendHeadRequest(int serverPort, SdkAsyncHttpClient client) {
74+
return sendRequest(serverPort, client, SdkHttpMethod.HEAD);
75+
}
8276

83-
@Override
84-
public void onError(Throwable error) {
85-
}
86-
})
87-
.request(SdkHttpFullRequest.builder()
88-
.method(SdkHttpMethod.GET)
89-
.protocol("https")
90-
.host("127.0.0.1")
91-
.port(serverPort)
92-
.build())
93-
.requestContentPublisher(new EmptyPublisher())
94-
.build();
95-
96-
return client.execute(req);
77+
private static CompletableFuture<byte[]> sendRequest(int serverPort,
78+
SdkAsyncHttpClient client,
79+
SdkHttpMethod httpMethod) {
80+
ByteArrayOutputStream data = new ByteArrayOutputStream();
81+
return client.execute(AsyncExecuteRequest.builder()
82+
.responseHandler(new SdkAsyncHttpResponseHandler() {
83+
@Override
84+
public void onHeaders(SdkHttpResponse headers) {
85+
}
86+
87+
@Override
88+
public void onStream(Publisher<ByteBuffer> stream) {
89+
Flowable.fromPublisher(stream).forEach(b -> {
90+
data.write(BinaryUtils.copyAllBytesFrom(b));
91+
});
92+
}
93+
94+
@Override
95+
public void onError(Throwable error) {
96+
}
97+
})
98+
.request(SdkHttpFullRequest.builder()
99+
.method(httpMethod)
100+
.protocol("https")
101+
.host("127.0.0.1")
102+
.port(serverPort)
103+
.build())
104+
.requestContentPublisher(new EmptyPublisher())
105+
.build())
106+
.thenApply(v -> data.toByteArray());
97107
}
98108

99109
public static SdkHttpContentPublisher createProvider(String body) {

‎test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@ public void connectionReceiveCloseHeaderShouldNotReuseConnection() throws Interr
118118
assertThat(server.channels.size()).isEqualTo(2);
119119
}
120120

121+
@Test
122+
public void headRequestResponsesHaveNoPayload() {
123+
byte[] responseData = HttpTestUtils.sendHeadRequest(server.port(), client).join();
124+
assertThat(responseData).hasSize(0);
125+
}
126+
121127
private static class Server extends ChannelInitializer<Channel> {
122128
private static final byte[] CONTENT = "helloworld".getBytes(StandardCharsets.UTF_8);
123129
private ServerBootstrap bootstrap;

0 commit comments

Comments
 (0)