diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index b08fd4537..58f68baf5 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -299,9 +299,17 @@ target_compile_definitions(matsdk_internal_config INTERFACE _USRDLL WINVER=_WIN32_WINNT_WIN7) target_compile_options(matsdk_internal_config INTERFACE /U_MBCS) +option(MATSDK_USE_WININET + "Use WinInet instead of WinHTTP as the Win32 desktop HTTP client" OFF) +if(MATSDK_USE_WININET) + target_compile_definitions(matsdk_internal_config INTERFACE HAVE_MAT_WININET_HTTP_CLIENT) +endif() list(APPEND SRCS http/HttpClient_WinInet.cpp http/HttpClient_WinInet.hpp + http/HttpClient_WinHttp.cpp + http/HttpClient_WinHttp.hpp + http/IBoundedHttpClientCancel.hpp pal/desktop/WindowsDesktopDeviceInformationImpl.cpp pal/desktop/WindowsDesktopNetworkInformationImpl.cpp pal/desktop/WindowsDesktopSystemInformationImpl.cpp @@ -666,7 +674,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "Android") target_link_libraries(mat PUBLIC log) endif() elseif(PAL_IMPLEMENTATION STREQUAL "WIN32") - target_link_libraries(mat PUBLIC wininet crypt32 ws2_32) + target_link_libraries(mat PRIVATE wininet winhttp crypt32) elseif(APPLE) target_link_libraries(mat PUBLIC "-framework CoreFoundation" diff --git a/lib/http/HttpClientFactory.cpp b/lib/http/HttpClientFactory.cpp index 5419f161d..b58175e1a 100644 --- a/lib/http/HttpClientFactory.cpp +++ b/lib/http/HttpClientFactory.cpp @@ -18,6 +18,8 @@ #include "http/HttpClient_WinRt.hpp" #elif defined(HAVE_MAT_WININET_HTTP_CLIENT) #include "http/HttpClient_WinInet.hpp" + #elif defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) + #include "http/HttpClient_WinHttp.hpp" #endif #elif defined(MATSDK_PAL_CPP11) #if TARGET_OS_IPHONE || (defined(__APPLE__) && defined(APPLE_HTTP)) @@ -49,6 +51,13 @@ namespace MAT_NS_BEGIN { return std::make_shared(); } +#elif defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) + /* Win32 WinHTTP client (default) */ + std::shared_ptr HttpClientFactory::Create() { + LOG_TRACE("Creating HttpClient_WinHttp"); + return std::make_shared(); + } + #endif #elif defined(HAVE_MAT_CURL_HTTP_CLIENT) std::shared_ptr HttpClientFactory::Create() { diff --git a/lib/http/HttpClientFactory.hpp b/lib/http/HttpClientFactory.hpp index c96bc2ab0..08cbe2cc0 100644 --- a/lib/http/HttpClientFactory.hpp +++ b/lib/http/HttpClientFactory.hpp @@ -25,8 +25,17 @@ class HttpClientFactory // TODO: [maxgolov] - remove this once there is a better way to pass HTTP client configuration #if defined(MATSDK_PAL_WIN32) && !defined(_WINRT_DLL) -#define HAVE_MAT_WININET_HTTP_CLIENT -#include "http/HttpClient_WinInet.hpp" + #if defined(HAVE_MAT_WININET_HTTP_CLIENT) + #include "http/HttpClient_WinInet.hpp" + #else + // WinHTTP is the default Win32 desktop transport: unlike WinInet, it does + // not depend on a logged-on interactive user or that user's Internet + // Explorer settings, so it works in services and other non-interactive + // processes without extra configuration. Define HAVE_MAT_WININET_HTTP_CLIENT + // to opt back into WinInet (e.g. for IE-integrated proxy/cookie behavior). + #define HAVE_MAT_WINHTTP_HTTP_CLIENT + #include "http/HttpClient_WinHttp.hpp" + #endif #endif #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT diff --git a/lib/http/HttpClient_Curl.cpp b/lib/http/HttpClient_Curl.cpp index b910cdf28..4633b2fc3 100644 --- a/lib/http/HttpClient_Curl.cpp +++ b/lib/http/HttpClient_Curl.cpp @@ -83,11 +83,9 @@ namespace MAT_NS_BEGIN { auto curlOperation = std::make_shared(curlRequest->m_method, curlRequest->m_url, callback, requestHeaders, curlRequest->m_body, false, HTTP_CONN_TIMEOUT, m_sslVerify, sslCaInfo); curlRequest->SetOperation(curlOperation); - - // The lifetime of curlOperation is guarnteed by the call to result.wait() in the d'tor. - curlOperation->SendAsync([this, callback, requestId](CurlHttpOperation& operation) { - this->EraseRequest(requestId); + curlOperation->SendAsync([this, callback, requestId](CurlHttpOperation& operation) { + EraseRequest(requestId); auto response = std::unique_ptr(new SimpleHttpResponse(requestId)); response->m_result = HttpResult_OK; @@ -161,4 +159,3 @@ namespace MAT_NS_BEGIN { } MAT_NS_END #endif - diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index 7d599dec9..461dd01f7 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -17,11 +17,15 @@ #include #include #include +#include #include #include -#include +#include #include +#include +#include +#include #include #include @@ -71,13 +75,6 @@ class HttpClient_Curl : public IHttpClient { class CurlHttpOperation { public: - static long GetPreferredHttpVersion() - { - const curl_version_info_data* versionInfo = curl_version_info(CURLVERSION_NOW); - return (versionInfo != nullptr && (versionInfo->features & CURL_VERSION_HTTP2) != 0) - ? CURL_HTTP_VERSION_2_0 - : CURL_HTTP_VERSION_1_1; - } void DispatchEvent(HttpStateEvent type) { @@ -97,13 +94,29 @@ class CurlHttpOperation { * @param httpConnTimeout HTTP connection timeout in seconds * @param httpReadTimeout HTTP read timeout in seconds */ + // Selects HTTP/2 only when the libcurl we are actually linked against was + // built with HTTP/2 support. Setting CURLOPT_HTTP_VERSION to + // CURL_HTTP_VERSION_2_0 against a libcurl without HTTP/2 does not silently + // downgrade -- it fails the transfer with CURLE_UNSUPPORTED_PROTOCOL -- so + // the version has to be probed at runtime rather than assumed. + static long GetPreferredHttpVersion() noexcept + { + const curl_version_info_data* versionInfo = curl_version_info(CURLVERSION_NOW); + if (versionInfo != nullptr && (versionInfo->features & CURL_VERSION_HTTP2) != 0) + { + return CURL_HTTP_VERSION_2_0; + } + return CURL_HTTP_VERSION_1_1; + } + CurlHttpOperation( std::string method, std::string url, IHttpResponseCallback* callback, - // requestHeaders is copied into the curl_slist during construction - // and need not outlive this operation. requestBody is stored by - // reference and read by Send(), so it must outlive this operation. + // requestHeaders is copied into the curl_slist during construction and + // need not outlive this operation. requestBody is stored by reference; + // CurlHttpRequest destroys this operation (which joins the worker) before + // destroying its inherited request-body storage. const std::map& requestHeaders, const std::vector& requestBody, // Default connectivity and response size options @@ -141,35 +154,21 @@ class CurlHttpOperation { #if 0 // Be verbose - if (!SetOption(CURLOPT_VERBOSE, 1L)) + curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L); #else - if (!SetOption(CURLOPT_VERBOSE, 0L)) + curl_easy_setopt(curl, CURLOPT_VERBOSE, 0); #endif - { - DispatchEvent(OnCreateFailed); - return; - } // Specify target URL - if (!SetOption(CURLOPT_URL, m_url.c_str()) - || !SetOption(CURLOPT_SSL_VERIFYPEER, sslVerify ? 1L : 0L) - || !SetOption(CURLOPT_SSL_VERIFYHOST, sslVerify ? 2L : 0L)) - { - DispatchEvent(OnCreateFailed); - return; - } - - if (!m_sslCaInfo.empty() && !SetOption(CURLOPT_CAINFO, m_sslCaInfo.c_str())) - { - DispatchEvent(OnCreateFailed); - return; - } + curl_easy_setopt(curl, CURLOPT_URL, m_url.c_str()); - if (!SetOption(CURLOPT_HTTP_VERSION, GetPreferredHttpVersion())) - { - DispatchEvent(OnCreateFailed); - return; + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, sslVerify ? 1L : 0L); + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, sslVerify ? 2L : 0L); + if (!m_sslCaInfo.empty()) { + curl_easy_setopt(curl, CURLOPT_CAINFO, m_sslCaInfo.c_str()); } + // HTTP/2 when the linked libcurl supports it, otherwise HTTP/1.1 + curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, GetPreferredHttpVersion()); // Headers are copied into m_headersChunk during construction and the // curl_slist is kept alive until destruction, so the original map does @@ -177,24 +176,15 @@ class CurlHttpOperation { for (const auto& kv : requestHeaders) { std::string header = kv.first + ": " + kv.second; - curl_slist* appended = curl_slist_append(m_headersChunk, header.c_str()); - if (appended == nullptr) - { - res = CURLE_OUT_OF_MEMORY; - DispatchEvent(OnCreateFailed); - return; - } - m_headersChunk = appended; + m_headersChunk = curl_slist_append(m_headersChunk, header.c_str()); } - if(m_headersChunk != nullptr && !SetOption(CURLOPT_HTTPHEADER, m_headersChunk)) + if(m_headersChunk != nullptr) { - DispatchEvent(OnCreateFailed); - return; + curl_easy_setopt(curl, CURLOPT_HTTPHEADER, m_headersChunk); } TRACE("method=%s, url=%s\n", this->m_method.c_str(), this->m_url.c_str()); - m_isConfigured = true; DispatchEvent(OnCreated); } @@ -203,18 +193,24 @@ class CurlHttpOperation { */ virtual ~CurlHttpOperation() { - // Given the request has not been aborted we should wait for completion here - // This guarantees the lifetime of this request. - if (result.valid()) + if (m_worker.joinable()) { - result.wait(); + if (m_worker.get_id() == std::this_thread::get_id()) + { + // The completion callback can release the owning request on this + // worker. Detach rather than joining the current thread; Send() has + // finished and the worker does not touch this operation afterward. + m_worker.detach(); + } + else + { + m_worker.join(); + } } - DispatchEvent(OnDestroy); + + DispatchDestroyEvent(); res = CURLE_OK; - if (curl != nullptr) - { - curl_easy_cleanup(curl); - } + curl_easy_cleanup(curl); curl_slist_free_all(m_headersChunk); ReleaseResponse(); } @@ -230,14 +226,10 @@ class CurlHttpOperation { // Request buffer const void *request = requestBody.empty() ? nullptr : requestBody.data(); const size_t reqSize = requestBody.size(); - int socketWaitResult = 0; - if(!curl || !m_isConfigured) + if(!curl) { - if (res == CURLE_OK) - { - res = CURLE_FAILED_INIT; - } + res = CURLE_FAILED_INIT; DispatchEvent(OnSendFailed); goto cleanup; } @@ -246,49 +238,37 @@ class CurlHttpOperation { // curl_easy_setopt(curl, CURLOPT_LOCALPORT, dcf_port); // Perform initial connect, handling the timeout if needed - if (!SetOption(CURLOPT_CONNECT_ONLY, 1L)) - { - DispatchEvent(OnConnectFailed); - goto cleanup; - } + curl_easy_setopt(curl, CURLOPT_CONNECT_ONLY, 1L); DispatchEvent(OnConnecting); + res = curl_easy_perform(curl); + if(CURLE_OK != res) { - const CURLcode curlResult = curl_easy_perform(curl); - res = static_cast(curlResult); - if(CURLE_OK != curlResult) - { - DispatchEvent(OnConnectFailed); // couldn't connect - stage 1 - TRACE("Error #1: %s\n", curl_easy_strerror(curlResult)); - goto cleanup; - } + DispatchEvent(OnConnectFailed); // couldn't connect - stage 1 + TRACE("Error #1: %s\n", curl_easy_strerror(res)); + goto cleanup; } - { - CURLcode infoResult; + /* Extract the socket from the curl handle - we'll need it for waiting. + * Note that this API takes a pointer to a 'long' while we use + * curl_socket_t for sockets otherwise. + */ + #if LIBCURL_VERSION_NUM >= 0x072D00 // Version 7.45.00 - infoResult = curl_easy_getinfo(curl, CURLINFO_ACTIVESOCKET, &sockextr); + res = curl_easy_getinfo(curl, CURLINFO_ACTIVESOCKET, &sockextr); #else - long lastSocket = -1; - infoResult = curl_easy_getinfo(curl, CURLINFO_LASTSOCKET, &lastSocket); - if (infoResult == CURLE_OK) - { - sockextr = static_cast(lastSocket); - } + res = curl_easy_getinfo(curl, CURLINFO_LASTSOCKET, &sockextr); #endif - if(CURLE_OK != infoResult || sockextr == CURL_SOCKET_BAD) - { - res = static_cast( - infoResult != CURLE_OK ? infoResult : CURLE_COULDNT_CONNECT); - DispatchEvent(OnConnectFailed); // couldn't connect - stage 2 - TRACE("Error #2: %s\n", curl_easy_strerror(static_cast(res))); - goto cleanup; - } + + if(CURLE_OK != res) + { + DispatchEvent(OnConnectFailed); // couldn't connect - stage 2 + TRACE("Error #2: %s\n", curl_easy_strerror(res)); + goto cleanup; } /* wait for the socket to become ready for sending */ sockfd = sockextr; - socketWaitResult = WaitOnSocket(sockfd, 0, HTTP_CONN_TIMEOUT * 1000L); - if(socketWaitResult <= 0 || isAborted) + if( !WaitOnSocket(sockfd, 0, HTTP_CONN_TIMEOUT * 1000L) || isAborted) { TRACE("Error #3: timeout, aborted=%u\n", isAborted.load() ); res = CURLE_OPERATION_TIMEDOUT; @@ -297,46 +277,27 @@ class CurlHttpOperation { } // once connection is there - switch back to easy perform for HTTP post - if (!SetOption(CURLOPT_CONNECT_ONLY, 0L)) - { - DispatchEvent(OnSendFailed); - goto cleanup; - } + curl_easy_setopt(curl, CURLOPT_CONNECT_ONLY, 0); // send all data to our callback function if (rawResponse) { - if (!SetOption(CURLOPT_HEADER, 1L) - || !SetOption(CURLOPT_WRITEFUNCTION, - static_cast(&WriteMemoryCallback)) - || !SetOption(CURLOPT_WRITEDATA, static_cast(&response))) - { - DispatchEvent(OnSendFailed); - goto cleanup; - } - } - else if (!SetOption(CURLOPT_WRITEFUNCTION, - static_cast(&WriteVectorCallback)) - || !SetOption(CURLOPT_HEADERFUNCTION, - static_cast(&WriteVectorCallback)) - || !SetOption(CURLOPT_HEADERDATA, static_cast(&respHeaders)) - || !SetOption(CURLOPT_WRITEDATA, static_cast(&respBody))) - { - DispatchEvent(OnSendFailed); - goto cleanup; + curl_easy_setopt(curl, CURLOPT_HEADER, true); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, (void *)&WriteMemoryCallback); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&response); + } else { + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, (void *)&WriteVectorCallback); + curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)&respHeaders); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&respBody); } // TODO: only two methods supported for now - POST and GET if (m_method.compare("POST") == 0) { // POST - if (!SetOption(CURLOPT_POST, 1L) - || !SetOption(CURLOPT_POSTFIELDS, static_cast(request)) - || !SetOption(CURLOPT_POSTFIELDSIZE_LARGE, static_cast(reqSize))) - { - DispatchEvent(OnSendFailed); - goto cleanup; - } + curl_easy_setopt(curl, CURLOPT_POST, true); + curl_easy_setopt(curl, CURLOPT_POSTFIELDS, static_cast(request)); + curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, reqSize); } else if (m_method.compare("GET") == 0) { @@ -348,23 +309,16 @@ class CurlHttpOperation { goto cleanup; } - if (!SetOption(CURLOPT_LOW_SPEED_TIME, 30L) - || !SetOption(CURLOPT_LOW_SPEED_LIMIT, 4096L)) + curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, 30L); + curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, 4096); + DispatchEvent(OnSending); + res = curl_easy_perform(curl); + if(CURLE_OK != res) { DispatchEvent(OnSendFailed); + TRACE("Error: %s\n", curl_easy_strerror(res)); goto cleanup; } - DispatchEvent(OnSending); - { - const CURLcode curlResult = curl_easy_perform(curl); - res = static_cast(curlResult); - if(CURLE_OK != curlResult) - { - DispatchEvent(OnSendFailed); - TRACE("Error: %s\n", curl_easy_strerror(curlResult)); - goto cleanup; - } - } /* Code snippet to parse raw HTTP response. This might come in handy * if we ever consider to handle the raw upload instead of curl_easy_perform @@ -378,17 +332,7 @@ class CurlHttpOperation { */ /* libcurl is nice enough to parse the response code itself: */ - { - long responseCode = 0; - const CURLcode infoResult = curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &responseCode); - if (infoResult != CURLE_OK) - { - res = static_cast(infoResult); - DispatchEvent(OnSendFailed); - goto cleanup; - } - res = responseCode; - } + curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &res); // We got some response from server. Dump the contents. TRACE("HTTP response code %d\n", res); DispatchEvent(OnResponse); @@ -402,14 +346,46 @@ class CurlHttpOperation { return res; } - std::future & SendAsync(std::function callback = nullptr) { - result = std::async(std::launch::async, [this, callback] { - long result = Send(); - if (callback!=nullptr) - callback(*this); - return result; - }); - return result; + void SendAsync(std::function callback = nullptr) { + // A newly created std::thread may run before it is assigned to m_worker. + // Hold this gate until the assignment completes so a fast failure cannot + // destroy the operation from its callback while SendAsync still uses it. + { + std::lock_guard startGuard(m_workerStartMtx); + if (m_sendAttempted) + { + throw std::logic_error("CurlHttpOperation is single-use"); + } + m_sendAttempted = true; + + try + { + m_worker = std::thread([this, callback]() { + { + std::lock_guard startGuard(m_workerStartMtx); + } + try + { + Send(); + } + catch (...) + { + // std::async stored worker exceptions in its unobserved + // future. A raw thread must contain them. + res = CURLE_FAILED_INIT; + } + Complete(callback); + }); + return; + } + catch (...) + { + // Callable allocation/copy or std::thread creation failed. + } + } + + res = CURLE_FAILED_INIT; + Complete(callback); } /** @@ -521,19 +497,16 @@ class CurlHttpOperation { const size_t httpConnTimeout; // Timeout for connect. Default: 5s CURL *curl; // Local curl instance - long res = CURLE_OK; // Curl result OR HTTP status code if successful - + CURLcode res = CURLE_OK; // Curl result OR HTTP status code if successful + IHttpResponseCallback* m_callback = nullptr; // Request values std::string m_method; std::string m_url; std::string m_sslCaInfo; - bool m_isConfigured = false; - // The SDK upload path keeps the owning IHttpRequest alive through the - // callback context until Send() completes; copying this body would duplicate - // every upload payload. Unlike CURLOPT_CAINFO, the body pointer is set and - // consumed during Send(), not retained from construction. + // The owning CurlHttpRequest destroys this operation before its inherited + // request-body storage, and cross-thread destruction joins the worker. const std::vector& requestBody; struct curl_slist *m_headersChunk = nullptr; @@ -544,26 +517,51 @@ class CurlHttpOperation { // Socket parameters curl_socket_t sockfd = 0; - curl_socket_t sockextr = CURL_SOCKET_BAD; + long sockextr = 0; curl_off_t nread = 0; size_t sendlen = 0; // # bytes sent by client size_t acklen = 0; // # bytes ack by server - std::future result; + std::mutex m_workerStartMtx; + bool m_sendAttempted = false; + std::thread m_worker; + std::atomic m_destroyEventDispatched { false }; - template - bool SetOption(CURLoption option, TValue value) + void DispatchDestroyEvent() noexcept { - const CURLcode optionResult = curl_easy_setopt(curl, option, value); - if (optionResult != CURLE_OK) + bool expected = false; + if (m_destroyEventDispatched.compare_exchange_strong( + expected, true, std::memory_order_acq_rel)) { - res = static_cast(optionResult); - TRACE("curl_easy_setopt(%d) failed: %s\n", - static_cast(option), curl_easy_strerror(optionResult)); - return false; + try + { + DispatchEvent(OnDestroy); + } + catch (...) + { + // State observers must not terminate the worker or destructor. + } + } + } + + void Complete(const std::function& callback) noexcept + { + // Preserve the documented state event while m_callback is still valid. + // The completion callback can release the last owner, so this must remain + // the worker's final access to the operation. + DispatchDestroyEvent(); + try + { + if (callback != nullptr) + { + callback(*this); + } + } + catch (...) + { + // Match the old unobserved-future behavior at the thread boundary. } - return true; } /** @@ -607,7 +605,7 @@ class CurlHttpOperation { * @param userp * @return */ - static size_t WriteMemoryCallback(char *contents, size_t size, size_t nmemb, void *userp) + static size_t WriteMemoryCallback(void *contents, size_t size, size_t nmemb, void *userp) { // Guard the size * nmemb product against size_t overflow before using it. if (nmemb != 0 && size > static_cast(-1) / nmemb) { @@ -651,15 +649,14 @@ class CurlHttpOperation { * @param data * @return */ - static size_t WriteVectorCallback(char *ptr, size_t size, size_t nmemb, void* userp) + static size_t WriteVectorCallback(void *ptr, size_t size, size_t nmemb, std::vector* data) { // Guard the size * nmemb product against size_t overflow before using it. if (nmemb != 0 && size > static_cast(-1) / nmemb) { return 0; } - size_t realsize = size * nmemb; - auto* data = static_cast*>(userp); if (data != nullptr) { + size_t realsize = size * nmemb; // SECURITY: bound the buffered response (see kMaxResponseBytes). Compare // overflow-safely (data->size() is always <= kMaxResponseBytes here). // Returning a short count aborts the transfer with CURLE_WRITE_ERROR. @@ -667,11 +664,11 @@ class CurlHttpOperation { TRACE("Response exceeds max buffered size (%zu bytes); aborting transfer\n", kMaxResponseBytes); return 0; } - const auto* begin = reinterpret_cast(ptr); + const auto* begin = static_cast(ptr); const auto* end = begin + realsize; data->insert( data->end(), begin, end); } - return realsize; + return size * nmemb; } }; diff --git a/lib/http/HttpClient_WinHttp.cpp b/lib/http/HttpClient_WinHttp.cpp new file mode 100644 index 000000000..e5f8af0e9 --- /dev/null +++ b/lib/http/HttpClient_WinHttp.cpp @@ -0,0 +1,715 @@ +// clang-format off +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#include "mat/config.h" + +#ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT +#include "HttpClient_WinHttp.hpp" +#include "utils/StringConversion.hpp" +#include "utils/StringUtils.hpp" + +#include +#include + +#include +#include +#include +#include +#include + +#pragma comment(lib, "winhttp.lib") + +namespace MAT_NS_BEGIN { + +class WinHttpRequestWrapper; + +struct WinHttpCallbackContext +{ + explicit WinHttpCallbackContext(std::shared_ptr request) + : request(std::move(request)) + { + } + + std::weak_ptr request; +}; + +class WinHttpRequestWrapper : public std::enable_shared_from_this +{ + protected: + HttpClient_WinHttp& m_parent; + std::string m_id; + IHttpResponseCallback* m_appCallback {nullptr}; + HINTERNET m_hConnect {nullptr}; + HINTERNET m_hRequest {nullptr}; + SimpleHttpRequest* m_request; + std::vector m_bodyBuffer; + std::vector m_readBuffer; + std::atomic isCallbackCalled {false}; + bool isAborted {false}; + WinHttpCallbackContext* m_callbackContext {nullptr}; + + public: + WinHttpRequestWrapper(HttpClient_WinHttp& parent, SimpleHttpRequest* request) + : m_parent(parent), + m_id(request->GetId()), + m_request(request) + { + LOG_TRACE("%p WinHttpRequestWrapper()", this); + } + + WinHttpRequestWrapper(WinHttpRequestWrapper const&) = delete; + WinHttpRequestWrapper& operator=(WinHttpRequestWrapper const&) = delete; + + ~WinHttpRequestWrapper() noexcept + { + LOG_TRACE("%p ~WinHttpRequestWrapper()", this); + if (m_hRequest != nullptr) + { + ::WinHttpCloseHandle(m_hRequest); + } + if (m_hConnect != nullptr) + { + ::WinHttpCloseHandle(m_hConnect); + } + } + + /// + /// Asynchronously cancel pending request. + /// + /// Unlike WinInet's InternetCloseHandle, WinHttpCloseHandle on a request + /// with a pending async operation blocks the calling thread until that + /// operation's completion callback has finished running -- and that + /// callback runs on a *different* WinHTTP-internal thread. Holding + /// m_parent.m_requestsMutex across the call (WinInet's pattern, safe there + /// because its callback runs synchronously on the calling thread) would + /// deadlock here: this thread would block inside WinHttpCloseHandle holding + /// the lock, while the completion callback blocks on the same thread's + /// erase() needing that same lock. So the handle is captured and closed + /// without holding the lock. This wrapper is only reachable through a + /// shared_ptr (see HttpClient_WinHttp::m_requests / CancelRequestAsync), so + /// releasing the lock here cannot race with the object being freed -- + /// the caller already holds its own shared_ptr keeping *this* alive. + /// + void cancel() + { + HINTERNET hRequestToClose = nullptr; + { + std::lock_guard lock(m_parent.m_requestsMutex); + isAborted = true; + hRequestToClose = m_hRequest; + } + if (hRequestToClose != nullptr) + { + ::WinHttpCloseHandle(hRequestToClose); + // WinHttpCloseHandle waits for any callback to finish. Some + // cancellation paths report only HANDLE_CLOSING, so complete the + // request here if no callback delivered the terminal result. + if (!isCallbackCalled) + { + m_hRequest = nullptr; + onRequestComplete(ERROR_WINHTTP_OPERATION_CANCELLED); + } + } + } + + /// + /// Verify that the server end-point certificate is MS-Rooted. + /// Unlike WinInet's INTERNET_OPTION_SERVER_CERT_CHAIN_CONTEXT (which hands + /// back a ready-made chain), WinHttpQueryOption only returns the leaf server + /// certificate context, so the chain must be built explicitly before running + /// the same CERT_CHAIN_POLICY_MICROSOFT_ROOT policy check WinInet performs. + /// + bool isMsRootCert() + { + PCCERT_CONTEXT pCertContext = nullptr; + DWORD dwSize = sizeof(pCertContext); + if (!::WinHttpQueryOption(m_hRequest, WINHTTP_OPTION_SERVER_CERT_CONTEXT, &pCertContext, &dwSize)) + { + // Downlevel/unsupported: proceed without cert validation. This behavior + // is identical to WinInet's fallback when its cert-chain option is + // unavailable, to avoid regressions for downlevel OS. + LOG_TRACE("WinHttpQueryOption(SERVER_CERT_CONTEXT) failed to obtain cert"); + return true; + } + + bool result = true; + PCCERT_CHAIN_CONTEXT pChainCtx = nullptr; + CERT_CHAIN_PARA chainPara = { sizeof(chainPara) }; + if (::CertGetCertificateChain(NULL, pCertContext, NULL, pCertContext->hCertStore, &chainPara, 0, NULL, &pChainCtx)) + { + CERT_CHAIN_POLICY_STATUS pps = { 0, 0, 0, 0, nullptr }; + pps.cbSize = sizeof(pps); + // Verify that the cert chain roots up to the Microsoft application root at top level + CERT_CHAIN_POLICY_PARA policyPara = { 0, 0, nullptr }; + policyPara.cbSize = sizeof(policyPara); + policyPara.dwFlags = MICROSOFT_ROOT_CERT_CHAIN_POLICY_CHECK_APPLICATION_ROOT_FLAG; + policyPara.pvExtraPolicyPara = nullptr; + + BOOL policyChecked = ::CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_MICROSOFT_ROOT, pChainCtx, &policyPara, &pps); + if (!policyChecked) + { + LOG_WARN("CertVerifyCertificateChainPolicy() failed: unable to verify"); + result = false; + } + else if (pps.dwError != ERROR_SUCCESS) + { + LOG_WARN("CertVerifyCertificateChainPolicy() failed: invalid root CA - %d", pps.dwError); + result = false; + } + ::CertFreeCertificateChain(pChainCtx); + } + else + { + // Unable to build the chain -- proceed without cert validation, same + // fallback philosophy as the "downlevel OS" case above. + LOG_TRACE("CertGetCertificateChain() failed to build cert chain"); + } + ::CertFreeCertificateContext(pCertContext); + return result; + } + + void DispatchEvent(HttpStateEvent type) + { + if (m_appCallback != nullptr) + { + m_appCallback->OnHttpStateEvent(type, static_cast(m_hRequest), 0); + } + } + + // Asynchronously send HTTP request and invoke response callback. + // Ownership semantics: send(...) method self-destroys *this* upon + // reaching the terminal WinHTTP callback. There must be absolutely no + // methods that attempt to use the object after triggering send on it. + // Send operation on request may be issued no more than once. + // + // Held under m_parent.m_requestsMutex (a recursive_mutex, matching + // HttpClient_WinInet's model) for the whole method, exactly like cancel(): + // that serializes send() and cancel() completely, so cancel() can never + // interleave mid-way through handle creation and be silently lost, and a + // synchronous/reentrant completion on this same thread can safely re-enter + // the lock rather than deadlock. + void send(IHttpResponseCallback* callback) + { + std::lock_guard lock(m_parent.m_requestsMutex); + m_appCallback = callback; + m_parent.m_requests[m_id] = shared_from_this(); + + if (isAborted) + { + // Request force-aborted before creating a WinHTTP handle. + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_WINHTTP_OPERATION_CANCELLED); + return; + } + + DispatchEvent(OnConnecting); + + std::wstring wUrl = to_utf16_string(m_request->m_url); + URL_COMPONENTS urlc; + memset(&urlc, 0, sizeof(urlc)); + urlc.dwStructSize = sizeof(urlc); + wchar_t hostname[256] = { 0 }; + urlc.lpszHostName = hostname; + urlc.dwHostNameLength = ARRAYSIZE(hostname); + wchar_t path[1024] = { 0 }; + urlc.lpszUrlPath = path; + urlc.dwUrlPathLength = ARRAYSIZE(path); + if (!::WinHttpCrackUrl(wUrl.c_str(), static_cast(wUrl.size()), 0, &urlc)) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.c_str()); + // Invalid URL passed to WinHTTP API + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_WINHTTP_OPERATION_CANCELLED); + return; + } + + // TODO: connect handle for the same target should be cached across + // requests to enable keep-alive (same pre-existing opportunity noted + // in HttpClient_WinInet.cpp; out of scope for this transport swap). + m_hConnect = ::WinHttpConnect(m_parent.m_hSession, hostname, urlc.nPort, 0); + if (m_hConnect == nullptr) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpConnect() failed: %d", dwError); + // Cannot connect to host + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_WINHTTP_OPERATION_CANCELLED); + return; + } + + std::wstring wMethod = to_utf16_string(m_request->m_method); + bool isHttps = (urlc.nScheme == INTERNET_SCHEME_HTTPS); + m_hRequest = ::WinHttpOpenRequest( + m_hConnect, wMethod.c_str(), path, NULL, WINHTTP_NO_REFERER, + WINHTTP_DEFAULT_ACCEPT_TYPES, + WINHTTP_FLAG_REFRESH | (isHttps ? WINHTTP_FLAG_SECURE : 0)); + if (m_hRequest == nullptr) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpOpenRequest() failed: %d", dwError); + // Request cannot be opened to given URL because of some connectivity issue + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_WINHTTP_OPERATION_CANCELLED); + return; + } + + // Unlike WinInet, WinHTTP has no automatic cookie jar to suppress (it + // never manages cookies on the caller's behalf) and never shows UI, so + // neither INTERNET_FLAG_NO_COOKIES nor INTERNET_FLAG_NO_UI has a WinHTTP + // equivalent to set here. + + /* Perform optional MS Root certificate check for certain end-point URLs */ + if (m_parent.IsMsRootCheckRequired()) + { + if (!isMsRootCert()) + { + // Request cannot be completed: end-point certificate is not MS-Rooted + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_WINHTTP_SECURE_INVALID_CERT); + return; + } + } + + // WinHttpSetStatusCallback returns the PREVIOUS callback function + // pointer (typically NULL here, since this is the first registration + // on a freshly opened request handle) -- not a BOOL -- and signals + // failure only via the distinct WINHTTP_INVALID_STATUS_CALLBACK + // sentinel. Treating a null "previous callback" as failure would + // reject every request immediately after this call. + if (::WinHttpSetStatusCallback(m_hRequest, &WinHttpRequestWrapper::winHttpCallback, + WINHTTP_CALLBACK_FLAG_ALL_COMPLETIONS, 0) == WINHTTP_INVALID_STATUS_CALLBACK) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpSetStatusCallback() failed: %d", dwError); + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_WINHTTP_OPERATION_CANCELLED); + return; + } + + std::ostringstream os; + for (auto const& header : m_request->m_headers) { + os << header.first << ": " << header.second << "\r\n"; + } + std::wstring wHeaders = to_utf16_string(os.str()); + + if (!wHeaders.empty() && + !::WinHttpAddRequestHeaders(m_hRequest, wHeaders.c_str(), static_cast(wHeaders.size()), + WINHTTP_ADDREQ_FLAG_ADD | WINHTTP_ADDREQ_FLAG_REPLACE)) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpAddRequestHeaders() failed: %d", dwError); + // Unable to add request headers. There's no point in proceeding with upload because + // our server is expecting those custom request headers to always be there. + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_WINHTTP_OPERATION_CANCELLED); + return; + } + + // Try to send headers and request body to server + DispatchEvent(OnSending); + void* data = m_request->m_body.empty() ? nullptr : static_cast(m_request->m_body.data()); + DWORD size = static_cast(m_request->m_body.size()); + m_callbackContext = new WinHttpCallbackContext(shared_from_this()); + DWORD_PTR context = reinterpret_cast(m_callbackContext); + BOOL bResult = ::WinHttpSendRequest( + m_hRequest, WINHTTP_NO_ADDITIONAL_HEADERS, 0, data, size, size, context); + if (!bResult) + { + DWORD dwError = ::GetLastError(); + delete m_callbackContext; + m_callbackContext = nullptr; + LOG_WARN("WinHttpSendRequest() failed: %d", dwError); + // Unable to send request + DispatchEvent(OnSendFailed); + onRequestComplete(dwError); + return; + } + // Async request has been queued; completion arrives via winHttpCallback. + } + + // Drives the WinHTTP async state machine: SendRequest -> ReceiveResponse -> + // (QueryDataAvailable -> ReadData)* -> onRequestComplete. Unlike WinInet + // (whose async completions all report through the single + // INTERNET_STATUS_REQUEST_COMPLETE code, and whose synchronous API calls + // signal a pending async op via a FALSE return + GetLastError()== + // ERROR_IO_PENDING), WinHTTP has one distinct callback status per stage, + // and a FALSE return from any of these calls on an async handle is always a + // genuine synchronous failure -- never "pending" -- so every failure path + // here reports immediately instead of waiting for a further callback. + static void CALLBACK winHttpCallback(HINTERNET hInternet, DWORD_PTR dwContext, DWORD dwInternetStatus, LPVOID lpvStatusInformation, DWORD dwStatusInformationLength) + { + UNREFERENCED_PARAMETER(hInternet); + + WinHttpCallbackContext* context = reinterpret_cast(dwContext); + if (context == nullptr) + { + return; + } + + if (dwInternetStatus == WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING) + { + // The callback context outlives the request wrapper and is released + // only by WinHTTP's final notification. + delete context; + return; + } + + std::shared_ptr self = context->request.lock(); + if (self == nullptr) + { + return; + } + + LOG_TRACE("winHttpCallback: hInternet %p, self %p, dwInternetStatus %u", hInternet, self.get(), dwInternetStatus); + + switch (dwInternetStatus) + { + case WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE: + if (!::WinHttpReceiveResponse(self->m_hRequest, NULL)) + { + self->onRequestComplete(::GetLastError()); + } + return; + + case WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE: + if (!::WinHttpQueryDataAvailable(self->m_hRequest, NULL)) + { + self->onRequestComplete(::GetLastError()); + } + return; + + case WINHTTP_CALLBACK_STATUS_DATA_AVAILABLE: + { + DWORD bytesAvailable = (lpvStatusInformation != nullptr) + ? *static_cast(lpvStatusInformation) : 0; + if (bytesAvailable == 0) + { + // No more data: response is complete. + self->onRequestComplete(ERROR_SUCCESS); + return; + } + // SECURITY: refuse an over-large response instead of buffering it + // (see MAX_HTTP_RESPONSE_SIZE) so a hostile/MITM'd collector cannot + // exhaust process memory. Checked before every read so the buffer + // never exceeds the cap; reported as an invalid server response -> + // NetworkFailure (retried). + if (self->m_bodyBuffer.size() + bytesAvailable > MAX_HTTP_RESPONSE_SIZE) + { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + self->onRequestComplete(ERROR_WINHTTP_INVALID_SERVER_RESPONSE); + return; + } + self->m_readBuffer.resize(bytesAvailable); + if (!::WinHttpReadData(self->m_hRequest, self->m_readBuffer.data(), bytesAvailable, NULL)) + { + self->onRequestComplete(::GetLastError()); + } + return; + } + + case WINHTTP_CALLBACK_STATUS_READ_COMPLETE: + // dwStatusInformationLength is the number of bytes actually placed + // into the buffer passed to WinHttpReadData (may be less than the + // bytesAvailable that was requested). + self->m_bodyBuffer.insert(self->m_bodyBuffer.end(), + self->m_readBuffer.begin(), self->m_readBuffer.begin() + dwStatusInformationLength); + if (!::WinHttpQueryDataAvailable(self->m_hRequest, NULL)) + { + self->onRequestComplete(::GetLastError()); + } + return; + + case WINHTTP_CALLBACK_STATUS_REQUEST_ERROR: + { + WINHTTP_ASYNC_RESULT* result = static_cast(lpvStatusInformation); + DWORD dwError = (result != nullptr) ? result->dwError : ERROR_WINHTTP_INTERNAL_ERROR; + self->onRequestComplete(dwError); + return; + } + + default: + return; + } + } + + void onRequestComplete(DWORD dwError) + { + if (isCallbackCalled.exchange(true)) + { + return; + } + + std::unique_ptr response(new SimpleHttpResponse(m_id)); + + if (dwError == ERROR_SUCCESS) { + response->m_body = m_bodyBuffer; + response->m_result = HttpResult_OK; + + DWORD statusCode = 0; + DWORD dwSize = sizeof(statusCode); + if (!::WinHttpQueryHeaders(m_hRequest, WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER, + WINHTTP_HEADER_NAME_BY_INDEX, &statusCode, &dwSize, WINHTTP_NO_HEADER_INDEX)) + { + LOG_WARN("WinHttpQueryHeaders(STATUS_CODE) failed: %d", ::GetLastError()); + } + response->m_statusCode = statusCode; + + // Raw headers, as "Name: Value\r\n..." pairs -- the same shape WinInet + // hands back via HTTP_QUERY_RAW_HEADERS_CRLF. + DWORD headerBytes = 0; + ::WinHttpQueryHeaders(m_hRequest, WINHTTP_QUERY_RAW_HEADERS_CRLF, + WINHTTP_HEADER_NAME_BY_INDEX, WINHTTP_NO_OUTPUT_BUFFER, &headerBytes, WINHTTP_NO_HEADER_INDEX); + DWORD headerErr = ::GetLastError(); + if (headerBytes > 0 && headerErr == ERROR_INSUFFICIENT_BUFFER) + { + std::wstring wHeaders(headerBytes / sizeof(wchar_t), L'\0'); + if (::WinHttpQueryHeaders(m_hRequest, WINHTTP_QUERY_RAW_HEADERS_CRLF, + WINHTTP_HEADER_NAME_BY_INDEX, &wHeaders[0], &headerBytes, WINHTTP_NO_HEADER_INDEX)) + { + // WinHttpQueryHeaders includes the buffer's trailing NUL(s) in + // the byte count; trim at the first one before converting. + size_t nul = wHeaders.find(L'\0'); + if (nul != std::wstring::npos) + { + wHeaders.resize(nul); + } + parseHeaders(to_utf8_string(wHeaders), *response); + } + else + { + LOG_WARN("WinHttpQueryHeaders(RAW_HEADERS_CRLF) failed twice: %d", ::GetLastError()); + } + } + // This event handler covers the only positive case when we actually got some server response. + // We may still invoke OnHttpResponse(...) below for this positive as well as other negative + // cases where there was a short-read, connection failure or timeout on reading the response. + DispatchEvent(OnResponse); + + } else { + switch (dwError) { + case ERROR_WINHTTP_OPERATION_CANCELLED: + response->m_result = HttpResult_Aborted; + break; + + case ERROR_WINHTTP_TIMEOUT: + case ERROR_WINHTTP_NAME_NOT_RESOLVED: + case ERROR_WINHTTP_CANNOT_CONNECT: + case ERROR_WINHTTP_CONNECTION_ERROR: + case ERROR_WINHTTP_RESEND_REQUEST: + case ERROR_WINHTTP_SECURE_CERT_DATE_INVALID: + case ERROR_WINHTTP_SECURE_CERT_CN_INVALID: + case ERROR_WINHTTP_CLIENT_AUTH_CERT_NEEDED: + case ERROR_WINHTTP_SECURE_INVALID_CA: + case ERROR_WINHTTP_SECURE_CERT_REV_FAILED: + case ERROR_WINHTTP_SECURE_CHANNEL_ERROR: + case ERROR_WINHTTP_SECURE_INVALID_CERT: + case ERROR_WINHTTP_SECURE_CERT_REVOKED: + case ERROR_WINHTTP_SECURE_CERT_WRONG_USAGE: + case ERROR_WINHTTP_SECURE_FAILURE: + case ERROR_WINHTTP_REDIRECT_FAILED: + case ERROR_WINHTTP_INVALID_SERVER_RESPONSE: + case ERROR_WINHTTP_RESPONSE_DRAIN_OVERFLOW: + response->m_result = HttpResult_NetworkFailure; + break; + + default: + response->m_result = HttpResult_LocalFailure; + break; + } + } + + { + auto callback = m_appCallback; + auto requestId = m_id; + auto keepAlive = shared_from_this(); + // Remove the request before entering application code. The callback + // can synchronously tear down the client and destroy this wrapper. + m_parent.erase(requestId); + callback->OnHttpResponse(response.release()); + keepAlive.reset(); + } + } + + private: + // Parses "Name: Value\r\n"-formatted raw headers (as returned by + // WINHTTP_QUERY_RAW_HEADERS_CRLF / HTTP_QUERY_RAW_HEADERS_CRLF) into an + // HttpHeaders map. Shared shape with HttpClient_WinInet's inline parser. + static void parseHeaders(std::string const& raw, SimpleHttpResponse& response) + { + char const* ptr = raw.c_str(); + while (*ptr) { + char const* colon = strchr(ptr, ':'); + if (!colon) { + break; + } + std::string name(ptr, colon); + + ptr = colon + 1; + while (*ptr == ' ') { + ptr++; + } + + char const* eol = strstr(ptr, "\r\n"); + if (!eol) { + break; + } + std::string value(ptr, eol); + + response.m_headers.add(name, value); + ptr = eol + 2; + } + } +}; + +//--- + +unsigned HttpClient_WinHttp::s_nextRequestId = 0; + +HttpClient_WinHttp::HttpClient_WinHttp() : + m_msRootCheck(false) +{ + // WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY (Windows 8.1+) resolves the proxy + // without depending on a logged-on interactive user or that user's + // Internet Explorer settings -- unlike WinInet's + // INTERNET_OPEN_TYPE_PRECONFIG, which requires one. This is why WinHTTP, + // not WinInet, is Microsoft's documented recommendation for services and + // other non-interactive processes. On an older OS that rejects this access + // type, fall back to no proxy rather than failing to construct at all. + m_hSession = ::WinHttpOpen( + NULL, WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY, + WINHTTP_NO_PROXY_NAME, WINHTTP_NO_PROXY_BYPASS, WINHTTP_FLAG_ASYNC); + if (m_hSession == nullptr) + { + LOG_WARN("WinHttpOpen(AUTOMATIC_PROXY) failed: %d; retrying with no proxy", ::GetLastError()); + m_hSession = ::WinHttpOpen( + NULL, WINHTTP_ACCESS_TYPE_NO_PROXY, + WINHTTP_NO_PROXY_NAME, WINHTTP_NO_PROXY_BYPASS, WINHTTP_FLAG_ASYNC); + } +} + +HttpClient_WinHttp::~HttpClient_WinHttp() +{ + CancelAllRequests(); + ::WinHttpCloseHandle(m_hSession); +} + +/** + * This method is called exclusively from onRequestComplete. + * No other code paths that lead to request destruction. + */ +void HttpClient_WinHttp::erase(std::string const& id) +{ + // Drop the map's shared_ptr reference under the lock. If a concurrent + // cancel() call (see its comment) is holding its own shared_ptr copy, the + // wrapper's actual destruction is deferred until that copy also goes out + // of scope -- never while any caller still holds a live reference. + { + std::lock_guard lock(m_requestsMutex); + m_requests.erase(id); + } + m_requestsCv.notify_all(); +} + +IHttpRequest* HttpClient_WinHttp::CreateRequest() +{ + std::string id = "WH-" + toString(::InterlockedIncrement(&s_nextRequestId)); + return new SimpleHttpRequest(id); +} + +void HttpClient_WinHttp::SendRequestAsync(IHttpRequest* request, IHttpResponseCallback* callback) +{ + // Note: 'request' is never owned by IHttpClient and gets deleted in EventsUploadContext.clear() + auto wrapper = std::make_shared(*this, static_cast(request)); + wrapper->send(callback); +} + +void HttpClient_WinHttp::CancelRequestAsync(std::string const& id) +{ + // Copy the shared_ptr out of the map while holding the lock only for the + // lookup, then call cancel() without the lock held (cancel() blocks in + // WinHttpCloseHandle waiting for a completion callback on another thread + // that needs this same lock -- see cancel()'s comment). The local copy + // keeps the wrapper alive for the duration of this call even if erase() + // concurrently removes the map's own reference. + std::shared_ptr request; + { + std::lock_guard lock(m_requestsMutex); + auto it = m_requests.find(id); + if (it != m_requests.end()) { + request = it->second; + } + } + if (request) { + request->cancel(); + } +} + +void HttpClient_WinHttp::CancelAllRequests() +{ + CancelAllRequests(std::chrono::milliseconds::zero()); +} + +void HttpClient_WinHttp::CancelAllRequests(std::chrono::milliseconds bestEffortTimeout) +{ + // vector of all request IDs + std::vector ids; + { + std::lock_guard lock(m_requestsMutex); + for (auto const& item : m_requests) { + ids.push_back(item.first); + } + } + // cancel all requests one-by-one not holding the lock + for (const auto& id : ids) + CancelRequestAsync(id); + + // Wait for all destructors to run, signaled from erase() rather than + // polled -- unlike a sleep-and-recheck loop, this drains the common case + // in microseconds and never busy-spins. A positive timeout is the bounded, + // best-effort path used during pause; zero is the full shutdown barrier. + std::unique_lock lock(m_requestsMutex); + if (bestEffortTimeout > std::chrono::milliseconds::zero()) + { + m_requestsCv.wait_for(lock, bestEffortTimeout, [this]() noexcept -> bool { + return m_requests.empty(); + }); + } + else + { + m_requestsCv.wait(lock, [this]() noexcept -> bool { + return m_requests.empty(); + }); + } +} + +/// +/// Enforces MS-root server certificate check. +/// +/// if set to true [enforce verification that server cert is MS-Rooted]. +void HttpClient_WinHttp::ApplySettings(ILogConfiguration& config) +{ + SetMsRootCheck(config[CFG_MAP_HTTP][CFG_BOOL_HTTP_MS_ROOT_CHECK]); +} + +void HttpClient_WinHttp::SetMsRootCheck(bool enforceMsRoot) +{ + m_msRootCheck = enforceMsRoot; +} + +/// +/// Determines whether MS-Rooted server cert check required. +/// +/// +/// true if [MS-Rooted server cert check required]; otherwise, false. +/// +bool HttpClient_WinHttp::IsMsRootCheckRequired() +{ + return m_msRootCheck; +} + +} MAT_NS_END +#endif // HAVE_MAT_DEFAULT_HTTP_CLIENT +// clang-format on diff --git a/lib/http/HttpClient_WinHttp.hpp b/lib/http/HttpClient_WinHttp.hpp new file mode 100644 index 000000000..b7d1e2990 --- /dev/null +++ b/lib/http/HttpClient_WinHttp.hpp @@ -0,0 +1,69 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#ifndef HTTPCLIENT_WINHTTP_HPP +#define HTTPCLIENT_WINHTTP_HPP + +#ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT + +#include "IHttpClient.hpp" +#include "IBoundedHttpClientCancel.hpp" +#include "pal/PAL.hpp" + +#include "ILogManager.hpp" + +#include +#include + +namespace MAT_NS_BEGIN { + +#ifndef _WINHTTPX_ +typedef void* HINTERNET; +#endif + +class WinHttpRequestWrapper; + +// WinHTTP-based HTTP client. Unlike WinInet, WinHTTP does not depend on a +// logged-on interactive user or that user's Internet Explorer settings, so +// it is Microsoft's recommended transport for services and other +// non-interactive processes (see +// https://learn.microsoft.com/windows/win32/winhttp/porting-wininet-applications-to-winhttp). +// This is the default Win32 desktop transport; HttpClient_WinInet remains +// available as an explicit opt-in for callers that need IE-integrated proxy +// or cookie behavior. +class HttpClient_WinHttp : public IHttpClient, public IBoundedHttpClientCancel { + public: + // Common IHttpClient methods + HttpClient_WinHttp(); + virtual ~HttpClient_WinHttp(); + virtual IHttpRequest* CreateRequest() final; + virtual void SendRequestAsync(IHttpRequest* request, IHttpResponseCallback* callback) final; + virtual void CancelRequestAsync(std::string const& id) final; + virtual void CancelAllRequests() final; + virtual void CancelAllRequests(std::chrono::milliseconds bestEffortTimeout) final; + + virtual void ApplySettings(ILogConfiguration& config) override; + + // Methods unique to WinHttp implementation. + void SetMsRootCheck(bool enforceMsRoot); + bool IsMsRootCheckRequired(); + + protected: + void erase(std::string const& id); + + protected: + HINTERNET m_hSession; + std::recursive_mutex m_requestsMutex; + std::condition_variable_any m_requestsCv; + std::map> m_requests; + static unsigned s_nextRequestId; + bool m_msRootCheck; + friend class WinHttpRequestWrapper; +}; + +} MAT_NS_END + +#endif // HAVE_MAT_DEFAULT_HTTP_CLIENT + +#endif // HTTPCLIENT_WINHTTP_HPP diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index 2ec8be9b0..21672aeb2 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -14,7 +14,6 @@ #include #include -#include #include #include #include @@ -593,7 +592,7 @@ void HttpClient_WinInet::SetMsRootCheck(bool enforceMsRoot) } /// -/// Determines whether MS-Roted server cert check required. +/// Determines whether an MS-Rooted server certificate check is required. /// /// /// true if [MS-Rooted server cert check required]; otherwise, false. diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index 12ac6aa00..062c90318 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/lib/http/HttpClient_WinRt.cpp @@ -11,9 +11,7 @@ #include "http/HttpClient_WinRt.hpp" #include "utils/StringUtils.hpp" -#include #include -#include #include #include @@ -21,7 +19,6 @@ #include #include #include -#include using namespace Windows::Foundation; using namespace Windows::Foundation::Collections; diff --git a/lib/offline/OfflineStorageHandler.cpp b/lib/offline/OfflineStorageHandler.cpp index 52ce15515..559c8f977 100644 --- a/lib/offline/OfflineStorageHandler.cpp +++ b/lib/offline/OfflineStorageHandler.cpp @@ -161,11 +161,31 @@ namespace MAT_NS_BEGIN { return count; } + void OfflineStorageHandler::SignalFlushComplete() + { + LOCKGUARD(m_flushLock); + m_flushHandle.Cancel(); + m_flushComplete.post(); + m_flushPending = false; + } + void OfflineStorageHandler::Flush() { + // StartActivity() only keeps the LogManager alive for the duration of an + // asynchronously scheduled flush; it fails once teardown has begun pausing. + // Returning here without signalling would strand every thread blocked in + // WaitForFlush(): m_flushPending stays true and m_flushComplete is never + // posted, so Shutdown() waits on it forever. Always release the waiters. if (!m_logManager.StartActivity()) { + SignalFlushComplete(); return; } + FlushImpl(); + m_logManager.EndActivity(); + } + + void OfflineStorageHandler::FlushImpl() + { // Flush could be executed from context of worker thread, as well as from TPM and // after HTTP callback. Make sure it is atomic / thread-safe. LOCKGUARD(m_flushLock); @@ -221,7 +241,6 @@ namespace MAT_NS_BEGIN { // Flush is done, notify the waiters m_flushComplete.post(); m_flushPending = false; - m_logManager.EndActivity(); } bool OfflineStorageHandler::StoreRecord(StorageRecord const& record) diff --git a/lib/offline/OfflineStorageHandler.hpp b/lib/offline/OfflineStorageHandler.hpp index 9a1131aff..1e4aefaa4 100644 --- a/lib/offline/OfflineStorageHandler.hpp +++ b/lib/offline/OfflineStorageHandler.hpp @@ -100,6 +100,8 @@ namespace MAT_NS_BEGIN { private: void WaitForFlush(); + void FlushImpl(); + void SignalFlushComplete(); }; diff --git a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp index f01992940..3c8fe6baf 100644 --- a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp @@ -13,16 +13,12 @@ MATSDK_LOG_INST_COMPONENT_NS("DeviceInfo", "Win32 Desktop Device Information") -#include #include #include #include #include #include -#include -#include - #pragma comment(lib, "iphlpapi.lib") #pragma comment(lib, "AdvAPI32.Lib") @@ -149,4 +145,3 @@ namespace PAL_NS_BEGIN { } } PAL_NS_END - diff --git a/lib/pal/desktop/desktop.vcxitems b/lib/pal/desktop/desktop.vcxitems index 0d8ae8def..e827b6299 100644 --- a/lib/pal/desktop/desktop.vcxitems +++ b/lib/pal/desktop/desktop.vcxitems @@ -14,9 +14,12 @@ + + + ..\..;..\..\include;$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories);$(WindowsSDK_IncludePath) diff --git a/lib/utils/annex_k.hpp b/lib/utils/annex_k.hpp index 5aa4b73af..cfb6f6ba6 100644 --- a/lib/utils/annex_k.hpp +++ b/lib/utils/annex_k.hpp @@ -47,21 +47,13 @@ class BoundCheckFunctions private: static bool oneds_buffer_region_overlap(const char *buffer1, size_t buffer1_len, const char *buffer2, size_t buffer2_len) noexcept { - if (buffer2 >= buffer1) + // Two half-open ranges [b1, b1+len1) and [b2, b2+len2) overlap iff each + // starts before the other ends. Empty ranges never overlap. + if (buffer1_len == 0 || buffer2_len == 0) { - if (buffer1 + buffer1_len - 1 > buffer2 ) - { - return true; - } + return false; } - else - { - if (buffer2 + buffer2_len - 1 > buffer1) - { - return true; - } - } - return false; + return (buffer1 < buffer2 + buffer2_len) && (buffer2 < buffer1 + buffer1_len); } public: @@ -147,12 +139,16 @@ static errno_t oneds_strncpy_s(char * restrict dest, rsize_t destsz, const char // In case of error, the entire destination range [dest, dest+destsz) is zeroed out // (if both dest and destsz are valid)) +// +// NOTE: the constraint checks below are performed here rather than delegated to +// the platform's Annex K / CRT memcpy_s. On MSVC the CRT memcpy_s reports a +// constraint violation through the invalid parameter handler, whose default +// behaviour terminates the process (__fastfail / STATUS_STACK_BUFFER_OVERRUN) +// instead of returning EINVAL. Validating first keeps the documented +// "return EINVAL and zero the destination" contract on every platform. static errno_t oneds_memcpy_s( void *restrict dest, rsize_t destsz, const void *restrict src, rsize_t count ) noexcept { -#if (defined __STDC_LIB_EXT1__) || ( defined _MSC_VER) - return memcpy_s(dest, destsz, src, count); -#else if (dest == NULL) { return EINVAL; @@ -176,13 +172,8 @@ static errno_t oneds_memcpy_s( void *restrict dest, rsize_t destsz, memset(dest, 0, destsz); return EINVAL; } - void *result = memcpy(dest, src, count); - if (result == (void *)NULL) - { - return -1; - } + memcpy(dest, src, count); return 0; -#endif } }; } diff --git a/tests/functests/APITest.cpp b/tests/functests/APITest.cpp index baea0112e..fb64cd4cb 100644 --- a/tests/functests/APITest.cpp +++ b/tests/functests/APITest.cpp @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include "PayloadDecoder.hpp" @@ -673,37 +675,31 @@ constexpr static unsigned MAX_THREADS = 25; /// The configuration. void StressUploadLockMultiThreaded(ILogConfiguration& config) { - std::srand(static_cast(std::time(nullptr))); TestDebugEventListener debugListener; addAllListeners(debugListener); size_t numIterations = MAX_ITERATIONS_MT; - std::mutex m_threads_mtx; - std::atomic threadCount(0); - while (numIterations--) { ILogger *result = LogManager::Initialize(TEST_TOKEN, config); - // Keep spawning UploadNow threads while the main thread is trying to perform - // Initialize and Teardown, but no more than MAX_THREADS at a time. + std::vector uploadThreads; + uploadThreads.reserve(MAX_THREADS); for (size_t i = 0; i < MAX_THREADS; i++) { - if (threadCount++ < MAX_THREADS) + uploadThreads.emplace_back([]() { - auto t = std::thread([&]() - { - std::this_thread::yield(); - LogManager::UploadNow(); - const auto randTimeSub2ms = std::rand() % 2; - PAL::sleep(randTimeSub2ms); - threadCount--; - }); - t.detach(); - } - }; + std::this_thread::yield(); + LogManager::UploadNow(); + PAL::sleep(0); + }); + } EventProperties props = testing::CreateSampleEvent("event_name", EventPriority_Normal); result->LogEvent(props); + for (auto& uploadThread : uploadThreads) + { + uploadThread.join(); + } LogManager::FlushAndTeardown(); } removeAllListeners(debugListener); diff --git a/tests/functests/BasicFuncTests.cpp b/tests/functests/BasicFuncTests.cpp index bc879d3e6..f54bebd80 100644 --- a/tests/functests/BasicFuncTests.cpp +++ b/tests/functests/BasicFuncTests.cpp @@ -1364,7 +1364,9 @@ TEST_F(BasicFuncTests, sendManyRequestsAndCancel) configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; configuration[CFG_MAP_HTTP][CFG_BOOL_HTTP_COMPRESSION] = true; - configuration[CFG_STR_COLLECTOR_URL] = COLLECTOR_URL_PROD; + // Use a closed local port so this teardown stress test does not depend + // on external networking or overflow the fixture server's socket set. + configuration[CFG_STR_COLLECTOR_URL] = "http://127.0.0.1:1/"; configuration[CFG_INT_MAX_TEARDOWN_TIME] = (int64_t)(i % 2); configuration[CFG_INT_TRACE_LEVEL_MASK] = 0; configuration[CFG_INT_TRACE_LEVEL_MIN] = ACTTraceLevel_Warn; diff --git a/tests/unittests/HttpClientCurlTests.cpp b/tests/unittests/HttpClientCurlTests.cpp index 50a82a874..7f07a5b7c 100644 --- a/tests/unittests/HttpClientCurlTests.cpp +++ b/tests/unittests/HttpClientCurlTests.cpp @@ -13,6 +13,15 @@ #include "http/HttpClient_Curl.hpp" #include "config/RuntimeConfig_Default.hpp" +#include +#include +#include +#include +#include +#include +#include +#include + using namespace testing; using namespace MAT; @@ -184,6 +193,68 @@ TEST_F(HttpClientCurlTests, SetSslVerification_ConcurrentCallsNoRace) SUCCEED(); } +// --- Regression: EDEADLK self-join in ~CurlHttpOperation --- + +TEST_F(HttpClientCurlTests, SendAsync_DestroyOnWorkerThread_NoSelfJoin) +{ + struct TrackingCallback : public IHttpResponseCallback + { + std::atomic destroyEvents { 0 }; + void OnHttpResponse(IHttpResponse* response) override { delete response; } + void OnHttpStateEvent(HttpStateEvent state, void*, size_t) override + { + if (state == OnDestroy) + { + ++destroyEvents; + } + } + }; + + auto callback = std::make_shared(); + auto callbackDone = std::make_shared>(); + auto done = callbackDone->get_future(); + + auto op = std::make_shared( + "GET", "://malformed", callback.get(), m_headers, m_body, + false, 1 /*connTimeout*/, false /*sslVerify*/, ""); + + auto box = std::make_shared>(std::move(op)); + (*box)->SendAsync([box, callback, callbackDone](CurlHttpOperation&) { + box->reset(); + callbackDone->set_value(); + }); + + if (done.wait_for(std::chrono::seconds(5)) != std::future_status::ready) + { + ADD_FAILURE() << "curl worker did not finish before fixture teardown"; + std::abort(); + } + EXPECT_EQ(callback->destroyEvents.load(), 1); +} + +TEST_F(HttpClientCurlTests, SendAsync_CallbackCopyFailureStillCompletes) +{ + struct ThrowOnCopy + { + explicit ThrowOnCopy(bool& invoked) : invoked(&invoked) {} + ThrowOnCopy(ThrowOnCopy&&) = default; + ThrowOnCopy(const ThrowOnCopy&) { throw std::logic_error("copy failed"); } + void operator()(CurlHttpOperation&) const { *invoked = true; } + bool* invoked; + }; + + CurlHttpOperation op( + "GET", "://malformed", nullptr, m_headers, m_body, + false, 1 /*connTimeout*/, false /*sslVerify*/, ""); + bool callbackInvoked = false; + std::function callback { ThrowOnCopy(callbackInvoked) }; + + EXPECT_NO_THROW(op.SendAsync(std::move(callback))); + EXPECT_TRUE(callbackInvoked); + EXPECT_EQ(op.GetResponseCode(), CURLE_FAILED_INIT); + EXPECT_THROW(op.SendAsync(), std::logic_error); +} + // --- Response-size cap (memory-amplification DoS hardening) --- class HttpClientCurlResponseCapTests : public ::testing::Test, @@ -195,9 +266,7 @@ class HttpClientCurlResponseCapTests : public ::testing::Test, HttpClient_Curl m_client; // The client never takes ownership of the request (it only stores a raw pointer // and erases it); the fixture owns it and frees it in TearDown -- on the main - // thread, after the transfer has completed. Freeing it inside OnHttpResponse - // would destroy the CurlHttpOperation from within its own async task, whose - // destructor waits on that task (a self-join deadlock). + // thread, after the transfer has completed. std::unique_ptr m_request; std::string m_hostname; size_t m_responseBodySize {0}; diff --git a/tests/unittests/HttpClientTests.cpp b/tests/unittests/HttpClientTests.cpp index 4b17bcce5..951dac34a 100644 --- a/tests/unittests/HttpClientTests.cpp +++ b/tests/unittests/HttpClientTests.cpp @@ -10,6 +10,8 @@ #include "common/HttpServer.hpp" #include "http/HttpClientFactory.hpp" +#include + using namespace testing; using namespace MAT; @@ -29,6 +31,11 @@ class HttpClientTests : public ::testing::Test, enum RequestState { Planned, Sent, Processed, Done }; std::vector _countedRequests; std::mutex _lock; + std::condition_variable _responseCv; + std::condition_variable _blockedRequestCv; + std::mutex _blockedRequestLock; + bool _blockedRequestReceived {false}; + bool _releaseBlockedRequest {false}; public: HttpClientTests() @@ -59,6 +66,7 @@ class HttpClientTests : public ::testing::Test, _server.addHandler("/simple/", *this); _server.addHandler("/echo/", *this); _server.addHandler("/count/", *this); + _server.addHandler("/block/", *this); _server.start(); Clear(); @@ -66,6 +74,11 @@ class HttpClientTests : public ::testing::Test, virtual void TearDown() override { + { + std::lock_guard lock(_blockedRequestLock); + _releaseBlockedRequest = true; + } + _blockedRequestCv.notify_all(); _server.stop(); _client.reset(); Clear(); @@ -87,6 +100,17 @@ class HttpClientTests : public ::testing::Test, return 200; } + if (request.uri == "/block/") { + { + std::lock_guard lock(_blockedRequestLock); + _blockedRequestReceived = true; + } + _blockedRequestCv.notify_all(); + std::unique_lock lock(_blockedRequestLock); + _blockedRequestCv.wait(lock, [this]() { return _releaseBlockedRequest; }); + return 200; + } + if (request.uri.substr(0, 7) == "/count/") { int id = atoi(request.uri.substr(7).c_str()); if (id >= 0 && static_cast(id) < _countedRequests.size()) { @@ -119,6 +143,7 @@ class HttpClientTests : public ::testing::Test, { std::lock_guard lock(_lock); _responses.push_back(clone(inResponse)); + _responseCv.notify_all(); } }; @@ -128,6 +153,47 @@ std::vector Binary(std::string const& str) return std::vector(str.data(), str.data() + str.size()); } +TEST_F(HttpClientTests, HandlesCancellationWhileResponseIsInFlight) +{ + Clear(); + { + std::lock_guard lock(_blockedRequestLock); + _blockedRequestReceived = false; + _releaseBlockedRequest = false; + } + + std::unique_ptr request(_client->CreateRequest()); + std::string requestId = request->GetId(); + request->SetUrl("http://" + _hostname + "/block/"); + _client->SendRequestAsync(request.release(), this); + + { + std::unique_lock lock(_blockedRequestLock); + ASSERT_TRUE(_blockedRequestCv.wait_for(lock, std::chrono::seconds(10), + [this]() { return _blockedRequestReceived; })); + } + + _client->CancelRequestAsync(requestId); + { + std::lock_guard lock(_blockedRequestLock); + _releaseBlockedRequest = true; + } + _blockedRequestCv.notify_all(); + + std::unique_ptr response; + { + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(2), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + response.reset(_responses[0]); + _responses.clear(); + } + + EXPECT_THAT(response->GetId(), requestId); + EXPECT_THAT(response->GetResult(), HttpResult_Aborted); +} + //--- TEST_F(HttpClientTests, HandlesSimpleRequest) @@ -346,4 +412,3 @@ TEST_F(HttpClientTests, SurvivesManyRequests) } #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT -