Skip to content

Commit 9dce433

Browse files
committed
Fix property write semantics: handle non-extensible and non-writable cases
- Enforce non-extensible objects: prevent adding new own properties (throw TypeError) (`src/core/value.rs`) - Ensure assignment respects non-writable properties and throws TypeError when appropriate (`src/core/eval.rs`) - Honor "writable" in Object.defineProperty (mark properties non-writable) (`src/js_object.rs`) - Remove temporary debug logs from `js-scripts/object_tests_01.js`
1 parent 3de9362 commit 9dce433

File tree

5 files changed

+47
-2
lines changed

5 files changed

+47
-2
lines changed

.github/workflows/test262.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ jobs:
3232
env:
3333
FAIL_ON_FAILURE: 'true'
3434
run: |
35-
bash ci/run_test262.sh --limit 100 --fail-on-failure --focus language
35+
bash ci/run_test262.sh --limit 110 --fail-on-failure --focus language
3636
3737
- name: Upload results
3838
if: ${{ !cancelled() }}

js-scripts/object_tests_01.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,19 @@ function assert(mustBeTrue, message) {
5656
}
5757
}
5858

59+
{
60+
console.log("==== Test non-writable property ====");
61+
62+
var _8_7_2_3 = {};
63+
Object.defineProperty(_8_7_2_3, "b", {
64+
writable: false
65+
});
66+
try {
67+
_8_7_2_3.b = 11;
68+
assert(false, 'Assigning to a non-writable property should throw a TypeError');
69+
} catch (e) {
70+
assert(e instanceof TypeError);
71+
}
72+
}
73+
5974
return true;

src/core/eval.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6091,6 +6091,10 @@ fn set_property_with_accessors<'gc>(
60916091
"Cannot set property which has only a getter"
60926092
)));
60936093
}
6094+
// If the existing property is non-writable, TypeError should be thrown
6095+
if !obj.borrow().is_writable(key) {
6096+
return Err(EvalError::Js(crate::raise_type_error!("Cannot assign to read-only property")));
6097+
}
60946098
object_set_key_value(mc, obj, key, val).map_err(EvalError::Js)?;
60956099
Ok(())
60966100
}
@@ -6099,6 +6103,10 @@ fn set_property_with_accessors<'gc>(
60996103
"Cannot set property which has only a getter"
61006104
))),
61016105
_ => {
6106+
// For plain existing properties, respect writability
6107+
if !obj.borrow().is_writable(key) {
6108+
return Err(EvalError::Js(crate::raise_type_error!("Cannot assign to read-only property")));
6109+
}
61026110
object_set_key_value(mc, obj, key, val).map_err(EvalError::Js)?;
61036111
Ok(())
61046112
}

src/core/value.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -812,8 +812,22 @@ pub fn object_set_key_value<'gc>(
812812
) -> Result<(), JSError> {
813813
let key = key.into();
814814

815-
// Disallow creating new own properties on non-extensible objects
815+
// Debug log to help diagnose non-extensible assignment behavior
816816
let exists = obj.borrow().properties.contains_key(&key);
817+
let key_desc = match &key {
818+
PropertyKey::String(s) => s.clone(),
819+
PropertyKey::Symbol(_) => "<symbol>".to_string(),
820+
};
821+
let obj_addr = format!("{:p}", &*obj.borrow());
822+
log::debug!(
823+
"object_set_key_value: obj={} key={} key_exists={} extensible={}",
824+
obj_addr,
825+
key_desc,
826+
exists,
827+
obj.borrow().is_extensible()
828+
);
829+
830+
// Disallow creating new own properties on non-extensible objects
817831
if !exists && !obj.borrow().is_extensible() {
818832
return Err(raise_type_error!("Cannot add property to non-extensible object"));
819833
}

src/js_object.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,14 @@ fn define_property_internal<'gc>(
180180
setter: setter_opt,
181181
};
182182

183+
// If writable flag explicitly set to false, mark property as non-writable
184+
if let Some(wrc) = object_get_key_value(desc_obj, "writable")
185+
&& let Value::Boolean(is_writable) = &*wrc.borrow()
186+
&& !*is_writable
187+
{
188+
target_obj.borrow_mut(mc).set_non_writable(prop_key.clone());
189+
}
190+
183191
// Install property on target object
184192
object_set_key_value(mc, target_obj, &prop_key, prop_descriptor)?;
185193
Ok(())

0 commit comments

Comments
 (0)