From e37722263fa6c8f9bd96970e57cdd017c22d5ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 13 Jul 2026 04:21:30 +0200 Subject: [PATCH 1/3] fix(runtime,codegen): expose the Web Streams globals; resolve instanceof against a built-in held in a variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps that both surface the moment a bundler minifies. 1. `ReadableStream` / `WritableStream` / `TransformStream` were missing from `globalThis`. Perry implements all three — codegen lowers `new ReadableStream(…)` and `x instanceof ReadableStream`, and each has a class id — but the NAMES were never registered, so a bare `ReadableStream` identifier resolved to nothing: `typeof ReadableStream === "undefined"` and `"ReadableStream" in globalThis` was false, where Node exposes all three as functions. Libraries feature-detect exactly that (`typeof ReadableStream !== "undefined" ? … : …`) when deciding how to consume a `fetch()` body, so they silently took the wrong branch. 2. `js_instanceof_dynamic` returned `false` for every built-in reached through a *variable* rather than its bare name. `x instanceof Response` worked; the equivalent after `const R = Response` did not — and minifiers alias constructors as a matter of course, so in a bundle this was broadly broken. The dynamic path only understood user class objects; a built-in constructor is a `ClosureHeader` thunk, which it did not map back to a class id. It now recovers the id from the thunk's recorded name (ReadableStream / WritableStream / TransformStream / Response / Request / Headers / Blob) and defers to `js_instanceof`. Validation: node-vs-perry differential — `typeof` and `in globalThis` for the three stream constructors, `instanceof` against both the bare name and an alias, and `Response`/`Headers`/`Blob` through aliases — byte-identical to Node, where 9 of the 10 assertions previously disagreed. perry-runtime 1264 passed / 0 failed; perry-codegen 159 passed / 0 failed. --- crates/perry-codegen/src/expr/helpers.rs | 17 ++++++ .../src/object/global_this_tables.rs | 11 ++++ crates/perry-runtime/src/object/instanceof.rs | 60 +++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/crates/perry-codegen/src/expr/helpers.rs b/crates/perry-codegen/src/expr/helpers.rs index ecb8a3757d..6ea7f144e1 100644 --- a/crates/perry-codegen/src/expr/helpers.rs +++ b/crates/perry-codegen/src/expr/helpers.rs @@ -307,6 +307,23 @@ pub(crate) fn is_global_this_builtin_name(name: &str) -> bool { | "TextDecoderStream" | "CompressionStream" | "DecompressionStream" + // The three core Web Streams constructors. Perry already implements + // them (`new ReadableStream(…)` and `x instanceof ReadableStream` are + // both lowered, and `ReadableStream` has a class id), but the NAMES + // were never registered as globalThis builtins — so a bare + // `ReadableStream` identifier did not resolve: `typeof + // ReadableStream === "undefined"` and `"ReadableStream" in globalThis` + // was false, while Node exposes all three as functions. + // + // Libraries feature-detect exactly this (`typeof ReadableStream !== + // "undefined" ? … : …`) to decide how to consume a `fetch()` body. + // Getting "undefined" sent a large esbuild-bundled CLI app down the + // wrong branch, which then threw "The first argument must be a + // Readable, a ReadableStream, or an async iterable" and aborted its + // background tar-stream downloads entirely. + | "ReadableStream" + | "WritableStream" + | "TransformStream" | "Navigator" | "URL" | "URLSearchParams" diff --git a/crates/perry-runtime/src/object/global_this_tables.rs b/crates/perry-runtime/src/object/global_this_tables.rs index c65c8ee49a..907d4f2495 100644 --- a/crates/perry-runtime/src/object/global_this_tables.rs +++ b/crates/perry-runtime/src/object/global_this_tables.rs @@ -55,6 +55,17 @@ pub(crate) const GLOBAL_THIS_BUILTIN_CONSTRUCTORS: &[&str] = &[ "TextDecoderStream", "CompressionStream", "DecompressionStream", + // The three core Web Streams constructors. Perry implements them (codegen + // lowers `new ReadableStream(…)` and `x instanceof ReadableStream`, and the + // class has an id), but the NAMES were never registered here or in codegen's + // globalThis-builtin list — so a bare `ReadableStream` identifier resolved to + // nothing: `typeof ReadableStream === "undefined"` and `"ReadableStream" in + // globalThis` was false, whereas Node exposes all three as functions. + // Libraries feature-detect precisely that (`typeof ReadableStream !== + // "undefined" ? … : …`) when deciding how to consume a `fetch()` body. + "ReadableStream", + "WritableStream", + "TransformStream", "Navigator", "URL", "URLSearchParams", diff --git a/crates/perry-runtime/src/object/instanceof.rs b/crates/perry-runtime/src/object/instanceof.rs index 9c4f902e26..81eec9f0d8 100644 --- a/crates/perry-runtime/src/object/instanceof.rs +++ b/crates/perry-runtime/src/object/instanceof.rs @@ -93,6 +93,43 @@ fn is_native_module_namespace_value(value: f64, expected: &str) -> bool { /// `js_instanceof`. Returns FALSE for non-class-ref type values (matches /// JS spec: `1 instanceof 2` throws, but Perry returns false defensively). /// Refs #420 / #618 followup. +#[no_mangle] +/// Map a builtin constructor VALUE (the `ClosureHeader`-backed function installed +/// on `globalThis`) back to the class id that codegen passes to `js_instanceof` +/// for the static `x instanceof ` form. +/// +/// Only the natively-backed builtins need this: their instances are handles +/// (stream / fetch registries), not heap objects with a real prototype chain, so +/// `js_instanceof` brand-checks them via the kind probes rather than a chain walk. +/// Heap-backed builtins already resolve through the class-id / prototype paths. +fn builtin_ctor_class_id_from_value(type_ref: f64) -> Option { + let jv = crate::JSValue::from_bits(type_ref.to_bits()); + if !jv.is_pointer() { + return None; + } + let closure = jv.as_pointer::(); + if closure.is_null() { + return None; + } + if unsafe { (*closure).type_tag } != crate::closure::CLOSURE_MAGIC { + return None; + } + let name_value = crate::closure::closure_get_dynamic_prop(closure as usize, "name"); + let mut scratch = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + let (ptr, len) = crate::string::str_bytes_from_jsvalue(name_value, &mut scratch)?; + let name = std::str::from_utf8(unsafe { std::slice::from_raw_parts(ptr, len as usize) }).ok()?; + Some(match name { + "ReadableStream" => 0xFFFF_0060, + "WritableStream" => 0xFFFF_0061, + "TransformStream" => 0xFFFF_0062, + "Response" => 0xFFFF_0028, + "Request" => 0xFFFF_0029, + "Headers" => 0xFFFF_002A, + "Blob" => 0xFFFF_0026, + _ => return None, + }) +} + #[no_mangle] pub extern "C" fn js_instanceof_dynamic(value: f64, type_ref: f64) -> f64 { const TAG_FALSE: u64 = 0x7FFC_0000_0000_0003; @@ -206,6 +243,29 @@ pub extern "C" fn js_instanceof_dynamic(value: f64, type_ref: f64) -> f64 { return js_instanceof(value, class_id); } } + // A builtin constructor held in a VARIABLE — `const RS = ReadableStream; body + // instanceof RS` — arrives here as the ClosureHeader-backed function installed + // on `globalThis`, so none of the class-id paths above match and the prototype + // walk below returns false. Codegen only special-cases the *static identifier* + // form (`body instanceof ReadableStream`), where it hands the builtin class id + // straight to `js_instanceof`, which brand-checks these natively-backed values + // via the stream / fetch kind probes (their instances are handles, not heap + // objects with a real prototype chain). + // + // Minified bundles almost always alias constructors into locals, so the + // variable form is the common one in the wild: `x instanceof ` for + // ReadableStream / Response / Headers silently returned `false` while Node + // returns `true`. That made a large esbuild-bundled CLI app mis-detect its + // `fetch()` body, throw "The first argument must be a Readable, a + // ReadableStream, or an async iterable", and abort its background + // tar-stream downloads entirely. + // + // Recover the builtin's name from the constructor closure (recorded by + // `set_bound_native_closure_name` when globalThis is populated) and reuse the + // static path's class id, so both spellings agree. + if let Some(class_id) = builtin_ctor_class_id_from_value(type_ref) { + return js_instanceof(value, class_id); + } if let Some((module, method)) = unsafe { bound_native_callable_module_and_method(type_ref) } { if module == "stream" && matches!( From 4393bc980e721d607a52e32980c28bd85b30ff97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 13 Jul 2026 08:47:38 +0200 Subject: [PATCH 2/3] style: cargo fmt (lint gate) --- crates/perry-runtime/src/object/instanceof.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/object/instanceof.rs b/crates/perry-runtime/src/object/instanceof.rs index 81eec9f0d8..d5f6f8cef7 100644 --- a/crates/perry-runtime/src/object/instanceof.rs +++ b/crates/perry-runtime/src/object/instanceof.rs @@ -117,7 +117,8 @@ fn builtin_ctor_class_id_from_value(type_ref: f64) -> Option { let name_value = crate::closure::closure_get_dynamic_prop(closure as usize, "name"); let mut scratch = [0u8; crate::value::SHORT_STRING_MAX_LEN]; let (ptr, len) = crate::string::str_bytes_from_jsvalue(name_value, &mut scratch)?; - let name = std::str::from_utf8(unsafe { std::slice::from_raw_parts(ptr, len as usize) }).ok()?; + let name = + std::str::from_utf8(unsafe { std::slice::from_raw_parts(ptr, len as usize) }).ok()?; Some(match name { "ReadableStream" => 0xFFFF_0060, "WritableStream" => 0xFFFF_0061, From eca38431efc082d3b07dee78ffbf1d01606455de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 13 Jul 2026 13:30:33 +0200 Subject: [PATCH 3/3] review: canonical nanbox extractor, null-guard, identity check against the globalThis builtin CodeRabbit findings on the builtin-ctor brand map: use js_nanbox_get_pointer for the pointer payload; refuse a null string pointer before from_raw_parts; and require pointer identity with the constructor installed on globalThis so a user function merely NAMED Response/ReadableStream/... cannot satisfy the native brand check. --- crates/perry-runtime/src/object/instanceof.rs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/perry-runtime/src/object/instanceof.rs b/crates/perry-runtime/src/object/instanceof.rs index d5f6f8cef7..9a3a8bce80 100644 --- a/crates/perry-runtime/src/object/instanceof.rs +++ b/crates/perry-runtime/src/object/instanceof.rs @@ -103,11 +103,8 @@ fn is_native_module_namespace_value(value: f64, expected: &str) -> bool { /// `js_instanceof` brand-checks them via the kind probes rather than a chain walk. /// Heap-backed builtins already resolve through the class-id / prototype paths. fn builtin_ctor_class_id_from_value(type_ref: f64) -> Option { - let jv = crate::JSValue::from_bits(type_ref.to_bits()); - if !jv.is_pointer() { - return None; - } - let closure = jv.as_pointer::(); + let closure = + crate::value::js_nanbox_get_pointer(type_ref) as *const crate::closure::ClosureHeader; if closure.is_null() { return None; } @@ -117,9 +114,12 @@ fn builtin_ctor_class_id_from_value(type_ref: f64) -> Option { let name_value = crate::closure::closure_get_dynamic_prop(closure as usize, "name"); let mut scratch = [0u8; crate::value::SHORT_STRING_MAX_LEN]; let (ptr, len) = crate::string::str_bytes_from_jsvalue(name_value, &mut scratch)?; + if ptr.is_null() { + return None; + } let name = std::str::from_utf8(unsafe { std::slice::from_raw_parts(ptr, len as usize) }).ok()?; - Some(match name { + let class_id = match name { "ReadableStream" => 0xFFFF_0060, "WritableStream" => 0xFFFF_0061, "TransformStream" => 0xFFFF_0062, @@ -128,7 +128,15 @@ fn builtin_ctor_class_id_from_value(type_ref: f64) -> Option { "Headers" => 0xFFFF_002A, "Blob" => 0xFFFF_0026, _ => return None, - }) + }; + // The name alone is forgeable (`function Response() {}` in user code). + // Require IDENTITY with the builtin constructor installed on + // `globalThis`: only the genuine builtin value brand-checks instances. + let global_ctor = crate::object::js_get_global_this_builtin_value(name.as_ptr(), name.len()); + if crate::value::js_nanbox_get_pointer(global_ctor) as usize != closure as usize { + return None; + } + Some(class_id) } #[no_mangle]