Skip to content

Commit

Permalink
Treat zero-byte responses from HTTP clients as not having content whe…
Browse files Browse the repository at this point in the history
…n 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.
  • Loading branch information
millems committed Dec 14, 2020
1 parent e49ac56 commit e3cbe0f
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 65 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-757a115.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS SDK for Java v2",
"contributor": "",
"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."
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,12 @@ public void onError(Throwable err) {
public CompletableFuture<T> prepare() {
streamFuture = new CompletableFuture<>();
return streamFuture.thenCompose(baos -> {
ByteArrayInputStream content = new ByteArrayInputStream(baos.toByteArray());
// Ignore aborts - we already have all of the content.
AbortableInputStream abortableContent = AbortableInputStream.create(content);
httpResponse.content(abortableContent);
byte[] responseBytes = baos.toByteArray();
if (responseBytes.length > 0) {
// Ignore aborts - we already have all of the content.
httpResponse.content(AbortableInputStream.create(new ByteArrayInputStream(responseBytes)));
}

try {
return CompletableFuture.completedFuture(responseHandler.handle(crc32Validator.apply(httpResponse.build()),
executionAttributes));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public void teardown() throws InterruptedException {
@Test
public void serviceReturn500_newRequestShouldUseNewConnection() {
server.return500OnFirstRequest = true;
CompletableFuture<Void> firstRequest = sendGetRequest(server.port(), netty);
CompletableFuture<?> firstRequest = sendGetRequest(server.port(), netty);
firstRequest.join();

sendGetRequest(server.port(), netty).join();
Expand All @@ -100,7 +100,7 @@ public void serviceReturn500_newRequestShouldUseNewConnection() {
@Test
public void serviceReturn200_newRequestShouldReuseNewConnection() {
server.return500OnFirstRequest = false;
CompletableFuture<Void> firstRequest = sendGetRequest(server.port(), netty);
CompletableFuture<?> firstRequest = sendGetRequest(server.port(), netty);
firstRequest.join();

sendGetRequest(server.port(), netty).join();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.awssdk.services.s3;

import static org.assertj.core.api.Assertions.assertThat;
import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.zip.GZIPOutputStream;
import org.junit.BeforeClass;
import org.junit.Test;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.services.s3.model.HeadObjectRequest;
import software.amazon.awssdk.services.s3.model.HeadObjectResponse;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;

public class HeadObjectIntegrationTest extends S3IntegrationTestBase {
private static final String BUCKET = temporaryBucketName(HeadObjectIntegrationTest.class);

private static final String GZIPPED_KEY = "some-key";

@BeforeClass
public static void setupFixture() throws IOException {
createBucket(BUCKET);

ByteArrayOutputStream baos = new ByteArrayOutputStream();
GZIPOutputStream gzos = new GZIPOutputStream(baos);
gzos.write("Test".getBytes(StandardCharsets.UTF_8));

s3.putObject(PutObjectRequest.builder()
.bucket(BUCKET)
.key(GZIPPED_KEY)
.contentEncoding("gzip")
.build(),
RequestBody.fromBytes(baos.toByteArray()));
}

@Test
public void asyncClientSupportsGzippedObjects() {
HeadObjectResponse response = s3Async.headObject(r -> r.bucket(BUCKET).key(GZIPPED_KEY)).join();
assertThat(response.contentEncoding()).isEqualTo("gzip");
}

@Test
public void syncClientSupportsGzippedObjects() {
HeadObjectResponse response = s3.headObject(r -> r.bucket(BUCKET).key(GZIPPED_KEY));
assertThat(response.contentEncoding()).isEqualTo("gzip");
}
}
31 changes: 0 additions & 31 deletions services/s3/src/it/resources/log4j2.xml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.github.tomakehurst.wiremock.WireMockServer;
import io.reactivex.Flowable;
import java.io.ByteArrayOutputStream;
import java.io.InputStream;
import java.net.URL;
import java.nio.ByteBuffer;
Expand All @@ -38,6 +39,7 @@
import software.amazon.awssdk.http.async.SdkAsyncHttpClient;
import software.amazon.awssdk.http.async.SdkAsyncHttpResponseHandler;
import software.amazon.awssdk.http.async.SdkHttpContentPublisher;
import software.amazon.awssdk.utils.BinaryUtils;

public class HttpTestUtils {
private HttpTestUtils() {
Expand All @@ -64,36 +66,44 @@ public static KeyStore getSelfSignedKeyStore() throws Exception {
return keyStore;
}

public static CompletableFuture<Void> sendGetRequest(int serverPort, SdkAsyncHttpClient client) {
AsyncExecuteRequest req = AsyncExecuteRequest.builder()
.responseHandler(new SdkAsyncHttpResponseHandler() {
private SdkHttpResponse headers;

@Override
public void onHeaders(SdkHttpResponse headers) {
this.headers = headers;
}
public static CompletableFuture<byte[]> sendGetRequest(int serverPort, SdkAsyncHttpClient client) {
return sendRequest(serverPort, client, SdkHttpMethod.GET);
}

@Override
public void onStream(Publisher<ByteBuffer> stream) {
Flowable.fromPublisher(stream).forEach(b -> {
});
}
public static CompletableFuture<byte[]> sendHeadRequest(int serverPort, SdkAsyncHttpClient client) {
return sendRequest(serverPort, client, SdkHttpMethod.HEAD);
}

@Override
public void onError(Throwable error) {
}
})
.request(SdkHttpFullRequest.builder()
.method(SdkHttpMethod.GET)
.protocol("https")
.host("127.0.0.1")
.port(serverPort)
.build())
.requestContentPublisher(new EmptyPublisher())
.build();

return client.execute(req);
private static CompletableFuture<byte[]> sendRequest(int serverPort,
SdkAsyncHttpClient client,
SdkHttpMethod httpMethod) {
ByteArrayOutputStream data = new ByteArrayOutputStream();
return client.execute(AsyncExecuteRequest.builder()
.responseHandler(new SdkAsyncHttpResponseHandler() {
@Override
public void onHeaders(SdkHttpResponse headers) {
}

@Override
public void onStream(Publisher<ByteBuffer> stream) {
Flowable.fromPublisher(stream).forEach(b -> {
data.write(BinaryUtils.copyAllBytesFrom(b));
});
}

@Override
public void onError(Throwable error) {
}
})
.request(SdkHttpFullRequest.builder()
.method(httpMethod)
.protocol("https")
.host("127.0.0.1")
.port(serverPort)
.build())
.requestContentPublisher(new EmptyPublisher())
.build())
.thenApply(v -> data.toByteArray());
}

public static SdkHttpContentPublisher createProvider(String body) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ public void connectionReceiveCloseHeaderShouldNotReuseConnection() throws Interr
assertThat(server.channels.size()).isEqualTo(2);
}

@Test
public void headRequestResponsesHaveNoPayload() {
byte[] responseData = HttpTestUtils.sendHeadRequest(server.port(), client).join();
assertThat(responseData).hasSize(0);
}

private static class Server extends ChannelInitializer<Channel> {
private static final byte[] CONTENT = "helloworld".getBytes(StandardCharsets.UTF_8);
private ServerBootstrap bootstrap;
Expand Down

0 comments on commit e3cbe0f

Please sign in to comment.