From cb20e88a070d572c57e5dcbce42c4aff0b4162b4 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Tue, 3 Dec 2024 11:22:11 -0800 Subject: [PATCH 1/3] Revert "Fix ApacheHttpClient's handling of request bodies on DELETE, GET, HEAD & OPTIONS requests (#5704)" This reverts commit b2fcb7e18ab040c5b8feb23346c879120f179126. --- .changes/2.29.26.json | 6 ++ CHANGELOG.md | 4 + .../impl/ApacheHttpRequestFactory.java | 34 ++++----- ...nnectionHttpClientDefaultWireMockTest.java | 21 ------ .../http/SdkHttpClientDefaultTestSuite.java | 75 ++++--------------- .../suites/cases/rest-json-contenttype.json | 10 +-- 6 files changed, 45 insertions(+), 105 deletions(-) diff --git a/.changes/2.29.26.json b/.changes/2.29.26.json index 8542b6b586d7..7a104fdba5c3 100644 --- a/.changes/2.29.26.json +++ b/.changes/2.29.26.json @@ -109,6 +109,12 @@ "category": "Redshift Serverless", "contributor": "", "description": "Adds support for the ListManagedWorkgroups API to get an overview of existing managed workgroups." + }, + { + "type": "bugfix", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Reverted PR https://github.com/aws/aws-sdk-java-v2/pull/5704 that attached the request body entity to all HTTP methods because it caused regression issues for some services." } ] } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 311b6a4c9075..bdaac33020ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,10 @@ - ### Features - Adds support for the ListManagedWorkgroups API to get an overview of existing managed workgroups. +## __AWS SDK for Java v2__ +- ### Bugfixes + - Reverted PR https://github.com/aws/aws-sdk-java-v2/pull/5704 that attached the request body entity to all HTTP methods because it caused regression issues for some services. + # __2.29.25__ __2024-12-02__ ## __AWS End User Messaging Social__ - ### Features 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 b6642aeec611..cfb22343ba3f 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,7 +24,14 @@ 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; @@ -109,18 +116,23 @@ private void addRequestConfig(final HttpRequestBase base, private HttpRequestBase createApacheRequest(HttpExecuteRequest request, URI uri) { - SdkHttpMethod method = request.httpRequest().method(); - switch (method) { + switch (request.httpRequest().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); case PATCH: + return wrapEntity(request, new HttpPatch(uri)); case POST: + return wrapEntity(request, new HttpPost(uri)); case PUT: - return wrapEntity(request, new HttpRequestImpl(method, uri)); + return wrapEntity(request, new HttpPut(uri)); default: - throw new RuntimeException("Unknown HTTP method name: " + method); + throw new RuntimeException("Unknown HTTP method name: " + request.httpRequest().method()); } } @@ -180,18 +192,4 @@ private String getHostHeaderValue(SdkHttpRequest request) { ? request.host() + ":" + request.port() : request.host(); } - - private static final class HttpRequestImpl extends HttpEntityEnclosingRequestBase { - private final SdkHttpMethod method; - - private HttpRequestImpl(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 12960c82dbcf..66cbc4e3b814 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,16 +15,11 @@ 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 { @@ -37,20 +32,4 @@ protected SdkHttpClient createSdkHttpClient() { public void reset() { HttpsURLConnection.setDefaultSSLSocketFactory((SSLSocketFactory) SSLSocketFactory.getDefault()); } - - @Test - @Override - public void supportsRequestBodyOnGetRequest() 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 supportsRequestBodyOnPatchRequest() { - // HttpURLConnection does not support PATCH requests. - assertThatThrownBy(super::supportsRequestBodyOnPatchRequest) - .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 22b55cc6192b..e0e300b848d4 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, false); + testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD); } @Test @@ -76,7 +76,7 @@ public void supportsResponseCode403() throws Exception { @Test public void supportsResponseCode403Head() throws Exception { - testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD, false); + testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD); } @Test @@ -98,64 +98,22 @@ public void supportsResponseCode500() throws Exception { public void validatesHttpsCertificateIssuer() { SdkHttpClient client = createSdkHttpClient(); - SdkHttpFullRequest request = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), SdkHttpMethod.POST, true); + SdkHttpFullRequest request = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), SdkHttpMethod.POST); assertThatThrownBy(client.prepareRequest(HttpExecuteRequest.builder().request(request).build())::call) .isInstanceOf(SSLHandshakeException.class); } - @Test - public void supportsRequestBodyOnGetRequest() throws Exception { - testForResponseCode(200, SdkHttpMethod.GET, true); - } - - @Test - public void supportsRequestBodyOnPostRequest() throws Exception { - testForResponseCode(200, SdkHttpMethod.POST, true); - } - - @Test - public void supportsRequestBodyOnPutRequest() throws Exception { - testForResponseCode(200, SdkHttpMethod.PUT, true); - } - - @Test - public void supportsRequestBodyOnDeleteRequest() throws Exception { - testForResponseCode(200, SdkHttpMethod.DELETE, true); - } - - @Test - public void supportsRequestBodyOnHeadRequest() throws Exception { - testForResponseCode(200, SdkHttpMethod.HEAD, true); - } - - @Test - public void supportsRequestBodyOnPatchRequest() throws Exception { - testForResponseCode(200, SdkHttpMethod.PATCH, true); - } - - @Test - public void supportsRequestBodyOnOptionsRequest() throws Exception { - testForResponseCode(200, SdkHttpMethod.OPTIONS, true); - } - private void testForResponseCode(int returnCode) throws Exception { - testForResponseCode(returnCode, SdkHttpMethod.POST, true); + testForResponseCode(returnCode, SdkHttpMethod.POST); } - 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 { + private void testForResponseCode(int returnCode, SdkHttpMethod method) throws Exception { SdkHttpClient client = createSdkHttpClient(); stubForMockRequest(returnCode); - SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port(), method, includeBody); + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port(), method); HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() .request(req) .contentStreamProvider(req.contentStreamProvider() @@ -163,14 +121,14 @@ protected void testForResponseCode(int returnCode, .build()) .call(); - validateResponse(rsp, returnCode, expectedMethod, includeBody); + validateResponse(rsp, returnCode, method); } protected void testForResponseCodeUsingHttps(SdkHttpClient client, int returnCode) throws Exception { SdkHttpMethod sdkHttpMethod = SdkHttpMethod.POST; stubForMockRequest(returnCode); - SdkHttpFullRequest req = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), sdkHttpMethod, true); + SdkHttpFullRequest req = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), sdkHttpMethod); HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() .request(req) .contentStreamProvider(req.contentStreamProvider() @@ -178,7 +136,7 @@ protected void testForResponseCodeUsingHttps(SdkHttpClient client, int returnCod .build()) .call(); - validateResponse(rsp, returnCode, sdkHttpMethod, true); + validateResponse(rsp, returnCode, sdkHttpMethod); } private void stubForMockRequest(int returnCode) { @@ -193,20 +151,17 @@ private void stubForMockRequest(int returnCode) { mockServer.stubFor(any(urlPathEqualTo("/")).willReturn(responseBuilder)); } - private void validateResponse(HttpExecuteResponse response, - int returnCode, - SdkHttpMethod method, - boolean expectBody) throws IOException { + private void validateResponse(HttpExecuteResponse response, int returnCode, SdkHttpMethod method) 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 (expectBody) { - patternBuilder.withRequestBody(equalTo("Body")); - } else { + if (method == SdkHttpMethod.HEAD) { patternBuilder.withRequestBody(absent()); + } else { + patternBuilder.withRequestBody(equalTo("Body")); } mockServer.verify(1, patternBuilder); @@ -222,14 +177,14 @@ private void validateResponse(HttpExecuteResponse response, mockServer.resetMappings(); } - private SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method, boolean includeBody) { + private SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) { URI uri = URI.create(uriString); SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder() .uri(uri) .method(method) .putHeader("Host", uri.getHost()) .putHeader("User-Agent", "hello-world!"); - if (includeBody) { + if (method != SdkHttpMethod.HEAD) { byte[] content = "Body".getBytes(StandardCharsets.UTF_8); requestBuilder.putHeader("Content-Length", Integer.toString(content.length)); requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(content)); diff --git a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-contenttype.json b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-contenttype.json index 1833bb74f6fc..7260c5743e6b 100644 --- a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-contenttype.json +++ b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-contenttype.json @@ -230,11 +230,9 @@ "uri": "/no-payload", "method": "GET", "headers": { - "contains": { - "Content-Length": "0" - }, "doesNotContain": [ - "Content-Type" + "Content-Type", + "Content-Length" ] }, "body": { @@ -260,11 +258,11 @@ "method": "GET", "headers": { "contains": { - "Content-Length": "0", "x-amz-test-id": "t-12345" }, "doesNotContain": [ - "Content-Type" + "Content-Type", + "Content-Length" ] }, "body": { From 70750a15dbf6f75e71083d401f3a7b7746f56a08 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Tue, 3 Dec 2024 11:46:44 -0800 Subject: [PATCH 2/3] updated change logs --- .changes/2.29.26.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/2.29.26.json b/.changes/2.29.26.json index 7a104fdba5c3..4a229f9d0fa7 100644 --- a/.changes/2.29.26.json +++ b/.changes/2.29.26.json @@ -114,7 +114,7 @@ "type": "bugfix", "category": "AWS SDK for Java v2", "contributor": "", - "description": "Reverted PR https://github.com/aws/aws-sdk-java-v2/pull/5704 that attached the request body entity to all HTTP methods because it caused regression issues for some services." + "description": "Reverted PR https://github.com/aws/aws-sdk-java-v2/pull/5704, which modified all HTTP methods to include the request body entity, as it caused regression issues for some services." } ] } \ No newline at end of file From 094ddae78e7a2fd0690a69275565a04f62b24e07 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Tue, 3 Dec 2024 11:47:01 -0800 Subject: [PATCH 3/3] updated change log --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdaac33020ab..db73e6c72430 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,7 +74,7 @@ ## __AWS SDK for Java v2__ - ### Bugfixes - - Reverted PR https://github.com/aws/aws-sdk-java-v2/pull/5704 that attached the request body entity to all HTTP methods because it caused regression issues for some services. + - Reverted PR https://github.com/aws/aws-sdk-java-v2/pull/5704, which modified all HTTP methods to include the request body entity, as it caused regression issues for some services. # __2.29.25__ __2024-12-02__ ## __AWS End User Messaging Social__