fix(object): Object.entries/values skip non-enumerable descriptor slots (#5046) (#5049)

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.

Co-authored-by: Ralph Küpper <ralph@skelpo.com>

Author: proggeramlug

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,13 +1987,21 @@ 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) {
                 let key_val = crate::array::js_array_get(keys, i);
                 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,13 +2133,21 @@ 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) {
                 let key_val = crate::array::js_array_get(keys, i);
                 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);

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());
+    }
+}