From 7363ecc429f3ce211affac45e309c7d14b90b407 Mon Sep 17 00:00:00 2001 From: k1ee Date: Sun, 9 May 2021 20:24:10 +0800 Subject: [PATCH 1/2] If HTTP(S)_PROXY already exist in environment variable, do not use IE Proxy auto detection. --- src/vcpkg/build.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/vcpkg/build.cpp b/src/vcpkg/build.cpp index 7692cac354..e933dfbc91 100644 --- a/src/vcpkg/build.cpp +++ b/src/vcpkg/build.cpp @@ -406,8 +406,14 @@ namespace vcpkg::Build * Visual Studio, we even cannot set HTTP(S)_PROXY in CLI, if we want to open or close Proxy we need to * restart VS. */ + + // 2021-05-09 Fix: Detect If there's already HTTP(S)_PROXY presented in the environment variables. + // If so, we no longer overwrite them. + bool proxy_from_env = (System::get_environment_variable("HTTP_PROXY").has_value() || + System::get_environment_variable("HTTPS_PROXY").has_value()); + auto ieProxy = System::get_windows_ie_proxy_server(); - if (ieProxy.has_value()) + if (ieProxy.has_value() && !proxy_from_env) { std::string server = Strings::to_utf8(ieProxy.get()->server); @@ -424,10 +430,9 @@ namespace vcpkg::Build { auto protocol = kvp[0]; auto address = kvp[1]; - if (!Strings::contains(address, "://")) - { - address = Strings::concat(protocol, "://", address); - } + // No longer append protocol prefix to address. Because HTTPS_PROXY's address is not always + // an HTTPS proxy, an HTTP proxy can also proxy HTTPS requests without end-to-end security + // (As an HTTP Proxy can see your cleartext while an HTTPS proxy can't). protocol = Strings::concat(Strings::ascii_to_uppercase(protocol.c_str()), "_PROXY"); env.emplace(protocol, address); System::print2("-- Setting ", protocol, " environment variables to ", address, "\n"); @@ -460,6 +465,10 @@ namespace vcpkg::Build env.emplace("HTTPS_PROXY", server.c_str()); } } + else if (proxy_from_env) + { + System::print2("-- Using HTTP(S)_PROXY in environment variables.\n"); + } return {env}; }); From cfc8f088b2a7902ee2e7b287a8b98dad78b81766 Mon Sep 17 00:00:00 2001 From: k1ee Date: Tue, 11 May 2021 11:38:57 +0800 Subject: [PATCH 2/2] Meet review requirements --- src/vcpkg/build.cpp | 113 +++++++++++++++++++++++++------------------- 1 file changed, 65 insertions(+), 48 deletions(-) diff --git a/src/vcpkg/build.cpp b/src/vcpkg/build.cpp index e933dfbc91..2afc6d89f1 100644 --- a/src/vcpkg/build.cpp +++ b/src/vcpkg/build.cpp @@ -412,63 +412,80 @@ namespace vcpkg::Build bool proxy_from_env = (System::get_environment_variable("HTTP_PROXY").has_value() || System::get_environment_variable("HTTPS_PROXY").has_value()); - auto ieProxy = System::get_windows_ie_proxy_server(); - if (ieProxy.has_value() && !proxy_from_env) + if (proxy_from_env) { - std::string server = Strings::to_utf8(ieProxy.get()->server); - - // Separate settings in IE Proxy Settings, which is rare? - // Python implementation: - // https://github.com/python/cpython/blob/7215d1ae25525c92b026166f9d5cac85fb1defe1/Lib/urllib/request.py#L2655 - if (Strings::contains(server, "=")) + System::print2("-- Using HTTP(S)_PROXY in environment variables.\n"); + } + else + { + auto ieProxy = System::get_windows_ie_proxy_server(); + if (ieProxy.has_value() && !proxy_from_env) { - auto proxy_settings = Strings::split(server, ';'); - for (auto& s : proxy_settings) + std::string server = Strings::to_utf8(ieProxy.get()->server); + + // Separate settings in IE Proxy Settings, which is rare? + // Python implementation: + // https://github.com/python/cpython/blob/7215d1ae25525c92b026166f9d5cac85fb1defe1/Lib/urllib/request.py#L2655 + if (Strings::contains(server, "=")) { - auto kvp = Strings::split(s, '='); - if (kvp.size() == 2) + auto proxy_settings = Strings::split(server, ';'); + for (auto& s : proxy_settings) { - auto protocol = kvp[0]; - auto address = kvp[1]; - // No longer append protocol prefix to address. Because HTTPS_PROXY's address is not always - // an HTTPS proxy, an HTTP proxy can also proxy HTTPS requests without end-to-end security - // (As an HTTP Proxy can see your cleartext while an HTTPS proxy can't). - protocol = Strings::concat(Strings::ascii_to_uppercase(protocol.c_str()), "_PROXY"); - env.emplace(protocol, address); - System::print2("-- Setting ", protocol, " environment variables to ", address, "\n"); + auto kvp = Strings::split(s, '='); + if (kvp.size() == 2) + { + auto protocol = kvp[0]; + auto address = kvp[1]; + + /* Unlike Python's urllib implementation about this type of proxy configuration + * (http=addr:port;https=addr:port) + * https://github.com/python/cpython/blob/7215d1ae25525c92b026166f9d5cac85fb1defe1/Lib/urllib/request.py#L2682 + * we do not intentionally append protocol prefix to address. Because HTTPS_PROXY's + * address is not always an HTTPS proxy, an HTTP proxy can also proxy HTTPS requests + * without end-to-end security (As an HTTP Proxy can see your cleartext while an HTTPS + * proxy can't). + * + * If the prefix (http=http://addr:port;https=https://addr:port) already exists in the + * address, we should consider this address points to an HTTPS proxy, and assign to + * HTTPS_PROXY directly. However, if it doesn't exist, then we should NOT append an + * `https://` prefix to an `addr:port` as it could be an HTTP proxy, and the connection + * request will fail. + */ + + protocol = Strings::concat(Strings::ascii_to_uppercase(protocol.c_str()), "_PROXY"); + env.emplace(protocol, address); + System::print2("-- Setting ", protocol, " environment variables to ", address, "\n"); + } } } - } - // Specified http:// prefix - else if (Strings::starts_with(server, "http://")) - { - System::print2("-- Setting HTTP_PROXY environment variables to ", server, "\n"); - env.emplace("HTTP_PROXY", server); - } - // Specified https:// prefix - else if (Strings::starts_with(server, "https://")) - { - System::print2("-- Setting HTTPS_PROXY environment variables to ", server, "\n"); - env.emplace("HTTPS_PROXY", server); - } - // Most common case: "ip:port" style, apply to HTTP and HTTPS proxies. - // An HTTP(S)_PROXY means https requests go through that, it can be: - // http:// prefixed: the request go through an HTTP proxy without end-to-end security. - // https:// prefixed: the request go through an HTTPS proxy with end-to-end security. - // Nothing prefixed: don't know the default behaviour, seems considering HTTP proxy as default. - // We simply set "ip:port" to HTTP(S)_PROXY variables because it works on most common cases. - else - { - System::print2("-- Automatically setting HTTP(S)_PROXY environment variables to ", server, "\n"); + // Specified http:// prefix + else if (Strings::starts_with(server, "http://")) + { + System::print2("-- Setting HTTP_PROXY environment variables to ", server, "\n"); + env.emplace("HTTP_PROXY", server); + } + // Specified https:// prefix + else if (Strings::starts_with(server, "https://")) + { + System::print2("-- Setting HTTPS_PROXY environment variables to ", server, "\n"); + env.emplace("HTTPS_PROXY", server); + } + // Most common case: "ip:port" style, apply to HTTP and HTTPS proxies. + // An HTTP(S)_PROXY means https requests go through that, it can be: + // http:// prefixed: the request go through an HTTP proxy without end-to-end security. + // https:// prefixed: the request go through an HTTPS proxy with end-to-end security. + // Nothing prefixed: don't know the default behaviour, seems considering HTTP proxy as default. + // We simply set "ip:port" to HTTP(S)_PROXY variables because it works on most common cases. + else + { + System::print2( + "-- Automatically setting HTTP(S)_PROXY environment variables to ", server, "\n"); - env.emplace("HTTP_PROXY", server.c_str()); - env.emplace("HTTPS_PROXY", server.c_str()); + env.emplace("HTTP_PROXY", server.c_str()); + env.emplace("HTTPS_PROXY", server.c_str()); + } } } - else if (proxy_from_env) - { - System::print2("-- Using HTTP(S)_PROXY in environment variables.\n"); - } return {env}; });