Skip to content
Merged
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSCRTHTTPClient-60a104f.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS CRT HTTP Client",
"contributor": "",
"description": "Add content-length header to the Request if the Request with Request body does not have one."
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,22 @@

package software.amazon.awssdk.http.crt.internal.request;

import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.crt.http.HttpHeader;
import software.amazon.awssdk.crt.http.HttpRequest;
import software.amazon.awssdk.http.ContentStreamProvider;
import software.amazon.awssdk.http.Header;
import software.amazon.awssdk.http.HttpExecuteRequest;
import software.amazon.awssdk.http.SdkHttpRequest;
import software.amazon.awssdk.http.async.AsyncExecuteRequest;
import software.amazon.awssdk.http.crt.internal.CrtAsyncRequestContext;
import software.amazon.awssdk.http.crt.internal.CrtRequestContext;
import software.amazon.awssdk.utils.IoUtils;

@SdkInternalApi
public final class CrtRequestAdapter {
Expand Down Expand Up @@ -131,11 +134,22 @@ private static List<HttpHeader> createHttpHeaderList(URI uri, HttpExecuteRequest
crtHeaderList.add(new HttpHeader(Header.CONNECTION, Header.KEEP_ALIVE_VALUE));
}

// We assume content length was set by the caller if a stream was present, so don't set it here.
if (!sdkRequest.firstMatchingHeader(Header.CONTENT_LENGTH).isPresent()
&& sdkExecuteRequest.contentStreamProvider().isPresent()) {
String contentLength = String.valueOf(calculateContentLength(sdkExecuteRequest.contentStreamProvider().get()));
crtHeaderList.add(new HttpHeader(Header.CONTENT_LENGTH, contentLength));
}

// Add the rest of the Headers
sdkRequest.forEachHeader((key, value) -> value.stream().map(val -> new HttpHeader(key, val)).forEach(crtHeaderList::add));

return crtHeaderList;
}

public static long calculateContentLength(ContentStreamProvider contentStreamProvider) {
try {
return IoUtils.toByteArray(contentStreamProvider.newStream()).length ;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wont that buffer the whole body content in memory? Do we really want to do that?

@joviegas joviegas Feb 13, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to rework on this , due to internal comment from Matthew Miller to handle this in Marshallers. Will update the PR with that approach.

} catch (IOException e) {
throw new RuntimeException("Cannot get the content-length of the request content.");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/*
* 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.http.crt;

import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
import static com.github.tomakehurst.wiremock.client.WireMock.put;
import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
import static java.util.Collections.emptyMap;
import static org.assertj.core.api.Assertions.assertThat;

import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
import java.io.ByteArrayInputStream;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.Map;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import software.amazon.awssdk.crt.CrtResource;
import software.amazon.awssdk.crt.io.EventLoopGroup;
import software.amazon.awssdk.crt.io.HostResolver;
import software.amazon.awssdk.http.ExecutableHttpRequest;
import software.amazon.awssdk.http.HttpExecuteRequest;
import software.amazon.awssdk.http.HttpExecuteResponse;
import software.amazon.awssdk.http.SdkHttpClient;
import software.amazon.awssdk.http.SdkHttpFullRequest;
import software.amazon.awssdk.http.SdkHttpMethod;
import software.amazon.awssdk.http.SdkHttpRequest;

public class AwsCrtSyncWireMockTest {
private final WireMockServer mockServer = new WireMockServer(new WireMockConfiguration()
.dynamicPort()
.dynamicHttpsPort());
private SdkHttpClient client;

@BeforeEach
public void setup() {
mockServer.start();
mockServer.stubFor(put(urlMatching(".*")).willReturn(aResponse().withStatus(200).withBody("hello")));
client = AwsCrtHttpClient.builder().build();
}

@AfterEach
public void teardown() {
mockServer.stop();
client.close();
EventLoopGroup.closeStaticDefault();
HostResolver.closeStaticDefault();
CrtResource.waitForNoResources();
}

@Test
public void sdkAddsContentLength_For_Requests_with_RequestBody() throws Throwable {
URI uri = URI.create("http://localhost:" + mockServer.port());
String stringRequestBody = "Body";
byte[] reqBody = stringRequestBody.getBytes(StandardCharsets.UTF_8);
String resourcePath = "/server/test";

Map<String, String> params = emptyMap();
SdkHttpRequest request = SdkHttpFullRequest.builder()
.uri(uri)
.method(SdkHttpMethod.PUT)
.contentStreamProvider(() -> new ByteArrayInputStream(reqBody))
.encodedPath(resourcePath)
.applyMutation(b -> params.forEach(b::putRawQueryParameter))
.applyMutation(b ->
b.putHeader("Host", uri.getHost())).build();
HttpExecuteRequest.Builder executeRequestBuilder = HttpExecuteRequest.builder();
executeRequestBuilder.request(request);
executeRequestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(reqBody));
ExecutableHttpRequest executableRequest = client.prepareRequest(executeRequestBuilder.build());
HttpExecuteResponse response = executableRequest.call();
assertThat(response.httpResponse().statusCode()).isEqualTo(200);
mockServer.verify(putRequestedFor(urlPathEqualTo(resourcePath))
.withRequestBody(equalTo(stringRequestBody))
.withHeader("content-length",
equalTo(String.valueOf(stringRequestBody.length()))));
}

@Test
public void sdkDoesNotAddsContentLength_For_Requests_with_NoRequestBody() throws Throwable {
URI uri = URI.create("http://localhost:" + mockServer.port());
String stringRequestBody = "Body";
String resourcePath = "/server/test";

Map<String, String> params = emptyMap();
SdkHttpRequest request = SdkHttpFullRequest.builder()
.uri(uri)
.method(SdkHttpMethod.PUT)
.encodedPath(resourcePath)
.applyMutation(b -> params.forEach(b::putRawQueryParameter))
.applyMutation(b ->
b.putHeader("Host", uri.getHost())).build();
HttpExecuteRequest.Builder executeRequestBuilder = HttpExecuteRequest.builder();
executeRequestBuilder.request(request);
ExecutableHttpRequest executableRequest = client.prepareRequest(executeRequestBuilder.build());
HttpExecuteResponse response = executableRequest.call();
assertThat(response.httpResponse().statusCode()).isEqualTo(200);
mockServer.verify(putRequestedFor(urlPathEqualTo(resourcePath))
.withoutHeader("content-length"));
}
}