* fix(js): structuredClone cycles through Error.cause and clones own __proto__ (#419, #420) The structuredClone polyfill had two correctness/parity gaps in _structuredClone: - The Error branch recursed into `cause` before recording the clone in `seen`, so a cycle through the error (e.cause === e) recursed until the stack overflowed. Chrome clones such graphs and preserves identity. Fix: seen.set the clone before recursing into cause. (#419) - The generic-object branch built the clone on the source's prototype and copied properties by assignment, so an own enumerable `__proto__` data property (exactly what JSON.parse('{"__proto__":…}') produces) went through the inherited __proto__ setter and reparented the clone instead of copying the property. Fix: clone plain objects onto Object.prototype (like Chrome) and define each property instead of assigning it. (#420) Adds two parity tests to structured_clone_crypto_parity.rs; full obscura-cdp suite (73) green. * perf(js): keep structuredClone fast path for all keys but __proto__ The own-__proto__ fix copied every own property with Object.defineProperty to avoid triggering the inherited prototype setter. That is ~3x slower than plain assignment per key, so cloning a large object regressed (~75ms -> ~228ms for a 1000-key object x200). Only the __proto__ key needs defineProperty; every other key is safe to assign. Special-case it so the fast path applies to all other keys. Measured back to parity with main (85ms vs 85ms); the own-__proto__ correctness test still passes. --------- Co-authored-by: SGavrl <277843151+SGavrl@users.noreply.github.com>
This commit is contained in:
1 parent
f5dc97b131
commit
b2e4bb49a7
2 files changed
+89
-2
No files matched your search
@@ -183,6 +183,72 @@ async fn structured_clone_preserves_dataview() {
|
||||
assert_eq!(val["independent"], true, "clone must own its buffer, not alias the source");
|
||||
}
|
||||
|
||||
// A reference cycle through Error.cause must clone without crashing (issue
|
||||
// #419). The Error branch recursed into `cause` before recording itself in
|
||||
// `seen`, so a self-referential cause blew the stack. Chrome clones this and
|
||||
// preserves identity (clone.cause === clone).
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn structured_clone_handles_circular_error_cause() {
|
||||
let (mut ctx, sid) = setup().await;
|
||||
let v = eval(
|
||||
&mut ctx,
|
||||
2,
|
||||
r#"(async () => {
|
||||
const e = new Error("boom");
|
||||
e.cause = e;
|
||||
const out = {};
|
||||
try {
|
||||
const clone = structuredClone(e);
|
||||
out.ok = true;
|
||||
out.message = clone.message;
|
||||
out.selfCycle = clone.cause === clone;
|
||||
out.isError = clone instanceof Error;
|
||||
} catch (err) {
|
||||
out.ok = false;
|
||||
out.err = String(err && err.message || err);
|
||||
}
|
||||
return JSON.stringify(out);
|
||||
})()"#,
|
||||
&sid,
|
||||
)
|
||||
.await;
|
||||
let val = serde_json::from_str::<Value>(v["result"]["value"].as_str().unwrap()).unwrap();
|
||||
assert_eq!(val["ok"], true, "circular Error.cause crashed structuredClone: {:?}", val["err"]);
|
||||
assert_eq!(val["message"], "boom");
|
||||
assert_eq!(val["isError"], true, "clone must remain an Error");
|
||||
assert_eq!(val["selfCycle"], true, "cyclic cause must resolve to the clone, not a duplicate");
|
||||
}
|
||||
|
||||
// An own enumerable `__proto__` data property (what JSON.parse('{"__proto__":…}')
|
||||
// produces) must clone as an own data property, not be routed through the
|
||||
// inherited __proto__ setter (issue #420). Plain objects must also clone onto
|
||||
// Object.prototype, matching Chrome, rather than inheriting the source proto.
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn structured_clone_reproduces_own_proto_property() {
|
||||
let (mut ctx, sid) = setup().await;
|
||||
let v = eval(
|
||||
&mut ctx,
|
||||
2,
|
||||
r#"(async () => {
|
||||
const src = JSON.parse('{"__proto__":{"polluted":true},"a":1}');
|
||||
const clone = structuredClone(src);
|
||||
return JSON.stringify({
|
||||
hasOwnProto: Object.prototype.hasOwnProperty.call(clone, "__proto__"),
|
||||
plainProto: Object.getPrototypeOf(clone) === Object.prototype,
|
||||
polluted: clone.polluted === true,
|
||||
a: clone.a,
|
||||
});
|
||||
})()"#,
|
||||
&sid,
|
||||
)
|
||||
.await;
|
||||
let val = serde_json::from_str::<Value>(v["result"]["value"].as_str().unwrap()).unwrap();
|
||||
assert_eq!(val["hasOwnProto"], true, "own __proto__ data property was lost");
|
||||
assert_eq!(val["plainProto"], true, "plain object must clone onto Object.prototype");
|
||||
assert_eq!(val["polluted"], false, "clone prototype was reparented via the __proto__ setter");
|
||||
assert_eq!(val["a"], 1);
|
||||
}
|
||||
|
||||
// Functions and symbols are not structured-cloneable. The original early
|
||||
// `typeof !== "object"` return passed them through by reference instead of
|
||||
// throwing DataCloneError, so this guards both the throw and the name.
|
||||
|
||||
Vendored
+23
-2
@@ -5296,6 +5296,9 @@ function _structuredClone(value, seen) {
|
||||
if (value instanceof Error) {
|
||||
const Ctor = value.constructor || Error;
|
||||
const e = new Ctor(value.message);
|
||||
// Record the clone before recursing into `cause`, otherwise a cycle
|
||||
// through the error (e.cause === e) recurses until the stack overflows.
|
||||
seen.set(value, e);
|
||||
if (value.name) e.name = value.name;
|
||||
if (value.stack) e.stack = value.stack;
|
||||
if (value.cause !== undefined) e.cause = _structuredClone(value.cause, seen);
|
||||
@@ -5308,11 +5311,29 @@ function _structuredClone(value, seen) {
|
||||
const hook = globalThis.__obscura_clone_hooks[value[Symbol.toStringTag]];
|
||||
if (typeof hook === "function") return hook(value, seen);
|
||||
}
|
||||
const out = Array.isArray(value) ? [] : Object.create(Object.getPrototypeOf(value));
|
||||
// Plain objects clone onto Object.prototype (like Chrome), not the source's
|
||||
// prototype. Define each property instead of assigning it: a source with an
|
||||
// own enumerable `__proto__` data prop (what JSON.parse('{"__proto__":…}')
|
||||
// yields) would otherwise hit the inherited __proto__ setter and reparent
|
||||
// the clone instead of copying the property.
|
||||
const out = Array.isArray(value) ? [] : {};
|
||||
seen.set(value, out);
|
||||
for (const k in value) {
|
||||
if (Object.prototype.hasOwnProperty.call(value, k)) {
|
||||
out[k] = _structuredClone(value[k], seen);
|
||||
const cloned = _structuredClone(value[k], seen);
|
||||
// Only `__proto__` needs defineProperty: plain assignment would hit the
|
||||
// inherited prototype setter and reparent the clone instead of adding an
|
||||
// own data property. Every other key takes the fast assignment path.
|
||||
if (k === "__proto__") {
|
||||
Object.defineProperty(out, k, {
|
||||
value: cloned,
|
||||
writable: true,
|
||||
enumerable: true,
|
||||
configurable: true,
|
||||
});
|
||||
} else {
|
||||
out[k] = cloned;
|
||||
}
|
||||
}
|
||||
}
|
||||
// Symbols are not enumerable via for-in; copy own symbol-keyed properties.
|
||||
|
||||
Reference in new issue
Block a user