From b21177118118507601f6b5800055ee1f497c6c2b Mon Sep 17 00:00:00 2001
From: Matas <lauzmata@amazon.com>
Date: Mon, 15 Apr 2024 15:04:43 -0400
Subject: [PATCH] fix: lazily resolve proxy environment variables (#1066)

---
 .../6ffd0a2b-9801-4814-a6d1-9d84196ff130.json |  8 ++++++++
 .../http/engine/HttpEngineConfigImplTest.kt   | 20 +++++++++++++++++++
 .../http/engine/EnvironmentProxySelector.kt   |  8 +++-----
 .../engine/EnvironmentProxySelectorTest.kt    |  3 ++-
 4 files changed, 33 insertions(+), 6 deletions(-)
 create mode 100644 .changes/6ffd0a2b-9801-4814-a6d1-9d84196ff130.json

diff --git a/.changes/6ffd0a2b-9801-4814-a6d1-9d84196ff130.json b/.changes/6ffd0a2b-9801-4814-a6d1-9d84196ff130.json
new file mode 100644
index 000000000..92540e615
--- /dev/null
+++ b/.changes/6ffd0a2b-9801-4814-a6d1-9d84196ff130.json
@@ -0,0 +1,8 @@
+{
+    "id": "6ffd0a2b-9801-4814-a6d1-9d84196ff130",
+    "type": "bugfix",
+    "description": "Lazily resolve proxy environment variables",
+    "issues": [
+        "https://github.com/awslabs/aws-sdk-kotlin/issues/1281"
+    ]
+}
\ No newline at end of file
diff --git a/runtime/protocol/http-client-engines/http-client-engine-default/jvm/test/aws/smithy/kotlin/runtime/http/engine/HttpEngineConfigImplTest.kt b/runtime/protocol/http-client-engines/http-client-engine-default/jvm/test/aws/smithy/kotlin/runtime/http/engine/HttpEngineConfigImplTest.kt
index 167c92ffe..cf95617ee 100644
--- a/runtime/protocol/http-client-engines/http-client-engine-default/jvm/test/aws/smithy/kotlin/runtime/http/engine/HttpEngineConfigImplTest.kt
+++ b/runtime/protocol/http-client-engines/http-client-engine-default/jvm/test/aws/smithy/kotlin/runtime/http/engine/HttpEngineConfigImplTest.kt
@@ -130,6 +130,26 @@ class HttpEngineConfigImplTest {
             }
         }
     }
+
+    // reproduces https://github.com/awslabs/aws-sdk-kotlin/issues/1281
+    @Test
+    fun testCanConfigureProxySelectorWithInvalidEnvVarsPresent() {
+        try {
+            System.setProperty("http.proxyHost", "invalid!")
+            System.setProperty("https.proxyHost", "invalid!")
+
+            val builder = HttpEngineConfigImpl.BuilderImpl()
+            builder.buildHttpEngineConfig()
+
+            builder.httpClient {
+                proxySelector = ProxySelector.NoProxy
+            }
+            builder.buildHttpEngineConfig()
+        } finally {
+            System.clearProperty("http.proxyHost")
+            System.clearProperty("https.proxyHost")
+        }
+    }
 }
 
 private fun config(block: HttpEngineConfig.Builder.() -> Unit): HttpEngineConfig =
diff --git a/runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelector.kt b/runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelector.kt
index d656ff759..c3ad5d3bd 100644
--- a/runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelector.kt
+++ b/runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelector.kt
@@ -30,11 +30,9 @@ import aws.smithy.kotlin.runtime.util.PropertyProvider
  * - `no_proxy`, `NO_PROXY`
  */
 internal class EnvironmentProxySelector(provider: PlatformEnvironProvider = PlatformProvider.System) : ProxySelector {
-    private val httpProxy =
-        resolveProxyByProperty(provider, Scheme.HTTP) ?: resolveProxyByEnvironment(provider, Scheme.HTTP)
-    private val httpsProxy =
-        resolveProxyByProperty(provider, Scheme.HTTPS) ?: resolveProxyByEnvironment(provider, Scheme.HTTPS)
-    private val noProxyHosts = resolveNoProxyHosts(provider)
+    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) }
 
     override fun select(url: Url): ProxyConfig {
         if (httpProxy == null && httpsProxy == null || noProxy(url)) return ProxyConfig.Direct
diff --git a/runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelectorTest.kt b/runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelectorTest.kt
index 3ff4d0829..3db876f9a 100644
--- a/runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelectorTest.kt
+++ b/runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelectorTest.kt
@@ -102,8 +102,9 @@ class EnvironmentProxySelectorTest {
     fun testSelectFailures() {
         failCases.forEachIndexed { idx, failCase ->
             val testProvider = TestPlatformEnvironmentProvider(failCase.env, failCase.props)
+            val selector = EnvironmentProxySelector(testProvider)
             val exception = assertFailsWith<ClientException>("[idx=$idx] expected ClientException") {
-                EnvironmentProxySelector(testProvider)
+                selector.select(Url.parse("http://localhost:8000")) // call `select` because proxy selector resolves env vars lazily
             }
 
             val expectedError = (failCase.env + failCase.props).map { (k, v) -> """$k="$v"""" }.joinToString(", ")