fix(web): judge arguments, results and redirect targets by what they are

[frenzzy]

Closes #3196, #3197, #3198, #3199, #3200, #3201, #3202.

Seven defects across the server-function guards. Five were introduced by the guards themselves — #3168, #3170, #3175 and #3176 — which is the reason to fix them together: each one is a special case that turned out to be guarding nothing, and removing it closes the defect.

48 lines of code in, 19 out, across three commits: the fixes, a test pass that closed a hole in its own coverage, and a simplification pass that replaced three hand-placed one-name checks with one named set.

The five changes

The argument walk stops for no prototype (#3200) and strips the whole unsafe key set (#3202).

     } else {
-      const proto = Object.getPrototypeOf(v);
-      if (proto !== Object.prototype && proto !== null) continue;
-      if (Object.prototype.hasOwnProperty.call(v, "__proto__")) {
-        delete v["__proto__"];
-      }
+      for (const key of UNSAFE_ARGUMENT_KEYS) {
+        if (Object.prototype.hasOwnProperty.call(v, key)) delete v[key];
+      }
       for (const key of Object.keys(v)) stack.push(v[key]);
     }

stripOwnProtoKeys mutates in place and rebuilds nothing, so the continue was never protecting a rebuild — it only stopped the walk, which is the whole of #3200: a payload under a codec-revived Error kept its keys. constructor joins __proto__ because a recursive merge reaches Object.prototype through constructor.prototype, and #3168's rationale only covered the shallow spelling.

The guard shell carries only the flag that reaches the wire (#3196, #3198). It is scratch the codec reads once and the author never holds, so replicating a frozen source's writable/configurable bought nothing and made the write-back illegal; pinning them true lost enumerable: false and put a deliberately hidden field on the wire. One descriptor shape replaces a two-branch conditional.

The scheme floor asks the URL parser instead of a regex (#3201). A parser strips ASCII tab and newline before it begins, so java<TAB>script: read as scheme-less to the grammar and as javascript: to every consumer. The masked and no-JS roads already resolved through new URL(), which is why neither was ever fooled — this makes the third road do what the other two do, and deletes a hand-written scheme grammar. No base is threaded: across 19 targets x 3 real bases the verdict is identical to a constant stand-in, because an absolute scheme always beats the base and a relative target always inherits an http(s) one.

The event is awaited only when it is genuinely a promise, and a failure is answered rather than thrown (#3199). Awaiting anything wearing a then parked the request forever on a lazy-locals proxy and starved the whole event loop on a self-resolving one. An async createEvent — #3170's own case — returns a real promise and still works.

No framing header is forwarded onto a body the transport composed (#3197). The declared length described the source; the body is ours. Over a real socket the answer arrived as 13 of 815 bytes.

Content-Length turned out to be one name in a class. Measured on the first version of this branch, Content-Encoding: gzip still rode onto every composed body — telling the peer to decompress bytes nobody compressed — on both respond() and a returned Response. So the three hand-placed one-name checks became one COMPOSED_BODY_FRAMING set in response.ts, the leaf module all three sites already import, covering transfer-encoding too and making the content-length special case in fillsStubGap redundant.

Tests

Five spec files, 40 tests, table-driven over the adjacent shapes — deliberately, because three of these seven defects were a previous fix closing one shape while the hole moved to the shape next door. Each table asserts both halves:

• what the fix must close: every descriptor combination; every carrier the codec revives; every whitespace a URL parser strips, in every position, on every road; every thenable spelling; every producer that merges author headers onto an encoded body
• what it must leave alone: sealed and partially-frozen objects, frozen arrays and Maps, a Map keyed by the literal string "__proto__", a field named constructorName, relative and cross-origin and protocol-relative redirect targets, a genuinely async createEvent, and a Content-Length the runtime did not invalidate

before   53 files   586 passed | 2 skipped
after    58 files   627 passed | 2 skipped

Every one of the five files goes red (41 tests) against a handler that answers 500 as its first statement — two of them did not, until the second commit put ran=1 and the expected status into the row.

No existing test changed.

Deliberately not included

• The X-Content-Raw passthrough keeps its author-supplied Content-Length: there the runtime forwards the author's own body, so §8.6's "known to be incorrect" does not apply.
• decodeFlashCookie and the client's decodeResponse still have no key strip, so __proto__ survives both. That is Decoded arguments keep __proto__ as an own key, so an ordinary Object.assign merge in a handler re-prototypes the result #3168's defect one road over and wants its own decision about scope — happy to fold it in here or file it separately.

---

Why delete and not a copy — measured

The value handed to stripOwnProtoKeys comes straight from JSON.parse(args) or the codec decode, one or two lines earlier (server.ts:1204, :1230). Nothing else holds a reference, so mutating it is safe and copying the graph on every request would be pure cost.

The alternative that looks cleaner — stripping inside JSON.parse with a reviver — is three times slower, because a reviver runs a callback for every key of every object while delete touches only the rare object that carries one:

parse + delete      :  97 ms     (200k iterations)
parse with reviver  : 300 ms

delete does move the object off V8's fast path, and that is real but small: 16 ms vs 12 ms over two million property reads, and only for objects that actually had the key.

What the strip is and is not protecting against — measured

Three of the four idiomatic ways to copy a decoded object are already safe, which narrows this change and belongs in the record:

Object.assign({}, src)   prototype replaced: true      ← the only unsafe one
{ ...src }               prototype replaced: false
structuredClone(src)     prototype replaced: false
Object.fromEntries(...)  prototype replaced: false
Object.prototype itself polluted: false

Object.assign is unsafe because it merges through [[Set]], which wakes the inherited setter; spread and fromEntries define rather than set. The constructor road narrows identically — t[k] = v reaches Object.prototype, defineProperty does not.

So this part of the PR protects application code from a footgun the application can avoid by writing spread. That is a boundary decision, and reviewers should take it as one rather than as a clear-cut fix. The argument for the runtime owning it is that this boundary is already hardened in other ways — depth cap, RegExp revival disabled, argument count bounded. The argument against is that the strip is lossy: a field honestly named constructor disappears with no error. Refusing with a 400 that names the key would keep the protection without the silent loss, and I am happy to switch to that if it is the preferred shape.

Two claims from review that did not survive checking

• redirect() and reload() still forwarding a stale Content-Length — refuted. Measured on this branch: content-length=null for both. The merge filter already covers them.
• URL.parse() would remove the try/catch — rejected on evidence rather than taste. It is Node 22.1 / Safari 18, while this package still carries a fallback for a Headers implementation without getSetCookie (≈ Node 19.7), and the twin check decodeRedirectHeaderValue lives in shared.ts, which ships to browsers — the two halves of one floor would end up on different idioms.

[changeset-bot]

🦋 Changeset detected

Latest commit: afed2e0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
@solidjs/web	Patch
@solidjs/babel-plugin	Patch
@solidjs/element	Patch
@solidjs/h	Patch
@solidjs/html	Patch
test-integration	Patch
@solidjs/compiler	Patch
@solidjs/diagnostics	Patch
@solidjs/signals	Patch
solid-js	Patch
@solidjs/universal	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codspeed-hq]

Merging this PR will not alter performance

✅ 136 untouched benchmarks
⏩ 132 skipped benchmarks¹

---

_{Comparing frenzzy:fix/server-function-guard-simplification (afed2e0) with next (8d1a011)}

Footnotes

1. 132 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩