Fix bug in using nonProxyHosts with CRT http/s3 clients (#6511)

* WIP - TEMPORARY testing of the crt no_proxy_hosts

* Set the noProxyHosts on CRTs HttpProxyOptions

* Fix spotbugs

Author: alextwoods

.changes/next-release/bugfix-AWSSDKforJavav2-5972f21.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "Correctly use noProxyHosts from CrtProxyConfiguration."
+}

core/crt-core/src/main/java/software/amazon/awssdk/crtcore/CrtConfigurationUtils.java

@@ -15,11 +15,7 @@
 
 package software.amazon.awssdk.crtcore;
 
-import static software.amazon.awssdk.utils.StringUtils.lowerCase;
-
-import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
 import software.amazon.awssdk.annotations.SdkProtectedApi;
 import software.amazon.awssdk.crt.http.HttpMonitoringOptions;
 import software.amazon.awssdk.crt.http.HttpProxyOptions;
@@ -37,13 +33,14 @@ public static Optional<HttpProxyOptions> resolveProxy(CrtProxyConfiguration prox
         if (proxyConfiguration == null) {
             return Optional.empty();
         }
-        if (doesTargetMatchNonProxyHosts(proxyConfiguration.host(), proxyConfiguration.nonProxyHosts())) {
-            return Optional.empty();
-        }
+
         HttpProxyOptions clientProxyOptions = new HttpProxyOptions();
 
         clientProxyOptions.setHost(proxyConfiguration.host());
         clientProxyOptions.setPort(proxyConfiguration.port());
+        if (!proxyConfiguration.nonProxyHosts().isEmpty()) {
+            clientProxyOptions.setNoProxyHosts(String.join(",", proxyConfiguration.nonProxyHosts()));
+        }
 
         if ("https".equalsIgnoreCase(proxyConfiguration.scheme())) {
             clientProxyOptions.setTlsContext(tlsContext);
@@ -60,15 +57,6 @@ public static Optional<HttpProxyOptions> resolveProxy(CrtProxyConfiguration prox
         return Optional.of(clientProxyOptions);
     }
 
-    private static boolean doesTargetMatchNonProxyHosts(String target, Set<String> hostPatterns) {
-        return Optional.ofNullable(hostPatterns)
-                       .map(patterns ->
-                                patterns.stream()
-                                        .filter(Objects::nonNull)
-                                        .anyMatch(pattern -> target != null && lowerCase(target).matches(pattern)))
-                       .orElse(false);
-    }
-
     public static Optional<HttpMonitoringOptions> resolveHttpMonitoringOptions(CrtConnectionHealthConfiguration config) {
         if (config == null) {
             return Optional.empty();

core/crt-core/src/main/java/software/amazon/awssdk/crtcore/CrtProxyConfiguration.java

@@ -281,12 +281,18 @@ public interface Builder {
 
         /**
          * Configure the hosts that the client is allowed to access without going through the proxy.
+         * The only wildcard available is a single "*" character, which matches all hosts.
+         * IP addresses specified to this option can be provided using CIDR notation: an appended slash and number
+         * specifies the number of "network bits" out of the address to use in the comparison.
          */
         Builder nonProxyHosts(Set<String> nonProxyHosts);
 
 
         /**
          * Add a host that the client is allowed to access without going through the proxy.
+         * The only wildcard available is a single "*" character, which matches all hosts.
+         * IP addresses specified to this option can be provided using CIDR notation: an appended slash and number
+         * specifies the number of "network bits" out of the address to use in the comparison.
          */
         Builder addNonProxyHost(String nonProxyHost);
 

core/crt-core/src/test/java/software/amazon/awssdk/crtcore/CrtConnectionUtilsTest.java

@@ -58,19 +58,19 @@ void resolveProxy_emptyProxy_shouldReturnEmpty() {
         assertThat(CrtConfigurationUtils.resolveProxy(null, tlsContext)).isEmpty();
     }
 
-    @ParameterizedTest
-    @ValueSource(strings = {".*?.2.3.4", "1.*?.3.4", ".*?"})
-    void resolveProxy_withSingleNonProxyHostsWidCards_shouldReturnEmpty(String nonProxyHost) {
+    @Test
+    void resolveProxy_nonProxyHosts_setAsCommaSeperatedString() {
         TlsContext tlsContext = Mockito.mock(TlsContext.class);
         CrtProxyConfiguration configuration = new TestProxy.Builder().host("1.2.3.4")
                                                                      .port(123)
                                                                      .scheme("https")
                                                                      .password("bar")
                                                                      .username("foo")
-                                                                     .nonProxyHosts(Stream.of(nonProxyHost,"someRandom")
-                                                                                          .collect(Collectors.toSet()))
+                                                                     .addNonProxyHost("host1")
+                                                                     .addNonProxyHost("host2")
                                                                      .build();
-        assertThat(CrtConfigurationUtils.resolveProxy(configuration, tlsContext)).isEmpty();
+        assertThat(CrtConfigurationUtils.resolveProxy(configuration, tlsContext).get().getNoProxyHosts())
+            .isEqualTo("host1,host2");
     }
 
 

pom.xml

@@ -129,7 +129,7 @@
         <rxjava3.version>3.1.5</rxjava3.version>
         <commons-codec.verion>1.17.1</commons-codec.verion>
         <jmh.version>1.37</jmh.version>
-        <awscrt.version>0.38.9</awscrt.version>
+        <awscrt.version>0.39.4</awscrt.version>
 
         <!--Test dependencies -->
         <junit5.version>5.10.0</junit5.version>