From d94b749beaf79b6bf8b559c2612ca7e9c4297a97 Mon Sep 17 00:00:00 2001 From: Andy McCalib Date: Tue, 4 Aug 2026 13:02:31 -0700 Subject: [PATCH] Fix WinHttpProvider caching a null WinHTTP session permanently GetHSession() could return a null HINTERNET as a *successful* Result, which was then cached in m_hSessions forever. Result's single-argument constructor is the "construct successful result" overload, so callers' RETURN_IF_FAILED(result.hr) checks passed and a null session was handed to WinHttpConnection::Initialize. Because m_hSessions is keyed only by securityProtocolFlags and is cleared only by Suspend() (which never runs on desktop platforms), one transient failure poisoned the provider for the lifetime of the process. Every later request short-circuited on the cached null and failed silently, emitting no trace output at all. Three related fixes, all in GetHSession(): 1. When the fallback WinHttpOpen(WINHTTP_FLAG_ASYNC) returns null, return the error instead of warning and falling through to cache the null. 2. Only inspect GetLastError() after WinHttpOpen has actually failed. It was read unconditionally, so a stale ERROR_INVALID_PARAMETER from an unrelated call could cause a second session to be opened over a successfully opened first one, leaking the first handle. 3. Map a zero GetLastError() to E_FAIL on the initial open failure path. HRESULT_FROM_WIN32(0) is S_OK, which would construct a "successful" Result with no payload via the Result(HRESULT) overload, making Payload() assert and ExtractPayload() undefined. Verified with a standalone Win32 harness that injects WinHttpOpen / WinHttpSetOption failures via IAT patching, so the library builds unmodified. Before: after a simulated outage the provider failed every subsequent request against a healthy network. After: it recovers on the next request. Note that WinHttpSetOption(WINHTTP_OPTION_SECURE_PROTOCOLS) fails with ERROR_ACCESS_DENIED on current Windows whenever the session was opened with WINHTTP_FLAG_SECURE_DEFAULTS and the requested mask includes TLS 1.0/1.1 (which the Win32 default mask does), so the close-and-reopen fallback is the normal path for HTTPS sessions rather than a rare edge case. That reopen also drops SECURE_DEFAULTS; addressing that is left to a separate change. --- Source/HTTP/WinHttp/winhttp_provider.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Source/HTTP/WinHttp/winhttp_provider.cpp b/Source/HTTP/WinHttp/winhttp_provider.cpp index 54417605..4c6c6ff8 100644 --- a/Source/HTTP/WinHttp/winhttp_provider.cpp +++ b/Source/HTTP/WinHttp/winhttp_provider.cpp @@ -399,8 +399,7 @@ Result WinHttpProvider::GetHSession(uint32_t securityProtocolFlags, c openFlags ); - DWORD error = GetLastError(); - if (error == ERROR_INVALID_PARAMETER && isHttps) + if (hSession == nullptr && GetLastError() == ERROR_INVALID_PARAMETER && isHttps) { // WINHTTP_FLAG_SECURE_DEFAULTS exists only on newer Windows versions; // on earlier OS releases we will receive ERROR_INVALID_PARAMETER and should continue without it. @@ -414,7 +413,8 @@ Result WinHttpProvider::GetHSession(uint32_t securityProtocolFlags, c if (hSession == nullptr) { - HRESULT hr = HRESULT_FROM_WIN32(GetLastError()); + DWORD openErr = GetLastError(); + HRESULT hr = openErr != 0 ? HRESULT_FROM_WIN32(openErr) : E_FAIL; HC_TRACE_ERROR_HR(HTTPCLIENT, hr, "WinHttpProvider WinHttpOpen"); return hr; } @@ -452,8 +452,10 @@ Result WinHttpProvider::GetHSession(uint32_t securityProtocolFlags, c WINHTTP_FLAG_ASYNC); if (hSession == nullptr) { - HRESULT openHr = HRESULT_FROM_WIN32(GetLastError()); - HC_TRACE_WARNING_HR(HTTPCLIENT, openHr, "WinHttpProvider fallback WinHttpOpen with WINHTTP_FLAG_ASYNC failed; continuing without explicitly setting secure protocols"); + DWORD openErr = GetLastError(); + HRESULT openHr = openErr != 0 ? HRESULT_FROM_WIN32(openErr) : E_FAIL; + HC_TRACE_ERROR_HR(HTTPCLIENT, openHr, "WinHttpProvider fallback WinHttpOpen with WINHTTP_FLAG_ASYNC failed"); + return openHr; } else {