fix(fetch+runtime): generic object ops must not deref fetch-band Headers/Request handles#5606
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughIntroduces a handle-band-aware ChangesHeaders Handle-Band JSON Conversion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/perry-runtime/src/object/tests.rs (1)
761-789: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the missing
in-operator regression.This covers
js_object_to_string, but the PR also changesjs_object_has_property. Add a companion test that a pointer-tagged fetch handle returnsTAG_FALSEfor a string key without dereferencing the id.🧪 Suggested test coverage
#[test] fn object_to_string_rejects_handle_band_ids() { use crate::value::addr_class; for &id in &[ addr_class::FETCH_HANDLE_BAND_START, // 0x40000 @@ ); } } + +#[test] +fn object_has_property_rejects_fetch_handle_band_ids() { + use crate::value::{addr_class, JSValue}; + + let handle = crate::value::js_nanbox_pointer(addr_class::FETCH_HANDLE_BAND_START as i64); + let key_ptr = crate::string::js_string_from_bytes(b"x".as_ptr(), 1); + let key = f64::from_bits(JSValue::string_ptr(key_ptr).bits()); + + let result = js_object_has_property(handle, key); + assert_eq!(result.to_bits(), crate::value::TAG_FALSE); +}🤖 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/tests.rs` around lines 761 - 789, Add a companion test function to verify that js_object_has_property also safely rejects handle-band ids without dereferencing them. Create a new test (similar in structure to object_to_string_rejects_handle_band_ids) that iterates over the same handle-band id values, creates nanboxed pointers using js_nanbox_pointer, and calls js_object_has_property with a string key to verify that it returns TAG_FALSE without attempting to dereference the bogus pointer address.
🤖 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/field_get_set.rs`:
- Around line 2266-2278: The current handle-band check in the code block
starting at the is_handle_band call is catching both fetch handles and small
handles indiscriminately, which causes the small-handle property dispatcher to
be bypassed and always return false. Keep the crash guard for fetch handles, but
modify the logic to allow small handles to proceed to the registered
handle-property dispatcher before falling back to false. Refine the condition by
either checking specifically for fetch handle types (Web Fetch
Headers/Request/Response, net/http handles, zlib streams) instead of the entire
handle band, or by checking whether the handle is a small handle first and
allowing those to continue through the function before applying the fetch-handle
crash guard.
In `@crates/perry-runtime/src/object/global_fetch.rs`:
- Around line 157-158: The function js_fetch_headers_to_json calls
headers_init_json_ptr directly without handling null/undefined values, causing
incorrect serialization as "null" string instead of an empty object. Move the
null/undefined normalization logic that currently exists in
fetch_headers_json_ptr into the headers_init_json_ptr helper function itself, so
both js_fetch_headers_to_json and the other calling path (around lines 303-321)
share the same contract for handling null/undefined headers and consistently
produce the expected empty object JSON output.
---
Nitpick comments:
In `@crates/perry-runtime/src/object/tests.rs`:
- Around line 761-789: Add a companion test function to verify that
js_object_has_property also safely rejects handle-band ids without dereferencing
them. Create a new test (similar in structure to
object_to_string_rejects_handle_band_ids) that iterates over the same
handle-band id values, creates nanboxed pointers using js_nanbox_pointer, and
calls js_object_has_property with a string key to verify that it returns
TAG_FALSE without attempting to dereference the bogus pointer address.
🪄 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: af1a7b50-d524-499b-b0cf-3ec3151edf6a
📒 Files selected for processing (9)
crates/perry-codegen/src/expr/logical_collections.rscrates/perry-codegen/src/runtime_decls/stdlib_ffi.rscrates/perry-runtime/src/object/field_get_set.rscrates/perry-runtime/src/object/global_fetch.rscrates/perry-runtime/src/object/mod.rscrates/perry-runtime/src/object/tests.rscrates/perry-stdlib/src/common/dispatch.rscrates/perry-stdlib/src/fetch/headers.rscrates/perry-stdlib/src/fetch/mod.rs
added 2 commits
June 24, 2026 06:07
…lper
The fetch thunk serialized init.headers via the generic js_json_stringify,
which dereferenced a Headers handle (a fetch-band registry id) as a heap
pointer -> EXC_BAD_ACCESS in gc_obj_type. Add js_fetch_headers_to_json that
reads a Headers handle from its registry instead, and route the
fetch(url, { headers }) path through it. Same handle-band family as the
string_from_header / inline-.length guards.
(Advances the bundle's -p path past the json-stringify crash; a further
handle-band deref remains in js_object_has_property on a Headers handle.)
`key in <handle>` where the receiver is a Web Fetch Headers/Request/Response handle (a fetch-band registry id, e.g. 0x40007) dereferenced the id as a heap object -> EXC_BAD_ACCESS. Return false for handle-band receivers instead, same family as the string_from_header / inline-.length / json_stringify guards.
proggeramlug
pushed a commit
that referenced
this pull request
Jun 24, 2026
- js_object_has_property: narrow the handle-band crash guard to the fetch/zlib
bands (>= COMMON_HANDLE_BAND_END). Common/small handles now fall through to
the registered small-handle property path instead of always returning false.
- headers_init_json_ptr: normalize null/undefined headers to {} in the shared
helper so the codegen js_fetch_headers_to_json path matches the runtime thunk
(previously serialized headers:null as "null").
proggeramlug
force-pushed
the
fix/fetch-headers-handle-stringify
branch
from
9cbb9a8 to
d4ad5ba
Compare
- js_object_has_property: narrow the handle-band crash guard to the fetch/zlib
bands (>= COMMON_HANDLE_BAND_END). Common/small handles now fall through to
the registered small-handle property path instead of always returning false.
- headers_init_json_ptr: normalize null/undefined headers to {} in the shared
helper so the codegen js_fetch_headers_to_json path matches the runtime thunk
(previously serialized headers:null as "null").
- Move the headers_fetch_object_json test out of fetch/mod.rs into a sibling
headers_json_test.rs submodule so fetch/mod.rs is back under the 2000-LOC
CI limit (2020 -> 1992).
proggeramlug
force-pushed
the
fix/fetch-headers-handle-stringify
branch
from
d4ad5ba to
b1f1958
Compare
This was referenced Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A real HTTP client (axios's fetch adapter) passes a
Headers/Requestobject — a perry fetch-band registry handle (a small id like0x40007, not a heap pointer) — to generic object operations. Each one dereferences the id as a heap object →EXC_BAD_ACCESS(SIGSEGV), sofetch()crashes before reaching the network. Two sites, two commits:1.
js_json_stringifyon aHeadersvalue (43f212ec4)The fetch thunk serializes
init.headersto JSON forjs_fetch_with_options. Withheaders: new Headers(...), the genericjs_json_stringifywalker reached the handle and deref'd it → crash ingc_obj_type. Fix:js_fetch_headers_to_json/js_headers_fetch_object_jsonread aHeadershandle from its registry instead, and thefetch(url, { headers })path is routed through it.2.
js_object_has_propertyon a handle (9cbb9a87f)key in <handle>(theinoperator) deref'd the fetch-band id as a heap object.key in <handle>has no own-property meaning for a registry handle, so returnfalsefor handle-band receivers (mirrors the existing Proxy guard in the same function).Both are the same handle-band family as the
string_from_header/ inline-.length/js_get_iteratorguards (#5559/#5560/#5549): fast/native/generic paths must reject handle-band ids before treating them as heap pointers.Result
With these (plus the
fetch(Request)dispatch fix), a real bundle's-prequest path runs through startup → request dispatch → TLS handshake and opens an ESTABLISHED socket to the API host (...:443), instead of crashing.Tests
cargo test -p perry-runtime --liband-p perry-stdlib --libpass; regression coverage added for the Headers-handle stringify path. fmt + check clean.Summary by CodeRabbit
Bug Fixes
fetch(url, { headers })to reliably serializeHeadersinto the expected flat{name: value}JSON shape, avoiding failures caused by misinterpreting internal handle identifiers.Object.prototype.toStringfrom attempting to treat Web/registry handle-band values as heap objects.Tests
Headers → JSONconversion, including correct handling of unknown/invalid handles.