From 37f908408cf6fb8bbe0d5099741e46bf4e1979b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87=20=28via=20Copilot=29?= <223556219+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 10:30:06 -0700 Subject: [PATCH 1/3] Don't crash on missing local files on Unix and Apple Two pre-existing bugs caused a missing local `app:///` (or `file://`) target to crash the host process instead of producing a clean failure on the URL request: - **Unix** (`UrlRequest_Unix.cpp`): the worker `std::thread` calls `curl_check(curl_easy_perform(m_curl))`. `curl_check` throws `std::runtime_error` on any non-OK libcurl code -- including `CURLE_FILE_COULDNT_READ_FILE` (37) for a missing local file. The uncaught exception in the `std::thread` callable calls `std::terminate`. Wrap the libcurl invocation in try/catch and retain the default status code of 0 on failure. - **Apple** (`UrlRequest_Apple.mm`): `Open` throws synchronously with `"No file exists at local path"` when `[NSBundle pathForResource:]` returns nil for an `app:///` URL. This propagates back to the JS caller through `xhr.open()` rather than going through the normal async failure path. Set `m_url = nil` and let `SendAsync`'s existing `if (m_url == nil)` branch produce the same `status=0` + `loadend` outcome as the rest of the platforms. Both fixes now match the convention already in use by `UrlRequest_UWP.cpp` (`catch (winrt::hresult_error)` -> retain status 0) and `UrlRequest_Win32.cpp` (HTTP path returns `task_from_result<>()` on non-success status). Discovered while investigating https://github.com/BabylonJS/JsRuntimeHost/pull/165 -- with that XHR fix in place, JS-side observers will now reliably receive an `error` event on missing local files instead of seeing the host crash. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Source/UrlRequest_Apple.mm | 7 ++++++- Source/UrlRequest_Unix.cpp | 30 +++++++++++++++++++++--------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/Source/UrlRequest_Apple.mm b/Source/UrlRequest_Apple.mm index 22c8912..3c95b30 100644 --- a/Source/UrlRequest_Apple.mm +++ b/Source/UrlRequest_Apple.mm @@ -36,7 +36,12 @@ void Open(UrlMethod method, const std::string& url) NSString* path = [[NSBundle mainBundle] pathForResource:[m_url.path substringFromIndex:1] ofType:nil]; if (path == nil) { - throw std::runtime_error{"No file exists at local path"}; + // No bundled resource at this path. Don't throw -- let SendAsync's existing + // `if (m_url == nil)` branch complete the task and retain the default status + // code of 0 to indicate a client side error. This matches Win32 / UWP / Unix + // semantics for missing local files. + m_url = nil; + return; } m_url = [NSURL fileURLWithPath:path]; } diff --git a/Source/UrlRequest_Unix.cpp b/Source/UrlRequest_Unix.cpp index d0a0cb1..82f871a 100644 --- a/Source/UrlRequest_Unix.cpp +++ b/Source/UrlRequest_Unix.cpp @@ -178,18 +178,30 @@ namespace UrlLib m_thread.emplace([this, taskCompletionSource]() mutable { - curl_check(curl_easy_perform(m_curl)); - - long codep{}; - curl_check(curl_easy_getinfo(m_curl, CURLINFO_RESPONSE_CODE, &codep)); - if (codep == 0 && m_file) + try { - // File scheme always returns 0 - m_statusCode = UrlStatusCode::Ok; + curl_check(curl_easy_perform(m_curl)); + + long codep{}; + curl_check(curl_easy_getinfo(m_curl, CURLINFO_RESPONSE_CODE, &codep)); + if (codep == 0 && m_file) + { + // File scheme always returns 0 + m_statusCode = UrlStatusCode::Ok; + } + else + { + m_statusCode = static_cast(codep); + } } - else + catch (const std::exception&) { - m_statusCode = static_cast(codep); + // Retain the default status code of 0 to indicate a client side error, + // matching the convention of UrlRequest_UWP.cpp's catch(winrt::hresult_error) + // and UrlRequest_Apple.mm's `m_url == nil` branch. Without this catch any + // libcurl failure (e.g. CURLE_FILE_COULDNT_READ_FILE (37) for a missing local + // `file://` or rewritten `app:///` target) would escape this std::thread and + // call std::terminate. } taskCompletionSource.complete(); From d42a5098b1120ec26c5d15844398535b5b520f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87=20=28via=20Copilot=29?= <223556219+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 16:37:41 -0700 Subject: [PATCH 2/3] Reset m_statusCode at the start of Open() on Unix and Apple If a UrlRequest is reused (Open + Send + Open + Send) and the first request succeeded with a non-zero status, then the second request hits the new client-side-failure path (Unix worker exception, Apple missing `app:///` resource), the caller would observe the stale prior status instead of the intended `UrlStatusCode::None` (0). Reset the status at the start of `Open()` so client-side failures consistently report 0 regardless of prior history. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Source/UrlRequest_Apple.mm | 1 + Source/UrlRequest_Unix.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/Source/UrlRequest_Apple.mm b/Source/UrlRequest_Apple.mm index 3c95b30..e2a0b61 100644 --- a/Source/UrlRequest_Apple.mm +++ b/Source/UrlRequest_Apple.mm @@ -24,6 +24,7 @@ public: void Open(UrlMethod method, const std::string& url) { + m_statusCode = UrlStatusCode::None; m_method = method; m_url = [NSURL URLWithString:[[NSString stringWithUTF8String:url.data()] stringByAddingPercentEncodingWithAllowedCharacters:URLAllowedCharacterSet]]; if (!m_url || !m_url.scheme) diff --git a/Source/UrlRequest_Unix.cpp b/Source/UrlRequest_Unix.cpp index 82f871a..1074d37 100644 --- a/Source/UrlRequest_Unix.cpp +++ b/Source/UrlRequest_Unix.cpp @@ -39,6 +39,7 @@ namespace UrlLib { Cleanup(); + m_statusCode = UrlStatusCode::None; m_method = method; if (m_method == UrlMethod::Post) From 2d4ece215224b47a6a7a5a33ca0b4065008ad675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87=20=28via=20Copilot=29?= <223556219+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 09:09:35 -0700 Subject: [PATCH 3/3] Hoist per-request response state reset into ImplBase Address @bghgary's cross-platform consistency concern on PR #28 (discussion_r3242603881). The previous commit reset only `m_statusCode`, and only on Unix and Apple -- creating a 2/5-platform 1/5-field inconsistency. Win32, UWP, and Android still relied on the implicit "construct a new `UrlRequest` per request" convention. Other per-request response state (`m_responseUrl`, `m_responseString`, `m_headers`, `m_responseBuffer`) was not reset on any platform. Add a protected `ImplBase::ResetForOpen()` helper that resets the response state living in the base class. Each platform's `Open()` calls it at the top, and additionally resets its own platform-specific response buffer (different types across backends: `std::vector` on Unix/Android, `NSData*` on Apple, `Storage::Streams::IBuffer` + `std::vector` on Win32/UWP). This makes `UrlRequest` properly reusable across all 5 platforms with consistent semantics: after `Open()` returns, the previous response is fully gone regardless of what happened before. Request-side caller-owned state (`m_requestBody`, `m_requestHeaders`) is intentionally not reset to preserve the existing pattern of setting headers/body between `Open()` and `SendAsync()`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Source/UrlRequest_Android.cpp | 3 +++ Source/UrlRequest_Apple.mm | 4 +++- Source/UrlRequest_Base.h | 13 +++++++++++++ Source/UrlRequest_Unix.cpp | 4 +++- Source/UrlRequest_Windows_Shared.h | 4 ++++ 5 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Source/UrlRequest_Android.cpp b/Source/UrlRequest_Android.cpp index cabf3c1..8f97d95 100644 --- a/Source/UrlRequest_Android.cpp +++ b/Source/UrlRequest_Android.cpp @@ -34,6 +34,9 @@ namespace UrlLib public: void Open(UrlMethod method, const std::string& url) { + ResetForOpen(); + m_responseBuffer.clear(); + m_method = method; Uri uri{Uri::Parse(url.data())}; // If the URL string doesn't contain a scheme, the URI object's scheme will be null. We throw in this case diff --git a/Source/UrlRequest_Apple.mm b/Source/UrlRequest_Apple.mm index e2a0b61..31b09e1 100644 --- a/Source/UrlRequest_Apple.mm +++ b/Source/UrlRequest_Apple.mm @@ -24,7 +24,9 @@ public: void Open(UrlMethod method, const std::string& url) { - m_statusCode = UrlStatusCode::None; + ResetForOpen(); + m_responseBuffer = nil; + m_method = method; m_url = [NSURL URLWithString:[[NSString stringWithUTF8String:url.data()] stringByAddingPercentEncodingWithAllowedCharacters:URLAllowedCharacterSet]]; if (!m_url || !m_url.scheme) diff --git a/Source/UrlRequest_Base.h b/Source/UrlRequest_Base.h index c8a32a0..266f300 100644 --- a/Source/UrlRequest_Base.h +++ b/Source/UrlRequest_Base.h @@ -84,6 +84,19 @@ namespace UrlLib std::transform(s.cbegin(), s.cend(), s.begin(), [](auto c) { return static_cast(std::tolower(c)); }); } + // Reset the per-request response state that lives in ImplBase. Each platform's `Open()` + // calls this at the start so a single `UrlRequest` can be reused across requests without + // leaking prior status / URL / body / headers. Platform-specific response buffers live in + // each `Impl` (different types per backend) and must be reset by the platform's `Open()` + // alongside this call. + void ResetForOpen() + { + m_statusCode = UrlStatusCode::None; + m_responseUrl.clear(); + m_responseString.clear(); + m_headers.clear(); + } + arcana::cancellation_source m_cancellationSource{}; UrlResponseType m_responseType{UrlResponseType::String}; UrlMethod m_method{UrlMethod::Get}; diff --git a/Source/UrlRequest_Unix.cpp b/Source/UrlRequest_Unix.cpp index 1074d37..64b9074 100644 --- a/Source/UrlRequest_Unix.cpp +++ b/Source/UrlRequest_Unix.cpp @@ -39,7 +39,9 @@ namespace UrlLib { Cleanup(); - m_statusCode = UrlStatusCode::None; + ResetForOpen(); + m_responseBuffer.clear(); + m_method = method; if (m_method == UrlMethod::Post) diff --git a/Source/UrlRequest_Windows_Shared.h b/Source/UrlRequest_Windows_Shared.h index 1e910f5..6faefb5 100644 --- a/Source/UrlRequest_Windows_Shared.h +++ b/Source/UrlRequest_Windows_Shared.h @@ -46,6 +46,10 @@ namespace UrlLib public: void Open(UrlMethod method, const std::string& url) { + ResetForOpen(); + m_responseBuffer = nullptr; + m_fileResponseBuffer.clear(); + m_method = method; m_uri = Foundation::Uri{winrt::to_hstring(url)}; }