diff --git a/changelog.d/11697-private-and-symbol-method-not-deleted.md b/changelog.d/11697-private-and-symbol-method-not-deleted.md new file mode 100644 index 0000000000..225f2346aa --- /dev/null +++ b/changelog.d/11697-private-and-symbol-method-not-deleted.md @@ -0,0 +1 @@ +Fix a #11667 regression: after any `Object.setPrototypeOf` in the program, a `#private` method or a `[Symbol.iterator]` (or other well-known-symbol) method was treated as deleted, because `class_proto_key_deleted` looked for its string key on the prototype object, where such members never have one. This broke node-redis (`#validateOptions is not a function`) and mongodb (`@@iterator is not a function` in `new MongoClient`) compiled from source. Those members are now never considered deleted by the absence of a string key. A source method literally named `"@@iterator"` is still deletable. Tests: `test_gap_class_private_method_after_prototype_mutation.ts`, `test_gap_class_symbol_iterator_after_prototype_mutation.ts`. diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index 9a5c88c69e..bd7a68d4d5 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -57,7 +57,7 @@ pub(crate) fn class_proto_key_deleted(class_id: u32, name: &str) -> bool { let declared = name == "constructor" || class_own_accessor_ptrs(class_id, name).is_some() || super::super::native_module::class_has_own_method(class_id, name); - if !declared { + if !declared || proto_member_has_no_string_key(class_id, name) { return false; } let proto = class_decl_prototype_object(class_id); @@ -91,6 +91,29 @@ pub(crate) fn class_proto_key_deleted(class_id: u32, name: &str) -> bool { } } +/// A declared prototype member that the reflective prototype object never +/// carries under the string key `name`, so the absence of that key proves +/// nothing about a `delete` (#11692): a `#private` method (not a property at +/// all; `delete` cannot reach it) and the synthetic dispatch alias of a +/// well-known-symbol method (`@@iterator` stands in for `[Symbol.iterator]`, +/// which lives under its symbol key). A source method literally named +/// `"@@iterator"` has a string-member order registration and is a real key. +fn proto_member_has_no_string_key(class_id: u32, name: &str) -> bool { + if name.starts_with('#') { + return true; + } + internal_symbol_dispatch_alias(name) + && !CLASS_STRING_MEMBER_ORDERS + .read() + .ok() + .and_then(|guard| { + guard + .as_ref() + .map(|map| map.contains_key(&(class_id, false, name.to_string()))) + }) + .unwrap_or(false) +} + /// Has `delete` removed class `class_id`'s own static member `name` (a /// ClassBody static method or accessor, or the intrinsic `name` / `length`)? /// Derived from the class function object that owns it. diff --git a/test-files/test_gap_class_private_method_after_prototype_mutation.ts b/test-files/test_gap_class_private_method_after_prototype_mutation.ts new file mode 100644 index 0000000000..7d01132e6d --- /dev/null +++ b/test-files/test_gap_class_private_method_after_prototype_mutation.ts @@ -0,0 +1,67 @@ +// #11692: a `#private` method called through a subclass whose parent is a +// runtime value (`class extends Base {}` built by a helper) must keep +// resolving after any prototype mutation elsewhere in the program. #11667 +// derived "this declared prototype member was deleted" from "its string key is +// not on the prototype object once the prototype guards are invalidated", and a +// private method is never a string key there, so the first unrelated +// `Object.setPrototypeOf` made every such call throw `#<...#m> is not a +// function` (node-redis: `this.#validateOptions` in `new RedisClient`). + +class Client { + opts: any; + static #made = 0; + constructor(o: any) { + this.#validate(o); + this.opts = this.#init(o); + Client.#made = Client.#count() + 1; + } + #validate(o: any) { + if (o && o.bad) throw new TypeError("bad options"); + } + #init(o: any) { + return { ...o, ok: true }; + } + static #count() { + return Client.#made; + } + static made() { + return Client.#count(); + } +} + +function attach(Base: any) { + return class extends Base {}; +} + +const Sub = attach(Client); +console.log("before:", new Sub({ a: 1 }).opts.ok, Client.made()); + +// An unrelated prototype mutation invalidates every prototype fast guard. +const unrelated: any = {}; +Object.setPrototypeOf(unrelated, { x: 1 }); + +console.log("after setPrototypeOf:", new Sub({ a: 2 }).opts.ok, Client.made()); +console.log("direct:", new Client({}).opts.ok, Client.made()); +try { + new Sub({ bad: true }); +} catch (e: any) { + console.log("validate still runs:", e instanceof TypeError, e.message); +} + +// Deleting an ordinary method is still observed through the same path. +class WithMethod { + #secret() { + return "s"; + } + m() { + return "m"; + } + reveal() { + return this.#secret(); + } +} +const Sub2 = attach(WithMethod); +const w: any = new Sub2(); +console.log("method:", w.m(), w.reveal()); +delete (WithMethod.prototype as any).m; +console.log("deleted method:", typeof w.m, "private still:", w.reveal()); diff --git a/test-files/test_gap_class_symbol_iterator_after_prototype_mutation.ts b/test-files/test_gap_class_symbol_iterator_after_prototype_mutation.ts new file mode 100644 index 0000000000..6328946cbf --- /dev/null +++ b/test-files/test_gap_class_symbol_iterator_after_prototype_mutation.ts @@ -0,0 +1,71 @@ +// #11692: a class's `[Symbol.iterator]()` method must keep driving +// iteration after a prototype mutation elsewhere. #11667 derived "this +// declared prototype member was deleted" from "its string key is not on the +// prototype object once the prototype guards are invalidated"; a +// `[Symbol.iterator]` method is dispatched under the synthetic name +// `@@iterator` but lives under its symbol key, so the first +// `Object.setPrototypeOf` anywhere made `Array.from(x)` / `for...of` throw +// `@@iterator is not a function` (mongodb's parseOptions, via +// mongodb-connection-string-url re-parenting whatwg-url's URLSearchParams). + +class Impl { + _list: any[] = [["x", "1"], ["y", "2"]]; + [Symbol.iterator]() { + return this._list[Symbol.iterator](); + } +} + +class Params { + keys() { + return ["k1", "k2"][Symbol.iterator](); + } +} +class Upper extends Params {} + +const impl = new Impl(); +console.log("before:", JSON.stringify(Array.from(impl))); + +const p: any = new Params(); +Object.setPrototypeOf(p, Upper.prototype); + +console.log("after setPrototypeOf:", JSON.stringify(Array.from(impl))); +for (const [k, v] of impl) console.log("for-of:", k, v); +console.log("spread:", [...impl].length); +for (const k of p.keys()) console.log("re-parented keys:", k); + +// The mongodb-connection-string-url shape: a wrapper class built around a +// runtime parent re-parents an existing instance; its iterator forwards to the +// parent's through `super`. +const implKey = Symbol("impl"); +class SP { + constructor() { + (this as any)[implKey] = new Impl(); + } + entries() { + return Array.from((this as any)[implKey])[Symbol.iterator](); + } + [Symbol.iterator]() { + return this.entries(); + } +} +function caseInsensitive(Ctor: any) { + return class CI extends Ctor { + entries() { + return super.entries(); + } + }; +} +const sp: any = new SP(); +Object.setPrototypeOf(sp, caseInsensitive(SP).prototype); +for (const [k, v] of sp) console.log("wrapped:", k, v); + +// A method literally named "@@iterator" is an ordinary string key. +class Literal { + ["@@iterator"]() { + return "literal"; + } +} +const lit: any = new Literal(); +console.log("literal:", lit["@@iterator"]()); +delete (Literal.prototype as any)["@@iterator"]; +console.log("literal deleted:", typeof lit["@@iterator"]);