diff --git a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriNormalizationTest.java b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriNormalizationTest.java new file mode 100644 index 000000000000..fc6f754b410d --- /dev/null +++ b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriNormalizationTest.java @@ -0,0 +1,29 @@ +/* + * 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.apache; + +import software.amazon.awssdk.http.HttpClientUriNormalizationTestSuite; +import software.amazon.awssdk.http.SdkHttpClient; + +public class ApacheHttpClientUriNormalizationTest extends HttpClientUriNormalizationTestSuite { + + + @Override + protected SdkHttpClient createSdkHttpClient() { + return ApacheHttpClient.create(); + } +} + diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java index 4a200df15fef..3219b271998f 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java @@ -100,8 +100,6 @@ private void addRequestConfig(HttpUriRequestBase base, .setResponseTimeout(saturatedCast(requestConfig.socketTimeout().toMillis()), TimeUnit.MILLISECONDS); // TODO as part of removed API : .setLocalAddress(requestConfig.localAddress()); - Apache5Utils.disableNormalizeUri(requestConfigBuilder); - /* * Enable 100-continue support for PUT operations, since this is * where we're potentially uploading large amounts of data and want diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java index f5e78ea4a11e..a777b3d21d8c 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java @@ -29,29 +29,9 @@ import org.apache.hc.core5.http.io.entity.BufferedHttpEntity; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.http.apache5.ProxyConfiguration; -import software.amazon.awssdk.utils.Logger; -import software.amazon.awssdk.utils.ReflectionMethodInvoker; @SdkInternalApi public final class Apache5Utils { - private static final Logger logger = Logger.loggerFor(Apache5Utils.class); - private static final ReflectionMethodInvoker NORMALIZE_URI_INVOKER; - - static { - // Attempt to initialize the invoker once on class-load. If it fails, it will not be attempted again, but we'll - // use that opportunity to log a warning. - NORMALIZE_URI_INVOKER = - new ReflectionMethodInvoker<>(RequestConfig.Builder.class, - RequestConfig.Builder.class, - "setNormalizeUri", - boolean.class); - - try { - NORMALIZE_URI_INVOKER.initialize(); - } catch (NoSuchMethodException ignored) { - noSuchMethodThrownByNormalizeUriInvoker(); - } - } private Apache5Utils() { } @@ -79,37 +59,11 @@ public static HttpClientContext newClientContext(ProxyConfiguration proxyConfigu addPreemptiveAuthenticationProxy(clientContext, proxyConfiguration); RequestConfig.Builder builder = RequestConfig.custom(); - disableNormalizeUri(builder); - clientContext.setRequestConfig(builder.build()); return clientContext; } - /** - * From Apache v4.5.8, normalization should be disabled or AWS requests with special characters in URI path will fail - * with Signature Errors. - *

- * setNormalizeUri is added only in 4.5.8, so customers using the latest version of SDK with old versions (4.5.6 or less) - * of Apache httpclient will see NoSuchMethodError. Hence this method will suppress the error. - * - * Do not use Apache version 4.5.7 as it breaks URI paths with special characters and there is no option - * to disable normalization. - *

- * - * For more information, See https://github.com/aws/aws-sdk-java/issues/1919 - */ - public static void disableNormalizeUri(RequestConfig.Builder requestConfigBuilder) { - // For efficiency, do not attempt to call the invoker again if it failed to initialize on class-load - if (NORMALIZE_URI_INVOKER.isInitialized()) { - try { - NORMALIZE_URI_INVOKER.invoke(requestConfigBuilder, false); - } catch (NoSuchMethodException ignored) { - noSuchMethodThrownByNormalizeUriInvoker(); - } - } - } - /** * Returns a new Credentials Provider for use with proxy authentication. */ @@ -154,13 +108,4 @@ private static void addPreemptiveAuthenticationProxy(HttpClientContext clientCon } } - // Just log and then swallow the exception - private static void noSuchMethodThrownByNormalizeUriInvoker() { - // setNormalizeUri method was added in httpclient 4.5.8 - logger.warn(() -> "NoSuchMethodException was thrown when disabling normalizeUri. This indicates you are using " - + "an old version (< 4.5.8) of Apache http client. It is recommended to use http client " - + "version >= 4.5.9 to avoid the breaking change introduced in apache client 4.5.7 and " - + "the latency in exception handling. See https://github.com/aws/aws-sdk-java/issues/1919" - + " for more information"); - } } diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriNormalizationTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriNormalizationTest.java new file mode 100644 index 000000000000..592508ab1057 --- /dev/null +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriNormalizationTest.java @@ -0,0 +1,29 @@ +/* + * 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.apache5; + +import software.amazon.awssdk.http.HttpClientUriNormalizationTestSuite; +import software.amazon.awssdk.http.SdkHttpClient; + +public class Apache5HttpClientUriNormalizationTest extends HttpClientUriNormalizationTestSuite { + + + @Override + protected SdkHttpClient createSdkHttpClient() { + return Apache5HttpClient.create(); + } +} + diff --git a/test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3 b/test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3 index 8c3b1f284548..dd81179f8903 100644 --- a/test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3 +++ b/test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3 @@ -11,8 +11,11 @@ Method calls method in (FileStoreTlsKeyManagersProvider.java:52) Method calls method in (SystemPropertyTlsKeyManagersProvider.java:61) Method calls method in (ApacheHttpClient.java:699) +Method calls method in (Apache5HttpClient.java:741) Method calls method in (RepeatableInputStreamRequestEntity.java:113) Method calls method in (ApacheUtils.java:162) +Method calls method in (RepeatableInputStreamRequestEntity.java:131) +Method calls method in (RepeatableInputStreamRequestEntity.java:143) Method calls method in (NettyUtils.java:289) Method calls method in (UrlConnectionHttpClient.java:263) Method calls method in (CloudWatchMetricPublisher.java:293) diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java new file mode 100644 index 000000000000..e09eaf3cb8f9 --- /dev/null +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java @@ -0,0 +1,169 @@ +/* + * 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; + +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.any; +import static com.github.tomakehurst.wiremock.client.WireMock.anyRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import static org.assertj.core.api.Assertions.assertThat; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; +import java.net.URI; +import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +public abstract class HttpClientUriNormalizationTestSuite { + + protected static SdkHttpClient httpClient; + private static WireMockServer wireMockServer; + + @BeforeAll + static void setUp() { + wireMockServer = new WireMockServer(options().dynamicPort()); + wireMockServer.start(); + WireMock.configureFor("localhost", wireMockServer.port()); + } + + @BeforeEach + void prepare() { + wireMockServer.stubFor(any(urlMatching(".*")) + .willReturn(aResponse() + .withStatus(200) + .withBody("success"))); + } + + @AfterEach + void reset() { + wireMockServer.resetAll(); + } + + @AfterAll + static void tearDown() { + if (httpClient != null) { + httpClient.close(); + } + if (wireMockServer != null) { + wireMockServer.stop(); + } + } + + private static Stream uriTestCases() { + return Stream.of( + Arguments.of( + "Encoded spaces", + "/path/with%20spaces/file.txt", + "%20" + ), + Arguments.of( + "Encoded slashes", + "/path/with%2Fslash/file.txt", + "%2F" + ), + Arguments.of( + "Encoded plus", + "/path/with%2Bplus/file.txt", + "%2B" + ), + Arguments.of( + "Plus sign", + "/path/with+plus/file.txt", + "+" + ), + Arguments.of( + "Encoded question mark", + "/path/with%3Fquery/file.txt", + "%3F" + ), + Arguments.of( + "Encoded ampersand", + "/path/with%26ampersand/file.txt", + "%26" + ), + Arguments.of( + "Encoded equals", + "/path/with%3Dequals/file.txt", + "%3D" + ), + Arguments.of( + "AWS S3 style path", + "/my-bucket/folder%2Fsubfolder/file%20name.txt", + "%2F" + ) + ); + } + + @ParameterizedTest + @MethodSource("uriTestCases") + @DisplayName("Verify URI normalization is disabled (encoded characters are preserved)") + void testUriNormalizationDisabled(String testName, String path, String encodedChar) throws Exception { + httpClient = createSdkHttpClient(); + + // Create and execute request + HttpExecuteRequest request = createTestRequest(path); + ExecutableHttpRequest executableRequest = httpClient.prepareRequest(request); + HttpExecuteResponse response = executableRequest.call(); + + // Verify response was successful + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + + // Capture the actual request sent to server + List requests = wireMockServer.findAll(anyRequestedFor(anyUrl())); + assertThat(requests).hasSize(1); + + String actualPathSent = requests.get(0).getUrl(); + assertThat(actualPathSent).contains(encodedChar); + } + + private HttpExecuteRequest createTestRequest(String path) { + String baseUrl = "http://localhost:" + wireMockServer.port(); + return HttpExecuteRequest.builder() + .request(SdkHttpRequest.builder() + .method(SdkHttpMethod.GET) + .uri(URI.create(baseUrl + path)) + .build()) + .build(); + } + + @ParameterizedTest + @MethodSource("uriTestCases") + @DisplayName("Test end-to-end execution flow with client context") + void testExecuteFlowWithClientContext(String testName, String path, String encodedChar) throws Exception { + httpClient = createSdkHttpClient(); + HttpExecuteRequest request = createTestRequest(path); + HttpExecuteResponse response = httpClient.prepareRequest(request).call(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + List requests = wireMockServer.findAll(anyRequestedFor(anyUrl())); + assertThat(requests).hasSize(1); + + String actualUrl = requests.get(0).getUrl(); + assertThat(actualUrl).contains(encodedChar); + } + + protected abstract SdkHttpClient createSdkHttpClient(); +} \ No newline at end of file