Parsing env variable NO_PROXY

[a-brunelliere-maif]

Describe the bug

Hi,

I don't know if the content of the NO_PROXY environment variable is actually described by a standard, but a pattern like this is quite common : "localhost, .example.com"

The code software.amazon.awssdk.utils.http.SdkHttpUtils.extractNonProxyHosts() code relies on a block of code that only expects a localhost|*.example.com format

Expected Behavior

Format like "localhost, .example.com" is allowed

Current Behavior

Format like "localhost, .example.com" is misinterpreted

Reproduction Steps

Create a client with env var NO_PROXY=localhost, .example.com

Possible Solution

No response

Additional Information/Context

No response

AWS Java SDK version used

2.21.22

JDK version used

11

Operating System and version

windows 11

[debora-ito]

@a-brunelliere-maif thank you for reaching out.

The reason why we use '|' as separators is because it's the standard for Java System Properties - According to the Java SE 8 Networking Properties documentation:

http.nonProxyHosts (default: localhost|127.|[::1])
Indicates the hosts that should be accessed without going through the proxy. Typically this defines internal hosts. The value of this property is a list of hosts, separated by the '|' character. In addition the wildcard character '*' can be used for pattern matching. For example -Dhttp.nonProxyHosts=”.foo.com|localhost” will indicate that every hosts in the foo.com domain and the localhost should be accessed directly even if a proxy server is specified.

We use the same '|' separator in the Environment Variable for consistency.

I don't see this is mentioned in our Developer Guide, so it's an opportunity to improve it.

Can you use '|' instead of a comma ',' in the environment variable?

[a-brunelliere-maif]

Hi @debora-ito , thanks for you your answer. You provide me the right documentation for the system property "http.nonProxyHosts" and we respect this format for it.
But the env variable "NO_PROXY" has an another common format.
An exemple of this format can be found here : https://about.gitlab.com/blog/2021/01/27/we-need-to-talk-no-proxy/

no_proxy is a comma- or space-separated list of machine
or domain names, with optional :port part. If no :port
part is present, it applies to all ports on that domain.
Example: no_proxy="cern.ch,some.domain:8001"

The amazon client s3 can retrieve the both values, except it use the "http.nonProxyHosts" format for the "NO_PROXY" variable

[martinsefcik]

It is also issue for us because we use another applications which are using NO_PROXY environment variable and they supports only I would say more "standardized" comma separated list so we cannot change it to pipes.

So this issue is blocking us from upgrading from 2.20.x (which contains vulnerability in netty dependencies - CVE-2023-4586) to 2.21.x.

[debora-ito]

Just to set expectations, we can't simply change the separator to ',' because this would break people already relying on '|'. A non-breaking solution will take a little more work.

Community note: please vote by adding a 👍 reaction to the original issue to help us prioritize this request.

@martinsefcik this should not block you from upgrading to 2.21.x, there was no change in behavior of the NO_PROXY environment variable from 2.20.x - meaning if you use comma separated list then NO_PROXY was not working on 2.2.0.x and it is still not working in 2.21.x.

[martinsefcik]

@debora-ito I have configured HTTP(S)_PROXY env variables with my company proxy server ip and port. Then I have NO_PROXY configured with various hostnames which includes also localhost and 127.0.0.1. All of them are comma separated. If I use aws sdk 2.20.x (tested 2.20.76 and 2.20.162) I am able to connect to locally running localstack (s3, sns). If I switch to 2.21.x (2.21.0 or 2.21.27) then I get 403 http response and in response body I can see that this response is from my company proxy server. So my NO_PROXY configuration is not respected and It is trying to go via proxy even it should not be. If I replace commas with pipes in NO_PROXY variable value then everything start working correctly also with 2.21.x

So I'm not sure what exactly is a problem, but it looks very related to this issue and initially I thought that you drop support for commas in NO_PROXY parsing in 2.21.0

[debora-ito]

@martinsefcik I see, that looks like a regression.

Which HTTP client were you using with 2.20.x? Can you share a code sample I can use to reproduce?

[a-brunelliere-maif]

I think, there was a regression due to this PR : #4467

There was an add 6 weeks ago to use NO_PROXY (with the wrong format), this env variable wasn't used before by the client.

[abatkin]

Let's be clear: Parsing NO_PROXY environment variable with | instead of , is objectively wrong. It's unlikely that anyone is relying on this behavior, and if they are then they will need to break (I don't say that lightly).

Here's my justification:

• The PR was only merged 6 weeks agoo
• Since every other HTTP library on Earth uses , instead of | then someone would need to specifically set a NO_PROXY environment variable with the expectation that it would only ever be used by Java (i.e. it can't possibly be used by any other application) and it's more likely that they would use a system property
• Support Proxy Configuration from Environment Variables also refactor existing ProxyConfig #4467 is already a regression and shouldn't have been merged as-is (it broke a number of our applications due to this change) BUT had it parsed NO_PROXY correctly then we wouldn't have been impacted

[joviegas]

@a-brunelliere-maif , @martinsefcik @abatkin Appreciate your response and details provided to this issue. Thank you 👍
We have merged the fix.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.