fix: support http.nonProxyHost JVM system property (#1083)

Author: lauzadis

.changes/040b1587-ec85-4601-9eec-a5158cb0edac.json

@@ -0,0 +1,8 @@
+{
+    "id": "040b1587-ec85-4601-9eec-a5158cb0edac",
+    "type": "bugfix",
+    "description": "Support `http.nonProxyHosts` JVM system property",
+    "issues": [
+        "https://github.com/smithy-lang/smithy-kotlin/issues/1081"
+    ]
+}

runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelector.kt

@@ -22,6 +22,7 @@ import aws.smithy.kotlin.runtime.util.PropertyProvider
  * - `http.proxyPort`
  * - `https.proxyHost`
  * - `https.proxyPort`
+ * - `http.nonProxyHosts`
  * - `http.noProxyHosts`
  *
  * **Environment variables in the given order**:
@@ -32,10 +33,10 @@ import aws.smithy.kotlin.runtime.util.PropertyProvider
 internal class EnvironmentProxySelector(provider: PlatformEnvironProvider = PlatformProvider.System) : ProxySelector {
     private val httpProxy by lazy { resolveProxyByProperty(provider, Scheme.HTTP) ?: resolveProxyByEnvironment(provider, Scheme.HTTP) }
     private val httpsProxy by lazy { resolveProxyByProperty(provider, Scheme.HTTPS) ?: resolveProxyByEnvironment(provider, Scheme.HTTPS) }
-    private val noProxyHosts by lazy { resolveNoProxyHosts(provider) }
+    private val nonProxyHosts by lazy { resolveNonProxyHosts(provider) }
 
     override fun select(url: Url): ProxyConfig {
-        if (httpProxy == null && httpsProxy == null || noProxy(url)) return ProxyConfig.Direct
+        if (httpProxy == null && httpsProxy == null || nonProxy(url)) return ProxyConfig.Direct
 
         val proxyConfig = when (url.scheme) {
             Scheme.HTTP -> httpProxy
@@ -46,7 +47,7 @@ internal class EnvironmentProxySelector(provider: PlatformEnvironProvider = Plat
         return proxyConfig ?: ProxyConfig.Direct
     }
 
-    private fun noProxy(url: Url): Boolean = noProxyHosts.any { it.matches(url) }
+    private fun nonProxy(url: Url): Boolean = nonProxyHosts.any { it.matches(url) }
 }
 
 private fun resolveProxyByProperty(provider: PropertyProvider, scheme: Scheme): ProxyConfig? {
@@ -94,7 +95,7 @@ private fun resolveProxyByEnvironment(provider: EnvironmentProvider, scheme: Sch
             }
         }
 
-internal data class NoProxyHost(val hostMatch: String, val port: Int? = null) {
+internal data class NonProxyHost(val hostMatch: String, val port: Int? = null) {
     fun matches(url: Url): Boolean {
         // any host
         if (hostMatch == "*") return true
@@ -115,24 +116,29 @@ internal data class NoProxyHost(val hostMatch: String, val port: Int? = null) {
     }
 }
 
-private fun parseNoProxyHost(raw: String): NoProxyHost {
+private fun parseNonProxyHost(raw: String): NonProxyHost {
     val pair = raw.split(':', limit = 2)
     return when (pair.size) {
-        1 -> NoProxyHost(pair[0])
-        2 -> NoProxyHost(pair[0], pair[1].toInt())
-        else -> error("invalid no proxy host: $raw")
+        1 -> NonProxyHost(pair[0])
+        2 -> NonProxyHost(pair[0], pair[1].toInt())
+        else -> error("invalid non proxy host: $raw")
     }
 }
 
-private fun resolveNoProxyHosts(provider: PlatformEnvironProvider): Set<NoProxyHost> {
+private fun resolveNonProxyHosts(provider: PlatformEnvironProvider): Set<NonProxyHost> {
     // http.nonProxyHosts:a list of hosts that should be reached directly, bypassing the proxy. This is a list of
     // patterns separated by '|'. The patterns may start or end with a '*' for wildcards. Any host matching one of
     // these patterns will be reached through a direct connection instead of through a proxy.
-    val noProxyHostProps = provider.getProperty("http.noProxyHosts")
+
+    // NOTE: Both http.nonProxyHosts (correct value according to the spec) AND http.noProxyHosts (legacy behavior) are checked
+    // https://github.com/smithy-lang/smithy-kotlin/issues/1081
+    val nonProxyHostProperty = provider.getProperty("http.nonProxyHosts") ?: provider.getProperty("http.noProxyHosts")
+
+    val nonProxyHostProps = nonProxyHostProperty
         ?.split('|')
         ?.map { it.trim() }
         ?.map { it.trimStart('.') }
-        ?.map(::parseNoProxyHost)
+        ?.map(::parseNonProxyHost)
         ?.toSet() ?: emptySet()
 
     // `no_proxy` is a comma or space-separated list of machine or domain names, with optional :port part.
@@ -142,8 +148,8 @@ private fun resolveNoProxyHosts(provider: PlatformEnvironProvider): Set<NoProxyH
         .flatMap { it.split(',', ' ') }
         .map { it.trim() }
         .map { it.trimStart('.') }
-        .map(::parseNoProxyHost)
+        .map(::parseNonProxyHost)
         .toSet()
 
-    return noProxyHostProps + noProxyEnv
+    return nonProxyHostProps + noProxyEnv
 }

runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/HttpClientEngineConfig.kt

@@ -144,6 +144,7 @@ public interface HttpClientEngineConfig {
          * - `http.proxyPort`
          * - `https.proxyHost`
          * - `https.proxyPort`
+         * - `http.nonProxyHosts`
          * - `http.noProxyHosts`
          *
          * **Environment variables in the given order**:

runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelectorTest.kt

@@ -51,12 +51,17 @@ class EnvironmentProxySelectorTest {
         // no proxy
         TestCase(ProxyConfig.Direct, env = mapOf("no_proxy" to "aws.amazon.com") + httpsProxyEnv),
         TestCase(ProxyConfig.Direct, env = mapOf("no_proxy" to ".amazon.com") + httpsProxyEnv),
+
         TestCase(ProxyConfig.Direct, props = mapOf("http.noProxyHosts" to "aws.amazon.com") + httpsProxyProps),
         TestCase(ProxyConfig.Direct, props = mapOf("http.noProxyHosts" to ".amazon.com") + httpsProxyProps),
 
+        TestCase(ProxyConfig.Direct, props = mapOf("http.nonProxyHosts" to "aws.amazon.com") + httpsProxyProps),
+        TestCase(ProxyConfig.Direct, props = mapOf("http.nonProxyHosts" to ".amazon.com") + httpsProxyProps),
+
         // multiple no proxy hosts normalization
         TestCase(ProxyConfig.Direct, env = mapOf("no_proxy" to "example.com,.amazon.com") + httpsProxyEnv),
         TestCase(ProxyConfig.Direct, props = mapOf("http.noProxyHosts" to "example.com|.amazon.com") + httpsProxyProps),
+        TestCase(ProxyConfig.Direct, props = mapOf("http.nonProxyHosts" to "example.com|.amazon.com") + httpsProxyProps),
 
         // environment
         TestCase(expectedProxyConfig, env = httpsProxyEnv),
@@ -68,6 +73,12 @@ class EnvironmentProxySelectorTest {
 
         // no_proxy set but doesn't match
         TestCase(expectedProxyConfig, env = httpsProxyEnv + mapOf("no_proxy" to "example.com")),
+
+        // prioritize http.nonProxyHosts over http.noProxyHosts
+        TestCase(ProxyConfig.Direct, props = mapOf("http.nonProxyHosts" to "example.com|.amazon.com", "http.noProxyHosts" to "") + httpsProxyProps),
+
+        // even though http.noProxyHosts is configured to go ProxyConfig.Direct, nonProxyHosts is present and should be prioritized
+        TestCase(expectedProxyConfig, props = mapOf("http.nonProxyHosts" to "", "http.noProxyHosts" to "example.com|.amazon.com") + httpsProxyProps),
     )
 
     @Test

runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/engine/NoProxyHostTest.kt

@@ -45,7 +45,7 @@ class NoProxyHostTest {
     @Test
     fun testMatches() {
         tests.forEachIndexed { i, testCase ->
-            val noProxyHost = NoProxyHost(testCase.noProxyHost, testCase.noProxyPort)
+            val noProxyHost = NonProxyHost(testCase.noProxyHost, testCase.noProxyPort)
             val url = Url.parse(testCase.url)
 
             assertEquals(testCase.shouldMatch, noProxyHost.matches(url), "[idx=$i] expected $noProxyHost to match $url")