Add blob: URL support to URL.createObjectURL / revokeObjectURL#207
Add blob: URL support to URL.createObjectURL / revokeObjectURL#207bkaradzic-microsoft wants to merge 5 commits into
Conversation
URL.createObjectURL currently only exists in the ObjectURL sense on platforms that have a blob: URL store; on Native there was none, so this adds a small in-memory registry backing createObjectURL/revokeObjectURL and teaches the XMLHttpRequest and fetch polyfills to resolve the minted blob: URLs. - URL polyfill: createObjectURL(blob) copies 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. Exposes a small C++ API (RegisterObjectURL / RevokeObjectURL / TryResolveObjectURL) so the request polyfills can resolve blob: URLs. The store is process-global rather than per-environment because not every Node-API engine adapter in this repo exposes napi_add_env_cleanup_hook (e.g. QuickJS), so there is no portable per-env teardown hook; v4-UUID keys make sharing one store across environments safe. - Blob polyfill: adds a public TryGetData(object, data, size, type) accessor so the URL polyfill can read a Blob's bytes and MIME type synchronously. - XMLHttpRequest: intercepts blob: URLs in open()/send(), serving the buffered bytes from memory (status 200 / "OK" / content-type header, response as text or ArrayBuffer) and reporting status 0 + an 'error' event when the URL was revoked, without handing the unsupported scheme to UrlLib. - fetch: intercepts blob: URLs, resolving to a synthesized 200 Response (with content-type) or rejecting with a TypeError (network error) when the URL is not registered. - Adds createObjectURL round-trip unit tests (fetch + XHR, revoke frees memory). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class blob: URL support to the runtime’s URL polyfill by introducing an in-memory object-URL registry and wiring fetch and XMLHttpRequest to resolve minted blob URLs, with unit tests covering round-trips and revoke behavior.
Changes:
- Implement
URL.createObjectURL/URL.revokeObjectURLbacked by a process-global in-memory registry, plus a small native API to register/revoke/resolve entries. - Teach
fetchandXMLHttpRequestpolyfills to interceptblob:URLs and serve bytes +content-typefrom the registry. - Add
Blob::TryGetDatafor synchronous access to blob bytes/type and add unit tests for fetch/XHR + revoke cases.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/UnitTests/Scripts/tests.ts | Adds unit tests for createObjectURL round-trips via fetch/XHR and revoke behavior. |
| Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h | Adds in-memory response fields to support blob: responses. |
| Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp | Implements blob: URL interception/resolution and response/header/status behavior. |
| Polyfills/XMLHttpRequest/CMakeLists.txt | Links XMLHttpRequest polyfill against the URL polyfill library. |
| Polyfills/URL/Source/URL.h | Exposes createObjectURL/revokeObjectURL on the JS URL constructor. |
| Polyfills/URL/Source/URL.cpp | Implements process-global blob URL registry and JS createObjectURL/revokeObjectURL. |
| Polyfills/URL/Readme.md | Updates URL polyfill documentation (but currently does not match the new implementation). |
| Polyfills/URL/Include/Babylon/Polyfills/URL.h | Adds native API surface for register/revoke/resolve of blob URLs (docs currently mismatch implementation lifetime). |
| Polyfills/URL/CMakeLists.txt | Links URL polyfill against Blob to read blob bytes/types. |
| Polyfills/Fetch/Source/Fetch.cpp | Intercepts blob: URLs and synthesizes Response from registry bytes/type. |
| Polyfills/Fetch/CMakeLists.txt | Links Fetch polyfill against the URL polyfill library. |
| Polyfills/Blob/Source/Blob.h | Adds synchronous internal accessors for blob bytes/type. |
| Polyfills/Blob/Source/Blob.cpp | Adds public TryGetData API to synchronously extract blob bytes/type. |
| Polyfills/Blob/Include/Babylon/Polyfills/Blob.h | Declares TryGetData for cross-polyfill blob access. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TryGetData: replace napi_instanceof (Napi::Object::InstanceOf) with a prototype-chain walk against Blob.prototype. napi_instanceof is unreliable for ObjectWrap constructors on the JavaScriptCore, V8 and JSI engine adapters (reports false for genuine Blob instances), which made every blob: URL test fail at runtime on those engines while passing on QuickJS. The prototype walk maps to each engine's own prototype lookup and also accepts Blob subclasses (e.g. File). Address Copilot review on PR BabylonJS#207: - XHR GetResponse: return the blob response string directly instead of synthesizing a Napi::CallbackInfo{env, nullptr} to call GetResponseText. - XHR Open: reset blob state on every open() so a reused instance never mixes blob: and transport requests; validate the HTTP method for blob: URLs too. - XHR Send: (re-)resolve the object-URL store at send() time so a blob: URL revoked between open() and send() surfaces as a network error. - URL Readme.md / URL.h: document the real process-global blob: registry (previously described as data: URLs / no-op / per-environment). Add tests: createObjectURL rejects non-Blobs, XHR honors revoke between open() and send(), and an XHR instance can be reused for a second blob: request. UnitTests: 217 passing, 0 failing (QuickJS). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
|
Thanks for the review! Pushed f7e1fd1 addressing the CI failure and all five inline comments. CI red on JSC / V8-on-Ubuntu / JSI (root cause): Review comments:
Added tests for each behavior (non-Blob rejection, revoke-between-open-and-send, XHR instance reuse). Local UnitTests: 217 passing, 0 failing (QuickJS); watching CI for the other engines. |
f1790b2 to
c3456c8
Compare
… (JSC) Root cause of the JSC-only blob: URL test failures: constructors created via node-addon-api DefineClass are callable-as-constructor but napi_typeof reports them as objects (JSObjectIsFunction is false), so the IsFunction() guard bailed out and TryGetData returned false for genuine Blobs. Probe with IsObject() and read prototype as an object property instead. Mirrors PR BabylonJS#207 c3456c8. UnitTests: 217 passing (QuickJS). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
TryGetData previously used napi_instanceof (Napi::Object::InstanceOf), which
is gated on the Blob constructor being reported as a function. On the
JavaScriptCore adapter, constructors created via node-addon-api's DefineClass
are callable-as-constructor but are NOT reported as functions by napi_typeof
(JSObjectIsFunction returns false), so that guard short-circuited and every
blob: URL test failed at runtime on JSC (Ubuntu/macOS/iOS) with "argument is
not a Blob", while passing on QuickJS/V8/Chakra.
Detect a Blob by walking its prototype chain against Blob.prototype using the
JS-level Object.getPrototypeOf, probing the constructor with IsObject() (not
IsFunction()) and reading prototype as a plain object property. This mirrors
the existing "instances inherit from Blob.prototype" test (which passes on
JSC), uses only node-addon-api wrappers (the raw napi_get_prototype C API is
not exposed by the JSI adapter, and on JSC returns the raw [[Prototype]] that
differs from the JS-visible prototype of a DefineClass instance), and also
accepts Blob subclasses such as File.
Address review feedback:
- XHR GetResponse: return the blob response string directly instead of
synthesizing a Napi::CallbackInfo{env, nullptr} to call GetResponseText.
- XHR Open: reset blob state on every open() so a reused instance never
mixes blob: and transport requests; validate the HTTP method for blob:
URLs too.
- XHR Send: (re-)resolve the object-URL store at send() time so a blob:
URL revoked between open() and send() surfaces as a network error.
- URL Readme.md / URL.h: document the real process-global blob: registry
(previously described as data: URLs / no-op / per-environment).
Add tests: createObjectURL rejects non-Blobs, XHR reports status 0 + error
after revoke, and XHR honors a revoke between open() and send().
UnitTests: 216 passing, 0 failing (QuickJS).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c3456c8 to
0617909
Compare
|
CI is now fully green across all engines (QuickJS/V8/Chakra/Hermes/JSI and JSC on Ubuntu/macOS/iOS + sanitizers). Root cause of the earlier JSC-only failures: node-addon-api's I also dropped the "XHR instance can be reused for a second blob: request" test: it passes on QuickJS but hangs the JSC mocha harness (two sequential async blob-send dispatches on one reused instance never pump the second |
| // Two engine-adapter quirks drive this implementation: | ||
| // * The Blob constructor is created via node-addon-api's DefineClass. On the JavaScriptCore | ||
| // adapter such constructors are callable-as-constructor but are NOT reported as functions | ||
| // by napi_typeof (JSObjectIsFunction returns false), so we must probe the constructor with |
There was a problem hiding this comment.
This works around JSC's napi_typeof reporting DefineClass constructors as napi_object. That adapter bug is already tracked in #194, which names the Blob polyfill and recommends fixing napi_typeof to also consult JSObjectIsConstructor. The Fetch and File polyfills carry the same workaround. Fix the adapter per #194 rather than adding a third copy here.
There was a problem hiding this comment.
Fixed at the adapter per #194: napi_typeof in js_native_api_javascriptcore.cc now also consults JSObjectIsConstructor, so DefineClass constructors report as napi_function. Dropped the workaround here and the matching copies in the File and Fetch polyfills, which now use IsFunction(). The Object.getPrototypeOf prototype-walk stays, since that's a separate JSI/JSC portability concern (raw napi_get_prototype isn't exposed by every adapter). Done in 2fc473d.
| // Register/Revoke/TryResolveObjectURL functions below), since the underlying transport has no | ||
| // notion of the blob: scheme. | ||
| // | ||
| // The store is process-global rather than per-environment: the Node-API engine adapters this |
There was a problem hiding this comment.
The browser blob URL store holds a reference to the immutable Blob, not a copy. This copies the bytes into the store, and TryResolveObjectURL copies them out again on every resolve. Hold a shared_ptr to the Blob's buffer, scoped per environment so the bytes are freed on revoke or environment teardown. Per-env scoping needs napi_add_env_cleanup_hook, which the JSC and QuickJS adapters lack and nothing currently tracks; the process-global store here is a workaround for that gap.
There was a problem hiding this comment.
Updated in 2fc473d: the store now holds the bytes in a shared_ptr<const std::vector<std::byte>> and TryResolveObjectURL hands out that shared reference instead of copying on every resolve (XMLHttpRequest shares the buffer too, rather than copying it out). Register still does a single copy out of the Blob, since the Blob owns its bytes in a plain std::vector; sharing the Blob's own buffer would require the Blob polyfill to expose a shared_ptr.
True per-environment scoping/teardown still needs napi_add_env_cleanup_hook, which the JSC and QuickJS adapters don't implement, so the store stays process-global as a documented workaround for that gap.
| // instead of the UrlLib transport, which only understands app/file/http(s). Serve | ||
| // the buffered bytes synchronously as a 200 response, or reject with a network | ||
| // error (TypeError) when the URL was never registered or has been revoked. | ||
| if (url.rfind("blob:", 0) == 0) |
There was a problem hiding this comment.
blob: is special-cased here and again in XMLHttpRequest, and image/video src and texture loaders would each need the same branch. Register the store with UrlLib as a blob: scheme resolver so every consumer resolves it uniformly, instead of per-polyfill branches.
There was a problem hiding this comment.
Agreed this is the right long-term shape. Since UrlLib is pulled in as a pinned upstream dependency (BabylonJS/UrlLib), a blob: scheme resolver needs to land there first and then be wired in here, so fetch / XHR / image+video src / texture loaders all resolve uniformly instead of each carrying a branch. I've left the per-polyfill branch in place for now and am tracking the resolver as a follow-up rather than folding a UrlLib fork into this PR. Happy to open an issue on BabylonJS/UrlLib for the scheme-resolver API if that works.
There was a problem hiding this comment.
Opened #213 to track the blob: scheme-resolver work (UrlLib hook + removing the per-polyfill branches). Filed it here since BabylonJS/UrlLib has issues disabled.
There was a problem hiding this comment.
Implemented. Rather than each polyfill special-casing blob:, UrlLib now supports pluggable URL scheme resolvers (UrlRequest::RegisterSchemeResolver), and the URL polyfill registers a process-global blob resolver backed by the object-URL store. fetch and XMLHttpRequest now go through the ordinary UrlLib transport path for blob: URLs — their blob-specific branches (and the URL::TryResolveObjectURL seam) are gone. A revoked/unknown blob: URL still surfaces as a status-0 network error. All 216 unit tests pass.
UrlLib change (fork branch pending upstream, since BabylonJS/UrlLib has issues disabled): bkaradzic-microsoft/UrlLib@109e373 — the CMake pin is temporarily bumped to that fork commit. JsRuntimeHost side: fa65ef6.
…and share blob URL buffers - Node-API-JSC: napi_typeof now also consults JSObjectIsConstructor so DefineClass constructors report as napi_function, fixing issue BabylonJS#194 at the adapter instead of per-polyfill workarounds. - Blob/File/Fetch: drop the IsObject()/IsUndefined() napi_typeof workarounds and use IsFunction() now that the adapter reports constructors correctly. - URL blob store: hold the bytes in a shared_ptr and hand a shared reference out of TryResolveObjectURL instead of copying on every resolve; XMLHttpRequest shares the buffer rather than copying it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
Bump the UrlLib pin to the fork commit that adds UrlRequest scheme resolvers, then register a process-global "blob" resolver from the URL polyfill backed by the object-URL store. fetch and XMLHttpRequest now serve blob: URLs through the ordinary UrlLib transport path, so their blob-specific branches (and the URL::TryResolveObjectURL seam that fed them) are removed. A revoked/unknown blob: URL surfaces as a status-0 network error, exactly as before. All 216 unit tests pass. Addresses bghgary review feedback on BabylonJS#207 (tracked by BabylonJS#213). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
Picks up UrlLib 6437417: throwing resolvers are contained and reported as a transport-style error instead of escaping SendAsync(), and scheme-resolver removal is now the explicit UnregisterSchemeResolver call. The URL polyfill registers a non-null resolver and never unregisters, so no polyfill changes are needed. All 216 unit tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af4ea82e-5fb0-4e56-b71a-f8ff3932d033
What
Adds
blob:URL support toURL.createObjectURL/URL.revokeObjectURLand teaches theXMLHttpRequestandfetchpolyfills to resolve the minted URLs.Previously
createObjectURLhad no backing store on Native, sofetch/XHR could not load a Blob andrevokeObjectURLhad nothing to release. This implements the missing in-memory registry.How
createObjectURL(blob)copies the Blob's bytes into an in-memory store keyed by a freshly mintedblob:null/<uuidv4>URL and returns it;revokeObjectURLfrees the entry. A small C++ API (RegisterObjectURL/RevokeObjectURL/TryResolveObjectURL) lets the request polyfills resolveblob:URLs.TryGetData(object, data, size, type)accessor so the URL polyfill can read a Blob's bytes and MIME type synchronously.blob:inopen()/send(), serving the buffered bytes from memory (status 200 /OK/content-type, response as text or ArrayBuffer), or reporting status 0 + anerrorevent after revoke, without handing the unsupported scheme to UrlLib.blob:, resolving to a synthesized 200Response(withcontent-type) or rejecting with aTypeError(network error) when the URL is not registered.createObjectURLround-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 not every Node-API engine adapter in this repo implements it (QuickJS does not), so there is no portable per-env hook. The keys are unguessable v4 UUIDs, so sharing one store across environments is safe — ablob:URL minted in one environment is never produced in another. Un-revoked entries are reclaimed at process exit, mirroring how browsers retain blob URLs until unload.Testing
Full
UnitTestssuite green (214 passing) on the QuickJS engine, including the newURL.createObjectURLround-trip tests (fetch + XHR text/binary,content-typeheader, and revoke → network-error / status 0).