Repository navigation
fix(javascript): omit fetch and WebSocket requests refused at construction - #10529
Plastic-time wants to merge 3 commits into
Conversation
…ction The Request constructor rejects a URL with credentials, a method that is not a token or is forbidden, a GET or HEAD body and a header name that is not a token. The WebSocket constructor throws for a fragment, a scheme other than ws, wss, http or https, and an invalid or repeated subprotocol. Neither sends anything, so neither is a request.
A spread before the init or subprotocol argument can push the literal into a slot the host ignores. A scheme without // is relative on a page with the same scheme, so it proves no credentials.
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
 [Medium impact] The PR appears safe to merge; both previous findings are fixed and no new blocking issues were found. SummaryThis PR omits requests that
Reviews (2) · Last reviewed commit: "fix(javascript): read fetch header names..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…does A header record with its own iterator, a computed key, a spread or a __proto__ setter can supply other pairs, so its names stay undecided. The URL parser trims surrounding C0 controls and spaces and drops tabs and newlines before it finds credentials, so judge that string.
Summary
JavaScript static analysis reported a request for
fetch(...)andnew WebSocket(...)calls that the host refuses while it builds the Request or the socket. Nothing is sent for these calls, and they no longer produce arequestOperationsentry.Fixes #10523
Problem and expected behavior
Measured in Node 24.22.1 (undici) against 127.0.0.1:9:
fetch("http://user@host/x"),fetch("http://:pw@host/x")fetch(url, { method: "G ET" }),"","GET\n"fetch(url, { method: "CONNECT" }),"trace","Track"fetch(url, { body: "x" })with a string URL,{ method: "get", body: "" },{ method: "head", body: 0 }fetch(url, { headers: { "a b": "1" } }),"","x-é"new WebSocket("ws://host/x#a"),"ws://host/x#","/s#a"new WebSocket("ftp://host/x"),"file:///x","mailto:x"new WebSocket(url, "a b"),"","a/b",["chat", "chat"]Accepted neighbors, which keep their request:
{ method: "PATCH" },{ method: "M-SEARCH" },{ method: "GET", body: null },"http://:@host/x",new WebSocket("https://host/x"),"WSS://host/x",new WebSocket(url, null),new WebSocket(url, ["a", null]).The Fetch standard's Request constructor and the WebSockets standard's constructor require these same refusals, so browsers behave the same way.
Change and scope
javascriptHostRequestRefusals.tsholds the two checks.collectJavaScriptSemanticDataEffectsfilters the request candidates through it before response consumers are linked.rejected_request_targetalready states.fetchinput may carry its own method and body, so the default-GET body rule applies only to a string URL. Unknown members, spreads and computed header keys stay undecided. A spread argument stops the check for every later slot, because it can move a literal into a slot the host ignores.//. On a page with the same scheme,"http:user@host/x"is a relative path and names no user.mode,cacheorredirect, and header values.Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable.
Contract and boundary impact
Semantic owner and earliest changed stage: request candidates in
javascriptSemanticDataEffects.ts.Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: fewer false request facts; undecided calls are unchanged.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.
Evidence and regression coverage
src/domain/javascript/javascriptSemanticAnalysis.host_request_refusals.test.ts, one row per refused and accepted form.Validation performed
Windows 11 Home 10.0.26200 x64, Node 24.22.1, base 9e24b36.
npx vitest run src/domain/javascript/javascriptSemanticAnalysis.host_request_refusals.test.tson main source: 25 failed / 24 passed; with this change: 49 passed.npx vitest run src/domain/javascript src/application/javascript: 217 files, 6431 passed / 7 skipped.@@iterator, computed key, spread or__proto__setter can supply other pairs; credentials are judged on the string the URL parser reads after trimming C0 controls and spaces and dropping tabs and newlines.npm run check:fast: passed (typecheck, lint)git merge-treeagainst the open PRs that editjavascriptSemanticDataEffects.ts(fix(javascript): reject an http method token Node refuses at the call site #9683, fix(javascript): reject an http path Node refuses at the call site #9684, fix(javascript): omit requests with mismatched default protocols #10169, fix(javascript): qualify response consumers with uncertain receivers #10170, fix(javascript): omit file reads with rejected callbacks #10171): clean.Compatibility, safety, and release
Author readiness
Human owner: @Plastic-time
Before merge
type(scope): outcome.