SocketsHttpHandler Doesn't Respect Manual Proxy Bypass

[adamrodger]

Issue

.Net Core versions tested: 2.1.2, 2.1.6, 2.2.0
OS tested: Windows Server 2016 x64

If your Windows proxy settings are set to:

then SocketsHttpHandler doesn't respect proxy bypass preferences.

Reproduction

• In Internet Options:
  • Tick "Automatically detect settings"
  • Populate the proxy server details at the bottom
  • Click 'Advanced' and add some manual proxy exclusions
• Using a HttpClient, request a URL which should match the manual exclusion list.

Expected result: The request does not go via the proxy as it matches the bypass list
Actual Result: The bypass list is ignored and the request goes via the proxy

Cause

With the configuration above, WinInetProxyHelper has the following properties:

WinInetHttpProxyHelper:
        AutoConfigUrl:
        AutoDetect: True
        AutoSettingsUsed: True
        ManualSettingsOnly: False
        Proxy: myproxy.local:8080
        ProxyBypass: localhost;*.local

When WinInetProxyHelper.GetProxyForUrl() is called, it fails during auto-detection since there's no PAC file, and then falls back to the manual proxy settings (in this case myproxy.local:8080). The manual settings are then parsed and returned by HttpSystemProxy.GetProxy()

However, HttpSystemProxy.IsBypassed contains an assumption that HttpSystemProxy.GetProxy() will have returned null when AutoSettingsUsed is true, but that assumption is incorrect because the manual fallback settings were returned instead of null. This means the URL goes via the proxy even if it matches the bypass list.

Workaround

You can either:

• Untick "Automatically detect settings" to cause AutoSettingsUsed to be false, then HttpSystemProxy.GetProxy() will actually check the bypass list and return null if a bypass matches, or
• Set the environment variable DOTNET_SYSTEM_NET_HTTP_USESOCKETSHTTPHANDLER to 0 so that SocketsHttpHandler isn't used (which shows that this is a regression from the older WinHttpHandler implementation).

The longer term solution should probably be that if auto-detection is enabled but fails, set AutoSettingsUsed to false so that the normal manual behaviour is used.

[rmkerr]

Thanks for the super detailed report! This should make it a lot easier to track down and resolve the issue.

Since you've clearly put a lot of work into understanding what's going on here, do you have any interest in trying to make the fix?

[adamrodger]

Hi @rmkerr - I would definitely be interested in fixing it, I've just found it really difficult to actually get the solution built because it's so big and complicated. The way I diagnosed this was actually copying and pasting the relevant code into a new solution of my own to create a little console app that proves the problem... Oh dear 😄

I'll have another go and let you know if I get anywhere. At the moment, I'm thinking the best way to fix it would be to set AutoSettingsUsed to false if it falls back to manual settings

[adamrodger]

Well, I've been trying to fix that for 4 hours now and I'm not really much closer. The tests can only be compiled and executed via the CLI instead of in Visual Studio, which means I can't attach a debugger. Because the tests have to do fake interop that makes them very difficult to diagnose without the aid of running the tests via the debugger.

Coupled to that, every time I do build the tests via the CLI, my VS breaks and I have to recompile the whole repo from root again. It's very frustrating!

I have started writing a test though:

[Fact]
public void HttpProxy_FailedAutoDetection_Bypassed()
{
    const string proxy = "myproxy.local";
    const string bypass = "*.local";
    const string dontBypassUri = "http://example.org";
    const string bypassUri = "http://example.local";

    RemoteInvoke((proxyString, bypassString, dontBypassUrlString, bypassUrlString) =>
    {
        TestControl.ResetAll();

        // WinInet has "Automatically detect settings" ticked in the Internet Options dialog
        FakeRegistry.WinInetProxySettings.AutoDetect = true;

        // A manual proxy is supplied in the same dialog, with a bypass list
        FakeRegistry.WinInetProxySettings.Proxy = proxyString;
        FakeRegistry.WinInetProxySettings.ProxyBypass = bypass;

        // There is no PAC file to auto-detect
        TestControl.PACFileNotDetectedOnNetwork = true;

        WinInetProxyHelper proxyHelper = new WinInetProxyHelper();
        
        Assert.Equal(true, proxyHelper.AutoDetect);
        Assert.Equal(true, proxyHelper.AutoSettingsUsed);
        Assert.Equal(false, proxyHelper.ManualSettingsOnly);
        Assert.Null(proxyHelper.AutoConfigUrl);
        Assert.Equal(proxyString, proxyHelper.Proxy);
        Assert.Equal(bypassString, proxyHelper.ProxyBypass);
        Assert.Equal(false, proxyHelper.RecentAutoDetectionFailure);

        HttpSystemProxy.TryCreate(out IWebProxy webProxy);

        Assert.NotNull(webProxy.GetProxy(new Uri(dontBypassUri)));
        Assert.Null(webProxy.GetProxy(new Uri(bypassUri)));
        Assert.Equal(true, proxyHelper.RecentAutoDetectionFailure);
    }, proxy, bypass, dontBypassUri, bypassUri).Dispose();
}

[rmkerr]

Glad to hear you're taking a look at it!

I haven't tried debugging the tests in VS recently, so I'm not familiar with that particular issue. If you haven't seen it yet, the developer guide contains a ton of useful info, including how to build or run the tests for just a single project.

You may also be able to get help via the corefx gitter chat room. There are a lot of other contributors that check in regularly there, and it's likely someone knows more about the issues you're running into.

[adamrodger]

Thanks for that. I'd already read the dev guide but I've not been in the gitter channel so I'll give that a go. I would like to get it sorted myself ideally. I notice the issue has been tagged as version 3.0, is that correct? Just so I know which branch to base the fix on.

…

On Fri, 7 Dec 2018, 18:46 Max Kerr, ***@***.***> wrote: Glad to hear you're taking a look at it! I haven't tried debugging the tests in VS recently, so I'm not familiar with that particular issue. If you haven't seen it yet, the developer guide <https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md> contains a ton of useful info, including how to build or run the tests for just a single project. You may also be able to get help via the corefx gitter chat room <https://gitter.im/dotnet/corefx>. There are a lot of other contributors that check in regularly there, and it's likely someone knows more about the issues you're running into. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <https://github.com/dotnet/corefx/issues/33866#issuecomment-445327278>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAfCqleBpVgJQh1kNRv4vwIUtX3aaUtQks5u2rdzgaJpZM4ZGyam> .

[rmkerr]

Yep, 3.0 is correct. Development for 3.0 is currently out of the master branch, so you can just fix it there. There is a precedent for porting this kind of proxy fix back to the LTS releases, but getting it fixed in master is the first step.