Fix architecture test failures for apache5.x

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();
+    }
+}
+

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

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<RequestConfig.Builder, RequestConfig.Builder> 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.
-     * <p>
-     *    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.
-     * </p>
-     *
-     * 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");
-    }
 }

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();
+    }
+}
+

test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3

@@ -11,8 +11,11 @@ Method <software.amazon.awssdk.core.retry.ClockSkew.getServerTime(software.amazo
 Method <software.amazon.awssdk.http.FileStoreTlsKeyManagersProvider.keyManagers()> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier, java.lang.Throwable)> in (FileStoreTlsKeyManagersProvider.java:52)
 Method <software.amazon.awssdk.http.SystemPropertyTlsKeyManagersProvider.keyManagers()> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier, java.lang.Throwable)> in (SystemPropertyTlsKeyManagersProvider.java:61)
 Method <software.amazon.awssdk.http.apache.ApacheHttpClient$ApacheConnectionManagerFactory.getSslContext(software.amazon.awssdk.utils.AttributeMap)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (ApacheHttpClient.java:699)
+Method <software.amazon.awssdk.http.apache5.Apache5HttpClient$ApacheConnectionManagerFactory.getSslContext(software.amazon.awssdk.utils.AttributeMap)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (Apache5HttpClient.java:741)
 Method <software.amazon.awssdk.http.apache.internal.RepeatableInputStreamRequestEntity.parseContentLength(java.lang.String)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (RepeatableInputStreamRequestEntity.java:113)
 Method <software.amazon.awssdk.http.apache.internal.utils.ApacheUtils.noSuchMethodThrownByNormalizeUriInvoker()> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (ApacheUtils.java:162)
+Method <software.amazon.awssdk.http.apache5.internal.RepeatableInputStreamRequestEntity.parseContentLength(java.lang.String)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (RepeatableInputStreamRequestEntity.java:131)
+Method <software.amazon.awssdk.http.apache5.internal.RepeatableInputStreamRequestEntity.parseContentType(java.lang.String)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (RepeatableInputStreamRequestEntity.java:143)
 Method <software.amazon.awssdk.http.nio.netty.internal.utils.NettyUtils.warnIfNotInEventLoop(io.netty.channel.EventLoop)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier, java.lang.Throwable)> in (NettyUtils.java:289)
 Method <software.amazon.awssdk.http.urlconnection.UrlConnectionHttpClient.getSslContext(software.amazon.awssdk.utils.AttributeMap)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (UrlConnectionHttpClient.java:263)
 Method <software.amazon.awssdk.metrics.publishers.cloudwatch.CloudWatchMetricPublisher.publish(software.amazon.awssdk.metrics.MetricCollection)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier, java.lang.Throwable)> in (CloudWatchMetricPublisher.java:293)

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<Arguments> 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<LoggedRequest> 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<LoggedRequest> requests = wireMockServer.findAll(anyRequestedFor(anyUrl()));
+        assertThat(requests).hasSize(1);
+
+        String actualUrl = requests.get(0).getUrl();
+        assertThat(actualUrl).contains(encodedChar);
+    }
+
+    protected abstract SdkHttpClient createSdkHttpClient();
+}