diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index 415b069ce..7e90bd049 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -2964,18 +2964,28 @@ pub(crate) fn ordinary_function_prototype_value_for_read(func_value: f64) -> Opt // `'prototype' in C.prototype.m === false`). (Test262 definition method/accessor // prop-desc.) // - // #4973 / #3527 exception: bound NATIVE-MODULE *class* exports - // (`http.Server`, `http.IncomingMessage`, `http.ServerResponse`, …) are + // #4973 / #3527 / #5268 exception: bound NATIVE-MODULE *class* exports + // (`http.Server`, `fs.ReadStream`, `events.EventEmitter`, …) are // constructors in Node, and the util.inherits / `Object.create(Ctor. - // prototype)` subclass pattern reads their `.prototype` as a - // setPrototypeOf / Object.create operand. Returning None here made that - // read `undefined`, and `Object.create(undefined)` / - // `Object.setPrototypeOf(x, undefined)` then threw "Object prototype may - // only be an Object or null" — the exact blocker hit at Express init - // (`express/lib/request.js`: `Object.create(http.IncomingMessage. - // prototype)`). These exports are cached singleton closures - // (NATIVE_CALLABLE_EXPORTS), so the synthetic-class path below gives them - // a stable prototype object. + // prototype)` / `Object.setPrototypeOf(x, Ctor.prototype)` subclass + // pattern reads their `.prototype` as a setPrototypeOf / Object.create + // operand. Returning None here made that read `undefined`, and + // `Object.create(undefined)` / `Object.setPrototypeOf(x, undefined)` then + // threw "Object prototype may only be an Object or null" — the blocker hit + // at Express init (`express/lib/request.js`: + // `Object.create(http.IncomingMessage.prototype)`), graceful-fs's + // `ReadStream.prototype = Object.create(fs$ReadStream.prototype)`, and + // pino's `Object.setPrototypeOf(prototype, EventEmitter.prototype)`. + // + // A bound-native export is a constructor class when its method name uses + // Node's constructor-cased convention (a leading uppercase ASCII letter, + // e.g. `ReadStream`/`EventEmitter`/`Server`) AND it isn't explicitly + // marked non-constructable (built-in prototype methods like + // `String.prototype.charAt` carry that flag). Such exports are cached + // singleton closures (NATIVE_CALLABLE_EXPORTS), so the synthetic-class + // path below gives them a stable `.prototype` object. Non-constructor + // bound methods (`fs.readFile`, `path.join`, …) keep `prototype === + // undefined`, matching Node's built-in non-constructor functions. { let jv = crate::value::JSValue::from_bits(func_value.to_bits()); if jv.is_pointer() { @@ -2984,24 +2994,17 @@ pub(crate) fn ordinary_function_prototype_value_for_read(func_value: f64) -> Opt && is_valid_obj_ptr(cptr as *const u8) && crate::closure::closure_is_bound_method(cptr) { + if super::native_module::builtin_closure_is_non_constructable_value(func_value) { + return None; + } let is_native_class_export = unsafe { super::native_module::bound_native_callable_module_and_method(func_value) } - .map(|(module, method)| { - matches!( - (module.as_str(), method.as_str()), - // Shared http/https constructor classes. - ("http" | "https", "Server" | "Agent") - // http-only request/response constructor classes - // that userland subclasses (Express, util.inherits). - | ( - "http", - "IncomingMessage" - | "ServerResponse" - | "OutgoingMessage" - | "ClientRequest" - ) - ) + .map(|(_module, method)| { + method + .as_bytes() + .first() + .is_some_and(|b| b.is_ascii_uppercase()) }) .unwrap_or(false); if !is_native_class_export { diff --git a/crates/perry/tests/issue_5268_native_ctor_prototype_undefined.rs b/crates/perry/tests/issue_5268_native_ctor_prototype_undefined.rs new file mode 100644 index 000000000..6a0d58a8b --- /dev/null +++ b/crates/perry/tests/issue_5268_native_ctor_prototype_undefined.rs @@ -0,0 +1,109 @@ +//! Regression test for #5268: native-module constructor *class* exports must +//! expose a real `.prototype` object so the userland subclass idioms used by +//! graceful-fs / fs-extra / pino don't throw +//! `TypeError: Object prototype may only be an Object or null: undefined`. +//! +//! graceful-fs `graceful-fs.js`: +//! ```js +//! var fs$ReadStream = fs.ReadStream +//! if (fs$ReadStream) { +//! ReadStream.prototype = Object.create(fs$ReadStream.prototype) // throws pre-fix +//! } +//! ``` +//! pino `lib/proto.js`: +//! ```js +//! const { EventEmitter } = require('node:events') +//! Object.setPrototypeOf(prototype, EventEmitter.prototype) // throws pre-fix +//! ``` +//! +//! Pre-fix, `fs.ReadStream`/`fs.WriteStream`/`events.EventEmitter` were truthy +//! callable closures (bound-native exports) whose `.prototype` resolved to +//! `undefined`: `ordinary_function_prototype_value_for_read` +//! (crates/perry-runtime/src/object/class_registry.rs) returned `None` for +//! every bound-native export except a hardcoded http/https whitelist. Reading +//! `Object.create(undefined)` / `Object.setPrototypeOf(x, undefined)` then hit +//! the spec TypeError. Fix: recognize constructor-cased bound-native exports +//! (leading uppercase, not flagged non-constructable) and let the +//! synthetic-class path materialize a stable `.prototype` object — while +//! keeping non-constructor exports (`fs.readFile`, …) at `prototype === +//! undefined`, matching Node's built-in non-constructor functions. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(dir: &std::path::Path, source: &str) -> String { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed (pre-fix: 'TypeError: Object prototype may \ + only be an Object or null: undefined')\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +#[test] +fn native_ctor_class_exports_expose_real_prototype() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +import * as fs from "node:fs"; +import { EventEmitter } from "node:events"; + +// Native constructor classes expose an object .prototype (not undefined). +const rsProto = (fs as any).ReadStream.prototype; +const wsProto = (fs as any).WriteStream.prototype; +const eeProto = (EventEmitter as any).prototype; +console.log("rs:", typeof rsProto === "object" && rsProto !== null); +console.log("ws:", typeof wsProto === "object" && wsProto !== null); +console.log("ee:", typeof eeProto === "object" && eeProto !== null); + +// graceful-fs's ReadStream pattern: Object.create(Ctor.prototype). +const sub = Object.create((fs as any).ReadStream.prototype); +console.log("create:", typeof sub === "object" && sub !== null); +console.log("chain:", Object.getPrototypeOf(sub) === (fs as any).ReadStream.prototype); + +// pino's proto.js pattern: setPrototypeOf(prototype, EventEmitter.prototype). +const prototype: any = { child() { return null; } }; +Object.setPrototypeOf(prototype, (EventEmitter as any).prototype); +console.log("setproto:", true); + +// Non-constructor native exports keep prototype === undefined (no spurious +// synthesis), matching Node's built-in non-constructor functions. +console.log("nonctor:", (fs as any).readFile.prototype === undefined); +"#, + ); + assert_eq!( + stdout, + "rs: true\nws: true\nee: true\ncreate: true\nchain: true\nsetproto: true\nnonctor: true\n" + ); +} diff --git a/test-files/test_issue_5268_native_ctor_prototype_undefined.ts b/test-files/test_issue_5268_native_ctor_prototype_undefined.ts new file mode 100644 index 000000000..0b7e1d89f --- /dev/null +++ b/test-files/test_issue_5268_native_ctor_prototype_undefined.ts @@ -0,0 +1,47 @@ +// #5268: native-module constructor *class* exports must expose a real +// `.prototype` object so the userland subclass idioms +// +// ReadStream.prototype = Object.create(fs$ReadStream.prototype) // graceful-fs +// Object.setPrototypeOf(prototype, EventEmitter.prototype) // pino +// +// don't throw "Object prototype may only be an Object or null: undefined". +// +// Pre-fix, `fs.ReadStream`/`fs.WriteStream`/`events.EventEmitter` were +// truthy callable closures (bound-native exports) whose `.prototype` +// read resolved to `undefined`: `ordinary_function_prototype_value_for_read` +// short-circuited to `None` for every bound-native export except a +// hardcoded http/https whitelist. graceful-fs guards on truthiness of +// `fs$ReadStream` (truthy) then calls `Object.create(fs$ReadStream.prototype)` +// → `Object.create(undefined)` → throw. The fix recognizes constructor-cased +// bound-native exports (leading uppercase, not flagged non-constructable) and +// lets the synthetic-class path materialize a stable `.prototype` object. + +import * as fs from "node:fs"; +import { EventEmitter } from "node:events"; + +// 1. Native constructor classes expose an object `.prototype` (not undefined). +const rsProto = (fs as any).ReadStream.prototype; +const wsProto = (fs as any).WriteStream.prototype; +const eeProto = (EventEmitter as any).prototype; +console.log("fs.ReadStream.prototype is object:", typeof rsProto === "object" && rsProto !== null); +console.log("fs.WriteStream.prototype is object:", typeof wsProto === "object" && wsProto !== null); +console.log("EventEmitter.prototype is object:", typeof eeProto === "object" && eeProto !== null); + +// 2. graceful-fs's clone() ReadStream pattern: `Object.create(Ctor.prototype)` +// must not throw and must yield an object whose proto is Ctor.prototype. +const sub = Object.create((fs as any).ReadStream.prototype); +console.log("Object.create(ReadStream.prototype) ok:", typeof sub === "object" && sub !== null); +console.log( + "subclass proto chains to ReadStream.prototype:", + Object.getPrototypeOf(sub) === (fs as any).ReadStream.prototype, +); + +// 3. pino's proto.js pattern: `Object.setPrototypeOf(prototype, EventEmitter.prototype)` +// must not throw on the (now real) EventEmitter.prototype operand. +const prototype: any = { child() { return null; } }; +Object.setPrototypeOf(prototype, (EventEmitter as any).prototype); +console.log("setPrototypeOf(obj, EventEmitter.prototype) ok:", true); + +// 4. Non-constructor native exports keep `prototype === undefined`, matching +// Node's built-in non-constructor functions (no spurious synthesis). +console.log("fs.readFile.prototype is undefined:", (fs as any).readFile.prototype === undefined); diff --git a/test-files/test_issue_pino_prototype_undefined.ts b/test-files/test_issue_pino_prototype_undefined.ts index faad8a8a4..a7536f62e 100644 --- a/test-files/test_issue_pino_prototype_undefined.ts +++ b/test-files/test_issue_pino_prototype_undefined.ts @@ -75,9 +75,9 @@ const eeKind = typeof (EventEmitter as any); console.log("ee_truthy:", eeKind !== "undefined"); // 2. Reading `.prototype` on the destructured EventEmitter must not -// throw. Returning `undefined` here is acceptable (Perry doesn't -// materialize a real EventEmitter.prototype object yet); the -// contract is just "doesn't trip the TypeError". +// throw. As of #5268 Perry materializes a real EventEmitter.prototype +// object (constructor-cased bound-native exports get a synthesized +// `.prototype`); the contract is just "doesn't trip the TypeError". const proto = (EventEmitter as any).prototype; console.log("proto_read_ok:", proto !== "__threw__");