diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-c212259.json b/.changes/next-release/bugfix-AWSSDKforJavav2-c212259.json
new file mode 100644
index 000000000000..aa0eca4ab0fe
--- /dev/null
+++ b/.changes/next-release/bugfix-AWSSDKforJavav2-c212259.json
@@ -0,0 +1,6 @@
+{
+ "type": "bugfix",
+ "category": "AWS SDK for Java v2",
+ "contributor": "Xtansia",
+ "description": "Fix ApacheHttpClient's handling of request bodies on DELETE, GET, HEAD & OPTIONS requests"
+}
diff --git a/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java b/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java
index cfb22343ba3f..a9979152e553 100644
--- a/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java
+++ b/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java
@@ -24,14 +24,7 @@
import org.apache.http.HttpEntity;
import org.apache.http.HttpHeaders;
import org.apache.http.client.config.RequestConfig;
-import org.apache.http.client.methods.HttpDelete;
import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpHead;
-import org.apache.http.client.methods.HttpOptions;
-import org.apache.http.client.methods.HttpPatch;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.client.methods.HttpPut;
import org.apache.http.client.methods.HttpRequestBase;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.http.HttpExecuteRequest;
@@ -116,28 +109,29 @@ private void addRequestConfig(final HttpRequestBase base,
private HttpRequestBase createApacheRequest(HttpExecuteRequest request, URI uri) {
- switch (request.httpRequest().method()) {
+ SdkHttpMethod method = request.httpRequest().method();
+ switch (method) {
case HEAD:
- return new HttpHead(uri);
case GET:
- return new HttpGet(uri);
case DELETE:
- return new HttpDelete(uri);
case OPTIONS:
- return new HttpOptions(uri);
+ if (hasContentStream(request)) {
+ return createEntityEnclosingRequest(request, method, uri);
+ }
+ return new HttpRequestImpl(method, uri);
case PATCH:
- return wrapEntity(request, new HttpPatch(uri));
case POST:
- return wrapEntity(request, new HttpPost(uri));
case PUT:
- return wrapEntity(request, new HttpPut(uri));
+ return createEntityEnclosingRequest(request, method, uri);
default:
- throw new RuntimeException("Unknown HTTP method name: " + request.httpRequest().method());
+ throw new RuntimeException("Unknown HTTP method name: " + method);
}
}
- private HttpRequestBase wrapEntity(HttpExecuteRequest request,
- HttpEntityEnclosingRequestBase entityEnclosingRequest) {
+
+
+ private HttpRequestBase createEntityEnclosingRequest(HttpExecuteRequest request, SdkHttpMethod method, URI uri) {
+ HttpEntityEnclosingRequestBase entityEnclosingRequest = new HttpEntityEnclosingRequestImpl(method, uri);
/*
* We should never reuse the entity of the previous request, since
@@ -149,7 +143,7 @@ private HttpRequestBase wrapEntity(HttpExecuteRequest request,
* preparation for the retry. Eventually, these wrappers would
* return incorrect validation result.
*/
- if (request.contentStreamProvider().isPresent()) {
+ if (hasContentStream(request)) {
HttpEntity entity = new RepeatableInputStreamRequestEntity(request);
if (!request.httpRequest().firstMatchingHeader(HttpHeaders.CONTENT_LENGTH).isPresent() && !entity.isChunked()) {
entity = ApacheUtils.newBufferedHttpEntity(entity);
@@ -160,6 +154,10 @@ private HttpRequestBase wrapEntity(HttpExecuteRequest request,
return entityEnclosingRequest;
}
+ private static boolean hasContentStream(HttpExecuteRequest request) {
+ return request.contentStreamProvider().isPresent();
+ }
+
/**
* Configures the headers in the specified Apache HTTP request.
*/
@@ -192,4 +190,32 @@ private String getHostHeaderValue(SdkHttpRequest request) {
? request.host() + ":" + request.port()
: request.host();
}
+
+ private static final class HttpRequestImpl extends HttpRequestBase {
+ private final SdkHttpMethod method;
+
+ private HttpRequestImpl(SdkHttpMethod method, URI uri) {
+ this.method = method;
+ setURI(uri);
+ }
+
+ @Override
+ public String getMethod() {
+ return this.method.toString();
+ }
+ }
+
+ private static final class HttpEntityEnclosingRequestImpl extends HttpEntityEnclosingRequestBase {
+ private final SdkHttpMethod method;
+
+ private HttpEntityEnclosingRequestImpl(SdkHttpMethod method, URI uri) {
+ this.method = method;
+ setURI(uri);
+ }
+
+ @Override
+ public String getMethod() {
+ return this.method.toString();
+ }
+ }
}
diff --git a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientDefaultWireMockTest.java b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientDefaultWireMockTest.java
index 66cbc4e3b814..bd69df1422fa 100644
--- a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientDefaultWireMockTest.java
+++ b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientDefaultWireMockTest.java
@@ -15,11 +15,16 @@
package software.amazon.awssdk.http.urlconnection;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.net.ProtocolException;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLSocketFactory;
+import org.junit.Test;
import org.junit.jupiter.api.AfterEach;
import software.amazon.awssdk.http.SdkHttpClient;
import software.amazon.awssdk.http.SdkHttpClientDefaultTestSuite;
+import software.amazon.awssdk.http.SdkHttpMethod;
public class UrlConnectionHttpClientDefaultWireMockTest extends SdkHttpClientDefaultTestSuite {
@@ -32,4 +37,29 @@ protected SdkHttpClient createSdkHttpClient() {
public void reset() {
HttpsURLConnection.setDefaultSSLSocketFactory((SSLSocketFactory) SSLSocketFactory.getDefault());
}
+
+ @Test
+ @Override
+ public void supportsGetRequestWithRequestBody() throws Exception {
+ // HttpURLConnection is hard-coded to switch GET requests with a body to POST requests, in #getOutputStream0.
+ testForResponseCode(200, SdkHttpMethod.GET, SdkHttpMethod.POST, true);
+ }
+
+ @Test
+ @Override
+ public void supportsPatchRequestWithoutRequestBody() {
+ // HttpURLConnection does not support PATCH requests.
+ assertThatThrownBy(super::supportsPatchRequestWithoutRequestBody)
+ .hasRootCauseInstanceOf(ProtocolException.class)
+ .hasRootCauseMessage("Invalid HTTP method: PATCH");
+ }
+
+ @Test
+ @Override
+ public void supportsPatchRequestWithRequestBody() {
+ // HttpURLConnection does not support PATCH requests.
+ assertThatThrownBy(super::supportsPatchRequestWithRequestBody)
+ .hasRootCauseInstanceOf(ProtocolException.class)
+ .hasRootCauseMessage("Invalid HTTP method: PATCH");
+ }
}
diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientDefaultTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientDefaultTestSuite.java
index e0e300b848d4..d6859a4955cf 100644
--- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientDefaultTestSuite.java
+++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientDefaultTestSuite.java
@@ -61,7 +61,7 @@ public void supportsResponseCode200() throws Exception {
@Test
public void supportsResponseCode200Head() throws Exception {
// HEAD is special due to closing of the connection immediately and streams are null
- testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD);
+ testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD, false);
}
@Test
@@ -76,7 +76,7 @@ public void supportsResponseCode403() throws Exception {
@Test
public void supportsResponseCode403Head() throws Exception {
- testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD);
+ testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD, false);
}
@Test
@@ -98,22 +98,99 @@ public void supportsResponseCode500() throws Exception {
public void validatesHttpsCertificateIssuer() {
SdkHttpClient client = createSdkHttpClient();
- SdkHttpFullRequest request = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), SdkHttpMethod.POST);
+ SdkHttpFullRequest request = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), SdkHttpMethod.POST, true);
assertThatThrownBy(client.prepareRequest(HttpExecuteRequest.builder().request(request).build())::call)
.isInstanceOf(SSLHandshakeException.class);
}
+ @Test
+ public void supportsDeleteRequestWithoutRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.DELETE, false);
+ }
+
+ @Test
+ public void supportsDeleteRequestWithRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.DELETE, true);
+ }
+
+ @Test
+ public void supportsGetRequestWithoutRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.GET, false);
+ }
+
+ @Test
+ public void supportsGetRequestWithRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.GET, true);
+ }
+
+ @Test
+ public void supportsHeadRequestWithoutRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.HEAD, false);
+ }
+
+ @Test
+ public void supportsHeadRequestWithRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.HEAD, true);
+ }
+
+ @Test
+ public void supportsOptionsRequestWithoutRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.OPTIONS, false);
+ }
+
+ @Test
+ public void supportsOptionsRequestWithRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.OPTIONS, true);
+ }
+
+ @Test
+ public void supportsPatchRequestWithoutRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.PATCH, false);
+ }
+
+ @Test
+ public void supportsPatchRequestWithRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.PATCH, true);
+ }
+
+ @Test
+ public void supportsPostRequestWithoutRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.POST, false);
+ }
+
+ @Test
+ public void supportsPostRequestWithRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.POST, true);
+ }
+
+ @Test
+ public void supportsPutRequestWithoutRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.PUT, false);
+ }
+
+ @Test
+ public void supportsPutRequestWithRequestBody() throws Exception {
+ testForResponseCode(200, SdkHttpMethod.PUT, true);
+ }
+
private void testForResponseCode(int returnCode) throws Exception {
- testForResponseCode(returnCode, SdkHttpMethod.POST);
+ testForResponseCode(returnCode, SdkHttpMethod.POST, true);
}
- private void testForResponseCode(int returnCode, SdkHttpMethod method) throws Exception {
+ protected void testForResponseCode(int returnCode, SdkHttpMethod method, boolean includeBody) throws Exception {
+ testForResponseCode(returnCode, method, method, includeBody);
+ }
+
+ protected void testForResponseCode(int returnCode,
+ SdkHttpMethod method,
+ SdkHttpMethod expectedMethod,
+ boolean includeBody) throws Exception {
SdkHttpClient client = createSdkHttpClient();
stubForMockRequest(returnCode);
- SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port(), method);
+ SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port(), method, includeBody);
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
.request(req)
.contentStreamProvider(req.contentStreamProvider()
@@ -121,14 +198,14 @@ private void testForResponseCode(int returnCode, SdkHttpMethod method) throws Ex
.build())
.call();
- validateResponse(rsp, returnCode, method);
+ validateResponse(rsp, returnCode, expectedMethod, includeBody);
}
protected void testForResponseCodeUsingHttps(SdkHttpClient client, int returnCode) throws Exception {
SdkHttpMethod sdkHttpMethod = SdkHttpMethod.POST;
stubForMockRequest(returnCode);
- SdkHttpFullRequest req = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), sdkHttpMethod);
+ SdkHttpFullRequest req = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), sdkHttpMethod, true);
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
.request(req)
.contentStreamProvider(req.contentStreamProvider()
@@ -136,7 +213,7 @@ protected void testForResponseCodeUsingHttps(SdkHttpClient client, int returnCod
.build())
.call();
- validateResponse(rsp, returnCode, sdkHttpMethod);
+ validateResponse(rsp, returnCode, sdkHttpMethod, true);
}
private void stubForMockRequest(int returnCode) {
@@ -151,17 +228,24 @@ private void stubForMockRequest(int returnCode) {
mockServer.stubFor(any(urlPathEqualTo("/")).willReturn(responseBuilder));
}
- private void validateResponse(HttpExecuteResponse response, int returnCode, SdkHttpMethod method) throws IOException {
+ private void validateResponse(HttpExecuteResponse response,
+ int returnCode,
+ SdkHttpMethod method,
+ boolean expectBody) throws IOException {
RequestMethod requestMethod = RequestMethod.fromString(method.name());
RequestPatternBuilder patternBuilder = RequestPatternBuilder.newRequestPattern(requestMethod, urlMatching("/"))
.withHeader("Host", containing("localhost"))
.withHeader("User-Agent", equalTo("hello-world!"));
- if (method == SdkHttpMethod.HEAD) {
- patternBuilder.withRequestBody(absent());
+ if (expectBody) {
+ patternBuilder.withRequestBody(equalTo("Body"))
+ .withHeader("Content-Length", equalTo("4"));
} else {
- patternBuilder.withRequestBody(equalTo("Body"));
+ patternBuilder.withRequestBody(absent());
+ if (method != SdkHttpMethod.PATCH && method != SdkHttpMethod.POST && method != SdkHttpMethod.PUT) {
+ patternBuilder.withoutHeader("Content-Length");
+ }
}
mockServer.verify(1, patternBuilder);
@@ -177,14 +261,14 @@ private void validateResponse(HttpExecuteResponse response, int returnCode, SdkH
mockServer.resetMappings();
}
- private SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) {
+ private SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method, boolean includeBody) {
URI uri = URI.create(uriString);
SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder()
.uri(uri)
.method(method)
.putHeader("Host", uri.getHost())
.putHeader("User-Agent", "hello-world!");
- if (method != SdkHttpMethod.HEAD) {
+ if (includeBody) {
byte[] content = "Body".getBytes(StandardCharsets.UTF_8);
requestBuilder.putHeader("Content-Length", Integer.toString(content.length));
requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(content));