fix(runtime): #5961 — URLSearchParams methods reachable through dynamic access#5964
Conversation
…ic access The native URLSearchParams is an ordinary object (class_id == 0, `_entries`/`_owner` slots — the shape try_read_as_search_params guards on) whose method surface existed only via static type-directed lowering (perry-codegen url_main.rs). The moment the receiver's static type was lost (`any`, heterogeneous containers, bundled/minified code where inference can't track the value) the generic dynamic paths found nothing: `typeof sp.append` read `undefined` and `sp.append(...)` threw "append is not a function". Observed in a real-world bundle whose auth-URL builder does eleven `url.searchParams.append(...)` calls — works extracted, failed bundled. Fix, mirroring the webcrypto_method_value pattern: - url/search_params.rs: `shape_is_url_search_params` (class_id == 0 + leading `_entries` key), `url_search_params_dynamic_call` (fused-call dispatch: append/set/get/has/delete/toString), and `url_search_params_method_value` (bound-closure thunks for property reads, receiver carried in a GC-traced nanboxed capture). `get` and `toString` rebuild their boxed results here — the raw natives return `*mut StringHeader` for the static path. `has` boxes a real boolean. - object/native_call_method.rs: dispatch arm for the covered names before the generic field-scan (fused `inst.m(...)` calls resolve here, not via get_field). - object/field_get_set/get_field_by_name_tail.rs: final-miss hook so `typeof sp.append` is "function", method extraction (`const m = sp.append`) yields a callable bound method, and `size` reads as a number. New e2e test covers typeof, fused calls, get/has/delete/toString/size, owner-URL sync after mutation, and extracted-method calls. Fixes #5961. Claude-Session: https://claude.ai/code/session_01K1w6WLaFNKhf4aNxeSWHy6
📝 WalkthroughWalkthroughThis PR adds dynamic dispatch support for native ChangesURLSearchParams Dynamic Dispatch
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Code as TS Code
participant FieldGet as get_field_by_name_object_tail
participant CallMethod as js_native_call_method
participant SearchParams as url_search_params module
Code->>FieldGet: read o.append (property access)
FieldGet->>SearchParams: shape_is_url_search_params(receiver)
SearchParams-->>FieldGet: true
FieldGet->>SearchParams: url_search_params_method_value("append")
SearchParams-->>FieldGet: bound closure
Code->>CallMethod: call o.append(a, 1)
CallMethod->>SearchParams: shape_is_url_search_params(receiver)
SearchParams-->>CallMethod: true
CallMethod->>SearchParams: url_search_params_dynamic_call("append", args)
SearchParams-->>CallMethod: Some(result)
CallMethod-->>Code: result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@crates/perry-runtime/src/object/native_call_method.rs`:
- Around line 725-740: The URLSearchParams fast path in native_call_method is
using object.to_bits() to build recv_ptr before any receiver validation, which
can let primitive or string receivers reach shape_is_url_search_params with a
bogus pointer. Update the fast path in native_call_method to use the same
receiver guard as the other native-call branches before masking or dereferencing
the receiver, and only enter url_search_params_dynamic_call after confirming the
receiver is a valid object pointer for URLSearchParams.
In `@crates/perry-runtime/src/url/search_params.rs`:
- Around line 942-966: The method-value dispatch in
url_search_params_method_value is missing the toString surface, so
property-read-then-call does not match the fused call path covered by
url_search_params_dynamic_call. Add a toString thunk with arity 0 to the name
match in url_search_params_method_value, and ensure it follows the same closure
allocation and capture pattern as the existing append/set/get/has/delete entries
so decomposed calls stay in sync.
- Around line 817-832: The shape guard in shape_is_url_search_params is too
permissive and can misclassify plain objects as URLSearchParams. Update the
guard to match the structure produced by create_url_search_params_object by
checking that the first key is "_entries" and the second own key is "_owner"
before returning true. Keep the existing null/class_id/length checks, and only
allow dynamic dispatch when both keys are present in the expected order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 10d32a57-7571-47fb-acb2-85c542fcb3d5
📒 Files selected for processing (4)
crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rscrates/perry-runtime/src/object/native_call_method.rscrates/perry-runtime/src/url/search_params.rscrates/perry/tests/issue_5961_urlsearchparams_dynamic_dispatch.rs
| if matches!( | ||
| method_name, | ||
| "append" | "set" | "get" | "has" | "delete" | "toString" | ||
| ) { | ||
| let recv_ptr = (object.to_bits() & 0x0000_FFFF_FFFF_FFFF) as *mut ObjectHeader; | ||
| if crate::url::search_params::shape_is_url_search_params(recv_ptr) { | ||
| if let Some(result) = crate::url::search_params::url_search_params_dynamic_call( | ||
| recv_ptr, | ||
| method_name, | ||
| args_ptr, | ||
| args_len, | ||
| ) { | ||
| return result; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm primitive/string receivers can reach js_native_call_method with these names.
rg -nP -C3 'js_native_call_method' crates/perry-codegen/src -g '*.rs' | rg -i 'toString|method|call' | head -40
# Confirm shape_is_url_search_params has no heap-validity guard beyond null.
rg -nP -A16 'fn shape_is_url_search_params' crates/perry-runtime/src/url/search_params.rsRepository: PerryTS/perry
Length of output: 5096
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the target function around the suspect arm and neighboring pointer-guarded arms.
sed -n '680,770p' crates/perry-runtime/src/object/native_call_method.rs
printf '\n====\n'
sed -n '800,900p' crates/perry-runtime/src/object/native_call_method.rs
# Search for the specific method names and js_native_call_method lowering paths.
printf '\n==== SEARCH ====\n'
rg -n 'toString|get|has|append|set|delete|js_native_call_method|shape_is_url_search_params|url_search_params_dynamic_call' crates/perry-runtime/src crates/perry-codegen/src -g '*.rs'Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the full native_call_method control flow around the suspect block and later primitive handling.
sed -n '720,1010p' crates/perry-runtime/src/object/native_call_method.rs
# Inspect the NaN-box / pointer helpers used elsewhere for guard patterns.
printf '\n==== HELPERS ====\n'
rg -n 'is_pointer\(|is_above_handle_band|js_nanbox_get_pointer|to_bits\(' crates/perry-runtime/src -g '*.rs'Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Read the exact native_call_method block around the suspicious arm and nearby guards.
sed -n '720,760p' crates/perry-runtime/src/object/native_call_method.rs
printf '\n==== LATER DISPATCH ====\n'
sed -n '960,1005p' crates/perry-runtime/src/object/native_call_method.rs
# 2) Read the URLSearchParams helper directly.
printf '\n==== SEARCH_PARAMS HELPER ====\n'
sed -n '810,835p' crates/perry-runtime/src/url/search_params.rs
# 3) Find the native table entry for URLSearchParams to see what call shapes are expected.
printf '\n==== CODEGEN ENTRIES ====\n'
rg -n -A8 -B4 'url_search_params|URLSearchParams|append|delete|toString|get|has|set' crates/perry-codegen/src/lower_call/native_table crates/perry-codegen/src -g '*.rs' | head -200Repository: PerryTS/perry
Length of output: 21602
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrowly inspect URLSearchParams lowering and any receiver-type guards around it.
rg -n -A6 -B4 'URLSearchParams|url_search_params|search_params|append|delete|toString|get|has|set' crates/perry-codegen/src/lower_call crates/perry-codegen/src -g '*.rs' | head -200
printf '\n==== RUNTIME CALLER TYPES ====\n'
rg -n -A4 -B4 'primitive_methods::dispatch_primitive|string_methods::dispatch_string|handle_methods::dispatch_handle' crates/perry-runtime/src/object/native_call_method.rsRepository: PerryTS/perry
Length of output: 16089
Guard the URLSearchParams fast path before masking the receiver
crates/perry-runtime/src/object/native_call_method.rs:725-740 runs before primitive/string/handle dispatch. recv_ptr is built from object.to_bits() with no pointer check, and shape_is_url_search_params dereferences class_id after only a null check, so toString/get/has on primitive receivers can hit a bogus address and crash. Add the same receiver guard used by the other native-call arms.
🤖 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/object/native_call_method.rs` around lines 725 -
740, The URLSearchParams fast path in native_call_method is using
object.to_bits() to build recv_ptr before any receiver validation, which can let
primitive or string receivers reach shape_is_url_search_params with a bogus
pointer. Update the fast path in native_call_method to use the same receiver
guard as the other native-call branches before masking or dereferencing the
receiver, and only enter url_search_params_dynamic_call after confirming the
receiver is a valid object pointer for URLSearchParams.
| pub(crate) fn shape_is_url_search_params(obj: *const ObjectHeader) -> bool { | ||
| if obj.is_null() { | ||
| return false; | ||
| } | ||
| unsafe { | ||
| if (*obj).class_id != 0 { | ||
| return false; | ||
| } | ||
| let keys_arr = (*obj).keys_array; | ||
| if keys_arr.is_null() || (*keys_arr).length == 0 { | ||
| return false; | ||
| } | ||
| let key0 = crate::array::js_array_get_f64(keys_arr, 0); | ||
| get_string_content(key0) == "_entries" | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the real URLSearchParams layout and check MIMEParams / JSON.parse collisions.
rg -nP -C4 'fn create_url_search_params_object|_owner|URL_SEARCH_PARAMS_ENTRIES' crates/perry-runtime/src/url/search_params.rs | head -80
# MIMEParams internal key layout
rg -nP -C3 '_entries|MIMEParams|mime.*params' crates/perry-runtime/src -g '*.rs' | rg -i 'mime' | head -40Repository: PerryTS/perry
Length of output: 6997
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the shape guard implementation and its callers.
sed -n '380,460p' crates/perry-runtime/src/url/search_params.rs
printf '\n--- call sites ---\n'
rg -n 'shape_is_url_search_params|URLSearchParams' crates/perry-runtime/src -g '*.rs'
printf '\n--- MIMEParams layout ---\n'
sed -n '234,260p' crates/perry-runtime/src/util_mime.rs
printf '\n--- object shape fallback ---\n'
rg -n 'class_id == 0|JSON.parse|AnonShape|keys_array' crates/perry-runtime/src -g '*.rs'Repository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '800,835p' crates/perry-runtime/src/url/search_params.rsRepository: PerryTS/perry
Length of output: 1744
Shape guard is too broad for dynamic dispatch. It accepts any class_id == 0 object whose first own key is _entries, so a plain object like {"_entries":[]} can be routed into the URLSearchParams natives. Require _owner as the second key as well, matching create_url_search_params_object.
🤖 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/url/search_params.rs` around lines 817 - 832, The
shape guard in shape_is_url_search_params is too permissive and can misclassify
plain objects as URLSearchParams. Update the guard to match the structure
produced by create_url_search_params_object by checking that the first key is
"_entries" and the second own key is "_owner" before returning true. Keep the
existing null/class_id/length checks, and only allow dynamic dispatch when both
keys are present in the expected order.
| /// Bound-method value for a dynamic property read on a native URLSearchParams | ||
| /// object. Returns `None` for names outside the covered surface so unknown | ||
| /// properties still read as `undefined`. | ||
| pub(crate) fn url_search_params_method_value(obj: *const ObjectHeader, name: &str) -> Option<f64> { | ||
| let (func_ptr, arity): (*const u8, u32) = match name { | ||
| "append" => (usp_append_thunk as *const u8, 2), | ||
| "set" => (usp_set_thunk as *const u8, 2), | ||
| "get" => (usp_get_thunk as *const u8, 1), | ||
| "has" => (usp_has_thunk as *const u8, 1), | ||
| "delete" => (usp_delete_thunk as *const u8, 1), | ||
| _ => return None, | ||
| }; | ||
| crate::closure::js_register_closure_arity(func_ptr, arity); | ||
| let closure = crate::closure::js_closure_alloc(func_ptr, 1); | ||
| if closure.is_null() { | ||
| return Some(f64::from_bits(crate::value::TAG_UNDEFINED)); | ||
| } | ||
| // Nanboxed (not raw-ptr) capture so the GC traces the receiver. | ||
| crate::closure::js_closure_set_capture_f64( | ||
| closure, | ||
| 0, | ||
| crate::value::js_nanbox_pointer(obj as i64), | ||
| ); | ||
| Some(crate::value::js_nanbox_pointer(closure as i64)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Dispatch surface mismatch: toString/size are callable when fused but not readable as method values.
url_search_params_dynamic_call (Line 929) covers toString, and size is handled in the field-get path, but url_search_params_method_value only reifies append/set/get/has/delete. So o.toString() works while the property-read-then-call form const f = o.toString; f() reads undefined and throws — the exact decomposed shape this PR set out to fix for the other methods. Consider adding a toString thunk (arity 0) here so the two paths stay in sync.
🤖 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/url/search_params.rs` around lines 942 - 966, The
method-value dispatch in url_search_params_method_value is missing the toString
surface, so property-read-then-call does not match the fused call path covered
by url_search_params_dynamic_call. Add a toString thunk with arity 0 to the name
match in url_search_params_method_value, and ensure it follows the same closure
allocation and capture pattern as the existing append/set/get/has/delete entries
so decomposed calls stay in sync.
…ceivers PR #5964 made native URLSearchParams methods reachable through dynamic access by probing the receiver in js_native_call_method for the method names append/set/get/has/delete/toString. That probe masks the receiver bits into an *ObjectHeader and calls shape_is_url_search_params, which reads class_id / keys_array / length at ObjectHeader offsets. But the receiver may be any pointer-tagged value, not an ordinary object. A Date value is a pointer to a GC_TYPE_DATE_CELL, whose bytes at those offsets are garbage. Reading keys_array as a pointer and dereferencing its length segfaulted -- so any two Date.toString() calls (the probe fires on toString) crashed: language/expressions/addition/S11.6.1_A2.2_T2.js (the test does date.toString() + date.toString()). Gate shape_is_url_search_params on the GC header type: only proceed when the allocation is GC_TYPE_OBJECT. A Date cell (GC_TYPE_DATE_CELL), array, buffer, closure, etc. now returns false immediately instead of reading ObjectHeader fields off a foreign layout. This protects every caller of the shared probe, not just the new toString arm. Verified on an internal Linux sweep host: the addition test passes, Date.toString()+Date.toString() no longer segfaults, and URLSearchParams dynamic get/toString/size still work. The addition slice is clean; the remaining Date runtime-fails are pre-existing unrelated gaps. Refs #5961
…ceivers (#5997) PR #5964 made native URLSearchParams methods reachable through dynamic access by probing the receiver in js_native_call_method for the method names append/set/get/has/delete/toString. That probe masks the receiver bits into an *ObjectHeader and calls shape_is_url_search_params, which reads class_id / keys_array / length at ObjectHeader offsets. But the receiver may be any pointer-tagged value, not an ordinary object. A Date value is a pointer to a GC_TYPE_DATE_CELL, whose bytes at those offsets are garbage. Reading keys_array as a pointer and dereferencing its length segfaulted -- so any two Date.toString() calls (the probe fires on toString) crashed: language/expressions/addition/S11.6.1_A2.2_T2.js (the test does date.toString() + date.toString()). Gate shape_is_url_search_params on the GC header type: only proceed when the allocation is GC_TYPE_OBJECT. A Date cell (GC_TYPE_DATE_CELL), array, buffer, closure, etc. now returns false immediately instead of reading ObjectHeader fields off a foreign layout. This protects every caller of the shared probe, not just the new toString arm. Verified on an internal Linux sweep host: the addition test passes, Date.toString()+Date.toString() no longer segfaults, and URLSearchParams dynamic get/toString/size still work. The addition slice is clean; the remaining Date runtime-fails are pre-existing unrelated gaps. Refs #5961 Co-authored-by: Ralph Küpper <ralph@skelpo.com>
…l_search_params The receiver gate (#5964) validates the OBJECT header, but keys_array was dereferenced raw — a GC_TYPE_OBJECT reached mid-transition (or with a typed layout) can carry a non-heap word there, and reading (*keys_arr).length SIGSEGV'd during Next.js request handling (config.js method dispatch probes arbitrary receivers through this shape check; first request killed the server). Gate keys_array with the same try_read_gc_header check (#5429/#5943 family).
… method dispatch (#6046) * fix(runtime): AbortSignal parity — accept in events helpers + dynamic method dispatch Node treats `AbortSignal` as an `EventTarget`; Perry represents it as its own native object (url/abort.rs) that two dispatch layers didn't know: 1. The module-level `node:events` helpers. `setMaxListeners(n, controller.signal)` — how SDKs raise the MaxListenersExceededWarning threshold on a shared signal — threw `ERR_INVALID_ARG_TYPE: The "eventTargets" argument must be an instance of EventEmitter or EventTarget`, rejecting the caller's whole request path. The four helpers (`setMaxListeners`, `getMaxListeners`, `listenerCount`, `getEventListeners`) now recognize signals in both perry-stdlib and perry-ext-events: set is a faithful no-op (the warning threshold is the call's only Node-observable effect and Perry never emits that warning for signals), get reports the EventTarget default, and the listener queries report the signal's registered "abort" listeners through two new runtime helpers (`js_abort_signal_listener_count`, `js_abort_signal_listeners_copy`). 2. Dynamic receivers. The statically-typed form (`c.signal.addEventListener(...)`) lowers to the native call, but the same method through an untyped local — the shape minified SDK code takes — resolved to `undefined` and threw `addEventListener is not a function` (the #5964 URLSearchParams dynamic-dispatch class). Both dynamic paths are covered: property GET returns a bound-method closure (`abort_signal_method_bind` in get_field_by_name_tail), and the fused dynamic method CALL pre-dispatches to the natives in js_native_call_method (the #5961 precedent) for `addEventListener` / `removeEventListener` / `throwIfAborted`. `options` is accepted and ignored — a signal only ever fires "abort" once, so `{ once: true }` is behaviorally implied. Two e2e tests cover the helpers surface and the dynamic-receiver forms (including listeners actually firing on abort and plain objects still being rejected). * fix(runtime): guard AbortSignal dynamic-dispatch against small-handle receivers A native handle (Headers/fetch/timer/...) is nanbox-pointer-tagged but its "pointer" is a small integer id, not a heap ObjectHeader. Guard the class_id read in the fused dynamic-call AbortSignal arm with is_small_handle so a handle receiver never gets dereferenced as an object. --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
Problem — #5961
The native URLSearchParams is an ordinary object (
class_id == 0,_entries/_ownerslots) whose method surface existed only via static type-directed lowering (perry-codegen/expr/url_main.rs). With the receiver's static type lost —any, containers, bundled/minified code where inference can't follow the value — the generic dynamic paths found nothing:typeof sp.appendreadundefined, and every method call threwX is not a function. Real-world hit: a bundle's auth-URL builder doing elevenurl.searchParams.append(...)calls failed at runtime while the identical extracted code passed.Fix
Mirrors the
webcrypto_method_valuepattern, covering the three dynamic entry points:url/search_params.rs—shape_is_url_search_params(thetry_read_as_search_paramsguard without materializing entries),url_search_params_dynamic_call(fused-call dispatch:append/set/get/has/delete/toString),url_search_params_method_value(bound-closure thunks for property reads; receiver in a GC-traced nanboxed capture).get/toStringrebuild boxed results (the raw natives return*mut StringHeaderfor the static path);hasboxes a real boolean.object/native_call_method.rs— dispatch arm for covered names before the generic field-scan (fusedinst.m(...)calls resolve here, not through get_field — property-read fixes alone leave calls broken).object/field_get_set/get_field_by_name_tail.rs— final-miss hook:typeof sp.append === "function", extracted methods (const m = sp.append; m(...)) are callable and receiver-bound,sizereads as a number.Uncovered names still read as
undefined/ fall through unchanged. All guards are shape-checked (class_id == 0+ leading_entrieskey) soutil.MIMEParamsand plain objects are untouched.Validation
New e2e test
crates/perry/tests/issue_5961_urlsearchparams_dynamic_dispatch.rs: typeof, fused calls,get(hit +nullmiss), booleanhas, WHATWGtoString(two+words),size, owner-URL sync afterset/delete, and extracted-method invocation — output matches node. Passes locally (cargo test --release -p perry --test issue_5961_...→ 1 passed). Also validated in the originating 13 MB bundle: the auth-URL builder now runs (compile in flight for the fully-unpatched confirmation).Fixes #5961.
https://claude.ai/code/session_01K1w6WLaFNKhf4aNxeSWHy6
Summary by CodeRabbit
New Features
URLSearchParamssupport when values are accessed dynamically, including method calls and property reads.sizeand common methods such asappend,set,get,has,delete, andtoString.Bug Fixes
URLSearchParamsmethods could stop working after type information was lost.Tests
URLSearchParamsaccess and method behavior.