Skip to content

fix(runtime): guard URLSearchParams dynamic-call fast-path against handle receivers#6049

Closed
proggeramlug wants to merge 1 commit into
mainfrom
fix/dynamic-call-small-handle-guard
Closed

fix(runtime): guard URLSearchParams dynamic-call fast-path against handle receivers#6049
proggeramlug wants to merge 1 commit into
mainfrom
fix/dynamic-call-small-handle-guard

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Problem

A fused dynamic method call — v.append(...) where v's static Web-Fetch type was erased (stored in an any/untyped local, or a destructured binding) — routes through js_native_call_method. Its #5961 URLSearchParams fast-path matches the method names append/set/get/has/delete/toString and dereferences the receiver as a heap ObjectHeader to sniff the _entries shape:

let recv_ptr = (object.to_bits() & POINTER_MASK) as *mut ObjectHeader;
if shape_is_url_search_params(recv_ptr) { ... }   // reads (*recv_ptr).class_id

But a Headers instance — and every fetch / timer / ... value — is a native handle: nanbox-pointer-tagged, yet its "pointer" is a small integer id in the low handle band, not a heap object. Dereferencing that id as an ObjectHeader SIGSEGVs (fault address in the low handle band).

The WHATWG NullableHeaders builder hits this directly — destructured (type-erased) names flow into Headers methods:

for (const [name, value] of entries) {   // destructured -> type-erased receiver below
  if (value === null) K.delete(name);
  else K.append(name, value);
}

Minimal repro (SIGSEGV before the fix):

const K = new Headers();
K.append("a", "1");
const v = K;          // untyped alias
v.append("b", "2");   // fused dynamic call -> deref handle id as ObjectHeader -> crash

Fix

Guard the fast-path with jsval.is_pointer() + !is_small_handle(recv_ptr) before the deref, so a handle receiver falls through to handle_method_dispatch below (which owns the fetch/Headers method surface). Real URLSearchParams instances are heap objects and are unaffected.

Test

crates/perry/tests/issue_dynamic_call_headers_handle.rs compiles + runs the NullableHeaders builder shape (destructured names into delete/append, dedup across lists) and reads the result back through an untyped local: ct=application/json beta=on hasCT=true.

Summary by CodeRabbit

  • Bug Fixes

    • Improved safety for certain dynamic method calls on URL search parameter-like objects, preventing incorrect handling of non-object receiver values.
    • Fixed dynamic dispatch behavior for header-related method calls when the receiver is type-erased, restoring correct results for common operations like reading, checking, adding, and removing values.
  • Tests

    • Added regression coverage for dynamic header method dispatch to verify the runtime output matches expected behavior.

…t handle receivers

A fused dynamic method call (`v.append(...)` where `v`'s static Web-Fetch
type was erased — an `any`/untyped local, or a destructured binding) routes
through `js_native_call_method`. Its #5961 URLSearchParams fast-path matches
the method names `append`/`set`/`get`/`has`/`delete`/`toString` and derefs the
receiver as a heap `ObjectHeader` to sniff the `_entries` shape.

But a `Headers` instance (and every fetch/timer/... value) is a native
*handle*: nanbox-pointer-tagged, yet its "pointer" is a small integer id in
the low handle band, not a heap object. Dereferencing it as an `ObjectHeader`
SIGSEGV'd. The WHATWG NullableHeaders builder triggers exactly this, calling
`K.delete(name)` / `K.append(name, value)` on destructured (type-erased)
names.

Add an `is_pointer()` + `is_small_handle` guard before the deref so handle
receivers fall through to `handle_method_dispatch`, which owns the
fetch/Headers method surface. Regression test compiles + runs the
NullableHeaders builder shape and reads the result back through an untyped
local.
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cc1e7eac-ca4a-46f6-a74f-978b5490d185

📥 Commits

Reviewing files that changed from the base of the PR and between 9016b44 and c869e03.

📒 Files selected for processing (2)
  • crates/perry-runtime/src/object/native_call_method.rs
  • crates/perry/tests/issue_dynamic_call_headers_handle.rs

📝 Walkthrough

Walkthrough

Adds a guard in js_native_call_method to skip small-handle receivers before invoking shape_is_url_search_params, preventing potential invalid pointer dereferences during URLSearchParams-style dynamic dispatch. Adds a new regression test verifying correct dynamic dispatch for Headers methods on type-erased receivers.

Changes

Small Handle Guard and Regression Test

Layer / File(s) Summary
Dispatch guard for URLSearchParams shape check
crates/perry-runtime/src/object/native_call_method.rs
Adds an is_small_handle check before calling shape_is_url_search_params on the receiver pointer, avoiding invalid ObjectHeader dereferences for small handle receivers while still routing valid shapes to url_search_params_dynamic_call.
Headers dynamic dispatch regression test
crates/perry/tests/issue_dynamic_call_headers_handle.rs
Adds a new test file with documentation of the prior crash, a compile_and_run helper that compiles and executes a TS snippet via the perry binary, and a test asserting correct get/has/append/delete dispatch on a type-erased Headers receiver.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • PerryTS/perry#5964: Both PRs modify the URLSearchParams dynamic-dispatch path in native_call_method.rs's js_native_call_method.
  • PerryTS/perry#5997: Both PRs strengthen receiver validation around shape_is_url_search_params in the same dispatch function.
  • PerryTS/perry#6004: Both PRs adjust dynamic dispatch routing logic within js_native_call_method.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the bug, fix, and test, but it does not follow the required template sections like Summary, Changes, Related issue, and Test plan. Reformat it to the repository template and add Summary, Changes, Related issue (or n/a), and a concrete Test plan/checklist.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main runtime fix for the URLSearchParams fast-path guard.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dynamic-call-small-handle-guard

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

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

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Closing — this turned out to be a non-issue on main. The crash only reproduces on an old local checkout; on current main the callee shape_is_url_search_params already classifies the receiver address via try_read_gc_header and rejects small handles / non-object types (added in #5964/#5989), so a Headers handle never reaches the ObjectHeader deref. The extra guard in the caller is redundant. Verified: the Headers dynamic-dispatch repro runs cleanly on main without this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant