add back parsing for simple configuration #31314
Conversation
|
@dotnet-bot test outerloop linux x64 debug build |
| ParseProxyConfig(proxyHelper.Proxy, out _insecureProxyUri, out _secureProxyUri); | ||
| if (_insecureProxyUri == null && _secureProxyUri == null) | ||
| { | ||
| // If advance parsing by protocol fails, fall-back to simplified parsing. |
| WinInetProxyHelper proxyHelper = new WinInetProxyHelper(); | ||
|
|
||
| Assert.True(HttpSystemProxy.TryCreate(out p)); | ||
| Assert.NotNull(p); |
There was a problem hiding this comment.
Instead of just Assert'ing for NotNull, you should Assert that the URI created is what you expected. I.e. from the rawProxyString="localhost:1234", you get "http://localhost:1234" etc. You would need to add the expected URI string as an additional InlineData parameter.
| @@ -80,9 +80,11 @@ public void HttpProxy_SystemProxy_Loaded(string rawProxyString) | |||
|
|
|||
| Assert.True(HttpSystemProxy.TryCreate(out p)); | |||
| Assert.NotNull(p); | |||
| Assert.Equal(expectedString, p.GetProxy(new Uri(fooHttp)).ToString()); | |||
| Assert.Equal(expectedString, p.GetProxy(new Uri(fooHttps)).ToString()); | |||
There was a problem hiding this comment.
Why is "fooHttp" and "fooHttps" being used here? Shouldn't it be "expectedString" ?
There was a problem hiding this comment.
p.GetProxy() expects destination URI to pass in and returns proxyURI.
It does not really mater here but we used the foo* as test target elsewhere so it does not mix with proxy values.
* add back simplified proxy configuration * add tracing * feedback from review
* add back simplified proxy configuration * add tracing * feedback from review Commit migrated from dotnet/corefx@2c5f290
it seems like this got broken by #28671.
Proxy setting can contain separate http and https, but it does not in simplifies case.
In scenario from #31242 we simply get "localhost:3128" string without any protocol specification.
Based on my testing, this would apply to both http & https.
fixes #31242