Provide better experience to Windows proxy users.

[reitowo]

There are several places I modified:

1. tls12-download.c
   This file doesn't use Windows proxy settings when HTTPS_PROXY env variable not found. The correct manner should be use access_type = IsWindows8Point1OrGreater() ? WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY : WINHTTP_ACCESS_TYPE_DEFAULT_PROXY;, not just setting WINHTTP_ACCESS_TYPE_NO_PROXY as it is inconvenient to set env variable on Windows.

2. src/vcpkg/base/downloads.cpp
   VersionHelper.h (IsWindows8Point1OrGreater, etc..) is deprecated. See this and remark section in this. Applications not manifested for Windows 8.1 or Windows 10 return false, even if the current operating is 8.1 or 10. So we need to do use RtlGetVersion to get correct OS version. Currently the program doesn't seems to have a manifest, and wrongly setting proxies.

3. pch.h
   Where i will use registry functions

4. system.proxy.h/.cpp
   Write an helper function to determine whether there's a active Windows proxy setting.

5. build.cpp
   At System::Environment& EnvCache::get_action_env I added the proxy detection and add the HTTP_PROXY and HTTPS_PROXY variables to env list.

[ghost]

All CLA requirements met.

[BillyONeal]

This file doesn't use Windows proxy settings when HTTPS_PROXY env variable not found.

I did that because I could not find an HTTPS proxy to test against, and have never seen an HTTPS proxy deployed in the wild. Plain HTTP proxies are common but HTTPS ones aren't. Proxies kinda defeat the purpose of TLS in most cases.

Moreover, the "automatic proxy" behavior creates differences in behavior in the tool; the vast majority of the vcpkg catalog use download mechanisms that don't respect WinHTTP's proxy settings or WPAD.

VersionHelper.h (IsWindows8Point1OrGreater, etc..) is deprecated. See this and remark section in this.

Neither of those indicate that IsWindows8Point1OrGreater are deprecated. They indicate that one should normally use feature testing rather than operating system version checks. As in, we should try to use the new feature, and if that fails, fall back to not using the new feature. Changing it to use RtlGetVersion instead doesn't resolve what any of that is complaining about. If you want the normal functions to have the desired behavior the correct fix is to add a manifest indicating the highest tested operating system version ( https://docs.microsoft.com/en-us/windows/win32/sysinfo/targeting-your-application-at-windows-8-1 ).

Do you have an actual HTTPS proxy against which the behavior can be tested? When I wrote tls12-download.exe I explicitly chose no proxy to avoid the aforementioned WPAD hangs and because I could not find an HTTPS proxy available to test for correct behavior.

[reitowo]

@BillyONeal

I did that because I could not find an HTTPS proxy to test against, and have never seen an HTTPS proxy deployed in the wild. Plain HTTP proxies are common but HTTPS ones aren't. Proxies kinda defeat the purpose of TLS in most cases.

I use a v2ray proxy by setting HTTPS_PROXY to http://127.0.0.1:10809 which works well, does that mean a "HTTPS proxy"?

Neither of those indicate that IsWindows8Point1OrGreater are deprecated. They indicate that one should normally use feature testing rather than operating system version checks. As in, we should try to use the new feature, and if that fails, fall back to not using the new feature. Changing it to use RtlGetVersion instead doesn't resolve what any of that is complaining about.

According to the link you provided they say:

In Windows 8.1 and Windows 10, the GetVersion and GetVersionEx functions have been deprecated. In Windows 10, the VerifyVersionInfo function has also been deprecated. While you can still call the deprecated functions, if your application does not specifically target Windows 8.1 or Windows 10, the functions will return the Windows 8 version (6.2).

Which clearly says that these API are deprecated, and providing an manifest seems just a workaround. It seems no replacing API is provided by Windows. However use RtlGetVersion will give correct OS version value. I also don't know why should and how to add an manifest.

[BillyONeal]

I use a v2ray proxy by setting HTTPS_PROXY to http://127.0.0.1:10809 which works well, does that mean a "HTTPS proxy"?

That's not an HTTPS proxy. That's an HTTP proxy. (It even says http)

According to the link you provided they say:

In Windows 8.1 and Windows 10, the GetVersion and GetVersionEx functions have been deprecated. In Windows 10, the VerifyVersionInfo function has also been deprecated. While you can still call the deprecated functions, if your application does not specifically target Windows 8.1 or Windows 10, the functions will return the Windows 8 version (6.2).

GetVersion and GetVersionEx are deprecated in favor of IsXxxOrGreater.

However use RtlGetVersion will give correct OS version value. I also don't know why should and how to add an manifest.

Manifests are the documented / supported way of doing this.

[reitowo]

Also, i write an proxy detection code by detecting the registry, and set the proxyserver to env list. This is because cmake file(DOWNLOAD doesn't detect windows proxy settings and only way to do that is setting HTTP(S)_PROXY.

[reitowo]

@BillyONeal

That's not an HTTPS proxy. That's an HTTP proxy. (It even says http)

That's true, but should a HTTP proxy supports https requests? Because all https downloads are proxy-able by this proxy.

[reitowo]

@BillyONeal
When I set HTTPS_PROXY to https://127.0.0.1:10809 is won't work (obviously), but http://127.0.0.1:10809 works. I think this indicates that the v2ray starts an HTTP proxy, but it DO supports proxying HTTPS requests. So I think it is ok to proxy to that address.

Also, please check my latest commit which reads Windows proxy settings and manually set the server address to environment list, I think it is way more user friendly to Windows proxy users.

[BillyONeal]

Also, i write an proxy detection code by detecting the registry, and set the proxyserver to env list. This is because cmake file(DOWNLOAD doesn't detect windows proxy settings and only way to do that is setting HTTP(S)_PROXY.

This argues even more for not turning on WPAD because it makes different parts of vcpkg inconsistent with each other since some parts would be doing WPAD and some parts would not be.

That's true, but should a HTTP proxy supports https requests? Because all https downloads are proxy-able by this proxy.

I tried proxies configured in this way when I wrote tls12-download.exe and the result I observed was that TLS negotiation failed to the proxy, so WinHTTP gave up and used no proxy (or failed). I was going to add support for HTTPS_PROXY but when I couldn't find an example to verify that it worked I didn't add such support.

Overall I think it should go like this:

+ Do we use WPAD?
  If yes, manifest both programs and replicate the `IsXxxOrGreater` in `vcpkg.exe` to `tls12-download.exe`.
  If no, remove the `IsXxxOrGreater` check from `vcpkg.exe`.
+ Do we support `HTTPS_PROXY`?
  If yes, we need a server that can succeed to test against and add the code to handle it to both places.
  If no, we should change nothing

The vcpkg probably has the IsXxxOrGreater only by virtue of copy/paste from an old cpprestsdk. (Where it should be noted we had to disable WPAD by default because it caused too many programs to break)

Personally I think we should not support WPAD and should support HTTPS_PROXY for consistency with cmake which we use to download lots of other things. I would have already done that to tls12-download.exe if I had been able to verify that it worked. (And would still be interested in such a PR with instructions on verifying that it works)

[BillyONeal]

I'll bring this up with other maintainers to get answers to / confirmation of the above questions.

[reitowo]

@BillyONeal
Thanks!

I tested on these IsXXXOrGreater and they do failed on OS versions above 8.1, which cause some if statements always false. We should either provide manifest or use other Rtl APIs.

Also, using HTTP(S)_PROXY does have consistency, but I also find out that vcpkg will generate environment variable list when starting other executables, where we can detect Windows IE Proxy settings in registry and manually insert HTTP(S)_PROXY variables to that list (Just like the last few commits do). This doesn't break the consistency and provide better experience.

Like this:

[BillyONeal]

I tested on these IsXXXOrGreater and they do failed on OS versions above 8.1, which cause some if statements always false. We should either provide manifest or use other Rtl APIs.

Right, they still require a manifest. The purpose of the manifest is to tell the OS what the latest version of the OS we tested with is.

Also, using HTTP(S)_PROXY does have consistency, but I also find out that vcpkg will generate environment variable list when starting other executables, where we can detect Windows IE Proxy settings in registry and manually insert HTTP(S)_PROXY variables to that list (Just like the last few commits do).

If we can create consistent behavior that would be fine by me -- the most important thing as far as I'm concerned is that we respond to proxies consistently in the product, and right now the only thing we can do reasonably consistently are the environment variables. If we can set the environment appropriately that would be fine too.

Although I don't see the output you describe and don't see anywhere in the tool where such behavior is being applied...

[reitowo]

@BillyONeal

Although I don't see the output you describe and don't see anywhere in the tool where such behavior is being applied...

It is in this PR's last 5 commits.

However tls12-download.exe is called by bootstrap.ps1. Users also need to set variables before calling bootstrap.

Does WPAD refer to WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY? If so, we can also read the registry just like my last few commits does and prevent WPAD.

It will be nice if we provide --proxy or --auto-detect-proxy etc. switches. Providing switches satisfies both kind of users.

[BillyONeal]

It is in this PR's last 5 commits.

🤦‍♂️ Sorry!

However tls12-download.exe is called by bootstrap.ps1. Users also need to set variables before calling bootstrap.

Right, if we're setting those variables making bootstrap respond to the settings in a consistent way is good.

Does WPAD refer to WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY? If so, we can also read the registry just like my last few commits does and prevent WPAD.

Yes. WPAD == Web Proxy Auto Discovery; it's what WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY requests: https://en.wikipedia.org/wiki/Web_Proxy_Auto-Discovery_Protocol

[reitowo]

@BillyONeal

Yes. WPAD == Web Proxy Auto Discovery; it's what WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY requests

Oh, thanks!

BTW, does the auto check failure my fault? It seems always stuck at Make Diff or Building rapidjson

[reitowo]

@BillyONeal
In the commit above I removed the WPAD and use registry check in tls12-download.c.

[reitowo]

Too complicated for a PR, will reopen a new one.