From fd8b09c60f9d074aca9ae3e8f14b47c8cc08ff62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 12 Jun 2026 23:07:18 +0200 Subject: [PATCH] fix(object): Object.entries/values skip non-enumerable descriptor slots (#5046) Object.keys consulted the per-property descriptor table and filtered keys defined with enumerable: false; js_object_values and js_object_entries walked the keys_array unfiltered, so Object.defineProperty(o, k, { value }) slots leaked into entries/values output (chalk's vendored ansi-styles: 55 entries under Perry vs Node's 45). Add a shared descriptor_marks_non_enumerable helper and gate it behind the same cheap any-descriptor probe js_object_keys uses, so descriptor-free objects stay on the fast path. --- .../perry-runtime/src/object/field_get_set.rs | 40 ++++++++++++++++ crates/perry-runtime/src/object/tests.rs | 46 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/crates/perry-runtime/src/object/field_get_set.rs b/crates/perry-runtime/src/object/field_get_set.rs index 44f7b1f597..29660dc9cd 100644 --- a/crates/perry-runtime/src/object/field_get_set.rs +++ b/crates/perry-runtime/src/object/field_get_set.rs @@ -1874,6 +1874,30 @@ pub(crate) unsafe fn instance_private_key_hidden( .unwrap_or(false) } +/// True when a per-property descriptor marks `key_val`'s name non-enumerable +/// (`Object.defineProperty(o, k, { enumerable: false })`). Mirrors the +/// slow-path filter in `js_object_keys` so `Object.values`/`Object.entries` +/// agree with `Object.keys` (#5046). Callers gate on a cheap "does this object +/// have any descriptors at all" probe so the common descriptor-free object +/// never pays the string extraction. +pub(crate) unsafe fn descriptor_marks_non_enumerable( + obj: *const ObjectHeader, + key_val: crate::JSValue, +) -> bool { + let mut buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + let bytes = match crate::string::js_string_key_bytes(key_val, &mut buf) { + Some(b) => b, + None => return false, + }; + let key_str = match std::str::from_utf8(bytes) { + Ok(s) => s, + Err(_) => return false, + }; + get_property_attrs(obj as usize, key_str) + .map(|attrs| !attrs.enumerable()) + .unwrap_or(false) +} + /// Returns an array of the object's field values #[no_mangle] pub extern "C" fn js_object_values(obj: *const ObjectHeader) -> *mut ArrayHeader { @@ -1963,6 +1987,11 @@ pub extern "C" fn js_object_values(obj: *const ObjectHeader) -> *mut ArrayHeader None => j as u32, } }; + // #5046: skip keys a descriptor marks non-enumerable, like + // `js_object_keys` does. Cheap any-descriptor probe first so + // descriptor-free objects stay on the fast path. + let has_descriptors = + PROPERTY_DESCRIPTORS.with(|m| m.borrow().keys().any(|(ptr, _)| *ptr == obj as usize)); for j in 0..count { let i = pos(j); if !keys.is_null() && i < crate::array::js_array_length(keys) { @@ -1970,6 +1999,9 @@ pub extern "C" fn js_object_values(obj: *const ObjectHeader) -> *mut ArrayHeader if instance_private_key_hidden(obj, key_val) { continue; } + if has_descriptors && descriptor_marks_non_enumerable(obj, key_val) { + continue; + } } let value = js_object_get_field(obj as *mut ObjectHeader, i); crate::array::js_array_push_f64(result, f64::from_bits(value.bits())); @@ -2101,6 +2133,11 @@ pub extern "C" fn js_object_entries(obj: *const ObjectHeader) -> *mut ArrayHeade None => j as u32, } }; + // #5046: skip keys a descriptor marks non-enumerable, like + // `js_object_keys` does. Cheap any-descriptor probe first so + // descriptor-free objects stay on the fast path. + let has_descriptors = + PROPERTY_DESCRIPTORS.with(|m| m.borrow().keys().any(|(ptr, _)| *ptr == obj as usize)); for j in 0..count { let i = pos(j); if !keys.is_null() && i < crate::array::js_array_length(keys) { @@ -2108,6 +2145,9 @@ pub extern "C" fn js_object_entries(obj: *const ObjectHeader) -> *mut ArrayHeade if instance_private_key_hidden(obj, key_val) { continue; } + if has_descriptors && descriptor_marks_non_enumerable(obj, key_val) { + continue; + } } // Create a pair array [key, value] let pair = crate::array::js_array_alloc(2); diff --git a/crates/perry-runtime/src/object/tests.rs b/crates/perry-runtime/src/object/tests.rs index 5a47be8996..318934c1c3 100644 --- a/crates/perry-runtime/src/object/tests.rs +++ b/crates/perry-runtime/src/object/tests.rs @@ -642,3 +642,49 @@ fn transition_cache_lookup_rejects_mutated_edge_target() { }; }); } + +#[test] +fn entries_and_values_skip_non_enumerable_descriptor_slots() { + // #5046: Object.defineProperty(o, 'hidden', { value: 1 }) defaults to + // enumerable: false. Object.keys filtered it; entries/values did not. + unsafe { + let obj = js_object_alloc(0, 0); + let hidden_key = crate::string::js_string_from_bytes(b"hidden".as_ptr(), 6); + let shown_key = crate::string::js_string_from_bytes(b"shown".as_ptr(), 5); + let value_key = crate::string::js_string_from_bytes(b"value".as_ptr(), 5); + + let descriptor = js_object_alloc(0, 0); + js_object_set_field_by_name(descriptor, value_key, 1.0); + + let obj_value = crate::value::js_nanbox_pointer(obj as i64); + let hidden_value = f64::from_bits(JSValue::string_ptr(hidden_key).bits()); + let descriptor_value = crate::value::js_nanbox_pointer(descriptor as i64); + js_object_define_property(obj_value, hidden_value, descriptor_value); + js_object_set_field_by_name(obj as *mut ObjectHeader, shown_key, 2.0); + + let keys = js_object_keys(obj); + assert_eq!(crate::array::js_array_length(keys), 1); + assert_eq!( + js_string_to_rust(crate::array::js_array_get(keys, 0).into()), + "shown" + ); + + let values = js_object_values(obj); + assert_eq!(crate::array::js_array_length(values), 1); + assert_eq!( + crate::array::js_array_get(values, 0).bits(), + 2.0f64.to_bits() + ); + + let entries = js_object_entries(obj); + assert_eq!(crate::array::js_array_length(entries), 1); + let pair = crate::value::js_nanbox_get_pointer(f64::from_bits( + crate::array::js_array_get(entries, 0).bits(), + )) as *const crate::array::ArrayHeader; + assert_eq!( + js_string_to_rust(crate::array::js_array_get(pair, 0).into()), + "shown" + ); + assert_eq!(crate::array::js_array_get(pair, 1).bits(), 2.0f64.to_bits()); + } +}