fix(runtime): a #private method or [Symbol.iterator] alias is not 'deleted' after an unrelated prototype mutation (#11692)

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`.

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.

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());

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"]);