Use "IE Proxy Settings" correctly on Windows in vcpkg.

[reitowo]

It is common in China or any other countries and regions that needs an proxy software (v2ray, shadowsocks, etc.) for faster connection speed. These softwares typically sets Windows IE Proxy Settings, but not setting environment variables HTTP(S)_PROXY.

I found out that when vcpkg starts an external application (like cmake), it builds an environment variable list. We can use this design to make life easier. Because it is annoying to manually set variables in CLI, or open Advanced Computer Settings and set environment variables there. When using vcpkg in Visual Studio, we even need to restart VS to make the proxy variables take effects. It is also not a good way to just leave HTTP(S)_PROXY in the Windows environment variables list, because it may cause unintended behaviour, and sometimes we do not want to use a proxy at all.

As mentioned in #48, vcpkg wants an coherence design about using proxy that only detects HTTPS_PROXY. Thats reasonable because many external applications vcpkg uses (cmake, etc..) detects HTTP(S)_PROXY. But why not make life easier? We can do better than that and still follow this design.

By reading IE Proxy Settings manually, and sets HTTP(S)_PROXY in the environment variable list building process, this will make all external applications happy.

if (System::get_windows_ie_proxy_enabled())
{
    auto proxy = System::get_windows_ie_proxy_server();
    if (proxy.has_value())
    {
        // Most HTTP proxy DOES proxy HTTPS requests.
        env.emplace("HTTP_PROXY", proxy.value().c_str());
        env.emplace("HTTPS_PROXY", proxy.value().c_str());

        System::print2(
            "-- Automatically setting HTTP(S)_PROXY environment variables to ", proxy.value(), "\n");
    }
}

Then what's left is vcpkg itself. I found two places that use proxy.

1. tls12-download.c
   This small application also tried to detect HTTPS_PROXY, we can add these code to detect IE Proxy Settings either.

// If HTTPS_PROXY not found, try use IE Proxy. This works on Windows 10 20H2, so there's no need to determine
// the OS version >= 8.1.
if (access_type == WINHTTP_ACCESS_TYPE_NO_PROXY)
{
    WINHTTP_CURRENT_USER_IE_PROXY_CONFIG ieProxy;
    if (WinHttpGetIEProxyConfigForCurrentUser(&ieProxy) && ieProxy.lpszProxy != NULL)
    {
        WINHTTP_PROXY_INFO proxy;
        proxy.dwAccessType = WINHTTP_ACCESS_TYPE_NAMED_PROXY;
        proxy.lpszProxy = ieProxy.lpszProxy;
        proxy.lpszProxyBypass = ieProxy.lpszProxyBypass;
        WinHttpSetOption(session, WINHTTP_OPTION_PROXY, &proxy, sizeof(proxy));

        write_message(std_out, L" (using IE proxy: ");
        write_message(std_out, proxy.lpszProxy);
        write_message(std_out, L")");

        GlobalFree(ieProxy.lpszProxy);
        GlobalFree(ieProxy.lpszProxyBypass);
        GlobalFree(ieProxy.lpszAutoConfigUrl);
    }
}

2. vcpkg/base/downloads.cpp
   This code uses deprecated API IsWindows8Point1OrGreater and IsWindows7OrGreater to determine whether to use IE Proxy Settings. This have two errors.

• These two API always returns false on OS >=8.1 unless an manifest is provided. (Provide better experience to Windows proxy users. #48) (See this and remark section in this)
• IE Proxy Settings also works on my Windows 10 20H2 and I don't know why not using it.

So I make two changes to that file.

• In line 92 I use WINHTTP_ACCESS_TYPE_NO_PROXY to prevent WPAD use. (Provide better experience to Windows proxy users. #48)
• In later I removed IsWindows8Point1OrGreater and IsWindows7OrGreater. Currently if there's no HTTPS_PROXY variable detected, it will detect IE Proxy Settings.

I think these changes will make vcpkg much easier to use, and also keeps the vcpkg's design language.
I have no idea why Checks failed at 'Create Diff' too. Run 'format' target before commit!

[strega-nil]

@cnSchwarzer it fails at "create diff" because the formatting is off; you can just download the diff file which was created and apply it to your code via git apply format.diff.

[ras0219]

Here's the python implementation, for reference: https://github.com/python/cpython/blob/7215d1ae25525c92b026166f9d5cac85fb1defe1/Lib/urllib/request.py#L2655

[BillyONeal]

The CI failure is that you need to clang-format various files. If you look in the artifacts for the build failure there's a format.diff with the changes to apply.

[reitowo]

@BillyONeal I find out that setting http://ip:port to HTTPS_PROXY means let https:// requests go through an HTTP proxy, and setting https://ip:port to HTTPS_PROXY means go through an HTTPS proxy.

Which means, HTTPS_PROXY doesn't have to be https:// prefixed. It doesn't mean an HTTPS proxy, but https:// requests go through this

We even don't need to worry about that, just remove the type:// prefix also works (setting ip:port to HTTP(S)_PROXY works on me)

[reitowo]

The difference between HTTP and HTTPS proxies is end-to-end security. My understanding is: When an https request go through HTTP proxy, the proxy can see plain request and then make a real secure https request at proxy server side. However HTTPS proxy accepts an CONNECT verb and can't see plain request, is that true? And is that a vcpkg's security concern (tls12-download only wants to accept real secure HTTPS proxy)?

[reitowo]

👀Any update on this? @BillyONeal

By the way, should I add an flag like --no-auto-ieproxy to disable this automatic behaviour? @strega-nil

[reitowo]

I'm new to PR rules and I don't get it why this PR stucks at this state? Anything should I do before it can get merged? 🙏Please point out, much thanks!

[BillyONeal]

👀Any update on this? @BillyONeal

Not right now.

By the way, should I add an flag like --no-auto-ieproxy to disable this automatic behaviour? @strega-nil

I don't think you need to do that although I wouldn't argue against it.

I'm new to PR rules and I don't get it why this PR stucks at this state? Anything should I do before it can get merged? 🙏Please point out, much thanks!

Sorry for the delay; maintainer folks have just been hyper busy over the last few days.

[reitowo]

🙏Sorry! Thanks all of you for your works! Take your time! @BillyONeal

[BillyONeal]

Don't need to be sorry! Completely reasonable to ask what the heck is going on with a review with a signoff sitting for like a week. I would probably have merged by now but I'm not technically a vcpkg maintainer right now.

[strega-nil]

Thanks @cnSchwarzer, sorry for not merging earlier :)

[BillyONeal]

microsoft/vcpkg#17688 <-- It looks like this change causes us to crash... maybe?

[reitowo]

microsoft/vcpkg#17688 <-- It looks like this change causes us to crash... maybe?

Oh, let me check it.

[reitowo]

microsoft/vcpkg#17688 <-- It looks like this change causes us to crash... maybe?

Not related, the current vcpkg's repository didn't update tls12-download.exe, it's still old Feb version.

[BillyONeal]

Yeah, sorry for the assumption :)