From 60e067e71aebebdda5d82955060f5f0821c9df1d Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Tue, 16 Jul 2019 07:55:55 -0700 Subject: [PATCH] Remove proxy settings detection behavior in "default proxy mode." (#1188) * Remove proxy settings detection behavior in "default proxy mode." This commit partially reverts https://github.com/microsoft/cpprestsdk/commit/eb108ada1ab23a46a09efb87f35c802e57e0832a in order to work around a reliability problem with WinHttpGetProxyForUrl. That function hangs unless there is an available thread pool thread to complete the WPAD request. As a result, if a customer issued ~512 concurrent HTTP requests, or otherwise needed that many thread pool threads, there would not be a thread available for WinHTTP to complete the operation, and the program would deadlock. Moreover this call to WinHttpGetDefaultProxyConfiguration is extremely expensive, taking ~20% of overall CPU for the entire program for some Azure Storage SDK customers. The function WinHttpGetProxyForUrlEx is supposed to help with this problem by being asynchronous, but that function was added in Windows 8, so we can't use it unconditionally. And on Windows 8.1 we already are using WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY instead of trying to do proxy autodetect ourselves. --- .../include/cpprest/details/web_utilities.h | 4 -- .../src/http/client/http_client_winhttp.cpp | 62 ++++++++++--------- 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/Release/include/cpprest/details/web_utilities.h b/Release/include/cpprest/details/web_utilities.h index aed7419c85..853d7614b1 100644 --- a/Release/include/cpprest/details/web_utilities.h +++ b/Release/include/cpprest/details/web_utilities.h @@ -10,10 +10,6 @@ ****/ #pragma once -#ifdef _WIN32 -#include -#endif // _WIN32 - #include "cpprest/asyncrt_utils.h" #include "cpprest/uri.h" diff --git a/Release/src/http/client/http_client_winhttp.cpp b/Release/src/http/client/http_client_winhttp.cpp index a6e5a226b1..249bfcd52c 100644 --- a/Release/src/http/client/http_client_winhttp.cpp +++ b/Release/src/http/client/http_client_winhttp.cpp @@ -243,6 +243,24 @@ enum msg_body_type transfer_encoding_chunked }; +static DWORD WinHttpDefaultProxyConstant() CPPREST_NOEXCEPT +{ +#if _WIN32_WINNT >= _WIN32_WINNT_VISTA +#if _WIN32_WINNT < _WIN32_WINNT_WINBLUE + if (!IsWindows8Point1OrGreater()) + { + // Not Windows 8.1 or later, use the default proxy setting + return WINHTTP_ACCESS_TYPE_DEFAULT_PROXY; + } +#endif // _WIN32_WINNT < _WIN32_WINNT_WINBLUE + + // Windows 8.1 or later, use the automatic proxy setting + return WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY; +#else // ^^^ _WIN32_WINNT >= _WIN32_WINNT_VISTA ^^^ // vvv _WIN32_WINNT < _WIN32_WINNT_VISTA vvv + return WINHTTP_ACCESS_TYPE_DEFAULT_PROXY; +#endif // _WIN32_WINNT >= _WIN32_WINNT_VISTA +} + // Additional information necessary to track a WinHTTP request. class winhttp_request_context final : public request_context { @@ -818,38 +836,30 @@ class winhttp_client final : public _http_client_communicator ie_proxy_config proxyIE; DWORD access_type; - LPCWSTR proxy_name; + LPCWSTR proxy_name = WINHTTP_NO_PROXY_NAME; LPCWSTR proxy_bypass = WINHTTP_NO_PROXY_BYPASS; + m_proxy_auto_config = false; utility::string_t proxy_str; http::uri uri; const auto& config = client_config(); - - if (config.proxy().is_disabled()) + const auto& proxy = config.proxy(); + if (proxy.is_default()) + { + access_type = WinHttpDefaultProxyConstant(); + } + else if (proxy.is_disabled()) { access_type = WINHTTP_ACCESS_TYPE_NO_PROXY; - proxy_name = WINHTTP_NO_PROXY_NAME; } - else if (config.proxy().is_default() || config.proxy().is_auto_discovery()) + else if (proxy.is_auto_discovery()) { - // Use the default WinHTTP proxy by default. - access_type = WINHTTP_ACCESS_TYPE_DEFAULT_PROXY; - proxy_name = WINHTTP_NO_PROXY_NAME; - -#if _WIN32_WINNT < _WIN32_WINNT_VISTA - if (config.proxy().is_auto_discovery()) + access_type = WinHttpDefaultProxyConstant(); + if (access_type != WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY) { + // Windows 8 or earlier, do proxy autodetection ourselves m_proxy_auto_config = true; - } -#else // ^^^ _WIN32_WINNT < _WIN32_WINNT_VISTA ^^^ // vvv _WIN32_WINNT >= _WIN32_WINNT_VISTA vvv - if (IsWindows8Point1OrGreater()) - { - // Windows 8.1 and newer supports automatic proxy discovery and auto-fallback to IE proxy settings - access_type = WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY; - } - else - { - // However, if it is not configured... + proxy_info proxyDefault; if (!WinHttpGetDefaultProxyConfiguration(&proxyDefault) || proxyDefault.dwAccessType == WINHTTP_ACCESS_TYPE_NO_PROXY) @@ -881,13 +891,7 @@ class winhttp_client final : public _http_client_communicator } } } - - if (config.proxy().is_auto_discovery()) - { - m_proxy_auto_config = true; - } } -#endif // _WIN32_WINNT < _WIN32_WINNT_VISTA } else { @@ -1007,9 +1011,7 @@ class winhttp_client final : public _http_client_communicator if (m_proxy_auto_config) { - WINHTTP_AUTOPROXY_OPTIONS autoproxy_options; - memset(&autoproxy_options, 0, sizeof(WINHTTP_AUTOPROXY_OPTIONS)); - + WINHTTP_AUTOPROXY_OPTIONS autoproxy_options {}; if (m_proxy_auto_config_url.empty()) { autoproxy_options.dwFlags = WINHTTP_AUTOPROXY_AUTO_DETECT;