From 4e6f10c50892dfdefef4cb05f84342acdf5aed96 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 14 Jul 2026 21:25:05 -0500 Subject: [PATCH 1/6] Cap HTTP response body size across WinInet, WinRt, and Apple transports Extends the memory-amplification DoS hardening (a hostile or MITM'd collector returning an oversized body to exhaust process memory) beyond the libcurl transport to the remaining platform transports. Introduces a single shared constant MAX_HTTP_RESPONSE_SIZE (16 MB) in IHttpClient.hpp so every transport uses the same generous ceiling, well above any legitimate OneCollector or config response. - WinInet: bound m_bodyBuffer in the InternetReadFile loop; over-cap aborts the read and the request is reported as a failure (retried). - WinRt: reject a ReadAsBufferAsync buffer whose length exceeds the cap without copying it; report NetworkFailure. - Apple (NSURLSession): reject a completion-handler NSData larger than the cap without copying it; report NetworkFailure. The libcurl transport is capped separately in its own focused change; a later cleanup can unify its constant onto MAX_HTTP_RESPONSE_SIZE. Files: - lib/include/public/IHttpClient.hpp - lib/http/HttpClient_WinInet.cpp - lib/http/HttpClient_WinRt.cpp - lib/http/HttpClient_Apple.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Apple.mm | 19 +++++++++++++++---- lib/http/HttpClient_WinInet.cpp | 8 ++++++++ lib/http/HttpClient_WinRt.cpp | 10 +++++++++- lib/include/public/IHttpClient.hpp | 13 +++++++++++++ 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index 449b4c9af..c0b968e45 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/lib/http/HttpClient_Apple.mm @@ -120,10 +120,21 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error) } else { - simpleResponse->m_result = HttpResult_OK; - auto body = static_cast([data bytes]); - simpleResponse->m_body.reserve(data.length); - std::copy(body, body + data.length, std::back_inserter(simpleResponse->m_body)); + // 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. Reported as a network failure (retried). + if (data.length > MAX_HTTP_RESPONSE_SIZE) + { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + simpleResponse->m_result = HttpResult_NetworkFailure; + } + else + { + simpleResponse->m_result = HttpResult_OK; + auto body = static_cast([data bytes]); + simpleResponse->m_body.reserve(data.length); + std::copy(body, body + data.length, std::back_inserter(simpleResponse->m_body)); + } } m_callback->OnHttpResponse(simpleResponse); } diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index eaefb2318..e9ec3609f 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -326,6 +326,14 @@ class WinInetRequestWrapper m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed); while (!m_readingData || m_bufferUsed != 0) { + // SECURITY: refuse an over-large response instead of buffering it (see + // MAX_HTTP_RESPONSE_SIZE). Each chunk is small, so checking here bounds + // the buffer to within one chunk of the cap. + if (m_bodyBuffer.size() > MAX_HTTP_RESPONSE_SIZE) { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + dwError = ERROR_NOT_ENOUGH_MEMORY; + break; + } BOOL bResult = ::InternetReadFile(m_hWinInetRequest, m_buffer, sizeof(m_buffer), &m_bufferUsed); m_readingData = true; if (!bResult) { diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index e46e4c49c..0eaaaca30 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/lib/http/HttpClient_WinRt.cpp @@ -223,7 +223,15 @@ namespace MAT_NS_BEGIN { auto buffer = task.get(); size_t length = buffer->Length; - if (length > 0) + // 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. The request is reported as a network failure (retried). + if (length > MAX_HTTP_RESPONSE_SIZE) + { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + response->m_result = HttpResult_NetworkFailure; + } + else if (length > 0) { response->m_body.reserve(length); response->m_body.resize(length); diff --git a/lib/include/public/IHttpClient.hpp b/lib/include/public/IHttpClient.hpp index 89e5e6cf0..7a8678ceb 100644 --- a/lib/include/public/IHttpClient.hpp +++ b/lib/include/public/IHttpClient.hpp @@ -14,11 +14,24 @@ #include #include #include +#include ///@cond INTERNAL_DOCS namespace MAT_NS_BEGIN { class ILogConfiguration; + + /// + /// SECURITY: upper bound (in bytes) on an HTTP response body that a transport + /// will buffer. OneCollector protocol responses are small (status, kill-switch + /// tokens, retry-after, small config), so this generous cap never rejects a + /// legitimate response, but it stops a hostile or MITM'd collector from driving + /// unbounded memory growth by returning an oversized body (a memory-amplification + /// DoS of the embedding process). A transport that would exceed it refuses the + /// response and reports the request as a network failure so it is retried. + /// + static constexpr std::size_t MAX_HTTP_RESPONSE_SIZE = 16u * 1024u * 1024u; // 16 MB + /// /// The HttpHeaders class contains a set of HTTP headers. /// From cc28b005d1171ada0d9e309d342ed2812fa4ab92 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 14 Jul 2026 21:43:55 -0500 Subject: [PATCH 2/6] Map WinInet oversize to NetworkFailure and guard Apple nil data (round 1) Address Copilot review: - WinInet: the oversize-response abort set ERROR_NOT_ENOUGH_MEMORY, which fell through to the default case (LocalFailure). Use ERROR_HTTP_INVALID_SERVER_- RESPONSE so it maps to HttpResult_NetworkFailure, consistent with the WinRt and Apple transports (still retried, but correctly classified). - Apple: guard the success-path copy on a non-zero length and cast data.length to size_t, so a nil NSData (bytes == nullptr) never performs pointer arithmetic on nullptr (undefined behavior). Files: - lib/http/HttpClient_WinInet.cpp - lib/http/HttpClient_Apple.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Apple.mm | 10 +++++++--- lib/http/HttpClient_WinInet.cpp | 5 +++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index c0b968e45..ac05254c9 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/lib/http/HttpClient_Apple.mm @@ -131,9 +131,13 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error) else { simpleResponse->m_result = HttpResult_OK; - auto body = static_cast([data bytes]); - simpleResponse->m_body.reserve(data.length); - std::copy(body, body + data.length, std::back_inserter(simpleResponse->m_body)); + const size_t length = static_cast(data.length); + if (length > 0) + { + auto body = static_cast([data bytes]); + simpleResponse->m_body.reserve(length); + std::copy(body, body + length, std::back_inserter(simpleResponse->m_body)); + } } } m_callback->OnHttpResponse(simpleResponse); diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index e9ec3609f..cdf816fa2 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -328,10 +328,11 @@ class WinInetRequestWrapper while (!m_readingData || m_bufferUsed != 0) { // SECURITY: refuse an over-large response instead of buffering it (see // MAX_HTTP_RESPONSE_SIZE). Each chunk is small, so checking here bounds - // the buffer to within one chunk of the cap. + // the buffer to within one chunk of the cap. Reported as an invalid + // server response -> NetworkFailure, so the upload is retried. if (m_bodyBuffer.size() > MAX_HTTP_RESPONSE_SIZE) { LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); - dwError = ERROR_NOT_ENOUGH_MEMORY; + dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE; break; } BOOL bResult = ::InternetReadFile(m_hWinInetRequest, m_buffer, sizeof(m_buffer), &m_bufferUsed); From ffeddf2e331cd72a7acf30a49f7a278cc4f72845 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 14 Jul 2026 22:16:04 -0500 Subject: [PATCH 3/6] Stream response bodies to enforce the cap before full materialization (round 2) Address Copilot review round 2: the previous checks ran after the framework had already materialized the whole response body, so an oversized response could still drive a large allocation. Rework each transport to bound memory to the cap: - WinInet: check before every append (pre-loop and in-loop) so m_bodyBuffer never exceeds MAX_HTTP_RESPONSE_SIZE, not "cap + one chunk". (Validated: the Windows `mat` library compiles.) - WinRt: request with HttpCompletionOption::ResponseHeadersRead (so the body is not pre-buffered) and stream it via ReadAsInputStreamAsync in 64 KB chunks, aborting the moment the cap would be exceeded. - Apple: replace the completionHandler NSURLSession API (which materializes the full NSData) with a streaming NSURLSessionDataDelegate that accumulates in didReceiveData: and cancels the task once the cap would be exceeded; an over-cap transfer is surfaced as NetworkFailure (retried). The WinRt and Apple rewrites target UWP/macOS toolchains that aren't available locally, so they are review-verified and must be built/tested on-device before merge. Files: - lib/http/HttpClient_WinInet.cpp - lib/http/HttpClient_WinRt.cpp - lib/http/HttpClient_Apple.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Apple.mm | 145 +++++++++++++++++++++++++++----- lib/http/HttpClient_WinInet.cpp | 61 ++++++++------ lib/http/HttpClient_WinRt.cpp | 91 +++++++++++++------- 3 files changed, 219 insertions(+), 78 deletions(-) diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index ac05254c9..e1fcff5ec 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/lib/http/HttpClient_Apple.mm @@ -15,6 +15,111 @@ #include "utils/StringUtils.hpp" #include "utils/Utils.hpp" +// Streams the response body in bounded chunks and enforces MAX_HTTP_RESPONSE_SIZE. +// The completionHandler-based NSURLSession APIs fully materialize the response body +// as an NSData before handing it over, so an attacker-controlled collector could force +// a large allocation. This delegate instead accumulates data incrementally in +// didReceiveData: and cancels the transfer as soon as the cap would be exceeded, so no +// more than the cap is ever buffered. Delegate callbacks may arrive on the session's +// delegate queue while a request thread registers a task, so shared state is guarded. +@interface MATStreamingSessionDelegate : NSObject +- (void)registerTask:(NSURLSessionTask*)task + handler:(void (^)(NSData* data, NSURLResponse* response, NSError* error))handler; +@end + +@implementation MATStreamingSessionDelegate { + NSMutableDictionary* _buffers; + NSMutableDictionary* _handlers; + NSMutableSet* _overCap; +} + +- (instancetype)init +{ + self = [super init]; + if (self) + { + _buffers = [NSMutableDictionary new]; + _handlers = [NSMutableDictionary new]; + _overCap = [NSMutableSet new]; + } + return self; +} + +- (void)registerTask:(NSURLSessionTask*)task + handler:(void (^)(NSData*, NSURLResponse*, NSError*))handler +{ + NSNumber* key = @(task.taskIdentifier); + @synchronized(self) + { + _buffers[key] = [NSMutableData new]; + _handlers[key] = [handler copy]; + } +} + +- (void)URLSession:(NSURLSession*)session + dataTask:(NSURLSessionDataTask*)dataTask + didReceiveData:(NSData*)data +{ + NSNumber* key = @(dataTask.taskIdentifier); + @synchronized(self) + { + if ([_overCap containsObject:key]) + { + return; + } + NSMutableData* buffer = _buffers[key]; + if (buffer == nil) + { + return; + } + if (buffer.length + data.length > MAT::MAX_HTTP_RESPONSE_SIZE) + { + // Refuse the over-large response: stop buffering and cancel the transfer. + [_overCap addObject:key]; + [dataTask cancel]; + return; + } + [buffer appendData:data]; + } +} + +- (void)URLSession:(NSURLSession*)session + task:(NSURLSessionTask*)task +didCompleteWithError:(NSError*)error +{ + NSNumber* key = @(task.taskIdentifier); + void (^handler)(NSData*, NSURLResponse*, NSError*) = nil; + NSData* body = nil; + BOOL overCap = NO; + @synchronized(self) + { + handler = _handlers[key]; + body = _buffers[key]; + overCap = [_overCap containsObject:key]; + [_handlers removeObjectForKey:key]; + [_buffers removeObjectForKey:key]; + [_overCap removeObjectForKey:key]; + } + if (handler == nil) + { + return; + } + if (overCap) + { + // Surface a non-cancellation error so the request maps to NetworkFailure + // (retried), not Aborted (which is reserved for caller-initiated cancels). + NSError* capError = [NSError errorWithDomain:@"MATResponseCap" + code:-1 + userInfo:@{ NSLocalizedDescriptionKey : @"HTTP response exceeds max buffered size" }]; + handler(nil, task.response, capError); + } + else + { + handler(body, task.response, error); + } +} +@end + namespace MAT_NS_BEGIN { static std::string NextReqId() @@ -31,6 +136,7 @@ static dispatch_once_t once; static NSURLSession* session; +static MATStreamingSessionDelegate* sessionDelegate; class HttpRequestApple : public SimpleHttpRequest { @@ -42,7 +148,10 @@ m_parent->Add(static_cast(this)); dispatch_once(&once, ^{ NSURLSessionConfiguration* sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration]; - session = [NSURLSession sessionWithConfiguration:sessionConfig]; + sessionDelegate = [MATStreamingSessionDelegate new]; + session = [NSURLSession sessionWithConfiguration:sessionConfig + delegate:sessionDelegate + delegateQueue:nil]; }); } @@ -75,15 +184,18 @@ void SendAsync(IHttpResponseCallback* callback) if(equalsIgnoreCase(m_method, "get")) { [m_urlRequest setHTTPMethod:@"GET"]; - m_dataTask = [session dataTaskWithRequest:m_urlRequest completionHandler:m_completionMethod]; + m_dataTask = [session dataTaskWithRequest:m_urlRequest]; } else { [m_urlRequest setHTTPMethod:@"POST"]; NSData* postData = [NSData dataWithBytes:m_body.data() length:m_body.size()]; - m_dataTask = [session uploadTaskWithRequest:m_urlRequest fromData:postData completionHandler:m_completionMethod]; + m_dataTask = [session uploadTaskWithRequest:m_urlRequest fromData:postData]; } + // Register before resume so the streaming delegate has the buffer and + // completion handler in place before any response data arrives. + [sessionDelegate registerTask:m_dataTask handler:m_completionMethod]; [m_dataTask resume]; } } @@ -120,24 +232,17 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error) } else { - // 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. Reported as a network failure (retried). - if (data.length > MAX_HTTP_RESPONSE_SIZE) - { - LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); - simpleResponse->m_result = HttpResult_NetworkFailure; - } - else + // The streaming delegate has already enforced MAX_HTTP_RESPONSE_SIZE + // (an over-cap response arrives here as a cap error, handled above), so + // data is bounded. Guard against a nil/empty body to avoid pointer + // arithmetic on a null [data bytes]. + simpleResponse->m_result = HttpResult_OK; + const size_t length = static_cast(data.length); + if (length > 0) { - simpleResponse->m_result = HttpResult_OK; - const size_t length = static_cast(data.length); - if (length > 0) - { - auto body = static_cast([data bytes]); - simpleResponse->m_body.reserve(length); - std::copy(body, body + length, std::back_inserter(simpleResponse->m_body)); - } + auto body = static_cast([data bytes]); + simpleResponse->m_body.reserve(length); + std::copy(body, body + length, std::back_inserter(simpleResponse->m_body)); } } m_callback->OnHttpResponse(simpleResponse); diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index cdf816fa2..b1d3b4013 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -324,36 +324,41 @@ class WinInetRequestWrapper // It might potentially be another async operation which will // trigger INTERNET_STATUS_REQUEST_COMPLETE again. - m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed); - while (!m_readingData || m_bufferUsed != 0) { - // SECURITY: refuse an over-large response instead of buffering it (see - // MAX_HTTP_RESPONSE_SIZE). Each chunk is small, so checking here bounds - // the buffer to within one chunk of the cap. Reported as an invalid - // server response -> NetworkFailure, so the upload is retried. - if (m_bodyBuffer.size() > MAX_HTTP_RESPONSE_SIZE) { - LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); - dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE; - break; - } - BOOL bResult = ::InternetReadFile(m_hWinInetRequest, m_buffer, sizeof(m_buffer), &m_bufferUsed); - m_readingData = true; - if (!bResult) { - dwError = GetLastError(); - if (dwError == ERROR_IO_PENDING) { - // Do not touch anything from this thread anymore. - // The buffer passed to InternetReadFile() and the - // read count will be filled asynchronously, so they - // must stay valid and writable until the next - // INTERNET_STATUS_REQUEST_COMPLETE callback comes - // (that's why those are member variables). - LOG_TRACE("InternetReadFile() failed: ERROR_IO_PENDING. Waiting for INTERNET_STATUS_REQUEST_COMPLETE to be called again"); - 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 append so the buffer never exceeds + // the cap; reported as an invalid server response -> NetworkFailure (retried). + if (m_bodyBuffer.size() + m_bufferUsed > MAX_HTTP_RESPONSE_SIZE) { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE; + } else { + m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed); + while (!m_readingData || m_bufferUsed != 0) { + BOOL bResult = ::InternetReadFile(m_hWinInetRequest, m_buffer, sizeof(m_buffer), &m_bufferUsed); + m_readingData = true; + if (!bResult) { + dwError = GetLastError(); + if (dwError == ERROR_IO_PENDING) { + // Do not touch anything from this thread anymore. + // The buffer passed to InternetReadFile() and the + // read count will be filled asynchronously, so they + // must stay valid and writable until the next + // INTERNET_STATUS_REQUEST_COMPLETE callback comes + // (that's why those are member variables). + LOG_TRACE("InternetReadFile() failed: ERROR_IO_PENDING. Waiting for INTERNET_STATUS_REQUEST_COMPLETE to be called again"); + return; + } + LOG_WARN("InternetReadFile() failed: %d", dwError); + break; } - LOG_WARN("InternetReadFile() failed: %d", dwError); - break; - } - m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed); + if (m_bodyBuffer.size() + m_bufferUsed > MAX_HTTP_RESPONSE_SIZE) { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE; + break; + } + m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed); + } } } diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index 0eaaaca30..dbdbea75e 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/lib/http/HttpClient_WinRt.cpp @@ -144,7 +144,7 @@ namespace MAT_NS_BEGIN { void SendHttpAsyncRequest(HttpRequestMessage ^req) { - IAsyncOperationWithProgress^ operation = m_parent.getHttpClient()->SendRequestAsync(req, HttpCompletionOption::ResponseContentRead); + IAsyncOperationWithProgress^ operation = m_parent.getHttpClient()->SendRequestAsync(req, HttpCompletionOption::ResponseHeadersRead); m_cancellationTokenSource = cancellation_token_source(); create_task(operation, m_cancellationTokenSource.get_token()). @@ -202,44 +202,75 @@ namespace MAT_NS_BEGIN { index++; } - auto operation = m_httpResponseMessage->Content->ReadAsBufferAsync(); - auto task = create_task(operation); - if (task.wait() == task_status::completed) + // Read content headers before streaming the body. + IMapView^ contentHeadersView = m_httpResponseMessage->Content->Headers->GetView(); + auto contentHeadersiterator = contentHeadersView->First(); + unsigned int contentHeadersIndex = 0; + while (contentHeadersIndex < contentHeadersView->Size) { - IMapView^ contentHeadersView = m_httpResponseMessage->Content->Headers->GetView(); + String^ Key = contentHeadersiterator->Current->Key; + String^ Value = contentHeadersiterator->Current->Value; - auto contentHeadersiterator = contentHeadersView->First(); - unsigned int contentHeadersIndex = 0; - while (contentHeadersIndex < contentHeadersView->Size) - { - String^ Key = contentHeadersiterator->Current->Key; - String^ Value = contentHeadersiterator->Current->Value; - - response->m_headers.add(from_platform_string(Key), from_platform_string(Value)); - contentHeadersiterator->MoveNext(); - contentHeadersIndex++; - } - - auto buffer = task.get(); - size_t length = buffer->Length; + response->m_headers.add(from_platform_string(Key), from_platform_string(Value)); + contentHeadersiterator->MoveNext(); + contentHeadersIndex++; + } - // 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. The request is reported as a network failure (retried). - if (length > MAX_HTTP_RESPONSE_SIZE) + // SECURITY: stream the body in bounded chunks and enforce + // MAX_HTTP_RESPONSE_SIZE. SendRequestAsync uses ResponseHeadersRead, so + // the framework does not pre-buffer the whole body; reading it here in + // chunks ensures an oversized response is never fully materialized in + // memory (a hostile/MITM'd collector cannot exhaust process memory). + IInputStream^ inputStream = nullptr; + { + auto streamOp = m_httpResponseMessage->Content->ReadAsInputStreamAsync(); + auto streamTask = create_task(streamOp, m_cancellationTokenSource.get_token()); + if (streamTask.wait() == task_status::completed) { - LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); - response->m_result = HttpResult_NetworkFailure; + inputStream = streamTask.get(); } - else if (length > 0) + } + + if (inputStream == nullptr) + { + response->m_result = HttpResult_NetworkFailure; + } + else + { + const unsigned int chunkSize = 64 * 1024; + for (;;) { - response->m_body.reserve(length); - response->m_body.resize(length); - DataReader^ dataReader = DataReader::FromBuffer(buffer); - dataReader->ReadBytes((Platform::ArrayReference(reinterpret_cast(response->m_body.data()), (DWORD)length))); + Buffer^ chunk = ref new Buffer(chunkSize); + auto readOp = inputStream->ReadAsync(chunk, chunkSize, InputStreamOptions::Partial); + auto readTask = create_task(readOp, m_cancellationTokenSource.get_token()); + if (readTask.wait() != task_status::completed) + { + response->m_result = HttpResult_NetworkFailure; + break; + } + + IBuffer^ readBuffer = readTask.get(); + unsigned int readLength = (readBuffer != nullptr) ? readBuffer->Length : 0; + if (readLength == 0) + { + break; // end of stream + } + + if (response->m_body.size() + readLength > MAX_HTTP_RESPONSE_SIZE) + { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + response->m_result = HttpResult_NetworkFailure; + break; + } + + const size_t oldSize = response->m_body.size(); + response->m_body.resize(oldSize + readLength); + DataReader^ dataReader = DataReader::FromBuffer(readBuffer); + dataReader->ReadBytes((Platform::ArrayReference(reinterpret_cast(response->m_body.data() + oldSize), readLength))); dataReader->DetachBuffer(); delete dataReader; } + delete inputStream; } } else From 0d61e8f577abed23a2ec85c30df65ce973aa6b26 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 14 Jul 2026 22:26:42 -0500 Subject: [PATCH 4/6] Harden streaming transports: guard WinRt read faults, fix Apple block cast (round 3) Address Copilot review round 3 on the streaming rework: - WinRt: concurrency::task::wait()/get() rethrow if ReadAsInputStreamAsync or a chunk ReadAsync faults (e.g., connection reset) even when the status looks completed. Wrap the whole streamed-read in try/catch so a fault maps to HttpResult_NetworkFailure instead of escaping onRequestComplete and crashing. - Apple: cast the dictionary value (stored as id) back to the concrete block type in didCompleteWithError: to avoid an incompatible-pointer-types warning (which fails builds under -Werror). Files: - lib/http/HttpClient_WinRt.cpp - lib/http/HttpClient_Apple.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Apple.mm | 2 +- lib/http/HttpClient_WinRt.cpp | 97 ++++++++++++++++++++--------------- 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index e1fcff5ec..227a57f7a 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/lib/http/HttpClient_Apple.mm @@ -93,7 +93,7 @@ - (void)URLSession:(NSURLSession*)session BOOL overCap = NO; @synchronized(self) { - handler = _handlers[key]; + handler = (void (^)(NSData*, NSURLResponse*, NSError*))_handlers[key]; body = _buffers[key]; overCap = [_overCap containsObject:key]; [_handlers removeObjectForKey:key]; diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index dbdbea75e..b37e986cd 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/lib/http/HttpClient_WinRt.cpp @@ -221,56 +221,69 @@ namespace MAT_NS_BEGIN { // the framework does not pre-buffer the whole body; reading it here in // chunks ensures an oversized response is never fully materialized in // memory (a hostile/MITM'd collector cannot exhaust process memory). - IInputStream^ inputStream = nullptr; - { - auto streamOp = m_httpResponseMessage->Content->ReadAsInputStreamAsync(); - auto streamTask = create_task(streamOp, m_cancellationTokenSource.get_token()); - if (streamTask.wait() == task_status::completed) - { - inputStream = streamTask.get(); - } - } - - if (inputStream == nullptr) - { - response->m_result = HttpResult_NetworkFailure; - } - else + // task::wait()/get() rethrow if a read faults, so guard the whole stream. + try { - const unsigned int chunkSize = 64 * 1024; - for (;;) + IInputStream^ inputStream = nullptr; { - Buffer^ chunk = ref new Buffer(chunkSize); - auto readOp = inputStream->ReadAsync(chunk, chunkSize, InputStreamOptions::Partial); - auto readTask = create_task(readOp, m_cancellationTokenSource.get_token()); - if (readTask.wait() != task_status::completed) - { - response->m_result = HttpResult_NetworkFailure; - break; - } - - IBuffer^ readBuffer = readTask.get(); - unsigned int readLength = (readBuffer != nullptr) ? readBuffer->Length : 0; - if (readLength == 0) + auto streamOp = m_httpResponseMessage->Content->ReadAsInputStreamAsync(); + auto streamTask = create_task(streamOp, m_cancellationTokenSource.get_token()); + if (streamTask.wait() == task_status::completed) { - break; // end of stream + inputStream = streamTask.get(); } + } - if (response->m_body.size() + readLength > MAX_HTTP_RESPONSE_SIZE) + if (inputStream == nullptr) + { + response->m_result = HttpResult_NetworkFailure; + } + else + { + const unsigned int chunkSize = 64 * 1024; + for (;;) { - LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); - response->m_result = HttpResult_NetworkFailure; - break; + Buffer^ chunk = ref new Buffer(chunkSize); + auto readOp = inputStream->ReadAsync(chunk, chunkSize, InputStreamOptions::Partial); + auto readTask = create_task(readOp, m_cancellationTokenSource.get_token()); + if (readTask.wait() != task_status::completed) + { + response->m_result = HttpResult_NetworkFailure; + break; + } + + IBuffer^ readBuffer = readTask.get(); + unsigned int readLength = (readBuffer != nullptr) ? readBuffer->Length : 0; + if (readLength == 0) + { + break; // end of stream + } + + if (response->m_body.size() + readLength > MAX_HTTP_RESPONSE_SIZE) + { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + response->m_result = HttpResult_NetworkFailure; + break; + } + + const size_t oldSize = response->m_body.size(); + response->m_body.resize(oldSize + readLength); + DataReader^ dataReader = DataReader::FromBuffer(readBuffer); + dataReader->ReadBytes((Platform::ArrayReference(reinterpret_cast(response->m_body.data() + oldSize), readLength))); + dataReader->DetachBuffer(); + delete dataReader; } - - const size_t oldSize = response->m_body.size(); - response->m_body.resize(oldSize + readLength); - DataReader^ dataReader = DataReader::FromBuffer(readBuffer); - dataReader->ReadBytes((Platform::ArrayReference(reinterpret_cast(response->m_body.data() + oldSize), readLength))); - dataReader->DetachBuffer(); - delete dataReader; + delete inputStream; } - delete inputStream; + } + catch (Platform::Exception^ ex) + { + LOG_WARN("Reading HTTP response body failed: 0x%08x", ex->HResult); + response->m_result = HttpResult_NetworkFailure; + } + catch (...) + { + response->m_result = HttpResult_NetworkFailure; } } else From 055d4f4ec514c114f15fb2b505cb5314018fa187 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 14 Jul 2026 22:42:37 -0500 Subject: [PATCH 5/6] WinRt: map cancel to Aborted and clear partial body on rejection (round 4) Address Copilot review round 4: HttpResponseDecoder processes any non-empty response body regardless of HttpResult (processBody runs when GetBody() is non-empty), so a partial body left on a rejected streamed response could be parsed for kill-switch/stats. In the WinRt streaming reader: - Map a caller-initiated cancellation (task_status::canceled, from cancel()) to HttpResult_Aborted instead of NetworkFailure. - Clear response->m_body on every non-success path (cancel, read failure, over-cap, and streaming exceptions) so no partial body is processed. (WinInet and Apple never attach a partial body to the response on rejection, so they need no change.) Files: - lib/http/HttpClient_WinRt.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_WinRt.cpp | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index b37e986cd..1efc1bb22 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/lib/http/HttpClient_WinRt.cpp @@ -228,17 +228,19 @@ namespace MAT_NS_BEGIN { { auto streamOp = m_httpResponseMessage->Content->ReadAsInputStreamAsync(); auto streamTask = create_task(streamOp, m_cancellationTokenSource.get_token()); - if (streamTask.wait() == task_status::completed) + auto status = streamTask.wait(); + if (status == task_status::completed) { inputStream = streamTask.get(); } + else + { + // Caller-initiated cancel maps to Aborted; anything else is a failure. + response->m_result = (status == task_status::canceled) ? HttpResult_Aborted : HttpResult_NetworkFailure; + } } - if (inputStream == nullptr) - { - response->m_result = HttpResult_NetworkFailure; - } - else + if (inputStream != nullptr) { const unsigned int chunkSize = 64 * 1024; for (;;) @@ -246,9 +248,12 @@ namespace MAT_NS_BEGIN { Buffer^ chunk = ref new Buffer(chunkSize); auto readOp = inputStream->ReadAsync(chunk, chunkSize, InputStreamOptions::Partial); auto readTask = create_task(readOp, m_cancellationTokenSource.get_token()); - if (readTask.wait() != task_status::completed) + auto status = readTask.wait(); + if (status != task_status::completed) { - response->m_result = HttpResult_NetworkFailure; + // Drop any partial body; caller cancel -> Aborted, else failure. + response->m_result = (status == task_status::canceled) ? HttpResult_Aborted : HttpResult_NetworkFailure; + response->m_body.clear(); break; } @@ -263,6 +268,7 @@ namespace MAT_NS_BEGIN { { LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); response->m_result = HttpResult_NetworkFailure; + response->m_body.clear(); break; } @@ -278,12 +284,15 @@ namespace MAT_NS_BEGIN { } catch (Platform::Exception^ ex) { + // A faulted read rethrows here; drop any partial body and fail the request. LOG_WARN("Reading HTTP response body failed: 0x%08x", ex->HResult); response->m_result = HttpResult_NetworkFailure; + response->m_body.clear(); } catch (...) { response->m_result = HttpResult_NetworkFailure; + response->m_body.clear(); } } else From 3744617993b1bd3f8abe9fb44453403274274157 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Wed, 15 Jul 2026 02:02:31 -0500 Subject: [PATCH 6/6] Fix Apple response-cap delegate cleanup: NSMutableSet uses removeObject: The streaming session delegate's didCompleteWithError: cleanup called [_overCap removeObjectForKey:key], but _overCap is an NSMutableSet, which has no removeObjectForKey: selector (that belongs to NSMutableDictionary). This was a copy-paste from the _handlers/_buffers dictionary cleanup two lines above and fails to compile, breaking the entire Apple/macOS mat build. Use the correct NSMutableSet selector, removeObject:. Validation (macOS arm64, Apple HTTP transport): - libmat builds clean; full host UnitTests 518/518 pass. - End-to-end test against a local HttpServer through the real HttpClient_Apple: under-cap (64 KB) -> HttpResult_OK with full body; over-cap (16 MB + 1 MB) -> HttpResult_NetworkFailure with an empty body and no crash; exactly MAX_HTTP_RESPONSE_SIZE (16 MB) -> HttpResult_OK with full body. Stable over 8 repeats (no delegate state races). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Apple.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index 227a57f7a..b7d6646a4 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/lib/http/HttpClient_Apple.mm @@ -98,7 +98,7 @@ - (void)URLSession:(NSURLSession*)session overCap = [_overCap containsObject:key]; [_handlers removeObjectForKey:key]; [_buffers removeObjectForKey:key]; - [_overCap removeObjectForKey:key]; + [_overCap removeObject:key]; } if (handler == nil) {