-
-
Notifications
You must be signed in to change notification settings - Fork 161
fix(fetch): accept a Proxy as a Headers record init #10275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Accept a Proxy as a `Headers` record init. `new Headers(new Proxy({ "x-a": "1" }, {}))` raised "Headers constructor: init is not iterable": the record path required a plain heap object, and a proxy value is a proxy id rather than one, so neither the iterable nor the record branch applied. The constructor now reads a proxied init's own string keys and their values through the proxy's `ownKeys` and `get` traps, matching how the spec reads a record init through the object's internal methods. Plain objects, arrays, maps, sets and string inits are unchanged. This unblocks OpenCode's request path, where the AI SDK hands the fetch layer a proxied header record (#10107). |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| //! `new Headers(init)` must accept a Proxy wrapping a record, reading the init's | ||
| //! own keys and values through the proxy's traps. perry took the record path only | ||
| //! for plain heap objects, so a proxied header record raised | ||
| //! "Headers constructor: init is not iterable" — OpenCode's request path hit this | ||
| //! on every `run` (tracker #10107). | ||
| //! Two neighbours are deliberately out of scope here: a proxy wrapping an *array* | ||
| //! (`Array.from` over such a value segfaults, #10270) and a proxied init passed | ||
| //! through `new Request(url, { headers })`, which takes a different path (#10274). | ||
|
|
||
| use std::path::PathBuf; | ||
| use std::process::Command; | ||
| use std::sync::Once; | ||
|
|
||
| fn perry_bin() -> PathBuf { | ||
| PathBuf::from(env!("CARGO_BIN_EXE_perry")) | ||
| } | ||
|
|
||
| fn workspace_root() -> PathBuf { | ||
| PathBuf::from(env!("CARGO_MANIFEST_DIR")) | ||
| .join("../..") | ||
| .canonicalize() | ||
| .expect("canonicalize workspace root") | ||
| } | ||
|
|
||
| fn runtime_dir() -> PathBuf { | ||
| static BUILD_RUNTIME: Once = Once::new(); | ||
| BUILD_RUNTIME.call_once(|| { | ||
| let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); | ||
| let build = Command::new(cargo) | ||
| .current_dir(workspace_root()) | ||
| .arg("build") | ||
| .arg("-p") | ||
| .arg("perry-runtime-static") | ||
| .arg("-p") | ||
| .arg("perry-stdlib-static") | ||
| .output() | ||
| .expect("build static runtime archives"); | ||
| assert!( | ||
| build.status.success(), | ||
| "static runtime build failed\nstdout:\n{}\nstderr:\n{}", | ||
| String::from_utf8_lossy(&build.stdout), | ||
| String::from_utf8_lossy(&build.stderr) | ||
| ); | ||
| }); | ||
| let target = std::env::var_os("CARGO_TARGET_DIR") | ||
| .map(PathBuf::from) | ||
| .unwrap_or_else(|| workspace_root().join("target")); | ||
| target.join("debug") | ||
| } | ||
|
|
||
| const SOURCE: &str = r#" | ||
| const t = (name: string, f: () => any) => { try { console.log(name, JSON.stringify(f())) } catch (e: any) { console.log(name, "THROW", e.message) } } | ||
| const dump = (h: any) => { const out: string[] = []; h.forEach((v: string, k: string) => out.push(k + "=" + v)); return out.sort() } | ||
| t("P1 plain proxy", () => dump(new Headers(new Proxy({ "x-a": "1", "x-b": "2" }, {}) as any))) | ||
| t("P2 proxy with get trap", () => dump(new Headers(new Proxy({ "x-a": "1" }, { get: (t: any, k: any) => (typeof k === "string" && k in t ? "trapped" : (t as any)[k]) }) as any))) | ||
| t("P3 proxy with ownKeys trap hiding a key", () => dump(new Headers(new Proxy({ "x-a": "1", "x-b": "2" }, { ownKeys: () => ["x-a"], getOwnPropertyDescriptor: () => ({ configurable: true, enumerable: true, value: "1" }) }) as any))) | ||
| t("P4 proxy over empty object", () => dump(new Headers(new Proxy({}, {}) as any))) | ||
| t("P6 nested proxy", () => dump(new Headers(new Proxy(new Proxy({ "x-a": "1" }, {}), {}) as any))) | ||
| t("P8 plain object still works", () => dump(new Headers({ "x-a": "1" }))) | ||
| t("P9 array still works", () => dump(new Headers([["x-a", "1"]]))) | ||
| t("P10 map still works", () => dump(new Headers(new Map([["x-a", "1"]]) as any))) | ||
| "#; | ||
|
|
||
| const EXPECTED: &str = "P1 plain proxy [\"x-a=1\",\"x-b=2\"]\nP2 proxy with get trap [\"x-a=trapped\"]\nP3 proxy with ownKeys trap hiding a key [\"x-a=1\"]\nP4 proxy over empty object []\nP6 nested proxy [\"x-a=1\"]\nP8 plain object still works [\"x-a=1\"]\nP9 array still works [\"x-a=1\"]\nP10 map still works [\"x-a=1\"]\n"; | ||
|
|
||
| #[test] | ||
| fn headers_accepts_a_proxied_record_init() { | ||
| let dir = tempfile::tempdir().expect("tempdir"); | ||
| let entry = dir.path().join("main.ts"); | ||
| let output = dir.path().join("main_bin"); | ||
| std::fs::write(&entry, SOURCE).expect("write entry"); | ||
| let compile = Command::new(perry_bin()) | ||
| .current_dir(dir.path()) | ||
| .arg("compile") | ||
| .arg(&entry) | ||
| .arg("-o") | ||
| .arg(&output) | ||
| .arg("--no-cache") | ||
| .env("PERRY_NO_AUTO_OPTIMIZE", "1") | ||
| .env("PERRY_RUNTIME_DIR", runtime_dir()) | ||
| .output() | ||
| .expect("run perry compile"); | ||
| assert!( | ||
| compile.status.success(), | ||
| "perry compile failed\nstdout:\n{}\nstderr:\n{}", | ||
| String::from_utf8_lossy(&compile.stdout), | ||
| String::from_utf8_lossy(&compile.stderr) | ||
| ); | ||
| let run = Command::new(&output) | ||
| .current_dir(dir.path()) | ||
| .output() | ||
| .expect("run compiled binary"); | ||
| assert!( | ||
| run.status.success(), | ||
| "compiled binary failed\nstdout:\n{}\nstderr:\n{}", | ||
| String::from_utf8_lossy(&run.stdout), | ||
| String::from_utf8_lossy(&run.stderr) | ||
| ); | ||
| assert_eq!(String::from_utf8_lossy(&run.stdout), EXPECTED); | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check property descriptors before reading Proxy keys.
[[OwnPropertyKeys]]includes non-enumerable keys. A Web IDL record must call[[GetOwnProperty]]and read a value only when the descriptor exists and is enumerable. The current path adds non-enumerable headers and calls thegettrap for keys hidden bygetOwnPropertyDescriptor. (webidl.spec.whatwg.org)Add the descriptor check before
js_proxy_get. Reuse the descriptor and enumerability sequence incrates/perry-runtime/src/object/alloc.rs:1404-1480. Add regressions for a non-enumerable target property and a descriptor trap that returnsundefined.🤖 Prompt for AI Agents