From c85083892290427d67fcd2d9c2dcc937185f9c60 Mon Sep 17 00:00:00 2001 From: Yorkie Makoto Date: Fri, 19 Jun 2026 22:48:13 +0800 Subject: [PATCH 1/3] Fix exotic get_own_property descriptor presence flags Set QuickJS JS_PROP_HAS_* bits when returning descriptors from exotic class get_own_property hooks, preventing descriptor consumers such as Object.keys() and Object.getOwnPropertyDescriptor() from reading invalid descriptor state. Extend the handwritten exotic class test to cover enumerable own property names, Object.keys(), writable data descriptors, and accessor descriptors. --- core/src/class.rs | 56 ++++++++++++++++++++++++++++++++++++++----- core/src/class/ffi.rs | 3 +++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/core/src/class.rs b/core/src/class.rs index c537023..d542439 100644 --- a/core/src/class.rs +++ b/core/src/class.rs @@ -998,6 +998,7 @@ mod test { println!("Got atom: {}", atom.to_string()?); if atom.to_string()? == "hello" || atom.to_string()? == "i" + || atom.to_string()? == "accessor" || atom.to_string()? == "toString" { return Ok(true); @@ -1023,14 +1024,31 @@ mod test { ) -> crate::Result>> { let name = atom.to_string()?; if name == "hello" || name == "i" { - let value = if name == "hello" { - "world".into_js(ctx)? + let (value, writable) = if name == "hello" { + ("world".into_js(ctx)?, false) } else { - this.borrow().i.into_js(ctx)? + (this.borrow().i.into_js(ctx)?, true) }; Ok(Some(super::PropertyDescriptor::new_value( - value, true, true, false, + value, true, true, writable, ))) + } else if name == "accessor" { + let getter = Function::new(ctx.clone(), || { + Ok::<&'static str, crate::Error>("accessor-value") + })? + .into_value(); + let setter = + Function::new(ctx.clone(), |_value: String| Ok::<(), crate::Error>(()))? + .into_value(); + Ok(Some(super::PropertyDescriptor { + value: crate::Value::new_undefined(ctx.clone()), + getter, + setter, + configurable: true, + enumerable: true, + writable: false, + is_getset: true, + })) } else { Ok(None) } @@ -1049,6 +1067,10 @@ mod test { atom: crate::Atom::from_str(ctx.clone(), "i")?, is_enumerable: true, }, + super::PropertyName { + atom: crate::Atom::from_str(ctx.clone(), "accessor")?, + is_enumerable: true, + }, ]) } } @@ -1096,6 +1118,7 @@ mod test { assert(exotic?.toString() === 'class Exotic { [native code] }', `exotic.toString() should be 'class Exotic { [native code] }' but is ${exotic?.toString()}`); assert('i' in exotic, 'i should be in exotic'); assert('hello' in exotic, 'hello should be in exotic'); + assert('accessor' in exotic, 'accessor should be in exotic'); assert(!('foo' in exotic), 'foo should not be in exotic'); try { @@ -1118,13 +1141,17 @@ mod test { // Test Object.getOwnPropertyNames() (uses get_own_property_names) let ownNames = Object.getOwnPropertyNames(exotic); - assert(ownNames.length === 2, `getOwnPropertyNames should return 2, got ${ownNames.length}`); + assert(ownNames.length === 3, `getOwnPropertyNames should return 3, got ${ownNames.length}`); assert(ownNames.includes('hello'), 'getOwnPropertyNames should include hello'); assert(ownNames.includes('i'), 'getOwnPropertyNames should include i'); + assert(ownNames.includes('accessor'), 'getOwnPropertyNames should include accessor'); // Test Object.keys() (uses get_own_property_names + get_own_property) let keys = Object.keys(exotic); - assert(keys.length === 2, `Object.keys should return 2 keys, got ${keys.length}`); + assert(keys.length === 3, `Object.keys should return 3 keys, got ${keys.length}`); + assert(keys.includes('hello'), 'Object.keys should include hello'); + assert(keys.includes('i'), 'Object.keys should include i'); + assert(keys.includes('accessor'), 'Object.keys should include accessor'); // Test Object.getOwnPropertyDescriptor() (uses get_own_property) let desc = Object.getOwnPropertyDescriptor(exotic, 'hello'); @@ -1134,6 +1161,23 @@ mod test { assert(desc.enumerable === true, 'hello should be enumerable'); assert(desc.writable === false, 'hello should not be writable'); + let writableDesc = Object.getOwnPropertyDescriptor(exotic, 'i'); + assert(writableDesc !== undefined, 'descriptor for i should exist'); + assert(writableDesc.value === 42, `descriptor value should be 42, got ${writableDesc.value}`); + assert(writableDesc.configurable === true, 'i should be configurable'); + assert(writableDesc.enumerable === true, 'i should be enumerable'); + assert(writableDesc.writable === true, 'i should be writable'); + + let accessorDesc = Object.getOwnPropertyDescriptor(exotic, 'accessor'); + assert(accessorDesc !== undefined, 'descriptor for accessor should exist'); + assert(accessorDesc.value === undefined, 'accessor should not have a value field'); + assert(typeof accessorDesc.get === 'function', 'accessor getter should be a function'); + assert(typeof accessorDesc.set === 'function', 'accessor setter should be a function'); + assert(accessorDesc.get() === 'accessor-value', 'accessor getter should return accessor-value'); + assert(accessorDesc.configurable === true, 'accessor should be configurable'); + assert(accessorDesc.enumerable === true, 'accessor should be enumerable'); + assert(accessorDesc.writable === undefined, 'accessor should not have a writable field'); + // Non-existent property returns undefined descriptor assert(Object.getOwnPropertyDescriptor(exotic, 'nonexistent') === undefined, 'nonexistent should be undefined'); diff --git a/core/src/class/ffi.rs b/core/src/class/ffi.rs index c6ccac5..48d7275 100644 --- a/core/src/class/ffi.rs +++ b/core/src/class/ffi.rs @@ -407,10 +407,13 @@ impl VTable { } if property.is_getset { flags |= qjs::JS_PROP_GETSET as qjs::c_int; + flags |= qjs::JS_PROP_HAS_GET as qjs::c_int; + flags |= qjs::JS_PROP_HAS_SET as qjs::c_int; (*desc).getter = property.getter.into_js_value(); (*desc).setter = property.setter.into_js_value(); (*desc).value = qjs::JS_UNDEFINED; } else { + flags |= qjs::JS_PROP_HAS_VALUE as qjs::c_int; if property.writable { flags |= qjs::JS_PROP_WRITABLE as qjs::c_int; } From 724ee8a4277d964a1aeed4119082464af84e8412 Mon Sep 17 00:00:00 2001 From: Yorkie Makoto Date: Fri, 19 Jun 2026 23:25:45 +0800 Subject: [PATCH 2/3] fix --- core/src/class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/class.rs b/core/src/class.rs index d542439..655d1e9 100644 --- a/core/src/class.rs +++ b/core/src/class.rs @@ -729,7 +729,7 @@ mod test { Function::new( ctx.clone(), |this: This>>| -> crate::Result { - Ok(format!("{:?}", &this.0.borrow().d)) + Ok(format!("{:?}", this.0.borrow().d)) }, ), )?; From a28a4695181339f15f63b5c8c7f6877685ac311a Mon Sep 17 00:00:00 2001 From: Yorkie Makoto Date: Sat, 20 Jun 2026 00:00:50 +0800 Subject: [PATCH 3/3] fixup --- core/src/class/ffi.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/class/ffi.rs b/core/src/class/ffi.rs index 48d7275..a633ba8 100644 --- a/core/src/class/ffi.rs +++ b/core/src/class/ffi.rs @@ -398,7 +398,9 @@ impl VTable { match C::exotic_get_own_property(&this_ptr.as_ref().data, &ctx, atom) { Ok(Some(property)) => { if !desc.is_null() { - let mut flags: qjs::c_int = 0; + let mut flags: qjs::c_int = (qjs::JS_PROP_HAS_CONFIGURABLE + | qjs::JS_PROP_HAS_ENUMERABLE) + as qjs::c_int; if property.configurable { flags |= qjs::JS_PROP_CONFIGURABLE as qjs::c_int; } @@ -414,6 +416,7 @@ impl VTable { (*desc).value = qjs::JS_UNDEFINED; } else { flags |= qjs::JS_PROP_HAS_VALUE as qjs::c_int; + flags |= qjs::JS_PROP_HAS_WRITABLE as qjs::c_int; if property.writable { flags |= qjs::JS_PROP_WRITABLE as qjs::c_int; }