Bypass all proxies for all hosts if no_proxy='*' is set
#2395
Conversation
E.g. no_proxy='.github.com' will now bypass github.com.
src/Test/L0/RunnerWebProxyL0.cs
Outdated
| Assert.False(proxy.IsBypassed(new Uri("https://example.com"))); | ||
| Assert.False(proxy.IsBypassed(new Uri("http://example.com:333"))); | ||
| Assert.False(proxy.IsBypassed(new Uri("http://192.168.0.123:123"))); | ||
| Assert.False(proxy.IsBypassed(new Uri("http://192.168.1.123/home"))); | ||
|
|
||
| Assert.True(proxy.IsBypassed(new Uri("https://google.com"))); |
There was a problem hiding this comment.
Hey, in this test why is "https://actions.com" not bypassed and "https://google.com" is?
There was a problem hiding this comment.
"actions.com" is not listed in the no_proxy ENV Var, so it will not be bypassed.
.google.com is present in no_proxy, and this change will make it so that such a format in no_proxy also bypasses the top level domain.
There was a problem hiding this comment.
Ahh right, I should've checked the whole file. Thank you! 🍫
src/Runner.Sdk/RunnerWebProxy.cs
Outdated
| // Strip leading '.' fron noproxy host | ||
| if (noProxyInfo.Host.StartsWith(".")) | ||
| { | ||
| noProxyInfo.Host = noProxyInfo.Host[1..]; |
There was a problem hiding this comment.
What by mistake someone set the host to sth like "cs."?
There was a problem hiding this comment.
Like no_proxy="example.com, cs."?
I don't think any valid Hostnames end with cs., end with a .. So, cs. would match nothing.
There was a problem hiding this comment.
Aha this is a range operation, so if the host will be equal to cs. this logic will set noProxyInfo.Host to just .
It's definitely not a valid host but it could be added by mistake. :)
|
Closing this to keep current behaviour which is consistent with |
fhammerl
changed the title
no_proxy='*' is set
| @@ -351,7 +351,7 @@ public void WebProxyFromEnvironmentVariablesNoProxy() | |||
| Assert.False(proxy.IsBypassed(new Uri("https://actions.com"))); | |||
| Assert.False(proxy.IsBypassed(new Uri("https://ggithub.com"))); | |||
| Assert.False(proxy.IsBypassed(new Uri("https://github.comm"))); | |||
| Assert.False(proxy.IsBypassed(new Uri("https://google.com"))); | |||
| Assert.False(proxy.IsBypassed(new Uri("https://google.com"))); // no_proxy has '.google.com', specifying only subdomains bypass | |||
There was a problem hiding this comment.
Not a change in behaviour in this PR. It's an additional test clarifying existing behaviour.
* Bypass top level domain even if no_proxy specified it with leading '.' E.g. no_proxy='.github.com' will now bypass github.com. * Bypass proxy on all hosts when no_proxy is * (wildcard) * Undo '.' stripping * Simplify unit tests * Respect wildcard even if it's one of many no_proxy items
The behaviour of
no_proxyis in no way standardised, and it is handled differently by the runner app vs in github actions (see actions/toolkit PR below). The behaviour in this PR is consistent withno_proxy='*'will now bypass any proxy for any host.