diff --git a/changelog.d/10643-webcrypto-method-identity.md b/changelog.d/10643-webcrypto-method-identity.md new file mode 100644 index 0000000000..2a127a7cbf --- /dev/null +++ b/changelog.d/10643-webcrypto-method-identity.md @@ -0,0 +1,3 @@ +### Fixed + +- `globalThis.crypto.randomUUID`, `.getRandomValues`, and `crypto.subtle`'s KEM methods (`encapsulateBits`/`decapsulateBits`/`encapsulateKey`/`decapsulateKey`) now have a stable identity across reads (`crypto.randomUUID === crypto.randomUUID` is `true`) instead of allocating a fresh closure on every property access. diff --git a/crates/perry-runtime/src/object/global_this/ctor_thunks.rs b/crates/perry-runtime/src/object/global_this/ctor_thunks.rs index 2468db130b..cb50412369 100644 --- a/crates/perry-runtime/src/object/global_this/ctor_thunks.rs +++ b/crates/perry-runtime/src/object/global_this/ctor_thunks.rs @@ -382,6 +382,16 @@ pub(crate) extern "C" fn cryptokey_usages_getter_thunk( cryptokey_property_getter(b"usages") } +/// #10427: `globalThis.crypto.` is a property READ, resolved fresh +/// on every access through `vt_get_own_field` (there is no real `ObjectHeader` +/// backing `globalThis.crypto` for the read to land an own slot on — see +/// `crypto.webcrypto`'s NATIVE_MODULE_CLASS_ID namespace). Plain +/// `js_closure_alloc` mints a brand-new `ClosureHeader` on every call, so +/// `crypto.randomUUID === crypto.randomUUID` was `false` and every read +/// allocated. `js_closure_alloc_singleton` (the same func-ptr-keyed cache PR +/// #10630 traced the closure-identity contract back to) returns the SAME +/// closure for the same `func_ptr` every time — the func_ptr IS the method +/// identity here since these thunks take no captures. pub(crate) fn webcrypto_method_value(property_name: &str) -> Option { let (func_ptr, arity) = match property_name { "getRandomValues" => (webcrypto_get_random_values_thunk as *const u8, 1), @@ -389,7 +399,7 @@ pub(crate) fn webcrypto_method_value(property_name: &str) -> Option { _ => return None, }; crate::closure::js_register_closure_arity(func_ptr, arity); - let closure = crate::closure::js_closure_alloc(func_ptr, 0); + let closure = crate::closure::js_closure_alloc_singleton(func_ptr); if closure.is_null() { return Some(f64::from_bits(crate::value::TAG_UNDEFINED)); } @@ -408,10 +418,14 @@ fn subtle_crypto_method_spec(property_name: &str) -> Option<(*const u8, u32)> { } } +/// Same per-read allocation defect as `webcrypto_method_value` above, for +/// `crypto.subtle`'s KEM methods (`encapsulateBits` and friends — the rest of +/// SubtleCrypto's surface is already cached via `bound_native_callable_export_value`, +/// see #10427's PR body for which paths were and weren't affected). pub(crate) fn subtle_crypto_method_value(property_name: &str) -> Option { let (func_ptr, length) = subtle_crypto_method_spec(property_name)?; crate::closure::js_register_closure_rest(func_ptr, 0); - let closure = crate::closure::js_closure_alloc(func_ptr, 0); + let closure = crate::closure::js_closure_alloc_singleton(func_ptr); if closure.is_null() { return Some(f64::from_bits(crate::value::TAG_UNDEFINED)); } diff --git a/test-files/test_gap_10427_webcrypto_method_identity.ts b/test-files/test_gap_10427_webcrypto_method_identity.ts new file mode 100644 index 0000000000..5571479eb6 --- /dev/null +++ b/test-files/test_gap_10427_webcrypto_method_identity.ts @@ -0,0 +1,61 @@ +// #10427: reading a Web Crypto method off `globalThis.crypto` (and +// `crypto.subtle`) allocated a fresh closure on every access, so the method +// had no stable identity (`crypto.randomUUID === crypto.randomUUID` was +// `false`) and every read allocated. Root cause: `webcrypto_method_value` / +// `subtle_crypto_method_value` in +// crates/perry-runtime/src/object/global_this/ctor_thunks.rs called plain +// `js_closure_alloc` instead of the func-ptr-keyed `js_closure_alloc_singleton` +// cache. This covers every member of `globalThis.crypto` (`randomUUID`, +// `getRandomValues`, `subtle` itself, and `subtle`'s methods including the +// KEM pair that shared the same defect) plus `node:crypto`'s default import +// as an already-correct control. +import nodeCrypto from "node:crypto"; +import { randomBytes as namedRandomBytes, randomUUID as namedRandomUUID } from "node:crypto"; + +// The issue's own repro, verbatim. +console.log("randomUUID stable:", globalThis.crypto.randomUUID === globalThis.crypto.randomUUID); +console.log("node:crypto randomUUID stable:", nodeCrypto.randomUUID === nodeCrypto.randomUUID); +const seen = new Set(); +for (let i = 0; i < 3; i++) seen.add(globalThis.crypto.randomUUID); +console.log("seen.size:", seen.size); + +// The other Web Crypto members the issue asked to check. +console.log("getRandomValues stable:", globalThis.crypto.getRandomValues === globalThis.crypto.getRandomValues); +console.log("crypto namespace stable:", globalThis.crypto === globalThis.crypto); +console.log("subtle namespace stable:", globalThis.crypto.subtle === globalThis.crypto.subtle); +console.log("crypto.subtle === crypto.subtle (2nd read pair):", crypto.subtle === crypto.subtle); + +// crypto.subtle's KEM methods went through the same broken per-read thunk as +// randomUUID/getRandomValues. +console.log("subtle.encapsulateBits stable:", crypto.subtle.encapsulateBits === crypto.subtle.encapsulateBits); +console.log("subtle.decapsulateBits stable:", crypto.subtle.decapsulateBits === crypto.subtle.decapsulateBits); +console.log("subtle.encapsulateKey stable:", crypto.subtle.encapsulateKey === crypto.subtle.encapsulateKey); +console.log("subtle.decapsulateKey stable:", crypto.subtle.decapsulateKey === crypto.subtle.decapsulateKey); + +// Controls: subtle's other methods were already cached via a different +// mechanism (bound_native_callable_export_value) — must stay stable too. +console.log("subtle.digest stable:", crypto.subtle.digest === crypto.subtle.digest); +console.log("subtle.encrypt stable:", crypto.subtle.encrypt === crypto.subtle.encrypt); +console.log("subtle.generateKey stable:", crypto.subtle.generateKey === crypto.subtle.generateKey); + +// Cross-read identity: the SAME closure across DIFFERENT expressions that +// resolve to the same property, not just repeated reads of one expression. +const a = globalThis.crypto.randomUUID; +const b = crypto.randomUUID; +console.log("cross-read identity:", a === b); + +// node:crypto (module-level) default vs named import identity — already +// correct before this fix; kept as a control so a future regression there +// shows up in the same test. +console.log("node:crypto default randomUUID === named randomUUID:", nodeCrypto.randomUUID === namedRandomUUID); +console.log("node:crypto randomBytes stable:", nodeCrypto.randomBytes === nodeCrypto.randomBytes); +console.log("node:crypto named randomBytes === default randomBytes:", namedRandomBytes === nodeCrypto.randomBytes); + +// Functional sanity: the cached closure still WORKS (shape check only — the +// value itself is random, so no exact value is printed). +const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; +const u = globalThis.crypto.randomUUID(); +console.log("randomUUID() shape ok:", uuidPattern.test(u)); +console.log("randomUUID() distinct across calls:", globalThis.crypto.randomUUID() !== globalThis.crypto.randomUUID()); +const bytes = globalThis.crypto.getRandomValues(new Uint8Array(8)); +console.log("getRandomValues() length ok:", bytes.length === 8);