Skip to content

Commit

Permalink
Remove proxy settings detection behavior in "default proxy mode." (#1188
Browse files Browse the repository at this point in the history
)

* Remove proxy settings detection behavior in "default proxy mode."

This commit partially reverts eb108ad 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.
  • Loading branch information
BillyONeal authored Jul 16, 2019
1 parent bed8fa5 commit 60e067e
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 34 deletions.
4 changes: 0 additions & 4 deletions Release/include/cpprest/details/web_utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@
****/
#pragma once

#ifdef _WIN32
#include <Windows.h>
#endif // _WIN32

#include "cpprest/asyncrt_utils.h"
#include "cpprest/uri.h"

Expand Down
62 changes: 32 additions & 30 deletions Release/src/http/client/http_client_winhttp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 60e067e

Please sign in to comment.