Skip to content

Commit f33c347

Browse files
Ralphclaude
andcommitted
fix(runtime): #5588 strict Set check on proxy-source symbol keys
Address CodeRabbit review: the proxy-source path wrote symbol keys via `js_object_set_symbol_property` directly, skipping the strict-`Set` rejection (read-only symbol data prop, setter-less accessor, or new symbol on a non-extensible target) the ordinary source path applies. A Proxy source's enumerable symbol key could thus bypass the required TypeError on a frozen/read-only target. Extract that block into `object_assign_throw_if_symbol_set_rejected` and call it from both the proxy and ordinary symbol-write paths. Extend the gap test with symbol-key copying, the frozen-target rejection, and an explicit ownKeys→getOwnPropertyDescriptor→get trap-order assertion — all byte-for-byte vs `node --experimental-strip-types`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e48e96f commit f33c347

2 files changed

Lines changed: 57 additions & 24 deletions

File tree

crates/perry-runtime/src/object/alloc.rs

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,34 @@ fn throw_object_assign_readonly(name: &str) -> ! {
938938
)
939939
}
940940

941+
/// Strict `Set(to, sym, value, true)` rejection check for a symbol-keyed
942+
/// `Object.assign` write: a non-writable existing symbol data property, an
943+
/// accessor symbol property with no setter, or a new symbol property on a
944+
/// non-extensible target each make the write fail, which under throwing `Set`
945+
/// semantics is a `TypeError`. The string-keyed counterpart is
946+
/// `object_assign_throw_if_set_rejected`.
947+
unsafe fn object_assign_throw_if_symbol_set_rejected(target: *mut ObjectHeader, sym_ptr: usize) {
948+
let owner = target as usize;
949+
let existing = crate::symbol::symbol_property_root_bits(owner, sym_ptr).is_some()
950+
|| crate::symbol::symbol_accessor_descriptor_bits(owner, sym_ptr).is_some();
951+
if existing {
952+
if let Some((_get, set)) = crate::symbol::symbol_accessor_descriptor_bits(owner, sym_ptr) {
953+
if set == 0 {
954+
throw_object_assign_readonly("Symbol()");
955+
}
956+
} else if let Some(attrs) = crate::symbol::get_symbol_property_attrs(owner, sym_ptr) {
957+
if !attrs.writable() {
958+
throw_object_assign_readonly("Symbol()");
959+
}
960+
}
961+
} else {
962+
let gc = gc_header_for(target);
963+
if (*gc)._reserved & crate::gc::OBJ_FLAG_NO_EXTEND != 0 {
964+
throw_object_assign_readonly("Symbol()");
965+
}
966+
}
967+
}
968+
941969
unsafe fn object_assign_set_string_key(
942970
target: *mut ObjectHeader,
943971
target_is_array: bool,
@@ -1041,6 +1069,9 @@ unsafe fn object_assign_proxy_source(
10411069
object_assign_set_string_key(target, target_is_array, key_ptr, value_f64);
10421070
}
10431071
} else if key.is_pointer() {
1072+
// Strict `Set` semantics for symbol keys, same as the ordinary path.
1073+
let sym_ptr = (key.bits() & crate::value::POINTER_MASK) as usize;
1074+
object_assign_throw_if_symbol_set_rejected(target, sym_ptr);
10441075
crate::symbol::js_object_set_symbol_property(target_f64, key_f64, value_f64);
10451076
}
10461077
}
@@ -1266,30 +1297,7 @@ pub unsafe extern "C" fn js_object_assign_one(target_f64: f64, source_f64: f64)
12661297
// copy already uses `[[Get]]` via `js_object_get_field_by_name`.
12671298
let value_f64 = crate::symbol::js_object_get_symbol_property(source_f64, sym_f64);
12681299
// Strict `Set` semantics for symbol-keyed writes too.
1269-
{
1270-
let owner = tgt_raw;
1271-
let existing = crate::symbol::symbol_property_root_bits(owner, sym_ptr).is_some()
1272-
|| crate::symbol::symbol_accessor_descriptor_bits(owner, sym_ptr).is_some();
1273-
if existing {
1274-
if let Some((_get, set)) =
1275-
crate::symbol::symbol_accessor_descriptor_bits(owner, sym_ptr)
1276-
{
1277-
if set == 0 {
1278-
throw_object_assign_readonly("Symbol()");
1279-
}
1280-
} else if let Some(attrs) = crate::symbol::get_symbol_property_attrs(owner, sym_ptr)
1281-
{
1282-
if !attrs.writable() {
1283-
throw_object_assign_readonly("Symbol()");
1284-
}
1285-
}
1286-
} else {
1287-
let gc = gc_header_for(target);
1288-
if (*gc)._reserved & crate::gc::OBJ_FLAG_NO_EXTEND != 0 {
1289-
throw_object_assign_readonly("Symbol()");
1290-
}
1291-
}
1292-
}
1300+
object_assign_throw_if_symbol_set_rejected(target, sym_ptr);
12931301
crate::symbol::js_object_set_symbol_property(target_f64, sym_f64, value_f64);
12941302
}
12951303

test-files/test_gap_5588_object_assign_proxy_source.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,28 @@ const doubler = new Proxy({ x: 2, y: 3 }, {
4444
get(t: any, k) { return typeof t[k] === "number" ? t[k] * 2 : t[k]; },
4545
});
4646
console.log(JSON.stringify(Object.assign({}, doubler))); // {"x":4,"y":6}
47+
48+
// ---- symbol keys are copied through the traps ----
49+
const sym = Symbol("s");
50+
const symSrc: any = { plain: 1 };
51+
symSrc[sym] = 42;
52+
const symOut: any = Object.assign({}, new Proxy(symSrc, {}));
53+
console.log("symbol copied: " + (symOut[sym] === 42 && symOut.plain === 1)); // true
54+
55+
// ---- strict Set: a symbol key onto a non-extensible target throws ----
56+
const frozenTarget = Object.preventExtensions({});
57+
const symProxy = new Proxy(symSrc, {});
58+
console.log(thrown(() => Object.assign(frozenTarget, symProxy))); // TypeError
59+
60+
// ---- trap order: ownKeys -> getOwnPropertyDescriptor -> get, per key ----
61+
const order: string[] = [];
62+
const ordered = new Proxy({ k: 7 }, {
63+
ownKeys(t) { order.push("ownKeys"); return Reflect.ownKeys(t); },
64+
getOwnPropertyDescriptor(t, k) {
65+
order.push("gopd:" + String(k));
66+
return Reflect.getOwnPropertyDescriptor(t, k);
67+
},
68+
get(t: any, k) { order.push("get:" + String(k)); return t[k]; },
69+
});
70+
Object.assign({}, ordered);
71+
console.log("trap order: " + order.join(",")); // ownKeys,gopd:k,get:k

0 commit comments

Comments
 (0)