fix(runtime): cap object keys-array length at capacity in property walks (prevents multi-GB / spin on a corrupt keys length)

[proggeramlug]

Problem

Two object property walks size their work by an object's keys-array length via js_array_length:

• the wide-key index in get_field_by_name (get_field_by_name_tail.rs), which does HashMap::with_capacity(key_count) + a 0..key_count build in wide_key_index_lookup; and
• Object.assign's source enumeration (js_object_assign_one), which does a 0..key_count copy loop.

A dense keys array's logical length can never exceed its capacity. But when a keys array is malformed and js_array_length reports a bogus, pointer-sized length (observed ≈ the keys pointer's own low bits — hundreds of millions), both walks are driven into unbounded work: a single missing-property read allocates a multi-hundred-million-bucket HashMap (multi-GB RSS), and an Object.assign spins for minutes walking a phantom tail through the slow sparse-array js_array_get path. On a large bundled application this manifested as the process pinning ~100% CPU at 9 GB RSS (the map build) or a flat-RSS spin (the copy loop), never making progress.

Fix

Add keys_array_len_capped_to_capacity (array/indexing.rs) — min(js_array_length(arr), (*arr).capacity) — and use it in both walks. For a well-formed dense keys array length <= capacity, so this is a no-op on the common path and a hard bound otherwise (work is limited to physically-present slots). Scoped to object keys/property arrays; general JS arrays may legitimately have length > capacity (sparse), so the helper is documented as keys-array-only.

Test

keys_len_capped_bounds_bogus_length_to_capacity forges an oversized length on a small array and asserts the cap returns capacity (not the bogus length). Full suite: cargo test --release -p perry-runtime → 1162 passed / 0 failed.

Note

This is defensive hardening at the two consumers. The upstream cause — js_array_length returning a pointer-sized value for a particular object's keys array (a length/pointer that looks transiently stale) — is a separate, deeper issue worth its own fix; other keys-array walks (field-set, arguments enumeration) share the same iterate-by-length pattern and would benefit from the same guard or, better, the root fix.

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of object key lists to avoid excessive work when key data is malformed.
  • Made object property copying and lookup more resilient by limiting key iteration to the actual array size.
  • Added coverage for normal and corrupted key-list cases to keep behavior stable.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new unsafe helper keys_array_len_capped_to_capacity in the array module that clamps a keys array's reported length to its allocated capacity, re-exports it, and updates js_object_assign_one and get_field_by_name_object_tail to use it instead of raw js_array_length when computing key counts.

Changes

Keys array length capping

Layer / File(s)	Summary
Capped-length helper and tests crates/perry-runtime/src/array/indexing.rs, crates/perry-runtime/src/array/mod.rs	New pub(crate) unsafe fn keys_array_len_capped_to_capacity clamps array length to capacity (returning raw length if the pointer is null), is re-exported from the array module, and is validated by new unit tests for normal and forged-length arrays.
Adoption in object assign and field lookup crates/perry-runtime/src/object/alloc.rs, crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs	js_object_assign_one's string-key copy loop and get_field_by_name_object_tail's key scan now derive key_count from the capped helper instead of raw js_array_length.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• PerryTS/perry#5527: Both PRs modify js_object_assign_one's keys-array iteration logic in crates/perry-runtime/src/object/alloc.rs.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR covers the fix and test, but it doesn't follow the required template and is missing Summary, Changes, Related issue, and Checklist sections.	Rewrite the description using the repository template and add Summary, Changes, Related issue, Test plan, Screenshots/output, and Checklist sections.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is specific and accurately summarizes the main runtime fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry-runtime/src/array/indexing.rs (1)

411-418: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider a debug-only guard for the dense-array invariant.

The doc comment correctly notes this helper is only valid for dense keys/property arrays (length <= capacity), not general sparse JS arrays. Since this is pub(crate) and could be reached by a future caller unaware of that constraint, a debug_assert! could catch misuse early without adding release-mode overhead.

🛡️ Optional debug guard

 pub(crate) unsafe fn keys_array_len_capped_to_capacity(arr: *const ArrayHeader) -> usize {
     let raw = js_array_length(arr) as usize;
     if arr.is_null() {
         raw
     } else {
+        debug_assert!(
+            raw <= (*arr).capacity as usize || cfg!(test),
+            "keys_array_len_capped_to_capacity called on a possibly-sparse array (length > capacity)"
+        );
         raw.min((*arr).capacity as usize)
     }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/array/indexing.rs` around lines 411 - 418, Add a
debug-only invariant check in keys_array_len_capped_to_capacity to catch misuse
of this helper on non-dense arrays. Keep the current release behavior unchanged,
but insert a debug_assert! that the array is either null or satisfies the
dense-array contract (length <= capacity) before using (*arr).capacity, so
future callers of this pub(crate) helper get an early signal if they violate the
documented constraint.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/perry-runtime/src/array/indexing.rs`:
- Around line 411-418: Add a debug-only invariant check in
keys_array_len_capped_to_capacity to catch misuse of this helper on non-dense
arrays. Keep the current release behavior unchanged, but insert a debug_assert!
that the array is either null or satisfies the dense-array contract (length <=
capacity) before using (*arr).capacity, so future callers of this pub(crate)
helper get an early signal if they violate the documented constraint.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2c79cccf-e0e0-4345-a914-3897a1ba0dfc

📥 Commits

Reviewing files that changed from the base of the PR and between 63a44a7 and b94b488.

📒 Files selected for processing (4)

• crates/perry-runtime/src/array/indexing.rs
• crates/perry-runtime/src/array/mod.rs
• crates/perry-runtime/src/object/alloc.rs
• crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs