Add pluggable URL scheme resolver (e.g. blob:) to UrlRequest - #37
Merged
bkaradzic-microsoft merged 6 commits intoJul 27, 2026
Merged
Conversation
Introduce UrlRequest::RegisterSchemeResolver so a consumer can register a process-global resolver for a non-transport URL scheme such as "blob:". When a UrlRequest is opened with a URL whose scheme has a registered resolver, the platform transport is bypassed and the resolver supplies the response at SendAsync() time. Resolution is deferred to SendAsync (rather than Open) so a blob: URL revoked between open() and send() is honored, and an unhandled URL (e.g. revoked) surfaces as a status-0 transport-style error. This lets every consumer (fetch, XMLHttpRequest, image/video src, texture loaders, ...) resolve such URLs uniformly through UrlRequest instead of each carrying its own branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
There was a problem hiding this comment.
Pull request overview
This PR adds a process-global, pluggable URL scheme resolver mechanism to UrlRequest, allowing non-transport schemes (e.g. blob:) to be resolved via a registered callback and served through the existing UrlRequest consumer surface (fetch/XHR/resource loaders) without per-consumer special-casing.
Changes:
- Introduces
UrlSchemeResolverResultandUrlSchemeResolver, plus the publicUrlRequest::RegisterSchemeResolver()API for process-global registration. - Diverts
UrlRequest::Open()when a registered scheme is detected, and performs resolver execution duringSendAsync()to honor revoke-after-open scenarios. - Adds shared-layer storage for resolver state and exposes resolved buffers via
ResponseBuffer()for scheme-resolved requests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Source/UrlRequest_Shared.h | Hooks scheme diversion into Open(), resolves via resolver path in SendAsync(), and routes ResponseBuffer() for resolved responses. |
| Source/UrlRequest_Base.h | Implements the resolver registry, scheme detection, and shared-layer scheme resolution response population. |
| Include/UrlLib/UrlLib.h | Adds the public resolver API surface (UrlSchemeResolverResult, UrlSchemeResolver, RegisterSchemeResolver). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Consume the pending resolver on the first ResolveScheme() call (move it out and clear it before invoking) so a second SendAsync() on the same request does not re-run the resolver -- avoiding repeated resolver side effects or clobbering the already-populated response. IsSchemeResolution() stays true so ResponseBuffer() keeps serving the resolved bytes. Add a UrlLibTests suite (SchemeResolver.cpp) covering: a handled resolver populating a String and a Buffer response (status, statusText, content-type, body, response URL); the handled == false transport-style error contract (status None, ErrorSymbol "SchemeResolverNotFound", empty buffer); and resolve-exactly-once across repeated sends. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
bghgary
approved these changes
Jul 24, 2026
A resolver that throws no longer escapes SendAsync() synchronously: the call is guarded and the exception is mapped to the same transport-style error surface as handled == false, reported as "SchemeResolverThrew" with the exception message as detail. Removal is now the explicit UnregisterSchemeResolver(scheme) rather than registering an empty std::function. RegisterSchemeResolver throws std::invalid_argument for a null resolver or empty scheme, so a moved-from or default-constructed resolver can no longer silently unregister a scheme whose captured state (e.g. a blob store) is still in use. Also adds a regression test showing the resolver path already inherits the canonical reason-phrase fallback in StatusText() when a resolver supplies a status code but no status text. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
The test sent a request through the real platform transport after unregistering the resolver, but used the resolver-path helper that asserts the send settles inline. On Apple backends the transport completes asynchronously, so the assert failed in CI (macOS_Xcode264). Assert the property actually under test instead: after unregistering, the resolver is never consulted again. The transport's outcome for an unknown scheme is platform-specific (throw at Open, fail inline, or fail async), so each of those is tolerated and an in-flight request is aborted. The completion flag is now held by shared_ptr rather than captured by reference, so a continuation that runs later cannot touch a dangling stack slot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
Explicitly aborting the (non-diverted) request perturbed the shared NSURLSession on the Apple backend, so the following test in the suite failed with a spurious NSURLErrorCancelled (-999). Let the request wind down through its own destructor instead, which the earlier CI run showed leaves subsequent tests unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
bghgary
reviewed
Jul 24, 2026
The previous rationale was wrong: ~ImplBase() calls the same non-virtual Abort() as the explicit call, so dropping the explicit Abort() changed nothing about cancellation, and on the Apple backend neither path stops a resumed NSURLSessionDataTask -- so an explicit abort could not have produced the NSURLErrorCancelled (-999) attributed to it. The real hazard is lifetime, not cancellation. UrlRequest_Apple.mm's completion handler writes m_statusCode / m_headers / m_responseBuffer and calls SetError(...) through a raw `this`, and nothing extends the Impl's lifetime. Abandoning the in-flight request let that handler write into freed memory, which is what corrupted a later test's request state. Wait for the request to settle before leaving scope, and fail explicitly if it does not, so the test never depends on that timing. The underlying backend gap is pre-existing and tracked separately in BabylonJS/JsRuntimeHost#214. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
bkaradzic-microsoft
pushed a commit
to bkaradzic-microsoft/JsRuntimeHost
that referenced
this pull request
Jul 27, 2026
BabylonJS/UrlLib#37 (the pluggable scheme resolver this change set depends on) has merged, so the temporary fork pin is no longer needed.
bkaradzic-microsoft
added a commit
to BabylonJS/JsRuntimeHost
that referenced
this pull request
Jul 28, 2026
### What Adds `blob:` URL support to `URL.createObjectURL` / `URL.revokeObjectURL`, and makes the minted URLs resolvable by every consumer of `UrlRequest` (`fetch`, `XMLHttpRequest`, and any downstream user of UrlLib). Previously `createObjectURL` had no backing store on Native, so `fetch`/XHR could not load a Blob and `revokeObjectURL` had nothing to release. This implements the missing in-memory registry. ### How - **URL polyfill** — `createObjectURL(blob)` puts the Blob's bytes into an in-memory store keyed by a freshly minted `blob:null/<uuidv4>` URL and returns it; `revokeObjectURL` frees the entry. The store's C++ surface is deliberately not exported: `RegisterObjectURL`/`RevokeObjectURL` are file-local to `URL.cpp`, since neither has a caller elsewhere and #215 would reshape the contract anyway. - **`blob:` scheme resolver** — rather than special-casing `blob:` inside each request polyfill, the URL polyfill registers a resolver with `UrlLib::UrlRequest::RegisterSchemeResolver` (added in BabylonJS/UrlLib#37). Every `UrlRequest` consumer therefore resolves `blob:` uniformly, with no per-polyfill interception: a hit yields status 200 with the stored `content-type`, and a miss (or a revoked URL) yields a network error, so `fetch` rejects with a `TypeError` and XHR reports status 0 plus an `error` event. - **Blob polyfill** — adds a `TryGetData(object) -> std::optional<BlobData>` accessor so the URL polyfill can read a Blob's bytes and MIME type. It lives behind a `BlobInternal` INTERFACE target (`InternalInclude/`, mirroring `JsRuntimeInternal`) rather than the public header, since it is an in-repo seam and not an embedder contract. `Blob` now holds its bytes in a `std::shared_ptr<const std::vector<std::byte>>`, so `createObjectURL` shares the buffer with the store instead of duplicating it, and the returned data no longer has a lifetime tied to the JS object. - **JavaScriptCore `napi_typeof` fix** — `Core/Node-API/Source/js_native_api_javascriptcore.cc` now consults `JSObjectIsConstructor` in addition to `JSObjectIsFunction`, so constructors made with `JSObjectMakeConstructor` report `napi_function` rather than `napi_object`. This is not specific to this feature — it affects every JSC consumer — but it lets `Fetch` and `File` drop their `IsUndefined()/IsNull()` workarounds and go back to a plain `IsFunction()` check. - Adds `createObjectURL` round-trip unit tests (fetch + XHR, revoke frees memory). ### Design note The store is **process-global** rather than per-environment. Per-env teardown would normally use `napi_add_env_cleanup_hook`, but no Node-API engine adapter in this repo implements it, so there is no portable per-env hook available today. The keys are unguessable v4 UUIDs, so sharing one store across environments is safe — a `blob:` URL minted in one environment is never produced in another. The cost is that entries which are never revoked stay alive for the lifetime of the process, and are not released when the environment that created them is torn down. This is a real leak, not a browser-like behavior: a browser drops a document's blob URLs when the document unloads. #215 tracks implementing `napi_add_env_cleanup_hook` across the adapters so the store can become per-environment. ### Testing Full `UnitTests` suite green (216 passing) on Windows/Chakra locally, with the remaining engines and platforms covered by CI, including the new `URL.createObjectURL` round-trip tests (fetch + XHR text/binary, `content-type` header, and revoke → network-error / status 0). --------- Co-authored-by: bkaradzic-microsoft <bkaradzic@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a pluggable URL scheme resolver hook to
UrlRequest, so a consumer can register a resolver for a non-transport URL scheme (e.g.blob:) and have everyUrlRequest-based consumer (fetch, XMLHttpRequest, image/videosrc, texture loaders, ...) resolve such URLs uniformly through the transport layer instead of each carrying its own branch.How
UrlRequest::RegisterSchemeResolver(std::string scheme, UrlSchemeResolver resolver), plus theUrlSchemeResolverResultstruct andUrlSchemeResolveralias.UrlRequestis opened with a URL whose (lower-cased) scheme has a registered resolver, the platform transport is bypassed. Resolution is deferred toSendAsync()(rather thanOpen()) so a URL revoked betweenopen()andsend()is honored.handled == false(e.g. a revokedblob:URL) leaves the status at0/Noneand records a transport-style error, mirroring how a genuine network failure surfaces.ImplBase+ the shared wrapper, so every platform backend (Win32/Unix/Apple/Android) inherits it with no per-platform changes.Testing
Builds clean with the Win32 backend (
UrlLibandUrlLibTeststargets). Downstream, this powersURL.createObjectURLblob:support in JsRuntimeHost — see BabylonJS/JsRuntimeHost#207 (tracked by BabylonJS/JsRuntimeHost#213), where all 216 unit tests pass with fetch/XMLHttpRequest resolvingblob:through this hook.