fix(web): support delegated listener objects - #3209
Merged
ryansolid merged 15 commits intoSep 2, 2026
Merged
Conversation
Seven defects across the server-function guards, five of them introduced by the guards themselves (solidjs#3168, solidjs#3170, solidjs#3175, solidjs#3176). Every fix removes a special case rather than adding one — 45 lines of code in, 28 out. - The argument walk stops for no prototype (solidjs#3200) and strips the whole unsafe key set (solidjs#3202). It mutates in place and rebuilds nothing, so the non-plain-prototype `continue` was never guarding a rebuild; it only hid a payload under a carrier the codec revives with own properties. `constructor` joins `__proto__` because a recursive merge reaches Object.prototype through it. - The guard shell carries only the flag that reaches the wire (solidjs#3196, solidjs#3198). Replicating a frozen source's `writable`/`configurable` made the write-back illegal; pinning them true lost `enumerable: false` and put a deliberately hidden field on the wire. One descriptor shape replaces the two-branch conditional. - The scheme floor asks the URL parser instead of a regex (solidjs#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, which is why neither was fooled. - The event is awaited only when it is genuinely a promise, and a failure is answered rather than thrown (solidjs#3199). Awaiting anything wearing a `then` parked the request forever on a lazy-locals proxy and starved the event loop on a self-resolving one. An `async createEvent` still works. - `Content-Length` is never forwarded onto a body the transport composed (solidjs#3197). The declared length described the source; the body is ours, so the answer arrived truncated at the socket — 13 of 815 bytes over a real connection. RFC 9110 §8.6. Tests are table-driven over the adjacent shapes each fix must close and the ones it must leave alone, so a later change cannot move a hole sideways: every descriptor combination, every carrier the codec revives, every whitespace a URL parser strips in every position on every road, every thenable spelling, and every producer that merges author headers onto an encoded body. Suite: 586 -> 626 passing.
… ran Three gaps a mutation pass found in the tests, plus one simplification. The solidjs#3200 fix had NO coverage: restoring the removed `continue` left the whole suite green. The reachable shape is a plain object one level UNDER a non-plain carrier — an unsafe key ON an `Error` is dropped by the codec at encode time, so the obvious test cannot fail. The added row encodes a real `Error` carrying the payload; with the fix reverted it reports `payloadKeys=["constructor","n"]` and `Object.prototype.polluted="viaCarrier"`. Two of the five tables passed when the handler dispatched nothing: `ship()` collapsed "threw", "500" and "no header" into one "refused", and the content-length rows never mentioned status, so `absent / 0 bytes` was a pass. Both now carry the observation that makes them fail — `ran=1` and the expected status. With a handler that answers 500 as its first statement, all five files now go red (41 tests) instead of two staying green. `Headers` rejects CR/LF in a value before the scheme floor is reached, so that protection comes from the platform rather than from the code under test. The helper now reports it as its own outcome instead of counting it as a refusal the floor made. Also `Object.prototype.hasOwnProperty.call` -> `Object.hasOwn` in the strip walk: same shadow-proofing (verified against a payload that shadows `hasOwnProperty`), one line shorter, and well inside the platform floor this package already assumes elsewhere.
… params Implementation review, all three measured against the branch. `Content-Length` was one name in three hand-placed checks, and the class it belongs to is wider: `Content-Encoding` still rode onto every body the transport composed and never compressed — measured `gzip` surviving on both `respond()` and a returned Response. One `COMPOSED_BODY_FRAMING` set in `response.ts`, the leaf module all three sites already import, replaces the three checks and covers `transfer-encoding` too. It also makes the `content-length` clause in `fillsStubGap` reachable-but-redundant, so that special case goes. The `base` threaded into `refusedTargetScheme` was doing nothing: 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. That removes a parameter, a signature change and a threaded argument, so the fix stops touching the dispatch tail entirely. `Object.hasOwn(v, key)` before `delete v[key]` changes no outcome — delete on an absent or inherited key is a no-op, and on a non-configurable own key both spellings throw the same TypeError. The guard was ceremony. The Content-Encoding rows are pinned: dropping the name from the set reddens them.
🦋 Changeset detectedLatest commit: 1c9ab9b The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
Seven defects across the server-function guards, five of them introduced by the guards themselves (solidjs#3168, solidjs#3170, solidjs#3175, solidjs#3176). Every fix removes a special case rather than adding one — 45 lines of code in, 28 out. - The argument walk stops for no prototype (solidjs#3200) and strips the whole unsafe key set (solidjs#3202). It mutates in place and rebuilds nothing, so the non-plain-prototype `continue` was never guarding a rebuild; it only hid a payload under a carrier the codec revives with own properties. `constructor` joins `__proto__` because a recursive merge reaches Object.prototype through it. - The guard shell carries only the flag that reaches the wire (solidjs#3196, solidjs#3198). Replicating a frozen source's `writable`/`configurable` made the write-back illegal; pinning them true lost `enumerable: false` and put a deliberately hidden field on the wire. One descriptor shape replaces the two-branch conditional. - The scheme floor asks the URL parser instead of a regex (solidjs#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, which is why neither was fooled. - The event is awaited only when it is genuinely a promise, and a failure is answered rather than thrown (solidjs#3199). Awaiting anything wearing a `then` parked the request forever on a lazy-locals proxy and starved the event loop on a self-resolving one. An `async createEvent` still works. - `Content-Length` is never forwarded onto a body the transport composed (solidjs#3197). The declared length described the source; the body is ours, so the answer arrived truncated at the socket — 13 of 815 bytes over a real connection. RFC 9110 §8.6. Tests are table-driven over the adjacent shapes each fix must close and the ones it must leave alone, so a later change cannot move a hole sideways: every descriptor combination, every carrier the codec revives, every whitespace a URL parser strips in every position on every road, every thenable spelling, and every producer that merges author headers onto an encoded body. Suite: 586 -> 626 passing.
… ran Three gaps a mutation pass found in the tests, plus one simplification. The solidjs#3200 fix had NO coverage: restoring the removed `continue` left the whole suite green. The reachable shape is a plain object one level UNDER a non-plain carrier — an unsafe key ON an `Error` is dropped by the codec at encode time, so the obvious test cannot fail. The added row encodes a real `Error` carrying the payload; with the fix reverted it reports `payloadKeys=["constructor","n"]` and `Object.prototype.polluted="viaCarrier"`. Two of the five tables passed when the handler dispatched nothing: `ship()` collapsed "threw", "500" and "no header" into one "refused", and the content-length rows never mentioned status, so `absent / 0 bytes` was a pass. Both now carry the observation that makes them fail — `ran=1` and the expected status. With a handler that answers 500 as its first statement, all five files now go red (41 tests) instead of two staying green. `Headers` rejects CR/LF in a value before the scheme floor is reached, so that protection comes from the platform rather than from the code under test. The helper now reports it as its own outcome instead of counting it as a refusal the floor made. Also `Object.prototype.hasOwnProperty.call` -> `Object.hasOwn` in the strip walk: same shadow-proofing (verified against a payload that shadows `hasOwnProperty`), one line shorter, and well inside the platform floor this package already assumes elsewhere.
… params Implementation review, all three measured against the branch. `Content-Length` was one name in three hand-placed checks, and the class it belongs to is wider: `Content-Encoding` still rode onto every body the transport composed and never compressed — measured `gzip` surviving on both `respond()` and a returned Response. One `COMPOSED_BODY_FRAMING` set in `response.ts`, the leaf module all three sites already import, replaces the three checks and covers `transfer-encoding` too. It also makes the `content-length` clause in `fillsStubGap` reachable-but-redundant, so that special case goes. The `base` threaded into `refusedTargetScheme` was doing nothing: 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. That removes a parameter, a signature change and a threaded argument, so the fix stops touching the dispatch tail entirely. `Object.hasOwn(v, key)` before `delete v[key]` changes no outcome — delete on an absent or inherited key is a no-op, and on a non-configurable own key both spellings throw the same TypeError. The guard was ceremony. The Content-Encoding rows are pinned: dropping the name from the set reddens them.
Merging this PR will not alter performance
Comparing 
Footnotes
|
Close cross-realm, descriptor, argument, and SSR response gaps found while auditing the server-function transport. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # .changeset/server-function-guard-simplification.md # packages/web/server-functions/src/server.ts # packages/web/test/server/server-functions-event-hook.spec.tsx # packages/web/test/server/server-functions-proto-keys.spec.tsx # packages/web/test/server/server-functions-result-descriptors.spec.tsx
Avoid exposing the shared framing-header set while preserving the audited server behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
…d-simplification fix(web): judge arguments, results and redirect targets by what they are
…-removal fix(web): remove capturing event listeners
Prevent a stale bound-tuple payload from changing how replacement listener objects are dispatched. Co-authored-by: Cursor <cursoragent@cursor.com>
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
3 participants
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.
Summary
addEvent()accepts both functions andEventListenerObjecthandlers, but the delegated event dispatcher always invoked handlers with.call(). As a result, delegatedEventListenerObjecthandlers threw instead of invoking theirhandleEvent()method. This change dispatches listener objects throughhandleEvent()while preserving the existing behavior for function and tuple handlers.How did you test this change?
Added a regression test that registers a delegated listener object and verifies that:
handleEvent()is called oncethisvalue is the listener objectCommands run:
pnpm buildpnpm typespnpm test-typesvitest run test/delegated-event-listener-object.spec.tsprettier --check src/client.ts test/delegated-event-listener-object.spec.ts ../../.changeset/support-delegated-event-listener-objects.md