smithy-lang / smithy-kotlin

Smithy code generator for Kotlin (in development)
Apache License 2.0
64 stars 26 forks source link

Incorrect "http.nonProxyHosts" property name #1081

Closed grassehh closed 2 months ago

grassehh commented 2 months ago

Describe the bug

Configuring the http.nonProxyHosts system property is not working.

Expected Behavior

As explained in the documentation the property http.nonProxyHosts should be used to determine non proxy hosts.

Current Behavior

Setting http.nonProxyHosts system property has no effect because http.noProxyHosts is used instead.

Steps to Reproduce

Simply configure a client using environment while having defined http.nonProxyHosts system property. For example:

S3Client.fromEnvironment {}

Then if you make a request to a host defined in the system property, the http client will still try to go through the proxy.

Possible Solution

Replace http.noProxyHosts with http.nonProxyHosts at this line : (https://github.com/smithy-lang/smithy-kotlin/blob/2c138edee0d33548d4ea6a8dfa99da4509b4c3a4/runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelector.kt#L131)

Context

We were trying to configure the proxy of an S3 client only by using the Java System properties.

Your Environment

lauzadis commented 2 months ago

Thanks for the bug report! You're right, we should be checking http.nonProxyHosts instead of http.noProxyHosts as the Java specification states.

To maintain backwards compatibility, we'll still need to support both system properties, but we will prioritize the new value http.nonProxyHosts.