From faf4902009d81a164395f115710080fcaa212b38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Tue, 29 Sep 2026 11:41:29 -0700 Subject: [PATCH 1/7] Report XHR URL-open failures asynchronously from send Preserve synchronous method and body validation, expose the original transport-opening error, and discard deferred failures after reopen or abort. Cover event order, duplicate sends, recovery, and cancellation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- Polyfills/XMLHttpRequest/Readme.md | 8 ++ .../XMLHttpRequest/Source/XMLHttpRequest.cpp | 58 +++++++++- .../XMLHttpRequest/Source/XMLHttpRequest.h | 5 + .../Source/Scripts/tests.xmlHttpRequest.ts | 105 ++++++++++++++++-- 4 files changed, 160 insertions(+), 16 deletions(-) diff --git a/Polyfills/XMLHttpRequest/Readme.md b/Polyfills/XMLHttpRequest/Readme.md index ea00d006..f071ffad 100644 --- a/Polyfills/XMLHttpRequest/Readme.md +++ b/Polyfills/XMLHttpRequest/Readme.md @@ -15,6 +15,14 @@ Unlike the web, XMLHttpRequest supports loading local files using two schemes: ## Other things to be aware of: * Only `GET` requests are currently supported * For `readyState`, we only support `UNSENT`, `OPENED`, and `DONE` +* If the platform transport rejects a URL during `open()`, the request still + enters `OPENED`. Calling `send()` reports `DONE`, `error`, and `loadend` + asynchronously, with `status === 0`. This lets asset loaders handle unsupported + or scheme-less Native URLs through their error callbacks rather than aborting + scene parsing. No document-relative URL resolution is added. +* Invalid methods, arguments, and unsupported request-body types still throw + synchronously. A deferred URL-open failure exposes `errorCode === "UrlOpenFailed"` + and the original error in `errorDetail`; reopening clears those diagnostics. ## Transport-error diagnostics (non-standard) A transport-level failure surfaces the standard way -- an `error` event followed by `loadend`, diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp index bac1f90f..f79b6b4c 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp @@ -179,14 +179,14 @@ namespace Babylon::Polyfills::Internal { // Stable symbolic token for a transport failure (e.g. "CURLE_COULDNT_CONNECT", // "NSURLErrorTimedOut", "AppResourceNotFound"); empty when there was no transport failure. - return Napi::String::New(Env(), std::string{m_request.ErrorSymbol()}); + return Napi::String::New(Env(), m_openError ? "UrlOpenFailed" : std::string{m_request.ErrorSymbol()}); } Napi::Value XMLHttpRequest::GetErrorDetail(const Napi::CallbackInfo&) { // Full normalized ":(): " string; empty when there was no // transport failure. - return Napi::String::New(Env(), std::string{m_request.ErrorString()}); + return Napi::String::New(Env(), m_openError.value_or(std::string{m_request.ErrorString()})); } Napi::Value XMLHttpRequest::GetResponseHeader(const Napi::CallbackInfo& info) @@ -254,24 +254,38 @@ namespace Babylon::Polyfills::Internal void XMLHttpRequest::Abort(const Napi::CallbackInfo&) { + ++m_requestGeneration; m_request.Abort(); } void XMLHttpRequest::Open(const Napi::CallbackInfo& info) { + UrlLib::UrlMethod method; + try + { + method = MethodType::StringToEnum(info[0].As().Utf8Value()); + } + catch (const std::exception& e) + { + throw Napi::Error::New(info.Env(), e.what()); + } + m_url = info[1].As(); + ++m_requestGeneration; + m_openError.reset(); + m_openErrorSent = false; try { - m_request.Open(MethodType::StringToEnum(info[0].As().Utf8Value()), m_url); + m_request.Open(method, m_url); } catch (const std::exception& e) { - throw Napi::Error::New(info.Env(), std::string{"Error opening URL: "} + e.what()); + m_openError = std::string{"Error opening URL: "} + e.what(); } catch (...) { - throw Napi::Error::New(info.Env(), "Unknown error opening URL"); + m_openError = "Unknown error opening URL"; } SetReadyState(ReadyState::Opened); @@ -284,6 +298,11 @@ namespace Babylon::Polyfills::Internal throw Napi::Error::New(info.Env(), "XMLHttpRequest must be opened before it can be sent"); } + if (m_openErrorSent) + { + throw Napi::Error::New(info.Env(), "XMLHttpRequest has already been sent"); + } + if (info.Length() > 0) { if (!info[0].IsString() && !info[0].IsUndefined() && !info[0].IsNull()) @@ -297,6 +316,35 @@ namespace Babylon::Polyfills::Internal } } + if (m_openError) + { + m_openErrorSent = true; + auto anchor = std::make_shared(Napi::Persistent(info.This().As())); + arcana::make_task(m_runtimeScheduler, arcana::cancellation::none(), + [this, anchor{std::move(anchor)}, generation{m_requestGeneration}]() { + if (generation != m_requestGeneration) + { + return; + } + + // Match an asynchronous transport failure, unless a callback reopens or aborts the request. + m_readyState = ReadyState::Done; + for (const auto event : {EventType::ReadyStateChange, EventType::Error, EventType::LoadEnd}) + { + if (generation != m_requestGeneration) + { + return; + } + RaiseEvent(event); + } + if (generation == m_requestGeneration) + { + m_eventHandlerRefs.clear(); + } + }); + return; + } + std::string traceName = (std::ostringstream{} << "XMLHttpRequest::Send [" << m_url << "]").str(); auto sendRegion = std::make_optional(traceName.c_str()); diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h index 74d2c3b9..608e9e87 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h @@ -6,6 +6,8 @@ #include #include +#include +#include #include namespace Babylon::Polyfills::Internal @@ -49,6 +51,9 @@ namespace Babylon::Polyfills::Internal void RaiseEvent(const char* eventType); std::string m_url{}; + std::optional m_openError{}; + uint64_t m_requestGeneration{}; + bool m_openErrorSent{}; UrlLib::UrlRequest m_request{}; JsRuntimeScheduler m_runtimeScheduler; ReadyState m_readyState{ReadyState::Unsent}; diff --git a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts index ab50e950..13ef72a8 100644 --- a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts +++ b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts @@ -104,23 +104,106 @@ describe("XMLHTTPRequest", function () { expect(xhr.errorDetail).to.equal(""); }); - it("should throw something when opening //", async function () { - function openDoubleSlash() { - const xhr = new XMLHttpRequest(); - xhr.open("GET", "//"); + for (const url of ["//", "noscheme.glb"]) { + it(`should report a URL-open failure asynchronously for ${url}`, async function () { + this.timeout(5000); + const xhr = new XMLHttpRequest() as XMLHttpRequest & { errorCode: string; errorDetail: string }; + const events: string[] = []; + xhr.addEventListener("readystatechange", () => events.push(`state:${xhr.readyState}`)); + xhr.open("GET", url); + expect(xhr.readyState).to.equal(XMLHttpRequest.OPENED); + expect(events).to.deep.equal(["state:1"]); xhr.send(); - } - expect(openDoubleSlash).to.throw(); + expect(() => xhr.send()).to.throw(); + expect(events).to.deep.equal(["state:1"]); + await new Promise((resolve) => { + xhr.addEventListener("error", () => events.push("error")); + xhr.addEventListener("loadend", () => { + events.push("loadend"); + resolve(); + }); + }); + expect(events).to.deep.equal(["state:1", "state:4", "error", "loadend"]); + expect(xhr.status).to.equal(0); + expect(xhr.statusText).to.equal(""); + expect(xhr.responseText).to.equal(""); + expect(xhr.errorCode).to.equal("UrlOpenFailed"); + expect(xhr.errorDetail).to.contain("Error opening URL:"); + }); + } + + it("should still reject an unsupported method synchronously", function () { + const xhr = new XMLHttpRequest(); + expect(() => xhr.open("INVALID", "noscheme.glb")).to.throw(); + expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT); }); - it("should throw something when opening a url with no scheme", function () { - function openNoProtocol() { + it("should still reject an unsupported body after a URL-open failure", async function () { + this.timeout(5000); + const xhr = new XMLHttpRequest(); + xhr.open("GET", "noscheme.glb"); + expect(() => xhr.send(new Uint8Array(1))).to.throw(); + const completed = new Promise((resolve) => xhr.addEventListener("loadend", () => resolve())); + xhr.send(); + await completed; + expect(xhr.status).to.equal(0); + }); + + it("should discard a pending URL-open failure when reopened", async function () { + this.timeout(5000); + const xhr = new XMLHttpRequest() as XMLHttpRequest & { errorCode: string; errorDetail: string }; + let errors = 0; + xhr.addEventListener("error", () => errors++); + xhr.open("GET", "noscheme.glb"); + xhr.send(); + xhr.open("GET", "app:///Assets/symlink_target.js"); + expect(xhr.errorCode).to.equal(""); + expect(xhr.errorDetail).to.equal(""); + const completed = new Promise((resolve) => xhr.addEventListener("loadend", () => resolve())); + xhr.send(); + await completed; + expect(errors).to.equal(0); + expect(xhr.status).to.equal(200); + expect(xhr.responseText).to.equal("var symlink_target_js = true;"); + }); + + it("should cancel a pending URL-open failure when aborted", async function () { + this.timeout(5000); + const xhr = new XMLHttpRequest(); + const events: string[] = []; + xhr.addEventListener("error", () => events.push("error")); + xhr.addEventListener("loadend", () => events.push("loadend")); + xhr.open("GET", "noscheme.glb"); + xhr.send(); + xhr.abort(); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(events).to.deep.equal([]); + }); + + for (const event of ["readystatechange", "error"]) { + it(`should preserve a request reopened from the failure ${event} callback`, async function () { + this.timeout(5000); const xhr = new XMLHttpRequest(); + let reopened = false; + let errors = 0; + xhr.addEventListener("error", () => errors++); + xhr.addEventListener(event, () => { + if (!reopened && xhr.readyState === XMLHttpRequest.DONE) { + reopened = true; + xhr.open("GET", "app:///Assets/symlink_target.js"); + xhr.send(); + } + }); + const completed = new Promise((resolve) => xhr.addEventListener("loadend", () => resolve())); xhr.open("GET", "noscheme.glb"); xhr.send(); - } - expect(openNoProtocol).to.throw(); - }); + await completed; + expect(reopened).to.equal(true); + expect(errors).to.equal(event === "error" ? 1 : 0); + expect(xhr.status).to.equal(200); + expect(xhr.responseText).to.equal("var symlink_target_js = true;"); + }); + } it("should throw something when sending before opening", function () { function sendWithoutOpening() { From c1f8d7995c5d72bf6f4db322df08fffa30aad64a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Tue, 29 Sep 2026 14:24:16 -0700 Subject: [PATCH 2/7] Release listeners after canceled XHR URL-opening failures Clean up deferred failure listeners after active dispatch unwinds, while preserving listeners when open() starts another request. Cover cancellation before dispatch and from callbacks, and clarify opening-error diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- Polyfills/XMLHttpRequest/Readme.md | 14 ++++---- .../XMLHttpRequest/Source/XMLHttpRequest.cpp | 14 +++++--- .../XMLHttpRequest/Source/XMLHttpRequest.h | 1 + .../Source/Scripts/tests.xmlHttpRequest.ts | 32 +++++++++++++++++++ 4 files changed, 50 insertions(+), 11 deletions(-) diff --git a/Polyfills/XMLHttpRequest/Readme.md b/Polyfills/XMLHttpRequest/Readme.md index f071ffad..7ae0fa46 100644 --- a/Polyfills/XMLHttpRequest/Readme.md +++ b/Polyfills/XMLHttpRequest/Readme.md @@ -29,11 +29,13 @@ A transport-level failure surfaces the standard way -- an `error` event followed with `status === 0` -- exactly as on the web. In addition, two **non-standard, additive** read-only properties expose the normalized `UrlLib` transport-error detail so BN-aware code can tell a DNS failure from a refused connection or a missing local asset: -* `errorCode` -- the stable symbolic token (e.g. `"CURLE_COULDNT_CONNECT"`, `"NSURLErrorTimedOut"`, +* `errorCode` -- the stable symbolic token (e.g. `"UrlOpenFailed"`, `"CURLE_COULDNT_CONNECT"`, `"NSURLErrorTimedOut"`, `"AppResourceNotFound"`) -* `errorDetail` -- the full normalized `":(): "` string +* `errorDetail` -- the original opening error for `UrlOpenFailed`, or the normalized + `":(): "` string for a failure during `send()` -Both are empty strings unless the request failed at the transport layer, and are populated only -on backends that expose the detail (Apple, Linux) -- empty on Windows/Android until those -backends populate `UrlLib`'s accessors. Browsers do not expose these properties, so -spec-conformant code is unaffected. +URL-opening errors populate both properties on every platform. Failures during +`send()` expose the diagnostics supplied by the platform's `UrlLib` backend; +those strings can be empty when the backend has no detail. Successful requests +leave both properties empty, and reopening clears an earlier opening error. +Browsers do not expose these properties, so spec-conformant code is unaffected. diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp index f79b6b4c..51e34514 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -271,7 +272,7 @@ namespace Babylon::Polyfills::Internal } m_url = info[1].As(); - ++m_requestGeneration; + m_openGeneration = ++m_requestGeneration; m_openError.reset(); m_openErrorSent = false; @@ -322,6 +323,13 @@ namespace Babylon::Polyfills::Internal auto anchor = std::make_shared(Napi::Persistent(info.This().As())); arcana::make_task(m_runtimeScheduler, arcana::cancellation::none(), [this, anchor{std::move(anchor)}, generation{m_requestGeneration}]() { + // Release after dispatch unwinds, without clearing a reopened request's listeners. + const auto releaseListeners = gsl::finally([this, generation]() { + if (m_openGeneration == generation) + { + m_eventHandlerRefs.clear(); + } + }); if (generation != m_requestGeneration) { return; @@ -337,10 +345,6 @@ namespace Babylon::Polyfills::Internal } RaiseEvent(event); } - if (generation == m_requestGeneration) - { - m_eventHandlerRefs.clear(); - } }); return; } diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h index 608e9e87..66640a66 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h @@ -53,6 +53,7 @@ namespace Babylon::Polyfills::Internal std::string m_url{}; std::optional m_openError{}; uint64_t m_requestGeneration{}; + uint64_t m_openGeneration{}; bool m_openErrorSent{}; UrlLib::UrlRequest m_request{}; JsRuntimeScheduler m_runtimeScheduler; diff --git a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts index 13ef72a8..f21235fe 100644 --- a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts +++ b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts @@ -180,6 +180,38 @@ describe("XMLHTTPRequest", function () { expect(events).to.deep.equal([]); }); + for (const event of ["before dispatch", "readystatechange", "error"]) { + it(`should release canceled URL-open listeners after aborting ${event}`, async function () { + this.timeout(5000); + const xhr = new XMLHttpRequest(); + let callbacks = 0; + const listener = () => { + callbacks++; + if (event !== "before dispatch" && xhr.readyState === XMLHttpRequest.DONE) { + xhr.abort(); + } + }; + xhr.addEventListener("readystatechange", listener); + xhr.addEventListener("error", listener); + if (event === "error") { + xhr.removeEventListener("readystatechange", listener); + } + xhr.open("GET", "noscheme.glb"); + xhr.send(); + if (event === "before dispatch") { + xhr.abort(); + } + await new Promise((resolve) => setTimeout(resolve, 10)); + const beforeReopen = callbacks; + xhr.open("GET", "app:///Assets/symlink_target.js"); + const completed = new Promise((resolve) => xhr.addEventListener("loadend", () => resolve())); + xhr.send(); + await completed; + expect(xhr.readyState).to.equal(XMLHttpRequest.DONE); + expect(callbacks).to.equal(beforeReopen); + }); + } + for (const event of ["readystatechange", "error"]) { it(`should preserve a request reopened from the failure ${event} callback`, async function () { this.timeout(5000); From f63b489a8e55f08e22f0acddeaf1b323eac516ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Tue, 29 Sep 2026 14:58:22 -0700 Subject: [PATCH 3/7] Track open identity separately for deferred XHR listener cleanup Capture the open generation independently of the event cancellation generation so aborting before send cannot skip releasing the listeners. Add an abort-before-first-send regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp | 6 +++--- Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp index 51e34514..e56c6390 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp @@ -322,10 +322,10 @@ namespace Babylon::Polyfills::Internal m_openErrorSent = true; auto anchor = std::make_shared(Napi::Persistent(info.This().As())); arcana::make_task(m_runtimeScheduler, arcana::cancellation::none(), - [this, anchor{std::move(anchor)}, generation{m_requestGeneration}]() { + [this, anchor{std::move(anchor)}, generation{m_requestGeneration}, openGeneration{m_openGeneration}]() { // Release after dispatch unwinds, without clearing a reopened request's listeners. - const auto releaseListeners = gsl::finally([this, generation]() { - if (m_openGeneration == generation) + const auto releaseListeners = gsl::finally([this, openGeneration]() { + if (m_openGeneration == openGeneration) { m_eventHandlerRefs.clear(); } diff --git a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts index f21235fe..5a9596f9 100644 --- a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts +++ b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts @@ -180,14 +180,14 @@ describe("XMLHTTPRequest", function () { expect(events).to.deep.equal([]); }); - for (const event of ["before dispatch", "readystatechange", "error"]) { + for (const event of ["before send", "before dispatch", "readystatechange", "error"]) { it(`should release canceled URL-open listeners after aborting ${event}`, async function () { this.timeout(5000); const xhr = new XMLHttpRequest(); let callbacks = 0; const listener = () => { callbacks++; - if (event !== "before dispatch" && xhr.readyState === XMLHttpRequest.DONE) { + if ((event === "readystatechange" || event === "error") && xhr.readyState === XMLHttpRequest.DONE) { xhr.abort(); } }; @@ -197,6 +197,9 @@ describe("XMLHTTPRequest", function () { xhr.removeEventListener("readystatechange", listener); } xhr.open("GET", "noscheme.glb"); + if (event === "before send") { + xhr.abort(); + } xhr.send(); if (event === "before dispatch") { xhr.abort(); From 9a1616f67f14855c276e09dc1cb22d8c5be25d0c Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Tue, 29 Sep 2026 21:18:11 -0700 Subject: [PATCH 4/7] Reset deferred XHR state when abort cancels delivery Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305 --- Polyfills/XMLHttpRequest/Readme.md | 2 ++ Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp | 8 ++++++-- Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts | 5 +++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Polyfills/XMLHttpRequest/Readme.md b/Polyfills/XMLHttpRequest/Readme.md index 7ae0fa46..c53425d4 100644 --- a/Polyfills/XMLHttpRequest/Readme.md +++ b/Polyfills/XMLHttpRequest/Readme.md @@ -23,6 +23,8 @@ Unlike the web, XMLHttpRequest supports loading local files using two schemes: * Invalid methods, arguments, and unsupported request-body types still throw synchronously. A deferred URL-open failure exposes `errorCode === "UrlOpenFailed"` and the original error in `errorDetail`; reopening clears those diagnostics. + Aborting its pending notification returns `readyState` to `UNSENT` without + dispatching failure events; a new `open()` is required before another `send()`. ## Transport-error diagnostics (non-standard) A transport-level failure surfaces the standard way -- an `error` event followed by `loadend`, diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp index e56c6390..b8e41495 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp @@ -185,8 +185,8 @@ namespace Babylon::Polyfills::Internal Napi::Value XMLHttpRequest::GetErrorDetail(const Napi::CallbackInfo&) { - // Full normalized ":(): " string; empty when there was no - // transport failure. + // Original opening error for UrlOpenFailed, or normalized + // ":(): " for a send failure; empty on success. return Napi::String::New(Env(), m_openError.value_or(std::string{m_request.ErrorString()})); } @@ -257,6 +257,10 @@ namespace Babylon::Polyfills::Internal { ++m_requestGeneration; m_request.Abort(); + if (m_openErrorSent && m_readyState == ReadyState::Opened) + { + m_readyState = ReadyState::Unsent; + } } void XMLHttpRequest::Open(const Napi::CallbackInfo& info) diff --git a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts index 5a9596f9..7da072a7 100644 --- a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts +++ b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts @@ -176,8 +176,11 @@ describe("XMLHTTPRequest", function () { xhr.open("GET", "noscheme.glb"); xhr.send(); xhr.abort(); + expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT); + expect(() => xhr.send()).to.throw(); await new Promise((resolve) => setTimeout(resolve, 10)); expect(events).to.deep.equal([]); + expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT); }); for (const event of ["before send", "before dispatch", "readystatechange", "error"]) { @@ -203,6 +206,8 @@ describe("XMLHTTPRequest", function () { xhr.send(); if (event === "before dispatch") { xhr.abort(); + expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT); + expect(() => xhr.send()).to.throw(); } await new Promise((resolve) => setTimeout(resolve, 10)); const beforeReopen = callbacks; From 0727092425b5b2e3b17f36122abe41e5a1737db2 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: Tue, 13 Oct 2026 18:28:39 -0700 Subject: [PATCH 5/7] Throw SyntaxError for malformed XHR URLs; keep unsupported methods synchronous open() now validates the URL against a placeholder special-scheme base: strings the URL parser rejects (e.g. "//", "http://", an out-of-range port) throw a SyntaxError synchronously as the XHR spec requires, while scheme-less relative references such as "noscheme.glb" still reach the transport and are reported through the deferred error events. If the transport rejects a non-GET method, probe whether a GET for the same URL opens; if so the method was the cause and open() throws synchronously instead of reporting UrlOpenFailed (e.g. POST on Unix, which UrlLib rejects before parsing the URL). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c658bb82-1999-4035-840f-54a1df553faf --- Polyfills/XMLHttpRequest/Readme.md | 10 +- .../XMLHttpRequest/Source/XMLHttpRequest.cpp | 246 +++++++++++++++++- .../Source/Scripts/tests.xmlHttpRequest.ts | 42 ++- 3 files changed, 293 insertions(+), 5 deletions(-) diff --git a/Polyfills/XMLHttpRequest/Readme.md b/Polyfills/XMLHttpRequest/Readme.md index c53425d4..e6d25b2b 100644 --- a/Polyfills/XMLHttpRequest/Readme.md +++ b/Polyfills/XMLHttpRequest/Readme.md @@ -20,8 +20,14 @@ Unlike the web, XMLHttpRequest supports loading local files using two schemes: asynchronously, with `status === 0`. This lets asset loaders handle unsupported or scheme-less Native URLs through their error callbacks rather than aborting scene parsing. No document-relative URL resolution is added. -* Invalid methods, arguments, and unsupported request-body types still throw - synchronously. A deferred URL-open failure exposes `errorCode === "UrlOpenFailed"` +* A URL that cannot be parsed (for example `//`, `http://`, or an invalid port) + throws a `SyntaxError` from `open()`, as in the XHR specification. Parsing is + checked as if the URL were resolved against a placeholder `http` base, so + scheme-less references such as `model.glb` are not malformed; the string + is still passed to the transport unchanged. +* Invalid methods, methods the platform transport does not support (for + example `POST` on Linux), invalid arguments, and unsupported request-body types + still throw synchronously. A deferred URL-open failure exposes `errorCode === "UrlOpenFailed"` and the original error in `errorDetail`; reopening clears those diagnostics. Aborting its pending notification returns `readyState` to `UNSENT` without dispatching failure events; a new `open()` is required before another `send()`. diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp index b8e41495..220c8364 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp @@ -3,8 +3,12 @@ #include #include #include +#include +#include +#include #include #include +#include namespace Babylon::Polyfills::Internal { @@ -61,6 +65,226 @@ namespace Babylon::Polyfills::Internal constexpr const char* LoadEnd = "loadend"; constexpr const char* Error = "error"; } + + bool TryOpenAsGet(UrlLib::UrlRequest& request, const std::string& url) + { + try + { + request.Open(UrlLib::UrlMethod::Get, url); + return true; + } + catch (...) + { + return false; + } + } + + namespace UrlSyntax + { + bool IsAlpha(char c) + { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); + } + + bool IsDigit(char c) + { + return c >= '0' && c <= '9'; + } + + bool IsHexDigit(char c) + { + return IsDigit(c) || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'); + } + + bool IsSlash(char c, bool special) + { + return c == '/' || (special && c == '\\'); + } + + bool IsSpecialScheme(const std::string& scheme) + { + return scheme == "http" || scheme == "https" || scheme == "ws" || scheme == "wss" || scheme == "ftp"; + } + + // The authority ends at the first '/', '?' or '#' ('\\' also ends it for special schemes). + std::string_view ExtractAuthority(std::string_view value, bool special) + { + size_t end = 0; + while (end < value.size() && !IsSlash(value[end], special) && value[end] != '?' && value[end] != '#') + { + ++end; + } + return value.substr(0, end); + } + + bool IsHostValid(std::string_view host) + { + for (size_t i = 0; i < host.size(); ++i) + { + const unsigned char c = static_cast(host[i]); + if (c <= 0x20 || c == 0x7F || std::strchr("#/<>?@[\\]^|", c) != nullptr) + { + return false; + } + if (c == '%' && (i + 2 >= host.size() || !IsHexDigit(host[i + 1]) || !IsHexDigit(host[i + 2]))) + { + return false; + } + } + return true; + } + + bool IsPortValid(std::string_view port) + { + if (port.size() > 5) + { + return false; + } + + uint32_t value = 0; + for (const char c : port) + { + if (!IsDigit(c)) + { + return false; + } + value = value * 10 + static_cast(c - '0'); + } + return value <= 65535; + } + + bool IsAuthorityValid(std::string_view authority, bool special) + { + const auto userInfoEnd = authority.rfind('@'); + if (userInfoEnd != std::string_view::npos) + { + authority.remove_prefix(userInfoEnd + 1); + if (authority.empty()) + { + return false; + } + } + + std::string_view host{authority}; + std::string_view port{}; + if (!authority.empty() && authority.front() == '[') + { + const auto close = authority.find(']'); + if (close == std::string_view::npos) + { + return false; + } + + host = authority.substr(1, close - 1); + const auto rest = authority.substr(close + 1); + if (!rest.empty()) + { + if (rest.front() != ':') + { + return false; + } + port = rest.substr(1); + } + + if (host.empty() || host.find_first_not_of("0123456789abcdefABCDEF:.") != std::string_view::npos) + { + return false; + } + } + else + { + const auto colon = authority.find(':'); + if (colon != std::string_view::npos) + { + host = authority.substr(0, colon); + port = authority.substr(colon + 1); + } + + if (!IsHostValid(host)) + { + return false; + } + } + + return IsPortValid(port) && (!special || !host.empty()); + } + + // Approximates whether the WHATWG URL parser accepts the string when resolved against a + // placeholder special-scheme base (so scheme-less relative references like "model.glb" parse, + // while "//" or "http://" do not). It only classifies the input -- the string passed to the + // transport is never rewritten -- so no document-relative URL resolution is introduced. + bool IsParseable(const std::string& input) + { + size_t first = 0; + size_t last = input.size(); + while (first < last && static_cast(input[first]) <= 0x20) + { + ++first; + } + while (last > first && static_cast(input[last - 1]) <= 0x20) + { + --last; + } + + std::string url; + for (size_t i = first; i < last; ++i) + { + if (input[i] != '\t' && input[i] != '\n' && input[i] != '\r') + { + url.push_back(input[i]); + } + } + + std::string_view rest{url}; + size_t schemeEnd = 0; + if (!url.empty() && IsAlpha(url[0])) + { + schemeEnd = 1; + while (schemeEnd < url.size() && (IsAlpha(url[schemeEnd]) || IsDigit(url[schemeEnd]) || url[schemeEnd] == '+' || url[schemeEnd] == '-' || url[schemeEnd] == '.')) + { + ++schemeEnd; + } + } + + if (schemeEnd > 0 && schemeEnd < url.size() && url[schemeEnd] == ':') + { + std::string scheme{url, 0, schemeEnd}; + std::transform(scheme.begin(), scheme.end(), scheme.begin(), [](unsigned char c) { return static_cast(std::tolower(c)); }); + rest.remove_prefix(schemeEnd + 1); + + if (scheme == "file") + { + return true; + } + + if (IsSpecialScheme(scheme)) + { + while (!rest.empty() && IsSlash(rest.front(), true)) + { + rest.remove_prefix(1); + } + return IsAuthorityValid(ExtractAuthority(rest, true), true); + } + + if (rest.size() >= 2 && rest[0] == '/' && rest[1] == '/') + { + return IsAuthorityValid(ExtractAuthority(rest.substr(2), false), false); + } + return true; + } + + if (rest.size() >= 2 && IsSlash(rest[0], true) && IsSlash(rest[1], true)) + { + while (!rest.empty() && IsSlash(rest.front(), true)) + { + rest.remove_prefix(1); + } + return IsAuthorityValid(ExtractAuthority(rest, true), true); + } + + return true; + } + } } void XMLHttpRequest::Initialize(Napi::Env env) @@ -266,16 +490,26 @@ namespace Babylon::Polyfills::Internal void XMLHttpRequest::Open(const Napi::CallbackInfo& info) { UrlLib::UrlMethod method; + const auto methodName = info[0].As().Utf8Value(); try { - method = MethodType::StringToEnum(info[0].As().Utf8Value()); + method = MethodType::StringToEnum(methodName); } catch (const std::exception& e) { throw Napi::Error::New(info.Env(), e.what()); } - m_url = info[1].As(); + // Per the XHR spec, a URL that cannot be parsed fails synchronously with SyntaxError; only + // requests the transport later fails to open or fetch are reported through error events. + const auto url = info[1].As().Utf8Value(); + if (!UrlSyntax::IsParseable(url)) + { + const auto syntaxError = info.Env().Global().Get("SyntaxError").As().New({Napi::String::New(info.Env(), "Failed to parse URL '" + url + "'")}); + throw Napi::Error{info.Env(), syntaxError}; + } + + m_url = url; m_openGeneration = ++m_requestGeneration; m_openError.reset(); m_openErrorSent = false; @@ -286,6 +520,14 @@ namespace Babylon::Polyfills::Internal } catch (const std::exception& e) { + // Some transports reject a method before looking at the URL (e.g. POST on Unix). If a GET + // for the same URL opens, the method was the cause, which is an unsupported operation + // rather than a URL failure, so it stays synchronous. + if (method != UrlLib::UrlMethod::Get && TryOpenAsGet(m_request, m_url)) + { + throw Napi::Error::New(info.Env(), "Unsupported url method for this platform: " + methodName + " (" + e.what() + ")"); + } + m_openError = std::string{"Error opening URL: "} + e.what(); } catch (...) diff --git a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts index 7da072a7..c444262d 100644 --- a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts +++ b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts @@ -104,7 +104,7 @@ describe("XMLHTTPRequest", function () { expect(xhr.errorDetail).to.equal(""); }); - for (const url of ["//", "noscheme.glb"]) { + for (const url of ["noscheme.glb", "textures/albedo.png"]) { it(`should report a URL-open failure asynchronously for ${url}`, async function () { this.timeout(5000); const xhr = new XMLHttpRequest() as XMLHttpRequest & { errorCode: string; errorDetail: string }; @@ -138,6 +138,46 @@ describe("XMLHTTPRequest", function () { expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT); }); + for (const url of ["//", "http://", "https:///", "http://host:99999", "http://[::1", "http://@/", "http://a b/"]) { + it(`should throw SyntaxError synchronously from open() for malformed URL ${url}`, function () { + const xhr = new XMLHttpRequest(); + expect(() => xhr.open("GET", url)).to.throw(SyntaxError); + expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT); + expect(() => xhr.send()).to.throw(); + }); + } + + it("should keep the previous request when open() throws SyntaxError", async function () { + this.timeout(5000); + const xhr = new XMLHttpRequest(); + xhr.open("GET", "app:///Assets/symlink_target.js"); + expect(() => xhr.open("GET", "//")).to.throw(SyntaxError); + expect(xhr.readyState).to.equal(XMLHttpRequest.OPENED); + const completed = new Promise((resolve) => xhr.addEventListener("loadend", () => resolve())); + xhr.send(); + await completed; + expect(xhr.status).to.equal(200); + }); + + it("should not report an unsupported method as a URL-open failure", async function () { + this.timeout(5000); + const xhr = new XMLHttpRequest() as XMLHttpRequest & { errorCode: string }; + let threw = false; + try { + xhr.open("POST", "noscheme.glb"); + } catch (e) { + // Transports that reject POST (e.g. Unix) surface it synchronously, not as UrlOpenFailed. + threw = true; + expect((e as Error).message).to.contain("POST"); + } + if (!threw) { + const completed = new Promise((resolve) => xhr.addEventListener("loadend", () => resolve())); + xhr.send(); + await completed; + expect(xhr.errorCode).to.equal("UrlOpenFailed"); + } + }); + it("should still reject an unsupported body after a URL-open failure", async function () { this.timeout(5000); const xhr = new XMLHttpRequest(); From 1403a3c6c38d01196f998cbbc9af8b090df4d134 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: Tue, 13 Oct 2026 20:45:01 -0700 Subject: [PATCH 6/7] Fix XHR SyntaxError throw on the JSI backend Napi::Error(env, Napi::Object) is ambiguous under JSI; pass a Napi::Value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c658bb82-1999-4035-840f-54a1df553faf --- Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp index 220c8364..83cee302 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp @@ -505,7 +505,7 @@ namespace Babylon::Polyfills::Internal const auto url = info[1].As().Utf8Value(); if (!UrlSyntax::IsParseable(url)) { - const auto syntaxError = info.Env().Global().Get("SyntaxError").As().New({Napi::String::New(info.Env(), "Failed to parse URL '" + url + "'")}); + const Napi::Value syntaxError = info.Env().Global().Get("SyntaxError").As().New({Napi::String::New(info.Env(), "Failed to parse URL '" + url + "'")}); throw Napi::Error{info.Env(), syntaxError}; } From f89e27604fe517b03e3ac42c41fb8a77e86c8ce8 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: Tue, 13 Oct 2026 20:56:49 -0700 Subject: [PATCH 7/7] Build the XHR SyntaxError from a Call result for JSI Napi::Value is move-only under JSI, so pass the Call() result directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c658bb82-1999-4035-840f-54a1df553faf --- Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp index 83cee302..26a3f656 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp @@ -505,8 +505,8 @@ namespace Babylon::Polyfills::Internal const auto url = info[1].As().Utf8Value(); if (!UrlSyntax::IsParseable(url)) { - const Napi::Value syntaxError = info.Env().Global().Get("SyntaxError").As().New({Napi::String::New(info.Env(), "Failed to parse URL '" + url + "'")}); - throw Napi::Error{info.Env(), syntaxError}; + const auto syntaxErrorCtor = info.Env().Global().Get("SyntaxError").As(); + throw Napi::Error{info.Env(), syntaxErrorCtor.Call({Napi::String::New(info.Env(), "Failed to parse URL '" + url + "'")})}; } m_url = url;