LHC: various minor fixes (#944)

jasonsandlin · web-flow · commit dae34407ddf3 · 2026-03-11T13:47:48.000-07:00
diff --git a/Include/httpClient/httpClient.h b/Include/httpClient/httpClient.h
@@ -1083,7 +1083,7 @@ STDAPI HCWebSocketGetEventFunctions(
     _Out_opt_ HCWebSocketMessageFunction* messageFunc,
     _Out_opt_ HCWebSocketBinaryMessageFunction* binaryMessageFunc,
     _Out_opt_ HCWebSocketCloseEventFunction* closeFunc,
-    _Out_ void** functionContext
+    _Out_opt_ void** functionContext
     ) noexcept;
 
 #if HC_PLATFORM == HC_PLATFORM_WIN32 || HC_PLATFORM == HC_PLATFORM_GDK
diff --git a/Include/httpClient/httpProvider.h b/Include/httpClient/httpProvider.h
@@ -284,7 +284,7 @@ STDAPI HCHttpCallRequestGetTimeout(
 /// </remarks>
 STDAPI HCHttpCallRequestGetRetryDelay(
     _In_opt_ HCCallHandle call,
-    _In_ uint32_t* retryDelayInSeconds
+    _Out_ uint32_t* retryDelayInSeconds
     ) noexcept;
 
 /// <summary>
diff --git a/Source/Common/Win/utils_win.cpp b/Source/Common/Win/utils_win.cpp
@@ -38,6 +38,11 @@ http_internal_string utf8_from_utf16(_In_reads_(size) PCWSTR utf16, size_t size)
         return "";
     }
 
+    if (size > INT_MAX)
+    {
+        throw std::exception("utf8_from_utf16 failed: input too large");
+    }
+
     // query for the buffer size
     auto queryResult = WideCharToMultiByte(
         CP_UTF8, WC_ERR_INVALID_CHARS,
@@ -49,13 +54,13 @@ http_internal_string utf8_from_utf16(_In_reads_(size) PCWSTR utf16, size_t size)
     {
 #if HC_TRACE_ERROR_ENABLE // to avoid unused variable warnings
         auto err = GetLastError();
-        HC_TRACE_ERROR(HTTPCLIENT, "utf16_from_uft8 failed during buffer size query with error: %u", err);
+        HC_TRACE_ERROR(HTTPCLIENT, "utf8_from_utf16 failed during buffer size query with error: %u", err);
 #endif
-        throw std::exception("utf16_from_utf8 failed");
+        throw std::exception("utf8_from_utf16 failed");
     }
 
     // allocate the output buffer, queryResult is the required size
-    http_internal_string utf8(static_cast<size_t>(queryResult), L'\0');
+    http_internal_string utf8(static_cast<size_t>(queryResult), '\0');
     auto conversionResult = WideCharToMultiByte(
         CP_UTF8, WC_ERR_INVALID_CHARS,
         utf16, static_cast<int>(size),
@@ -66,9 +71,9 @@ http_internal_string utf8_from_utf16(_In_reads_(size) PCWSTR utf16, size_t size)
     {
 #if HC_TRACE_ERROR_ENABLE // to avoid unused variable warnings
         auto err = GetLastError();
-        HC_TRACE_ERROR(HTTPCLIENT, "utf16_from_uft8 failed during conversion: %u", err);
+        HC_TRACE_ERROR(HTTPCLIENT, "utf8_from_utf16 failed during conversion: %u", err);
 #endif
-        throw std::exception("utf16_from_utf8 failed");
+        throw std::exception("utf8_from_utf16 failed");
     }
 
     return utf8;
@@ -82,6 +87,11 @@ http_internal_wstring utf16_from_utf8(_In_reads_(size) const char* utf8, size_t
         return L"";
     }
 
+    if (size > INT_MAX)
+    {
+        throw std::exception("utf16_from_utf8 failed: input too large");
+    }
+
     // query for the buffer size
     auto queryResult = MultiByteToWideChar(
         CP_UTF8, MB_ERR_INVALID_CHARS,
@@ -92,7 +102,7 @@ http_internal_wstring utf16_from_utf8(_In_reads_(size) const char* utf8, size_t
     {
 #if HC_TRACE_ERROR_ENABLE // to avoid unused variable warnings
         auto err = GetLastError();
-        HC_TRACE_ERROR(HTTPCLIENT, "utf16_from_uft8 failed during buffer size query with error: %u", err);
+        HC_TRACE_ERROR(HTTPCLIENT, "utf16_from_utf8 failed during buffer size query with error: %u", err);
 #endif
         throw std::exception("utf16_from_utf8 failed");
     }
@@ -108,7 +118,7 @@ http_internal_wstring utf16_from_utf8(_In_reads_(size) const char* utf8, size_t
     {
 #if HC_TRACE_ERROR_ENABLE // to avoid unused variable warnings
         auto err = GetLastError();
-        HC_TRACE_ERROR(HTTPCLIENT, "utf16_from_uft8 failed during conversion: %u", err);
+        HC_TRACE_ERROR(HTTPCLIENT, "utf16_from_utf8 failed during conversion: %u", err);
 #endif
         throw std::exception("utf16_from_utf8 failed");
     }
diff --git a/Source/Common/pch_common.h b/Source/Common/pch_common.h
@@ -160,7 +160,7 @@ HC_DECLARE_TRACE_AREA(WEBSOCKET);
     catch (std::exception const& e) { ::xbox::httpclient::detail::StdExceptionToResult(e, file, line); return errCode; } \
     catch (...) { ::xbox::httpclient::detail::UnknownExceptionToResult(file, line); return errCode; }
 
-#define RETURN_IF_PERFORM_CALLED(call) if (call->performCalled) return E_HC_PERFORM_ALREADY_CALLED;
+#define RETURN_IF_PERFORM_CALLED(call) if (call->performCalled.load()) return E_HC_PERFORM_ALREADY_CALLED;
 #define TO_ULL(x) static_cast<unsigned long long>(x)
 
 NAMESPACE_XBOX_HTTP_CLIENT_DETAIL_BEGIN
diff --git a/Source/Common/uri.cpp b/Source/Common/uri.cpp
@@ -289,15 +289,15 @@ String Uri::ToString() const
         break;
         case '%':
         {
-            if (chunkEnd > urlPart.size() - 3) // a % encoding is 3 characters long
+            if (chunkEnd + 3 > urlPart.size()) // a % encoding is 3 characters long
             {
-                //THROW(E_INVALIDARG, "Invalid % encode in url encoded string");
+                return http_internal_string();
             }
 
             uint8_t value = 0;
             if (!HexDecodePair(urlPart[chunkEnd + 1], urlPart[chunkEnd + 2], value))
             {
-                //THROW(E_INVALIDARG, "Invalid value for % encode in url encoded string");
+                return http_internal_string();
             }
 
             decoded.push_back(value);
@@ -619,6 +619,12 @@ bool Uri::ParsePort(String const& uri, String::const_iterator& it)
         return false;
     }
 
+    if (portV > 65535)
+    {
+        HC_TRACE_WARNING(HTTPCLIENT, "Port out of range in URI.");
+        return false;
+    }
+
     m_port = static_cast<uint16_t>(portV);
     it = portEnd;
 
diff --git a/Source/Global/global.cpp b/Source/Global/global.cpp
@@ -232,7 +232,8 @@ void http_singleton::set_retry_state(
 
 http_retry_after_api_state http_singleton::get_retry_state(_In_ uint32_t retryAfterCacheId)
 {
-    auto it = m_retryAfterCache.find(retryAfterCacheId); // STL is multithread read safe
+    std::lock_guard<std::recursive_mutex> lock(m_retryAfterCacheLock);
+    auto it = m_retryAfterCache.find(retryAfterCacheId);
     if (it != m_retryAfterCache.end())
     {
         return it->second; // returning a copy of state struct
diff --git a/Source/HTTP/WinHttp/winhttp_connection.cpp b/Source/HTTP/WinHttp/winhttp_connection.cpp
@@ -1068,7 +1068,7 @@ void WinHttpConnection::callback_status_data_available(
             nullptr))
         {
             DWORD dwError = GetLastError();
-            HC_TRACE_ERROR(HTTPCLIENT, "WinHttpConnection [ID %llu] [TID %ul] WinHttpReadData errorcode %d", TO_ULL(HCHttpCallGetId(pRequestContext->m_call)), GetCurrentThreadId(), GetLastError());
+            HC_TRACE_ERROR(HTTPCLIENT, "WinHttpConnection [ID %llu] [TID %ul] WinHttpReadData errorcode %d", TO_ULL(HCHttpCallGetId(pRequestContext->m_call)), GetCurrentThreadId(), dwError);
             pRequestContext->m_lock.unlock();
             pRequestContext->complete_task(E_FAIL, HRESULT_FROM_WIN32(dwError));
             return;
diff --git a/Source/HTTP/WinHttp/winhttp_connection.h b/Source/HTTP/WinHttp/winhttp_connection.h
@@ -97,7 +97,8 @@ class websocket_message_buffer
 
         if (dataByteCount > m_bufferByteCapacity)
         {
-            newBuffer = static_cast<uint8_t*>(http_memory::mem_alloc(dataByteCount));
+            // Allocate +1 for possible null terminator on text WebSocket messages
+            newBuffer = static_cast<uint8_t*>(http_memory::mem_alloc(dataByteCount + 1));
             if (newBuffer != nullptr)
             {
                 if (m_buffer != nullptr)
diff --git a/Source/HTTP/XMLHttp/xmlhttp_http_task.cpp b/Source/HTTP/XMLHttp/xmlhttp_http_task.cpp
@@ -26,7 +26,7 @@ xmlhttp_http_task::xmlhttp_http_task(
 xmlhttp_http_task::~xmlhttp_http_task()
 {
     m_hRequest = nullptr;
-    if (SUCCEEDED(m_hrCoInit))
+    if (m_hrCoInit == S_OK)
     {
         CoUninitialize();
     }
diff --git a/Source/HTTP/compression.cpp b/Source/HTTP/compression.cpp
@@ -29,7 +29,11 @@ void Compression::CompressToGzip(uint8_t* inData, size_t inDataSize, HCCompressi
     stream.opaque = Z_NULL;
 
     // deflateInit will use zlib (deflate) compression, so deflateInit2 with these flags is required for GZIP Compression
-    deflateInit2(&stream, compressionLevelValue, Z_DEFLATED, WINDOWBITS | GZIP_ENCODING, 8, Z_DEFAULT_STRATEGY);
+    int initResult = deflateInit2(&stream, compressionLevelValue, Z_DEFLATED, WINDOWBITS | GZIP_ENCODING, 8, Z_DEFAULT_STRATEGY);
+    if (initResult != Z_OK)
+    {
+        return;
+    }
 
     stream.next_in = inData;
     stream.avail_in = static_cast<uInt>(inDataSize);
@@ -75,7 +79,11 @@ void Compression::DecompressFromGzip(uint8_t* inData, size_t inDataSize, http_in
     stream.opaque = Z_NULL;
 
     // WINDOWBITS | GZIP_ENCODING - add 16 to decode only the gzip format 
-    inflateInit2(&stream, WINDOWBITS | GZIP_ENCODING);
+    int initResult = inflateInit2(&stream, WINDOWBITS | GZIP_ENCODING);
+    if (initResult != Z_OK)
+    {
+        return;
+    }
 
     stream.next_in = inData; 
     stream.avail_in = static_cast<uInt>(inDataSize); 
diff --git a/Source/HTTP/httpcall.cpp b/Source/HTTP/httpcall.cpp
@@ -77,7 +77,11 @@ HRESULT CALLBACK HC_CALL::PerfomAsyncProvider(XAsyncOp op, XAsyncProviderData co
     {
     case XAsyncOp::Begin:
     {
-        call->performCalled = true;
+        bool expected = false;
+        if (!call->performCalled.compare_exchange_strong(expected, true))
+        {
+            return E_HC_PERFORM_ALREADY_CALLED;
+        }
         call->m_performStartTime = chrono_clock_t::now();
 
         // Initialize work queues
@@ -184,11 +188,19 @@ void CALLBACK HC_CALL::CompressRequestBody(void* c, bool canceled)
     if (FAILED(hr) || !clientRequestBodyReadCallback)
     {
         HC_TRACE_ERROR(HTTPCLIENT, "HC_CALL::CompressRequestBody: Unable to get client's RequestBodyRead callback");
+        XAsyncComplete(context->asyncBlock, FAILED(hr) ? hr : E_UNEXPECTED, 0);
+        return;
+    }
+
+    if (requestBodySize == 0)
+    {
+        HC_TRACE_ERROR(HTTPCLIENT, "HC_CALL::CompressRequestBody: Request body is empty");
+        XAsyncComplete(context->asyncBlock, E_INVALIDARG, 0);
         return;
     }
 
     http_internal_vector<uint8_t> uncompressedRequestyBodyBuffer(requestBodySize);
-    uint8_t* bufferPtr = &uncompressedRequestyBodyBuffer.front();
+    uint8_t* bufferPtr = uncompressedRequestyBodyBuffer.data();
     size_t bytesWritten = 0;
 
     try
@@ -198,18 +210,21 @@ void CALLBACK HC_CALL::CompressRequestBody(void* c, bool canceled)
         if (FAILED(hr))
         {
             HC_TRACE_ERROR_HR(HTTPCLIENT, hr, "HC_CALL::CompressRequestBody: client RequestBodyRead callback failed");
+            XAsyncComplete(context->asyncBlock, hr, 0);
             return;
         }
 
         // Return error if client provides less bytes than expected.
         if (bytesWritten < requestBodySize)
         {
             HC_TRACE_ERROR(HTTPCLIENT, "HC_CALL::CompressRequestBody: Expected more data written by the client based on initial request body size provided.");
+            XAsyncComplete(context->asyncBlock, E_FAIL, 0);
             return;
         }
     }
     catch (...)
     {
+        XAsyncComplete(context->asyncBlock, E_FAIL, 0);
         return;
     }
 
@@ -223,7 +238,13 @@ void CALLBACK HC_CALL::CompressRequestBody(void* c, bool canceled)
     call->requestBodyReadFunctionContext = nullptr;
 
     // Directly setting compressed body bytes to HCCall
-    call->requestBodySize = (uint32_t)compressedRequestBodyBuffer.size();
+    if (compressedRequestBodyBuffer.size() > UINT32_MAX)
+    {
+        HC_TRACE_ERROR(HTTPCLIENT, "HC_CALL::CompressRequestBody: Compressed body size exceeds uint32_t max");
+        XAsyncComplete(context->asyncBlock, E_FAIL, 0);
+        return;
+    }
+    call->requestBodySize = static_cast<uint32_t>(compressedRequestBodyBuffer.size());
     call->requestBodyBytes = std::move(compressedRequestBodyBuffer);
     call->requestBodyString.clear();
 
diff --git a/Source/HTTP/httpcall.h b/Source/HTTP/httpcall.h
@@ -79,7 +79,7 @@ struct HC_CALL
     bool compressedResponse{ false };
 
     // Request metadata
-    bool performCalled{ false };
+    std::atomic<bool> performCalled{ false };
     bool retryAllowed{ false };
     uint32_t retryAfterCacheId{ 0 };
     uint32_t timeoutWindowInSeconds{ 0 };
diff --git a/Source/Logger/trace_internal.h b/Source/Logger/trace_internal.h
@@ -28,8 +28,8 @@ class TraceState
     {
         std::chrono::high_resolution_clock::time_point{}
     };
-    bool m_traceToDebugger = false;
-    bool m_etwEnabled = false;
+    std::atomic<bool> m_traceToDebugger{ false };
+    std::atomic<bool> m_etwEnabled{ false };
 };
 
 TraceState& GetTraceState() noexcept;
diff --git a/Source/Platform/GDK/PlatformComponents_GDK.cpp b/Source/Platform/GDK/PlatformComponents_GDK.cpp
@@ -83,15 +83,31 @@ HRESULT PlatformInitialize(PlatformComponents& components, HCInitArgs* initArgs)
 STDAPI_(void) HCWinHttpSuspend()
 {
     auto httpSingleton = get_http_singleton();
-    auto& winHttpProvider = dynamic_cast<WinHttp_WebSocketProvider*>(&httpSingleton->m_networkState->WebSocketProvider())->WinHttpProvider;
-    winHttpProvider->Suspend();
+    if (!httpSingleton)
+    {
+        return;
+    }
+    auto* winHttpWebSocketProvider = dynamic_cast<WinHttp_WebSocketProvider*>(&httpSingleton->m_networkState->WebSocketProvider());
+    if (!winHttpWebSocketProvider)
+    {
+        return;
+    }
+    winHttpWebSocketProvider->WinHttpProvider->Suspend();
 }
 
 STDAPI_(void) HCWinHttpResume()
 {
     auto httpSingleton = get_http_singleton();
-    auto& winHttpProvider = dynamic_cast<WinHttp_WebSocketProvider*>(&httpSingleton->m_networkState->WebSocketProvider())->WinHttpProvider;
-    winHttpProvider->Resume();
+    if (!httpSingleton)
+    {
+        return;
+    }
+    auto* winHttpWebSocketProvider = dynamic_cast<WinHttp_WebSocketProvider*>(&httpSingleton->m_networkState->WebSocketProvider());
+    if (!winHttpWebSocketProvider)
+    {
+        return;
+    }
+    winHttpWebSocketProvider->WinHttpProvider->Resume();
 }
 
 NAMESPACE_XBOX_HTTP_CLIENT_END
diff --git a/Source/Platform/Generic/PlatformComponents_Generic.cpp b/Source/Platform/Generic/PlatformComponents_Generic.cpp
@@ -61,7 +61,9 @@ class GenericWebSocketProvider : public IWebSocketProvider
 HRESULT PlatformInitialize(PlatformComponents& components, HCInitArgs* /*initArgs*/)
 {
     components.HttpProvider = http_allocate_unique<GenericHttpProvider>();
+#ifndef HC_NOWEBSOCKETS
     components.WebSocketProvider = http_allocate_unique<GenericWebSocketProvider>();
+#endif
 
     return S_OK;
 }
diff --git a/Source/WebSocket/websocket_publics.cpp b/Source/WebSocket/websocket_publics.cpp
@@ -310,7 +310,7 @@ STDAPI HCWebSocketGetEventFunctions(
     _Out_opt_ HCWebSocketMessageFunction* messageFunc,
     _Out_opt_ HCWebSocketBinaryMessageFunction* binaryMessageFunc,
     _Out_opt_ HCWebSocketCloseEventFunction* closeFunc,
-    _Out_ void** context
+    _Out_opt_ void** context
 ) noexcept
 try
 {

Original file line number	Diff line number	Diff line change
`@@ -289,15 +289,15 @@ String Uri::ToString() const`
`289`	`289`	`break;`
`290`	`290`	`case '%':`
`291`	`291`	`{`
`292`		`- if (chunkEnd > urlPart.size() - 3) // a % encoding is 3 characters long`
	`292`	`+ if (chunkEnd + 3 > urlPart.size()) // a % encoding is 3 characters long`
`293`	`293`	`{`
`294`		`- //THROW(E_INVALIDARG, "Invalid % encode in url encoded string");`
	`294`	`+ return http_internal_string();`
`295`	`295`	`}`
`296`	`296`
`297`	`297`	`uint8_t value = 0;`
`298`	`298`	`if (!HexDecodePair(urlPart[chunkEnd + 1], urlPart[chunkEnd + 2], value))`
`299`	`299`	`{`
`300`		`- //THROW(E_INVALIDARG, "Invalid value for % encode in url encoded string");`
	`300`	`+ return http_internal_string();`
`301`	`301`	`}`
`302`	`302`
`303`	`303`	`decoded.push_back(value);`
`@@ -619,6 +619,12 @@ bool Uri::ParsePort(String const& uri, String::const_iterator& it)`
`619`	`619`	`return false;`
`620`	`620`	`}`
`621`	`621`
	`622`	`+ if (portV > 65535)`
	`623`	`+ {`
	`624`	`+ HC_TRACE_WARNING(HTTPCLIENT, "Port out of range in URI.");`
	`625`	`+ return false;`
	`626`	`+ }`
	`627`	`+`
`622`	`628`	`m_port = static_cast<uint16_t>(portV);`
`623`	`629`	`it = portEnd;`
`624`	`630`
Original file line number	Diff line number	Diff line change
`@@ -232,7 +232,8 @@ void http_singleton::set_retry_state(`
`232`	`232`
`233`	`233`	`http_retry_after_api_state http_singleton::get_retry_state(_In_ uint32_t retryAfterCacheId)`
`234`	`234`	`{`
`235`		`- auto it = m_retryAfterCache.find(retryAfterCacheId); // STL is multithread read safe`
	`235`	`+ std::lock_guard<std::recursive_mutex> lock(m_retryAfterCacheLock);`
	`236`	`+ auto it = m_retryAfterCache.find(retryAfterCacheId);`
`236`	`237`	`if (it != m_retryAfterCache.end())`
`237`	`238`	`{`
`238`	`239`	`return it->second; // returning a copy of state struct`
Original file line number	Diff line number	Diff line change
`@@ -1068,7 +1068,7 @@ void WinHttpConnection::callback_status_data_available(`
`1068`	`1068`	`nullptr))`
`1069`	`1069`	`{`
`1070`	`1070`	`DWORD dwError = GetLastError();`
`1071`		`- HC_TRACE_ERROR(HTTPCLIENT, "WinHttpConnection [ID %llu] [TID %ul] WinHttpReadData errorcode %d", TO_ULL(HCHttpCallGetId(pRequestContext->m_call)), GetCurrentThreadId(), GetLastError());`
	`1071`	`+ HC_TRACE_ERROR(HTTPCLIENT, "WinHttpConnection [ID %llu] [TID %ul] WinHttpReadData errorcode %d", TO_ULL(HCHttpCallGetId(pRequestContext->m_call)), GetCurrentThreadId(), dwError);`
`1072`	`1072`	`pRequestContext->m_lock.unlock();`
`1073`	`1073`	`pRequestContext->complete_task(E_FAIL, HRESULT_FROM_WIN32(dwError));`
`1074`	`1074`	`return;`
Original file line number	Diff line number	Diff line change
`@@ -97,7 +97,8 @@ class websocket_message_buffer`
`97`	`97`
`98`	`98`	`if (dataByteCount > m_bufferByteCapacity)`
`99`	`99`	`{`
`100`		`- newBuffer = static_cast<uint8_t*>(http_memory::mem_alloc(dataByteCount));`
	`100`	`+ // Allocate +1 for possible null terminator on text WebSocket messages`
	`101`	`+ newBuffer = static_cast<uint8_t*>(http_memory::mem_alloc(dataByteCount + 1));`
`101`	`102`	`if (newBuffer != nullptr)`
`102`	`103`	`{`
`103`	`104`	`if (m_buffer != nullptr)`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ xmlhttp_http_task::xmlhttp_http_task(`
`26`	`26`	`xmlhttp_http_task::~xmlhttp_http_task()`
`27`	`27`	`{`
`28`	`28`	`m_hRequest = nullptr;`
`29`		`- if (SUCCEEDED(m_hrCoInit))`
	`29`	`+ if (m_hrCoInit == S_OK)`
`30`	`30`	`{`
`31`	`31`	`CoUninitialize();`
`32`	`32`	`}`