From a0270c6cecbeb9252fe8bda266e6c88ec7962fde Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Wed, 2 Sep 2026 11:44:23 +0700 Subject: [PATCH 01/14] fix(web): judge arguments, results and redirect targets by what they are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven defects across the server-function guards, five of them introduced by the guards themselves (#3168, #3170, #3175, #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 (#3200) and strips the whole unsafe key set (#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 (#3196, #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 (#3201). A parser strips ASCII tab and newline before it begins, so `javascript:` 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 (#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 (#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. --- .../server-function-guard-simplification.md | 13 + packages/web/server-functions/src/server.ts | 99 +++++-- packages/web/src/response.ts | 3 + packages/web/src/server.ts | 5 +- .../server-functions-content-length.spec.tsx | 191 +++++++++++++ .../server-functions-event-hook.spec.tsx | 210 ++++++++++++++ .../server-functions-proto-keys.spec.tsx | 198 +++++++++++++ .../server-functions-redirect-scheme.spec.tsx | 211 ++++++++++++++ ...rver-functions-result-descriptors.spec.tsx | 263 ++++++++++++++++++ 9 files changed, 1165 insertions(+), 28 deletions(-) create mode 100644 .changeset/server-function-guard-simplification.md create mode 100644 packages/web/test/server/server-functions-content-length.spec.tsx create mode 100644 packages/web/test/server/server-functions-event-hook.spec.tsx create mode 100644 packages/web/test/server/server-functions-proto-keys.spec.tsx create mode 100644 packages/web/test/server/server-functions-redirect-scheme.spec.tsx create mode 100644 packages/web/test/server/server-functions-result-descriptors.spec.tsx diff --git a/.changeset/server-function-guard-simplification.md b/.changeset/server-function-guard-simplification.md new file mode 100644 index 000000000..d9be0875d --- /dev/null +++ b/.changeset/server-function-guard-simplification.md @@ -0,0 +1,13 @@ +--- +"@solidjs/web": patch +--- + +Judge decoded arguments, guarded results and navigation targets by what they +are rather than by how they are spelled, and never forward a `Content-Length` +that describes a body the transport replaced. + +Seven defects, five of them introduced by the guards added in #3168, #3170, +#3175 and #3176. Each fix removes a special case rather than adding one: the +argument walk stops for no prototype, the guard shell keeps only the flag that +reaches the wire, the scheme floor asks the URL parser instead of a regex, and +the event is awaited only when it is genuinely a promise. diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 2787988f7..aaba244ce 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -1113,6 +1113,8 @@ function assertDecodeDepth(value) { * Sets; a value nested inside a revived class instance keeps its shape — * the codec owns that value's construction, not this guard. */ +const UNSAFE_ARGUMENT_KEYS = ["__proto__", "constructor", "prototype"]; + function stripOwnProtoKeys(value) { const stack = [value]; const seen = new Set(); @@ -1127,10 +1129,13 @@ function stripOwnProtoKeys(value) { } else if (v instanceof Set) { for (const member of v) stack.push(member); } else { - const proto = Object.getPrototypeOf(v); - if (proto !== Object.prototype && proto !== null) continue; - if (Object.prototype.hasOwnProperty.call(v, "__proto__")) { - delete v["__proto__"]; + // Every object is walked, whatever its prototype: this mutates in place + // and rebuilds nothing, so a non-plain prototype was never a reason to + // stop — it only hid a payload under a carrier the codec revives with + // own properties (#3200). `constructor` rides alongside `__proto__` + // because a recursive merge reaches Object.prototype through it (#3202). + 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]); } @@ -1427,7 +1432,10 @@ export function foldSetCookies(headers, setCookies) { // way response headers may merge here: never `get`/`set` folding. function mergeResponseHeaders(target, source) { source.forEach((value, key) => { - if (key !== "set-cookie") target.append(key, value); + // `content-length` describes the source's body and the caller is about to + // send a different one; forwarding a length known to be wrong truncates + // the answer at the socket (#3197, RFC 9110 §8.6). + if (key !== "set-cookie" && key !== "content-length") target.append(key, value); }); if (source.getSetCookie) { for (const cookie of source.getSetCookie()) target.append("Set-Cookie", cookie); @@ -1501,10 +1509,20 @@ const BOUNDED_COMPOSED_HEADERS = [REDIRECT_HEADER, "Location", REVALIDATE_HEADER // gives it an opt-in. The decoder enforces the same floor independently // (decodeRedirectHeaderValue), so a hostile peer cannot re-open the class // against integrations either. -function refusedTargetScheme(target) { - // explicit scheme present and not http(s) → refused - const match = /^[a-zA-Z][a-zA-Z0-9+.-]*:/.exec(target); - return match !== null && !/^https?:$/i.test(match[0]); +function refusedTargetScheme(target, base) { + // Judge the target the way a consumer will READ it, not the way its bytes + // are spelled: a URL parser strips ASCII tab and newline from anywhere and + // trims leading C0/space before it begins, so a scheme grammar over the raw + // value saw nothing where the parser sees `javascript:` (#3201). Resolving + // is what the masked and no-JS roads already did, which is why neither was + // fooled; the base keeps relative targets — the ordinary case — http(s). + let protocol; + try { + protocol = new URL(target, base).protocol; + } catch { + return true; + } + return protocol !== "http:" && protocol !== "https:"; } // The transport half of redirect()'s and initWithRevalidate's invariants: @@ -1519,7 +1537,7 @@ function refusedTargetScheme(target) { // or rewritten target is a DIFFERENT address, a dropped revalidate key is // a silently stale cache. Runs ahead of the stub fold so integration // cookies still ride the refusal (#3159). -function enforceComposedHeaderInvariants(response) { +function enforceComposedHeaderInvariants(response, base) { for (const name of BOUNDED_COMPOSED_HEADERS) { const value = response.headers.get(name); if (value === null) continue; @@ -1542,7 +1560,7 @@ function enforceComposedHeaderInvariants(response) { if (name === REVALIDATE_HEADER) continue; // REDIRECT_HEADER rides as " "; Location is the target const target = name === REDIRECT_HEADER ? value.slice(value.indexOf(" ") + 1) : value; - if (refusedTargetScheme(target)) { + if (refusedTargetScheme(target, base)) { return refuseComposedHeader( response, name, @@ -1822,13 +1840,12 @@ export function guardFailures(value, state) { // value needed no wrapping): only the rebuilt shell carries the // data property — pass the original through and the codec would // invoke the getter afresh, minting a new unguarded channel. - Object.defineProperty( - top.next, - items[i], - top.accessorRead === i - ? { enumerable: true, configurable: true, writable: true, value: guarded } - : { ...top.descriptors[items[i]], value: guarded } - ); + Object.defineProperty(top.next, items[i], { + value: guarded, + writable: true, + configurable: true, + enumerable: top.descriptors[items[i]].enumerable + }); top.changed = true; } top.i++; @@ -2029,7 +2046,17 @@ function enterGuard(value, state) { // rebuild. Getter-backed accessors are materialized by the driver (#3176): // invoked once there, rewritten as data properties on this shell. const descriptors = Object.getOwnPropertyDescriptors(value); - const next = Object.create(prototype, descriptors); + // The shell is scratch the codec reads once and the author never holds, so + // only `enumerable` has to survive — it is what the codec serializes. + // Replicating a frozen source's `writable`/`configurable` made the + // write-back below illegal (#3196) and pinning them true lost + // `enumerable: false` (#3198). + const rewritable = {}; + for (const key of Object.keys(descriptors)) { + rewritable[key] = { ...descriptors[key], writable: true, configurable: true }; + if ("get" in rewritable[key]) delete rewritable[key].writable; + } + const next = Object.create(prototype, rewritable); state.seen.set(value, next); return new Frame(OBJECT, value, next, Object.keys(descriptors), descriptors); } @@ -2784,14 +2811,32 @@ export async function handleServerFunctionRequest(request, options = {}) { } } - let event = options.createEvent ? options.createEvent(request) : { request, locals: {} }; // An async createEvent is out of contract (the type is synchronous), but - // handing a pending Promise downstream as the event is the worst failure - // available: the function runs, the caller sees 200, and every header the - // integration wrote on the real event's stub silently vanishes (#3170). - // Awaiting is strictly better than refusing — the resolved value IS the - // event the integration meant. - if (typeof (event as any)?.then === "function") event = await event; + // handing a pending Promise downstream is the worst failure available: the + // function runs, the caller sees 200, and every header the integration + // wrote on the stub vanishes (#3170). So it is awaited — but only when it + // is genuinely a Promise. The event is a datum an integration handed back, + // not something the runtime asked to be async, and awaiting anything + // wearing a `then` parked the request forever on a lazy-locals proxy and + // starved the event loop on a self-resolving one (#3199). A failure here + // is answered rather than thrown: no event exists yet, so there is no stub + // to fold and nothing downstream can report it. + let event; + try { + event = options.createEvent ? options.createEvent(request) : { request, locals: {} }; + if (event instanceof Promise) event = await event; + } catch (error) { + // tagged like every other error answer, so the transport reads it as ours + const headers = new Headers(); + headers.set( + ERROR_HEADER, + boundedErrorHeaderValue( + DEV ? String((error as any)?.message ?? error) : GENERIC_SERVER_ERROR_MESSAGE + ) + ); + const response = new Response(null, { status: 500, headers }); + return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); + } // Once an event exists, its response stub folds onto EVERY exit — the // refusals below included (#3159). A refusal that returned directly // dropped the stub silently: an integration's Set-Cookie written in @@ -3202,7 +3247,7 @@ export async function handleServerFunctionRequest(request, options = {}) { // foreign-response path at once (raw passthrough, unscripted returns and // throws, custom handleNoJS results, envelope-carried responses). const response = commitEventResponse( - enforceComposedHeaderInvariants(ownResponse(await dispatch())), + enforceComposedHeaderInvariants(ownResponse(await dispatch()), request.url), event ); return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); diff --git a/packages/web/src/response.ts b/packages/web/src/response.ts index 535f65660..77be09ee9 100644 --- a/packages/web/src/response.ts +++ b/packages/web/src/response.ts @@ -264,6 +264,9 @@ export function respond(value: T, init: ResponseHelperInit = {}) { // Carry the metadata bodiless; the server-function encoder answers the // void shapes with a real null-body response and reports value-carrying // ones legibly. + // The body below is ours, so an author-supplied length describes something + // else — and on a null-body status there is no body at all (#3197). + headers.delete("Content-Length"); if (NULL_BODY_STATUSES.has(responseInit.status)) { return new ResponseEnvelope(new Response(null, { ...responseInit, headers }), value); } diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index 37dbbcee9..ec5a9b223 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -4460,7 +4460,10 @@ const STUB_GAP_FILL_EXCLUDED = /*#__PURE__*/ new Set( SINGLE_FLIGHT_HEADER, REVALIDATE_HEADER, REDIRECT_HEADER, - "Location" + "Location", + // written before the body exists, so it can only describe a different + // one (#3197) + "Content-Length" ].map(header => header.toLowerCase()) ); diff --git a/packages/web/test/server/server-functions-content-length.spec.tsx b/packages/web/test/server/server-functions-content-length.spec.tsx new file mode 100644 index 000000000..cd5c2bb59 --- /dev/null +++ b/packages/web/test/server/server-functions-content-length.spec.tsx @@ -0,0 +1,191 @@ +/** + * A `Content-Length` an author did not compute must never describe a body + * the transport composed (#3197). + * + * Three producers merge author-supplied headers onto an answer whose body + * the runtime encodes itself — the `respond()` envelope, a returned/thrown + * `Response` on the scripted road, and the request event's response stub + * gap-fill — and a stale length is not a cosmetic mismatch: RFC 9112 §6.3 + * has a recipient with a valid `Content-Length` read exactly that many + * octets and stop, so the answer arrives truncated with no error anywhere, + * and RFC 9110 §8.6 forbids forwarding a length "known to be incorrect" at + * all. `createNoJSHandler` already reconciles this by deleting the header + * from the redirect it builds; the other producers did not. + * + * The invariant asserted here is one line wide and holds for every answer + * the handler can emit: either there is no `Content-Length`, or it equals + * the number of bytes the response body actually carries. The controls + * matter as much as the repros — a length the runtime did NOT invalidate + * (an unscripted passthrough of the author's own body) must survive, and + * the streaming road must stay length-free so it can still chunk. + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { createRequestEvent, respond } from "@solidjs/web"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const provideEvent = (_event: unknown, run: () => T): T => run(); + +type Road = "scripted" | "plain" | "form"; + +async function call(id: string, { road = "scripted" as Road, stub = null as string | null } = {}) { + const address = road === "scripted" ? `/_server/data/${id}` : `/_server/${id}`; + const form = road === "form"; + const response = await handleServerFunctionRequest( + new Request(`https://app.example${address}`, { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + ...(form + ? { + "Content-Type": "application/x-www-form-urlencoded", + "Sec-Fetch-Mode": "navigate", + Referer: "https://app.example/page" + } + : {}) + }, + ...(form ? { body: "a=1" } : {}) + }), + { + provideEvent, + createEvent: request => { + const event = createRequestEvent(request); + // a middleware parking a length on the response stub + if (stub !== null) event.response.headers.set("Content-Length", stub); + return event; + } + } + ); + const declared = response.headers.get("Content-Length"); + const bytes = response.body ? (await response.arrayBuffer()).byteLength : 0; + return { status: response.status, declared, bytes }; +} + +/** + * The whole contract in one line: a declared length is the length that was + * sent. Rendered as a row so a failure names every producer at once rather + * than only the first. + */ +function row(label: string, r: { declared: string | null; bytes: number }) { + return `${label}: Content-Length=${r.declared ?? "absent"} body=${r.bytes} bytes`; +} +function expected(label: string, r: { declared: string | null; bytes: number }) { + return `${label}: Content-Length=${r.declared === null ? "absent" : r.bytes} body=${r.bytes} bytes`; +} + +describe("Content-Length never describes a body the transport composed (#3197)", () => { + it("holds across every producer that merges author headers onto an encoded body", async () => { + // the proxy shape: `return await fetch(upstream)` — every fetch Response + // carries a Content-Length, and the author wrote none of it + registerServerFunction("cl-returned-response", async () => { + return new Response("upstream body", { + headers: { "Content-Type": "text/html", "Content-Length": "13" } + }); + }); + registerServerFunction("cl-thrown-response", async () => { + throw new Response("upstream body", { + headers: { "Content-Type": "text/html", "Content-Length": "13" } + }); + }); + registerServerFunction("cl-returned-envelope", async () => + respond({ ok: true, n: 42 }, { headers: { "Content-Length": "999" } }) + ); + registerServerFunction("cl-thrown-envelope", async () => { + throw respond({ ok: true, n: 42 }, { headers: { "Content-Length": "999" } }); + }); + registerServerFunction("cl-redirect", async () => { + throw new Response(null, { + status: 302, + headers: { Location: "/done", "Content-Length": "42" } + }); + }); + registerServerFunction("cl-string", async () => "seven!!"); + for (const status of [204, 205, 304]) { + registerServerFunction(`cl-null-${status}`, async () => + respond(undefined, { status, headers: { "Content-Length": "5" } }) + ); + } + + const cases: [string, () => Promise][] = [ + ["returned Response + Content-Length (codec road)", () => call("cl-returned-response")], + ["thrown Response + Content-Length", () => call("cl-thrown-response")], + ["returned respond() envelope (JSON road)", () => call("cl-returned-envelope")], + ["thrown respond() envelope", () => call("cl-thrown-envelope")], + ["thrown 302 carrying a Content-Length", () => call("cl-redirect")], + ["stub gap-fill: middleware sets 0", () => call("cl-string", { stub: "0" })], + ["stub gap-fill: middleware sets 999", () => call("cl-string", { stub: "999" })], + ["204 + Content-Length", () => call("cl-null-204")], + ["205 + Content-Length", () => call("cl-null-205")], + ["304 + Content-Length", () => call("cl-null-304")], + // unscripted: the same producers, plain-HTTP road + [ + "unscripted respond() envelope + Content-Length", + () => call("cl-returned-envelope", { road: "plain" }) + ], + ["unscripted stub gap-fill", () => call("cl-string", { road: "plain", stub: "999" })], + ["no-JS form post + Content-Length", () => call("cl-returned-response", { road: "form" })] + ]; + + const actual: string[] = []; + const want: string[] = []; + for (const [label, run] of cases) { + const r = await run(); + actual.push(row(label, r)); + want.push(expected(label, r)); + } + expect(actual).toEqual(want); + }); + + it("controls: nothing that was already correct changes", async () => { + registerServerFunction( + "cl-ctl-response", + async () => new Response("upstream body", { headers: { "Content-Type": "text/html" } }) + ); + registerServerFunction("cl-ctl-envelope", async () => respond({ ok: true, n: 42 })); + registerServerFunction("cl-ctl-string", async () => "seven!!"); + registerServerFunction("cl-ctl-stream", async function* () { + yield "a"; + yield "b"; + } as any); + // the author serves their OWN body on the plain road: the runtime never + // re-encodes it, so their length is the truth and must survive + registerServerFunction( + "cl-ctl-passthrough", + async () => + new Response("upstream body", { + headers: { "Content-Type": "text/html", "Content-Length": "13" } + }) + ); + + const passthrough = await call("cl-ctl-passthrough", { road: "plain" }); + expect(passthrough.declared).toBe("13"); + expect(passthrough.bytes).toBe(13); + + for (const [label, run] of [ + ["returned Response, no length", () => call("cl-ctl-response")], + ["respond() envelope, no length", () => call("cl-ctl-envelope")], + ["plain string result", () => call("cl-ctl-string")], + ["streaming result stays length-free", () => call("cl-ctl-stream")] + ] as [string, () => Promise][]) { + const r = await run(); + expect(`${label}: ${r.declared}`).toBe(`${label}: null`); + expect(r.bytes).toBeGreaterThan(0); + } + }); +}); diff --git a/packages/web/test/server/server-functions-event-hook.spec.tsx b/packages/web/test/server/server-functions-event-hook.spec.tsx new file mode 100644 index 000000000..9dc1c75a7 --- /dev/null +++ b/packages/web/test/server/server-functions-event-hook.spec.tsx @@ -0,0 +1,210 @@ +/** + * `createEvent`'s await (#3199). #3170 made the runtime tolerate an async + * `createEvent` by awaiting its return — but it duck-types the value: + * + * if (typeof event?.then === "function") event = await event; + * + * Anything carrying a `then` is treated as a promise. An event that merely + * LOOKS thenable — a lazy-locals Proxy answering any unknown key, a tracing + * wrapper — is awaited on a `then` nobody ever calls, and the request hangs + * with no response, no timeout and no log. One spelling is worse: a `then` + * that resolves with the event itself spins the promise-resolution + * procedure forever and starves the whole event loop, not just the request. + * + * The second half is independent: the call sits outside every try, so a + * rejecting `createEvent` — a session store that is down, which is exactly + * the condition the hook exists to survive — escapes `handleServerFunctionRequest` + * with no status at all. + * + * Like the other server-function specs, these run against the built + * bundles (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createRequestEvent } from "@solidjs/web"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const H = { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Format": "8", + "X-Server-Function-Instance": "server-function:test" +}; + +function scriptedPost(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: H + }); +} + +const stamp = (event: any) => { + event.response.headers.append("Set-Cookie", "sid=abc; Path=/"); + return event; +}; + +/** Answers within `ms` or reports the hang as a value, never as a timeout. */ +async function within(work: Promise, ms = 1000): Promise { + let timer: any; + const result = await Promise.race([ + work, + new Promise<"HUNG">(resolve => (timer = setTimeout(() => resolve("HUNG"), ms))) + ]); + clearTimeout(timer); + return result; +} + +describe("a thenable createEvent still dispatches (#3199)", () => { + // Every spelling of "carries a then" that an integration can arrive at by + // accident. None of them is a promise; all of them are awaited today. + const thenables: [string, (request: Request) => unknown][] = [ + [ + "an own `then` that never settles", + request => { + const event: any = stamp(createRequestEvent(request)); + event.then = () => {}; + return event; + } + ], + [ + "a non-enumerable own `then`", + request => { + const event: any = stamp(createRequestEvent(request)); + Object.defineProperty(event, "then", { value: () => {}, enumerable: false }); + return event; + } + ], + [ + "`then` behind a getter", + request => { + const event: any = stamp(createRequestEvent(request)); + Object.defineProperty(event, "then", { get: () => () => {}, configurable: true }); + return event; + } + ], + // the realistic shape: a lazy-locals / auto-stub proxy answers ANY + // unknown key with something, and `then` is an unknown key + [ + "a Proxy answering unknown keys", + request => { + const event: any = stamp(createRequestEvent(request)); + return new Proxy(event, { + get: (target, key) => + key in target ? (target as any)[key] : key === "then" ? () => {} : undefined + }); + } + ], + // NOTE: this row starves the event loop before the fix — the promise + // resolution procedure re-adopts the same thenable forever. Post-fix it + // is never awaited at all. + [ + "a `then` that resolves with the event itself", + request => { + const event: any = stamp(createRequestEvent(request)); + event.then = (resolve: (v: unknown) => void) => resolve(event); + return event; + } + ] + ]; + + test.each(thenables)("%s dispatches normally", async (label, createEvent) => { + const id = "event-thenable-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let ran = 0; + registerServerFunction(id, async () => { + ran++; + return "ok"; + }); + + const response = await within( + handleServerFunctionRequest(scriptedPost(id), { createEvent: createEvent as any }) + ); + + expect(response).not.toBe("HUNG"); + expect((response as Response).status).toBe(200); + expect(ran).toBe(1); + // the event is still the integration's event: its stub folds as always + expect((response as Response).headers.getSetCookie()).toContain("sid=abc; Path=/"); + }); +}); + +describe("a failing createEvent is answered, not escaped (#3199)", () => { + const failing: [string, (request: Request) => unknown][] = [ + [ + "a rejecting async createEvent", + async () => { + throw new Error("session store unreachable"); + } + ], + [ + "a synchronously throwing createEvent", + () => { + throw new Error("session store unreachable"); + } + ] + ]; + + test.each(failing)("%s answers a sanitized 500", async (label, createEvent) => { + const id = "event-failing-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let ran = 0; + registerServerFunction(id, async () => { + ran++; + return "unreached"; + }); + + const response = await within( + handleServerFunctionRequest(scriptedPost(id), { createEvent: createEvent as any }) + ); + + expect(response).not.toBe("HUNG"); + expect((response as Response).status).toBe(500); + // sanitized: the store's own message is not the caller's business + expect((response as Response).headers.get("X-Server-Function-Error")).toBe( + "Internal Server Error" + ); + expect(ran).toBe(0); + }); +}); + +describe("the awaited shapes #3170 added keep working (#3199 baseline)", () => { + const controls: [string, (request: Request) => unknown][] = [ + [ + "a genuinely async createEvent", + async (request: Request) => { + await Promise.resolve(); + return stamp(createRequestEvent(request)); + } + ], + ["a plain synchronous createEvent", request => stamp(createRequestEvent(request))] + ]; + + test.each(controls)("%s still lands its cookies on the wire", async (label, createEvent) => { + const id = "event-control-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let ran = 0; + registerServerFunction(id, async () => { + ran++; + return "ok"; + }); + + const response = await within( + handleServerFunctionRequest(scriptedPost(id), { createEvent: createEvent as any }) + ); + + expect(response).not.toBe("HUNG"); + expect((response as Response).status).toBe(200); + expect(ran).toBe(1); + expect((response as Response).headers.getSetCookie()).toContain("sid=abc; Path=/"); + }); +}); diff --git a/packages/web/test/server/server-functions-proto-keys.spec.tsx b/packages/web/test/server/server-functions-proto-keys.spec.tsx new file mode 100644 index 000000000..509d95b53 --- /dev/null +++ b/packages/web/test/server/server-functions-proto-keys.spec.tsx @@ -0,0 +1,198 @@ +/** + * The decode boundary strips the keys that turn an ordinary merge into + * prototype pollution — all of them, on every road (#3168, #3202). + * + * #3168 stripped `__proto__` because `Object.assign` merges by [[Set]], so + * the key fires the inherited setter and re-prototypes the merged copy. + * That reasoning covers a SHALLOW merge. A recursive merge — at least as + * common in configuration and patch handling — walks into an own + * `constructor`, finds `prototype`, and writes onto `Object.prototype` + * itself: strictly worse than the case that was fixed, because it escapes + * the copy and reaches the whole process. + * + * Half-covering the class is the failure mode this table exists to prevent: + * an author who read #3168 and concluded the boundary was handled is wrong + * for the recursive spelling. So the assertion is not "`__proto__` is + * gone" but "`Object.prototype` is untouched after a naive recursive merge + * of a decoded argument", across every key and every road that decodes one. + * + * A field named `constructorName` is the control: the strip must not eat + * ordinary data. + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { serializeString } from "@solidjs/web/server-functions/client"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; + delete (Object.prototype as any).polluted; +}); + +const provideEvent = (_event: unknown, run: () => T): T => run(); + +/** The naive recursive merge #3168's own rationale names as the sink. */ +function deepMerge(target: any, source: any) { + for (const key of Object.keys(source)) { + if (source[key] && typeof source[key] === "object") { + target[key] ??= {}; + deepMerge(target[key], source[key]); + } else target[key] = source[key]; + } + return target; +} + +/** + * A hostile peer's codec frame. The codec's OWN encoder refuses to + * serialize an object with an own `constructor` key, so no honest client + * can produce one — but the frame is just text, and nothing stops a peer + * from writing it by hand. Encode under placeholder names, rename them in + * the payload, re-length the frame header. + */ +const HOSTILE_RENAME: Record = { + ctorKey: "constructor", + protoKey: "__proto__", + prototypeKey: "prototype" +}; + +async function hostileFrame(value: unknown) { + const framed = await serializeString(value); + let json = framed.slice(framed.indexOf(";", 1) + 1); + for (const [from, to] of Object.entries(HOSTILE_RENAME)) + json = json.split(`"${from}"`).join(`"${to}"`); + const length = new TextEncoder().encode(json).byteLength; + return `;0x${length.toString(16).padStart(8, "0")};${json}`; +} + +type Road = "json-query" | "json-body" | "codec-query" | "codec-body"; + +let seq = 0; + +/** Runs one call and hands back the argument exactly as the function saw it. */ +async function decodeArgument(road: Road, payload: unknown) { + const id = `proto-keys-${seq++}`; + let seen: unknown; + registerServerFunction(id, async (first: unknown) => { + seen = first; + return "ok"; + }); + const address = `https://app.example/_server/data/${id}`; + const headers: Record = { "Sec-Fetch-Site": "same-origin" }; + let request: Request; + if (road === "json-query") { + request = new Request(`${address}?args=${encodeURIComponent(JSON.stringify([payload]))}`, { + method: "POST", + headers + }); + } else if (road === "json-body") { + request = new Request(address, { + method: "POST", + headers: { ...headers, "Content-Type": "application/json", "X-Server-Function-Format": "8" }, + body: JSON.stringify([payload]) + }); + } else if (road === "codec-query") { + request = new Request(`${address}?args=${encodeURIComponent(await hostileFrame([payload]))}`, { + method: "POST", + headers + }); + } else { + request = new Request(address, { + method: "POST", + headers: { ...headers, "Content-Type": "text/plain", "X-Server-Function-Format": "0" }, + body: await hostileFrame([payload]) + }); + } + const response = await handleServerFunctionRequest(request, { provideEvent }); + return { status: response.status, seen }; +} + +/** + * The same payload in the two spellings the two decode roads can carry: the + * JSON road takes the dangerous names literally; the codec road takes + * placeholders that `hostileFrame` renames back. + */ +const PAYLOADS: [string, unknown, unknown][] = [ + [ + "__proto__", + JSON.parse('{"__proto__":{"polluted":"viaProto"},"n":1}'), + { protoKey: { polluted: "viaProto" }, n: 1 } + ], + [ + "constructor", + JSON.parse('{"constructor":{"prototype":{"polluted":"viaCtor"}},"n":1}'), + { ctorKey: { prototypeKey: { polluted: "viaCtor" } }, n: 1 } + ], + [ + "prototype", + JSON.parse('{"prototype":{"polluted":"viaPrototype"},"n":1}'), + { prototypeKey: { polluted: "viaPrototype" }, n: 1 } + ], + [ + "constructor nested one level", + JSON.parse('{"a":{"constructor":{"prototype":{"polluted":"viaNested"}}},"n":1}'), + { a: { ctorKey: { prototypeKey: { polluted: "viaNested" } } }, n: 1 } + ], + [ + "constructor inside an array", + JSON.parse('[{"constructor":{"prototype":{"polluted":"viaArray"}}}]'), + [{ ctorKey: { prototypeKey: { polluted: "viaArray" } } }] + ] +]; + +const ROADS: Road[] = ["json-query", "json-body", "codec-query", "codec-body"]; + +describe("decoded arguments cannot reach Object.prototype (#3202)", () => { + it("no dangerous key survives any decode road into a recursive merge", async () => { + const rows: string[] = []; + for (const [name, jsonPayload, codecPayload] of PAYLOADS) { + for (const road of ROADS) { + const { status, seen } = await decodeArgument( + road, + road.startsWith("codec") ? codecPayload : jsonPayload + ); + // shallow merge — the sink #3168 closed + const shallow: any = {}; + Object.assign(shallow, seen); + const reprototyped = Object.getPrototypeOf(shallow) !== Object.prototype; + // recursive merge — the sink #3168 left open + deepMerge({}, seen); + const leaked = (Object.prototype as any).polluted; + delete (Object.prototype as any).polluted; + rows.push( + `${name} / ${road}: status=${status} reprototyped=${reprototyped} Object.prototype.polluted=${JSON.stringify( + leaked + )}` + ); + } + } + expect(rows).toEqual( + rows.map( + r => + `${r.slice(0, r.indexOf(": ") + 2)}status=200 reprototyped=false Object.prototype.polluted=undefined` + ) + ); + }); + + it("control: ordinary data with a similar name is not eaten", async () => { + const rows: string[] = []; + for (const road of ROADS) { + const { status, seen } = await decodeArgument(road, { constructorName: "Widget", n: 1 }); + rows.push(`${road}: status=${status} ${JSON.stringify(seen)}`); + } + expect(rows).toEqual( + ROADS.map(road => `${road}: status=200 {"constructorName":"Widget","n":1}`) + ); + }); +}); diff --git a/packages/web/test/server/server-functions-redirect-scheme.spec.tsx b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx new file mode 100644 index 000000000..d1ec19b20 --- /dev/null +++ b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx @@ -0,0 +1,211 @@ +/** + * The navigation-target scheme floor (#3175) must judge the target the way + * a URL parser will read it, not the way the bytes are spelled (#3201). + * + * `refusedTargetScheme` matched `/^[a-zA-Z][a-zA-Z0-9+.-]*:/` against the + * RAW header value. Every URL parser removes ASCII tab, LF and CR from a + * URL before it begins (WHATWG URL, "basic URL parser", step 2), so a TAB + * anywhere inside the scheme token made the regex see no scheme at all + * while the consumer sees `javascript:` — the floor was one character wide. + * + * Two of the roads were never fooled, because they resolve through + * `new URL()` first: the scripted mask (`maskRedirect`) and the no-JS + * handler, plus the client-side `decodeRedirectHeaderValue`. The roads that + * read a header RAW — an author's `Location` on a forwarded 3xx, and an + * author's or hook's `X-Server-Function-Redirect` — were not, which is + * exactly the road the floor was documented to backstop. + * + * The table is the point: every refused scheme, every whitespace character + * a URL parser strips, in every position it can sit, on every road. And + * the allowances the floor deliberately keeps — relative, same-origin + * absolute, CROSS-ORIGIN absolute (OAuth hand-offs), protocol-relative — + * are asserted alongside, because a fix that resolves through `new URL()` + * without a base refuses all of them. + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + REDIRECT_HEADER, + decodeRedirectHeaderValue, + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const provideEvent = (_event: unknown, run: () => T): T => run(); + +/** The four roads a navigation target can leave the transport on. */ +type Road = "location" | "redirect-header" | "masked" | "nojs"; + +let seq = 0; + +async function ship(road: Road, target: string) { + const id = `scheme-${seq++}`; + registerServerFunction(id, async () => { + // `Headers` refuses CR/LF in a value outright, so those variants never + // reach the floor at all — report that rather than hiding it as a pass + const headers = new Headers(); + if (road === "redirect-header") { + headers.set(REDIRECT_HEADER, `302 ${target}`); + return new Response(null, { status: 200, headers }); + } + headers.set("Location", target); + return new Response(null, { status: 302, headers }); + }); + const scripted = road === "masked"; + const form = road === "nojs"; + let response: Response; + try { + response = await handleServerFunctionRequest( + new Request(`https://app.example${scripted ? "/_server/data/" : "/_server/"}${id}`, { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + ...(form + ? { + "Content-Type": "application/x-www-form-urlencoded", + "Sec-Fetch-Mode": "navigate", + Referer: "https://app.example/page" + } + : {}) + }, + ...(form ? { body: "a=1" } : {}) + }), + { provideEvent } + ); + } catch { + return { shipped: null as string | null, refused: true, protocol: "n/a" }; + } + if (response.status === 500) return { shipped: null, refused: true, protocol: "n/a" }; + const carried = response.headers.get(REDIRECT_HEADER); + const shipped = carried + ? carried.slice(carried.indexOf(" ") + 1) + : response.headers.get("Location"); + if (shipped === null) return { shipped: null, refused: true, protocol: "n/a" }; + let protocol: string; + try { + protocol = new URL(shipped, "https://app.example/here").protocol; + } catch { + protocol = "unparseable"; + } + return { shipped, refused: false, protocol }; +} + +/** Every scheme the floor refuses. */ +const REFUSED_SCHEMES = ["javascript", "data", "vbscript", "file", "intent", "mailto", "myapp"]; + +/** + * Every spelling that reads back as that scheme: the URL parser strips + * ASCII TAB/LF/CR from anywhere, and leading C0-control-or-space before + * parsing, so all of these are the same target to a consumer. + */ +function spellings(scheme: string): [string, string][] { + const mid = Math.max(1, scheme.length >> 1); + const split = (c: string) => `${scheme.slice(0, mid)}${c}${scheme.slice(mid)}:x`; + return [ + ["plain", `${scheme}:x`], + ["TAB interior", split("\t")], + ["LF interior", split("\n")], + ["CR interior", split("\r")], + ["TAB before colon", `${scheme}\t:x`], + ["leading SP", ` ${scheme}:x`], + ["leading TAB", `\t${scheme}:x`], + ["leading LF", `\n${scheme}:x`] + ]; +} + +describe("non-http(s) navigation targets are refused however they are spelled (#3201)", () => { + it("every refused scheme, every stripped whitespace, every position, every road", async () => { + // The contract is not "the request fails" — the no-JS road legitimately + // answers a rejected target by redirecting BACK to the referer. It is + // that nothing carrying a non-http(s) scheme ever leaves the transport. + const rows: string[] = []; + for (const scheme of REFUSED_SCHEMES) { + for (const [name, target] of spellings(scheme)) { + for (const road of ["location", "redirect-header", "masked", "nojs"] as const) { + const r = await ship(road, target); + const safe = r.refused || r.protocol === "http:" || r.protocol === "https:"; + rows.push( + `${scheme}/${name}/${road}: ${ + safe + ? "http(s)-or-refused" + : `SHIPPED ${JSON.stringify(r.shipped)} reads as ${r.protocol}` + }` + ); + } + // the client-side decoder enforces the same floor independently + rows.push( + `${scheme}/${name}/decoder: ${ + decodeRedirectHeaderValue(`302 ${target}`) === undefined + ? "http(s)-or-refused" + : "DECODED a non-http(s) target" + }` + ); + } + } + expect(rows).toEqual(rows.map(r => `${r.slice(0, r.indexOf(":") + 1)} http(s)-or-refused`)); + }); + + it("keeps every allowance the floor deliberately grants", async () => { + const allowed: [string, string][] = [ + ["relative path", "/dashboard"], + ["relative path with query and hash", "/dashboard?next=1#top"], + ["schemeless relative segment", "dashboard"], + ["relative segment containing a colon", "./dashboard:tab"], + ["query only", "?only=query"], + ["hash only", "#only-hash"], + ["empty target", ""], + ["absolute same-origin https", "https://app.example/next"], + ["absolute same-origin http", "http://app.example/next"], + // cross-origin http(s) is DELIBERATE: the floor is a scheme floor, not + // an origin policy — OAuth hand-offs flow through it (#3175) + ["absolute cross-origin", "https://accounts.example.com/oauth"], + ["protocol-relative", "//accounts.example.com/oauth"] + ]; + const rows: string[] = []; + for (const [name, target] of allowed) { + for (const road of ["location", "masked"] as const) { + const r = await ship(road, target); + rows.push(`${name}/${road}: ${r.refused ? "REFUSED" : `shipped, reads as ${r.protocol}`}`); + } + } + expect(rows).toEqual([ + "relative path/location: shipped, reads as https:", + "relative path/masked: shipped, reads as https:", + "relative path with query and hash/location: shipped, reads as https:", + "relative path with query and hash/masked: shipped, reads as https:", + "schemeless relative segment/location: shipped, reads as https:", + "schemeless relative segment/masked: shipped, reads as https:", + "relative segment containing a colon/location: shipped, reads as https:", + "relative segment containing a colon/masked: shipped, reads as https:", + "query only/location: shipped, reads as https:", + "query only/masked: shipped, reads as https:", + "hash only/location: shipped, reads as https:", + "hash only/masked: shipped, reads as https:", + "empty target/location: shipped, reads as https:", + // an empty Location carries no navigation for a scripted caller to act + // on, so the mask emits no header at all + "empty target/masked: REFUSED", + "absolute same-origin https/location: shipped, reads as https:", + "absolute same-origin https/masked: shipped, reads as https:", + "absolute same-origin http/location: shipped, reads as http:", + "absolute same-origin http/masked: shipped, reads as http:", + "absolute cross-origin/location: shipped, reads as https:", + "absolute cross-origin/masked: shipped, reads as https:", + "protocol-relative/location: shipped, reads as https:", + "protocol-relative/masked: shipped, reads as https:" + ]); + }); +}); diff --git a/packages/web/test/server/server-functions-result-descriptors.spec.tsx b/packages/web/test/server/server-functions-result-descriptors.spec.tsx new file mode 100644 index 000000000..31ed7cfd7 --- /dev/null +++ b/packages/web/test/server/server-functions-result-descriptors.spec.tsx @@ -0,0 +1,263 @@ +/** + * The descriptor literal in guardFailures' accessor materialization + * (#3196, #3198). One line writes back every slot the walk rewrites: + * + * { value, writable: true, enumerable: true, configurable: true } + * + * Two defects come out of it, and one fix settles both. + * + * - `enumerable: true` is unconditional, so a property the author hid from + * serialization with `enumerable: false` is now materialized as an + * enumerable data property and SHIPPED (#3198). Before #3176 it never + * left the server. + * - The shell is rebuilt with `Object.create(prototype, descriptors)`, so + * a FROZEN original hands it non-configurable, non-writable slots — and + * redefining one is a TypeError. The call answers 500 after its side + * effects committed, carrying the mutation's own Set-Cookie on the same + * response (#3196). + * + * Both tables are the adjacent-shape map: the rows that fail today, and + * the rows that pass today and a careless fix must not break. + * + * Like the other server-function specs, these run against the built + * bundles (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createRequestEvent } from "@solidjs/web"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const H = { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Format": "8", + "X-Server-Function-Instance": "server-function:test" +}; + +function scriptedPost(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: H + }); +} + +/** Decodes a result response on whichever road it took. */ +async function decode(response: Response) { + if (response.headers.get("X-Server-Function-Format") === "8") return response.json(); + const { deserializeStream } = await import("@solidjs/web/server-functions/client"); + return deserializeStream(response); +} + +describe("a frozen result is not a failed call (#3196)", () => { + // Every row is a container whose walk REWRITES at least one slot — a + // channel the guard wraps, or an accessor it materializes — on the codec + // road. `Object.freeze` on a returned DTO is ordinary defensive + // authoring, and the same value without the freeze round-trips fine. + const rewriting: [string, () => unknown][] = [ + [ + "frozen object holding a promise", + () => Object.freeze({ id: 9, receipt: Promise.resolve("R-9") }) + ], + [ + "frozen object with a plain getter", + () => + Object.freeze({ + a: 1, + get computed() { + return "cheap"; + } + }) + ], + [ + "frozen object with a non-enumerable getter beside a Date", + () => { + const row: any = { name: "widget", createdAt: new Date(0) }; + Object.defineProperty(row, "hidden", { + get: () => "H", + enumerable: false, + configurable: true + }); + return Object.freeze(row); + } + ], + [ + "frozen object holding a ReadableStream", + () => + Object.freeze({ + s: new ReadableStream({ + start(c) { + c.enqueue("x"); + c.close(); + } + }) + }) + ], + [ + "frozen object holding an async iterable", + () => + Object.freeze({ + it: (async function* () { + yield 1; + })() + }) + ], + [ + "frozen object nested inside an ordinary result", + () => ({ outer: 1, inner: Object.freeze({ r: Promise.resolve("R") }) }) + ] + ]; + + // The shapes that already answer 200. A fix that reaches wider than the + // rewritten slot — freezing the shell, or refusing frozen containers — + // breaks these, so they are pinned as the baseline. + const working: [string, () => unknown][] = [ + ["sealed object holding a promise", () => Object.seal({ r: Promise.resolve("R") })], + [ + "writable:false alone", + () => { + const o: any = {}; + Object.defineProperty(o, "r", { + value: Promise.resolve("R"), + writable: false, + enumerable: true, + configurable: true + }); + return o; + } + ], + [ + "configurable:false alone", + () => { + const o: any = {}; + Object.defineProperty(o, "r", { + value: Promise.resolve("R"), + writable: true, + enumerable: true, + configurable: false + }); + return o; + } + ], + ["frozen array of a promise", () => Object.freeze([Promise.resolve("R")])], + ["frozen Map holding a promise", () => Object.freeze(new Map([["r", Promise.resolve("R")]]))], + ["frozen Set holding a promise", () => Object.freeze(new Set([Promise.resolve("R")]))], + ["frozen plain data (JSON road)", () => Object.freeze({ n: 1 })], + [ + "frozen object holding a Date (codec road, no slot rewritten)", + () => Object.freeze({ d: new Date(0) }) + ], + ["the same shape unfrozen", () => ({ id: 9, receipt: Promise.resolve("R-9") })] + ]; + + test.each([...rewriting, ...working])( + "%s answers 200 and never reports a committed call as failed", + async (label, make) => { + const id = "descriptors-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let committed = 0; + registerServerFunction(id, async () => { + committed++; + return make(); + }); + + const response = await handleServerFunctionRequest(scriptedPost(id), { + createEvent: (request: Request) => { + const event = createRequestEvent(request); + event.response.headers.append("Set-Cookie", "order=9; Path=/"); + return event; + } + }); + + expect(committed).toBe(1); + // the wire must not say "failed" and "succeeded" at once: the + // mutation's own Set-Cookie is on this response either way + expect(response.headers.getSetCookie()).toContain("order=9; Path=/"); + expect(response.headers.get("X-Server-Function-Error")).toBeNull(); + expect(response.status).toBe(200); + // and the value actually round-trips, so a 200 built by dropping the + // result would not pass either + await expect(decode(response)).resolves.toBeDefined(); + } + ); +}); + +describe("a non-enumerable accessor is not serialized (#3198)", () => { + const SECRET = "COST-SECRET-42"; + + // `enumerable: false` is the mechanism JSON.stringify honours and the one + // an author reaches for to keep a computed field server-side. Every row + // asserts the wire body against that same baseline. + const rows: [string, () => any, boolean][] = [ + // [label, factory, secret expected on the wire] + ["non-enumerable accessor beside a Date", () => hidden("accessor", new Date(0)), false], + ["non-enumerable accessor beside a Map", () => hidden("accessor", new Map([["a", 1]])), false], + ["non-enumerable accessor beside a Set", () => hidden("accessor", new Set([1])), false], + [ + "non-enumerable accessor beside a promise", + () => hidden("accessor", Promise.resolve("R")), + false + ], + ["non-enumerable accessor beside an undefined", () => hidden("accessor", undefined), false], + [ + "non-enumerable accessor nested one level down", + () => ({ wrap: hidden("accessor", new Date(0)) }), + false + ], + // controls that already pass and must keep passing + ["non-enumerable accessor alone (JSON road)", () => hidden("accessor", "plain"), false], + ["non-enumerable DATA property beside a Date", () => hidden("data", new Date(0)), false], + [ + "non-enumerable DATA property beside a promise", + () => hidden("data", Promise.resolve("R")), + false + ], + [ + "an ENUMERABLE accessor is still serialized", + () => { + const row: any = { name: "widget", extra: new Date(0) }; + Object.defineProperty(row, "visible", { + get: () => SECRET, + enumerable: true, + configurable: true + }); + return row; + }, + true + ] + ]; + + function hidden(kind: "accessor" | "data", companion: unknown) { + const row: any = { name: "widget", extra: companion }; + Object.defineProperty( + row, + "internalCostBasis", + kind === "accessor" + ? { get: () => SECRET, enumerable: false, configurable: true } + : { value: SECRET, enumerable: false, writable: true, configurable: true } + ); + return row; + } + + test.each(rows)("%s", async (label, make, onWire) => { + const id = "nonenum-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + registerServerFunction(id, async () => make()); + + const response = await handleServerFunctionRequest(scriptedPost(id)); + const body = await response.text(); + + expect(response.status).toBe(200); + expect(body.includes(SECRET)).toBe(onWire); + }); +}); From 8f59ebeb09730d4da93c671903484b950524bae4 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 1 Sep 2026 21:53:32 -0700 Subject: [PATCH 02/14] test(signals): isolate structural attribution controls Disable wall-clock diagnostics across benchmark-shaped structural tests so coverage and runner contention cannot produce unrelated failures. Co-authored-by: Cursor --- .../signals/tests/attribution-benchmark-eval.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/signals/tests/attribution-benchmark-eval.test.ts b/packages/signals/tests/attribution-benchmark-eval.test.ts index 0732284b2..969df95e8 100644 --- a/packages/signals/tests/attribution-benchmark-eval.test.ts +++ b/packages/signals/tests/attribution-benchmark-eval.test.ts @@ -30,7 +30,10 @@ afterEach(() => { function arm(opts: Parameters[0] = {}) { vi.spyOn(console, "warn").mockImplementation(() => {}); vi.spyOn(console, "log").mockImplementation(() => {}); - DEV!.attribution.enable({ log: false, ...opts }); + // These benchmark-shaped scenarios evaluate structural attribution. Keep + // the independent wall-clock detector out: coverage and runner contention + // can legitimately push a fine-grained scope over its default 8ms budget. + DEV!.attribution.enable({ log: false, hotTime: false, ...opts }); const diagnostics: DiagnosticEvent[] = []; DEV!.diagnostics.subscribe(e => diagnostics.push(e)); const reruns: RerunEvent[] = []; @@ -227,10 +230,7 @@ describe("healthy fine-grained benchmark implementation (false-positive control) }); flush(); - // This is a structural false-positive control. Wall-clock attribution is - // deliberately outside its claim: coverage/contended CI can push an - // otherwise fine-grained row effect over the default 8ms budget. - const { diagnostics, reruns } = arm({ hotTime: false }); + const { diagnostics, reruns } = arm(); setState(s => { for (let i = 0; i < n; i += 10) s.rows[i].label += " !!!"; }); From 2a6567d839f3348a41aa45c2e79da5d1c94c56a3 Mon Sep 17 00:00:00 2001 From: Nick Date: Tue, 1 Sep 2026 21:27:09 +0300 Subject: [PATCH 03/14] fix(web): track in-place class mutations --- .changeset/track-applied-classes.md | 5 +++ packages/web/src/client.ts | 25 ++++++++--- packages/web/test/class.spec.tsx | 50 ++++++++++++++++++++++ packages/web/test/hydration/class.spec.tsx | 41 ++++++++++++++++++ 4 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 .changeset/track-applied-classes.md create mode 100644 packages/web/test/class.spec.tsx create mode 100644 packages/web/test/hydration/class.spec.tsx diff --git a/.changeset/track-applied-classes.md b/.changeset/track-applied-classes.md new file mode 100644 index 000000000..5378c0a28 --- /dev/null +++ b/.changeset/track-applied-classes.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Update object-valued class bindings after in-place mutations by tracking a separate snapshot of the classes applied to each element. diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index 7ec8dd0e4..847e6fc45 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -562,27 +562,39 @@ export function setAttributeNS(node, namespace, name, value) { export function className(node: Element, value: JSX.ClassValue, prev?: JSX.ClassValue): void; export function className(node, value, prev) { - if (isHydrating(node)) return; // Numbers stringify like the compiler's static output (`class={1}` // inlines as `class="1"` in the template) so static and dynamic forms of // the same ClassValue behave identically (#3189). if (typeof value === "number") value = "" + value; if (typeof prev === "number") prev = "" + prev; + if (isHydrating(node)) { + // Seed applied state without touching the claimed DOM so later in-place + // mutations can still be diffed after hydration completes. + node._$classes = value && typeof value === "object" ? classListToObject(value) : undefined; + return; + } if (value == null || value === false) { - prev && node.removeAttribute("class"); + if (prev || node._$classes) { + node.removeAttribute("class"); + node._$classes = undefined; + } return; } if (typeof value === "string") { + node._$classes = undefined; value !== prev && node.setAttribute("class", value); return; } + // Track classes applied by className() itself. value/prev are user-owned + // and may be the same object on shared-effect reruns. + let applied; if (typeof prev === "string") { - prev = {}; + applied = {}; node.removeAttribute("class"); - } else prev = classListToObject(prev || {}); + } else applied = node._$classes || classListToObject(prev || {}); value = classListToObject(value); const classKeys = Object.keys(value || {}); - const prevKeys = Object.keys(prev); + const prevKeys = Object.keys(applied); let i, len; for (i = 0, len = prevKeys.length; i < len; i++) { const key = prevKeys[i]; @@ -592,9 +604,10 @@ export function className(node, value, prev) { for (i = 0, len = classKeys.length; i < len; i++) { const key = classKeys[i], classValue = !!value[key]; - if (!key || key === "undefined" || prev[key] === classValue || !classValue) continue; + if (!key || key === "undefined" || applied[key] === classValue || !classValue) continue; node.classList.add(key); } + node._$classes = value; } /** Compiler-emitted primitive; not for hand-written code. @internal */ export function addEvent( node: Element, diff --git a/packages/web/test/class.spec.tsx b/packages/web/test/class.spec.tsx new file mode 100644 index 000000000..180c51802 --- /dev/null +++ b/packages/web/test/class.spec.tsx @@ -0,0 +1,50 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ +import { describe, expect, test } from "vitest"; +import { render } from "@solidjs/web"; +import { createSignal, flush } from "solid-js"; + +describe("class", () => { + test("updates an object value after an in-place mutation", () => { + const classes = { before: true, after: false }; + const [value, setValue] = createSignal(classes, { equals: false }); + const container = document.createElement("div"); + const dispose = render(() =>
, container); + const element = container.firstElementChild as HTMLDivElement; + + expect(element.className).toBe("before"); + element.classList.add("external"); + + classes.before = false; + classes.after = true; + setValue(classes); + flush(); + + expect(element.className).toBe("external after"); + dispose(); + }); + + test("resets the applied snapshot when switching value forms", () => { + const [value, setValue] = createSignal | null>({ + first: true + }); + const container = document.createElement("div"); + const dispose = render(() =>
, container); + const element = container.firstElementChild as HTMLDivElement; + + setValue("second"); + flush(); + expect(element.className).toBe("second"); + + setValue({ third: true }); + flush(); + expect(element.className).toBe("third"); + + setValue(null); + flush(); + expect(element.hasAttribute("class")).toBe(false); + dispose(); + }); +}); diff --git a/packages/web/test/hydration/class.spec.tsx b/packages/web/test/hydration/class.spec.tsx new file mode 100644 index 000000000..82f3cce86 --- /dev/null +++ b/packages/web/test/hydration/class.spec.tsx @@ -0,0 +1,41 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { hydrate } from "@solidjs/web"; +import { createSignal, flush } from "solid-js"; + +describe("class hydration", () => { + const container = document.createElement("div"); + let dispose: (() => void) | undefined; + + beforeEach(() => { + (globalThis as any)._$HY = { events: [], completed: new WeakSet(), r: {}, fe() {} }; + document.body.appendChild(container); + }); + + afterEach(() => { + dispose?.(); + dispose = undefined; + container.remove(); + container.innerHTML = ""; + }); + + test("updates an object value after an in-place mutation", () => { + const classes = { before: true, after: false }; + const [value, setValue] = createSignal(classes, { equals: false }); + container.innerHTML = '
'; + + dispose = hydrate(() =>
, container); + const element = container.firstElementChild as HTMLDivElement; + element.classList.add("external"); + + classes.before = false; + classes.after = true; + setValue(classes); + flush(); + + expect(element.className).toBe("external after"); + }); +}); From e90f7761cc6224bf047909fa02bb0fd6af452093 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 1 Sep 2026 22:10:30 -0700 Subject: [PATCH 04/14] chore(size): account for applied class snapshots Document and ratchet the two app scenarios that retain the in-place class mutation fix. Co-authored-by: Cursor --- scripts/size/.size-limit.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index 552872c65..f3a6a7667 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -283,8 +283,13 @@ module.exports = [ // // Fold relocation pass (2026-09-01): 10.73 -> 10.72 KB, measured at // 10.71 — the core-floor relocation (see that note). + // + // In-place class mutation fix (#3188): 10.80 -> 10.84 KB, measured at + // 10.834. className() retains the last applied object/array snapshot so + // shared-reference reruns can diff mutations without deleting external + // classes. path: "minimal-app.js", - limit: "10.80 KB", + limit: "10.84 KB", modifyEsbuildConfig }, { @@ -334,7 +339,11 @@ module.exports = [ // Fold relocation pass (2026-09-01): 17.6 -> 17.56 KB, measured at // 17.54 — this bundle's import graph retained the scheduler-resident // ledger; the relocation lets it shake. - limit: "17.67 KB", + // + // In-place class mutation fix (#3188): 17.67 -> 17.68 KB, measured at + // 17.673. Hydration seeds the applied-class snapshot without mutating + // the claimed DOM so the first live in-place change still diffs. + limit: "17.68 KB", modifyEsbuildConfig }, { From e87cdcd0f98297d7f7da7bd7de5b372f0c17dd95 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 1 Sep 2026 21:53:24 -0700 Subject: [PATCH 05/14] test(compiler): normalize TSRX virtual paths on Windows Co-authored-by: Cursor --- .../tsrx-typecheck-projection.test.js | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/compiler/__tests__/tsrx-typecheck-projection.test.js b/packages/compiler/__tests__/tsrx-typecheck-projection.test.js index 4c471aa4d..5dae40697 100644 --- a/packages/compiler/__tests__/tsrx-typecheck-projection.test.js +++ b/packages/compiler/__tests__/tsrx-typecheck-projection.test.js @@ -2,6 +2,14 @@ const path = require("path"); const ts = require("typescript"); const { projectTsrxForTypecheck } = require(".."); +function sameFileName(left, right) { + const canonical = name => { + const resolved = ts.sys.resolvePath(name).replaceAll("\\", "/"); + return ts.sys.useCaseSensitiveFileNames ? resolved : resolved.toLowerCase(); + }; + return canonical(left) === canonical(right); +} + function typecheck(code) { const filename = path.join(__dirname, "__virtual-tsrx-projection.tsx"); const repository = path.resolve(__dirname, "../../.."); @@ -23,10 +31,10 @@ function typecheck(code) { }; const host = ts.createCompilerHost(options); const getSourceFile = host.getSourceFile.bind(host); - host.fileExists = name => name === filename || ts.sys.fileExists(name); - host.readFile = name => (name === filename ? code : ts.sys.readFile(name)); + host.fileExists = name => sameFileName(name, filename) || ts.sys.fileExists(name); + host.readFile = name => (sameFileName(name, filename) ? code : ts.sys.readFile(name)); host.getSourceFile = (name, languageVersion, onError, shouldCreateNewSourceFile) => - name === filename + sameFileName(name, filename) ? ts.createSourceFile(name, code, languageVersion, true, ts.ScriptKind.TSX) : getSourceFile(name, languageVersion, onError, shouldCreateNewSourceFile); const program = ts.createProgram([filename], options, host); @@ -61,14 +69,15 @@ function createLanguageService(code) { getDefaultLibFileName: compilerOptions => ts.getDefaultLibFilePath(compilerOptions), getScriptFileNames: () => [filename], getScriptSnapshot: name => { - if (name === filename) return ts.ScriptSnapshot.fromString(code); + if (sameFileName(name, filename)) return ts.ScriptSnapshot.fromString(code); const text = ts.sys.readFile(name); return text === undefined ? undefined : ts.ScriptSnapshot.fromString(text); }, getScriptVersion: () => "0", - fileExists: ts.sys.fileExists, + fileExists: name => sameFileName(name, filename) || ts.sys.fileExists(name), readDirectory: ts.sys.readDirectory, - readFile: ts.sys.readFile + readFile: name => (sameFileName(name, filename) ? code : ts.sys.readFile(name)), + useCaseSensitiveFileNames: () => ts.sys.useCaseSensitiveFileNames }; return { filename, service: ts.createLanguageService(host) }; } From 8e30fed91fa0abea94aabd8cb3292b3c872666ec Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Wed, 2 Sep 2026 12:22:50 +0700 Subject: [PATCH 06/14] test(web): cover the carrier case, and make the tables prove the call ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gaps a mutation pass found in the tests, plus one simplification. The #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. --- packages/web/server-functions/src/server.ts | 2 +- .../server-functions-content-length.spec.tsx | 47 +++++++++++-------- .../server-functions-proto-keys.spec.tsx | 31 ++++++++++++ .../server-functions-redirect-scheme.spec.tsx | 38 ++++++++++----- 4 files changed, 84 insertions(+), 34 deletions(-) diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index aaba244ce..3a45f5ac7 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -1135,7 +1135,7 @@ function stripOwnProtoKeys(value) { // own properties (#3200). `constructor` rides alongside `__proto__` // because a recursive merge reaches Object.prototype through it (#3202). for (const key of UNSAFE_ARGUMENT_KEYS) { - if (Object.prototype.hasOwnProperty.call(v, key)) delete v[key]; + if (Object.hasOwn(v, key)) delete v[key]; } for (const key of Object.keys(v)) stack.push(v[key]); } diff --git a/packages/web/test/server/server-functions-content-length.spec.tsx b/packages/web/test/server/server-functions-content-length.spec.tsx index cd5c2bb59..62a4a7e26 100644 --- a/packages/web/test/server/server-functions-content-length.spec.tsx +++ b/packages/web/test/server/server-functions-content-length.spec.tsx @@ -82,11 +82,13 @@ async function call(id: string, { road = "scripted" as Road, stub = null as stri * sent. Rendered as a row so a failure names every producer at once rather * than only the first. */ -function row(label: string, r: { declared: string | null; bytes: number }) { - return `${label}: Content-Length=${r.declared ?? "absent"} body=${r.bytes} bytes`; +function row(label: string, r: { status: number; declared: string | null; bytes: number }) { + return `${label}: status=${r.status} Content-Length=${r.declared ?? "absent"} body=${r.bytes} bytes`; } -function expected(label: string, r: { declared: string | null; bytes: number }) { - return `${label}: Content-Length=${r.declared === null ? "absent" : r.bytes} body=${r.bytes} bytes`; +// The status is part of the contract: without it a handler that answers +// nothing at all renders as `absent / 0 bytes` and passes. +function expected(label: string, status: number, r: { declared: string | null; bytes: number }) { + return `${label}: status=${status} Content-Length=${r.declared === null ? "absent" : r.bytes} body=${r.bytes} bytes`; } describe("Content-Length never describes a body the transport composed (#3197)", () => { @@ -122,32 +124,37 @@ describe("Content-Length never describes a body the transport composed (#3197)", ); } - const cases: [string, () => Promise][] = [ - ["returned Response + Content-Length (codec road)", () => call("cl-returned-response")], - ["thrown Response + Content-Length", () => call("cl-thrown-response")], - ["returned respond() envelope (JSON road)", () => call("cl-returned-envelope")], - ["thrown respond() envelope", () => call("cl-thrown-envelope")], - ["thrown 302 carrying a Content-Length", () => call("cl-redirect")], - ["stub gap-fill: middleware sets 0", () => call("cl-string", { stub: "0" })], - ["stub gap-fill: middleware sets 999", () => call("cl-string", { stub: "999" })], - ["204 + Content-Length", () => call("cl-null-204")], - ["205 + Content-Length", () => call("cl-null-205")], - ["304 + Content-Length", () => call("cl-null-304")], + const cases: [string, () => Promise, number][] = [ + ["returned Response + Content-Length (codec road)", () => call("cl-returned-response"), 200], + ["thrown Response + Content-Length", () => call("cl-thrown-response"), 200], + ["returned respond() envelope (JSON road)", () => call("cl-returned-envelope"), 200], + ["thrown respond() envelope", () => call("cl-thrown-envelope"), 200], + ["thrown 302 carrying a Content-Length", () => call("cl-redirect"), 200], + ["stub gap-fill: middleware sets 0", () => call("cl-string", { stub: "0" }), 200], + ["stub gap-fill: middleware sets 999", () => call("cl-string", { stub: "999" }), 200], + ["204 + Content-Length", () => call("cl-null-204"), 204], + ["205 + Content-Length", () => call("cl-null-205"), 205], + ["304 + Content-Length", () => call("cl-null-304"), 304], // unscripted: the same producers, plain-HTTP road [ "unscripted respond() envelope + Content-Length", - () => call("cl-returned-envelope", { road: "plain" }) + () => call("cl-returned-envelope", { road: "plain" }), + 200 ], - ["unscripted stub gap-fill", () => call("cl-string", { road: "plain", stub: "999" })], - ["no-JS form post + Content-Length", () => call("cl-returned-response", { road: "form" })] + ["unscripted stub gap-fill", () => call("cl-string", { road: "plain", stub: "999" }), 200], + [ + "no-JS form post + Content-Length", + () => call("cl-returned-response", { road: "form" }), + 303 + ] ]; const actual: string[] = []; const want: string[] = []; - for (const [label, run] of cases) { + for (const [label, run, status] of cases) { const r = await run(); actual.push(row(label, r)); - want.push(expected(label, r)); + want.push(expected(label, status, r)); } expect(actual).toEqual(want); }); diff --git a/packages/web/test/server/server-functions-proto-keys.spec.tsx b/packages/web/test/server/server-functions-proto-keys.spec.tsx index 509d95b53..03831d0e7 100644 --- a/packages/web/test/server/server-functions-proto-keys.spec.tsx +++ b/packages/web/test/server/server-functions-proto-keys.spec.tsx @@ -185,6 +185,37 @@ describe("decoded arguments cannot reach Object.prototype (#3202)", () => { ); }); + it("a non-plain carrier does not shelter the payload underneath it (#3200)", async () => { + // The reachable shape is not an unsafe key ON a carrier — seroval drops + // that at encode — but a plain object one level UNDER one. The walk used + // to stop at any non-plain prototype, so an `Error`, or any class the + // codec revives with own properties, hid everything beneath it. Codec + // road only: the JSON road cannot express a carrier. + const rows: string[] = []; + for (const road of ["codec-query", "codec-body"] as Road[]) { + const carrier = Object.assign(new Error("validation failed"), { + payload: { ctorKey: { prototypeKey: { polluted: "viaCarrier" } }, n: 1 } + }); + const { status, seen } = await decodeArgument(road, carrier); + const payload = (seen as any)?.payload; + deepMerge({}, payload ?? {}); + const leaked = (Object.prototype as any).polluted; + delete (Object.prototype as any).polluted; + rows.push( + `${road}: status=${status} payloadKeys=${JSON.stringify( + Object.keys(payload ?? {}) + )} Object.prototype.polluted=${JSON.stringify(leaked)}` + ); + } + expect(rows).toEqual( + rows.map( + r => + `${r.slice(0, r.indexOf(": ") + 2)}status=200 payloadKeys=["n"] ` + + `Object.prototype.polluted=undefined` + ) + ); + }); + it("control: ordinary data with a similar name is not eaten", async () => { const rows: string[] = []; for (const road of ROADS) { diff --git a/packages/web/test/server/server-functions-redirect-scheme.spec.tsx b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx index d1ec19b20..31a08aaf3 100644 --- a/packages/web/test/server/server-functions-redirect-scheme.spec.tsx +++ b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx @@ -53,16 +53,24 @@ let seq = 0; async function ship(road: Road, target: string) { const id = `scheme-${seq++}`; + let ran = 0; registerServerFunction(id, async () => { + ran++; // `Headers` refuses CR/LF in a value outright, so those variants never - // reach the floor at all — report that rather than hiding it as a pass + // reach the floor — that protection comes from the platform, not from the + // code under test, so it is reported as its own outcome rather than + // counted as a refusal the floor made. const headers = new Headers(); - if (road === "redirect-header") { - headers.set(REDIRECT_HEADER, `302 ${target}`); - return new Response(null, { status: 200, headers }); + try { + if (road === "redirect-header") { + headers.set(REDIRECT_HEADER, `302 ${target}`); + return new Response(null, { status: 200, headers }); + } + headers.set("Location", target); + return new Response(null, { status: 302, headers }); + } catch { + return new Response("HEADERS-REFUSED", { status: 200 }); } - headers.set("Location", target); - return new Response(null, { status: 302, headers }); }); const scripted = road === "masked"; const form = road === "nojs"; @@ -86,21 +94,21 @@ async function ship(road: Road, target: string) { { provideEvent } ); } catch { - return { shipped: null as string | null, refused: true, protocol: "n/a" }; + return { ran, shipped: null as string | null, refused: true, protocol: "n/a" }; } - if (response.status === 500) return { shipped: null, refused: true, protocol: "n/a" }; + if (response.status === 500) return { ran, shipped: null, refused: true, protocol: "n/a" }; const carried = response.headers.get(REDIRECT_HEADER); const shipped = carried ? carried.slice(carried.indexOf(" ") + 1) : response.headers.get("Location"); - if (shipped === null) return { shipped: null, refused: true, protocol: "n/a" }; + if (shipped === null) return { ran, shipped: null, refused: true, protocol: "n/a" }; let protocol: string; try { protocol = new URL(shipped, "https://app.example/here").protocol; } catch { protocol = "unparseable"; } - return { shipped, refused: false, protocol }; + return { ran, shipped, refused: false, protocol }; } /** Every scheme the floor refuses. */ @@ -138,7 +146,7 @@ describe("non-http(s) navigation targets are refused however they are spelled (# const r = await ship(road, target); const safe = r.refused || r.protocol === "http:" || r.protocol === "https:"; rows.push( - `${scheme}/${name}/${road}: ${ + `${scheme}/${name}/${road}: ran=${r.ran} ${ safe ? "http(s)-or-refused" : `SHIPPED ${JSON.stringify(r.shipped)} reads as ${r.protocol}` @@ -147,7 +155,7 @@ describe("non-http(s) navigation targets are refused however they are spelled (# } // the client-side decoder enforces the same floor independently rows.push( - `${scheme}/${name}/decoder: ${ + `${scheme}/${name}/decoder: ran=1 ${ decodeRedirectHeaderValue(`302 ${target}`) === undefined ? "http(s)-or-refused" : "DECODED a non-http(s) target" @@ -155,7 +163,11 @@ describe("non-http(s) navigation targets are refused however they are spelled (# ); } } - expect(rows).toEqual(rows.map(r => `${r.slice(0, r.indexOf(":") + 1)} http(s)-or-refused`)); + // `ran=1` is part of the contract: a handler that dispatches nothing + // renders every row as "refused" and would otherwise pass. + expect(rows).toEqual( + rows.map(r => `${r.slice(0, r.indexOf(":") + 1)} ran=1 http(s)-or-refused`) + ); }); it("keeps every allowance the floor deliberately grants", async () => { From db739366dfd67426638eba1d88bfc2ba99a7c6e5 Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Wed, 2 Sep 2026 12:39:33 +0700 Subject: [PATCH 07/14] refactor(web): name the framing headers once, and drop three needless params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/web/server-functions/src/server.ts | 20 ++++++------- packages/web/src/response.ts | 17 +++++++++-- packages/web/src/server.ts | 6 ++-- .../server-functions-content-length.spec.tsx | 30 ++++++++++++++++--- 4 files changed, 54 insertions(+), 19 deletions(-) diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 3a45f5ac7..9dd84de21 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -13,6 +13,7 @@ // mutation — and never what they carry. Which data a mutation invalidates, // and how an outcome reaches the UI, stay with the integration. import { + COMPOSED_BODY_FRAMING, NULL_BODY_STATUSES, RESPONSE_HEADER_VALUE_LIMIT, REVALIDATE_HEADER, @@ -1135,7 +1136,7 @@ function stripOwnProtoKeys(value) { // own properties (#3200). `constructor` rides alongside `__proto__` // because a recursive merge reaches Object.prototype through it (#3202). for (const key of UNSAFE_ARGUMENT_KEYS) { - if (Object.hasOwn(v, key)) delete v[key]; + delete v[key]; } for (const key of Object.keys(v)) stack.push(v[key]); } @@ -1432,10 +1433,7 @@ export function foldSetCookies(headers, setCookies) { // way response headers may merge here: never `get`/`set` folding. function mergeResponseHeaders(target, source) { source.forEach((value, key) => { - // `content-length` describes the source's body and the caller is about to - // send a different one; forwarding a length known to be wrong truncates - // the answer at the socket (#3197, RFC 9110 §8.6). - if (key !== "set-cookie" && key !== "content-length") target.append(key, value); + if (key !== "set-cookie" && !COMPOSED_BODY_FRAMING.has(key)) target.append(key, value); }); if (source.getSetCookie) { for (const cookie of source.getSetCookie()) target.append("Set-Cookie", cookie); @@ -1509,7 +1507,7 @@ const BOUNDED_COMPOSED_HEADERS = [REDIRECT_HEADER, "Location", REVALIDATE_HEADER // gives it an opt-in. The decoder enforces the same floor independently // (decodeRedirectHeaderValue), so a hostile peer cannot re-open the class // against integrations either. -function refusedTargetScheme(target, base) { +function refusedTargetScheme(target) { // Judge the target the way a consumer will READ it, not the way its bytes // are spelled: a URL parser strips ASCII tab and newline from anywhere and // trims leading C0/space before it begins, so a scheme grammar over the raw @@ -1518,7 +1516,9 @@ function refusedTargetScheme(target, base) { // fooled; the base keeps relative targets — the ordinary case — http(s). let protocol; try { - protocol = new URL(target, base).protocol; + // any http(s) base gives the same verdict: an absolute scheme wins over + // it, and a relative target inherits it + protocol = new URL(target, "http://base.invalid").protocol; } catch { return true; } @@ -1537,7 +1537,7 @@ function refusedTargetScheme(target, base) { // or rewritten target is a DIFFERENT address, a dropped revalidate key is // a silently stale cache. Runs ahead of the stub fold so integration // cookies still ride the refusal (#3159). -function enforceComposedHeaderInvariants(response, base) { +function enforceComposedHeaderInvariants(response) { for (const name of BOUNDED_COMPOSED_HEADERS) { const value = response.headers.get(name); if (value === null) continue; @@ -1560,7 +1560,7 @@ function enforceComposedHeaderInvariants(response, base) { if (name === REVALIDATE_HEADER) continue; // REDIRECT_HEADER rides as " "; Location is the target const target = name === REDIRECT_HEADER ? value.slice(value.indexOf(" ") + 1) : value; - if (refusedTargetScheme(target, base)) { + if (refusedTargetScheme(target)) { return refuseComposedHeader( response, name, @@ -3247,7 +3247,7 @@ export async function handleServerFunctionRequest(request, options = {}) { // foreign-response path at once (raw passthrough, unscripted returns and // throws, custom handleNoJS results, envelope-carried responses). const response = commitEventResponse( - enforceComposedHeaderInvariants(ownResponse(await dispatch()), request.url), + enforceComposedHeaderInvariants(ownResponse(await dispatch())), event ); return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); diff --git a/packages/web/src/response.ts b/packages/web/src/response.ts index 77be09ee9..26962dfdc 100644 --- a/packages/web/src/response.ts +++ b/packages/web/src/response.ts @@ -257,6 +257,19 @@ export const NULL_BODY_STATUSES: ReadonlySet = new Set([204, 205, 304]); * consumers without the client runtime (no-JS form posts, direct HTTP) * get real JSON, while integrations read `value` — no reparse. */ +/** + * Headers that describe how a body is framed on the wire. Whenever the + * transport composes a body of its own, an author-supplied value describes + * the body it replaced — a stale `Content-Length` truncates the answer at the + * socket, and a stale `Content-Encoding` tells the peer to decompress bytes + * nobody compressed (#3197, RFC 9110 §8.6). + */ +export const COMPOSED_BODY_FRAMING: ReadonlySet = /*#__PURE__*/ new Set([ + "content-length", + "content-encoding", + "transfer-encoding" +]); + export function respond(value: T, init: ResponseHelperInit = {}) { const { responseInit, headers } = initWithRevalidate(init); // A null-body status cannot carry the passthrough JSON body — building it @@ -264,9 +277,9 @@ export function respond(value: T, init: ResponseHelperInit = {}) { // Carry the metadata bodiless; the server-function encoder answers the // void shapes with a real null-body response and reports value-carrying // ones legibly. - // The body below is ours, so an author-supplied length describes something + // The body below is ours, so an author's framing headers describe something // else — and on a null-body status there is no body at all (#3197). - headers.delete("Content-Length"); + for (const header of COMPOSED_BODY_FRAMING) headers.delete(header); if (NULL_BODY_STATUSES.has(responseInit.status)) { return new ResponseEnvelope(new Response(null, { ...responseInit, headers }), value); } diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index ec5a9b223..83f72c427 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -21,7 +21,7 @@ import { // Wire-protocol header names for the commit fold's gap-fill denylist // (`commitEventResponse`): shared constants, not copies, so the fold can // never drift from what the server-function handler actually sends. -import { REVALIDATE_HEADER } from "./response.js"; +import { COMPOSED_BODY_FRAMING, REVALIDATE_HEADER } from "./response.js"; import { BODY_FORMAT_HEADER, ERROR_HEADER, @@ -4461,9 +4461,9 @@ const STUB_GAP_FILL_EXCLUDED = /*#__PURE__*/ new Set( REVALIDATE_HEADER, REDIRECT_HEADER, "Location", - // written before the body exists, so it can only describe a different + // written before the body exists, so they can only describe a different // one (#3197) - "Content-Length" + ...COMPOSED_BODY_FRAMING ].map(header => header.toLowerCase()) ); diff --git a/packages/web/test/server/server-functions-content-length.spec.tsx b/packages/web/test/server/server-functions-content-length.spec.tsx index 62a4a7e26..f0d344966 100644 --- a/packages/web/test/server/server-functions-content-length.spec.tsx +++ b/packages/web/test/server/server-functions-content-length.spec.tsx @@ -73,8 +73,9 @@ async function call(id: string, { road = "scripted" as Road, stub = null as stri } ); const declared = response.headers.get("Content-Length"); + const encoding = response.headers.get("Content-Encoding"); const bytes = response.body ? (await response.arrayBuffer()).byteLength : 0; - return { status: response.status, declared, bytes }; + return { status: response.status, declared, encoding, bytes }; } /** @@ -82,13 +83,23 @@ async function call(id: string, { road = "scripted" as Road, stub = null as stri * sent. Rendered as a row so a failure names every producer at once rather * than only the first. */ -function row(label: string, r: { status: number; declared: string | null; bytes: number }) { - return `${label}: status=${r.status} Content-Length=${r.declared ?? "absent"} body=${r.bytes} bytes`; +function row( + label: string, + r: { status: number; declared: string | null; encoding: string | null; bytes: number } +) { + return `${label}: status=${r.status} Content-Length=${r.declared ?? "absent"} Content-Encoding=${ + r.encoding ?? "absent" + } body=${r.bytes} bytes`; } // The status is part of the contract: without it a handler that answers // nothing at all renders as `absent / 0 bytes` and passes. function expected(label: string, status: number, r: { declared: string | null; bytes: number }) { - return `${label}: status=${status} Content-Length=${r.declared === null ? "absent" : r.bytes} body=${r.bytes} bytes`; + // A framing header the runtime did not compute must simply be absent: a + // stale length truncates the answer, a stale encoding tells the peer to + // decompress bytes nobody compressed. + return `${label}: status=${status} Content-Length=${ + r.declared === null ? "absent" : r.bytes + } Content-Encoding=absent body=${r.bytes} bytes`; } describe("Content-Length never describes a body the transport composed (#3197)", () => { @@ -118,6 +129,15 @@ describe("Content-Length never describes a body the transport composed (#3197)", }); }); registerServerFunction("cl-string", async () => "seven!!"); + // a Content-Encoding is the same defect wearing a different name: it + // describes a compression the transport never applied + registerServerFunction("cl-encoding-envelope", async () => + respond({ ok: true, n: 42 }, { headers: { "Content-Encoding": "gzip" } }) + ); + registerServerFunction( + "cl-encoding-response", + async () => new Response("upstream body", { headers: { "Content-Encoding": "gzip" } }) + ); for (const status of [204, 205, 304]) { registerServerFunction(`cl-null-${status}`, async () => respond(undefined, { status, headers: { "Content-Length": "5" } }) @@ -132,6 +152,8 @@ describe("Content-Length never describes a body the transport composed (#3197)", ["thrown 302 carrying a Content-Length", () => call("cl-redirect"), 200], ["stub gap-fill: middleware sets 0", () => call("cl-string", { stub: "0" }), 200], ["stub gap-fill: middleware sets 999", () => call("cl-string", { stub: "999" }), 200], + ["envelope + Content-Encoding", () => call("cl-encoding-envelope"), 200], + ["Response + Content-Encoding", () => call("cl-encoding-response"), 200], ["204 + Content-Length", () => call("cl-null-204"), 204], ["205 + Content-Length", () => call("cl-null-205"), 205], ["304 + Content-Length", () => call("cl-null-304"), 304], From b73635b9ae9ae4f1e90694e1c53755a4eb695609 Mon Sep 17 00:00:00 2001 From: Nick Date: Tue, 1 Sep 2026 21:00:51 +0300 Subject: [PATCH 08/14] fix(web): preserve reusable bound event tuples --- .changeset/preserve-bound-event-tuples.md | 5 ++ packages/web/src/client.ts | 10 ++- packages/web/test/event-handler.spec.tsx | 74 +++++++++++++++++++++++ 3 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 .changeset/preserve-bound-event-tuples.md create mode 100644 packages/web/test/event-handler.spec.tsx diff --git a/.changeset/preserve-bound-event-tuples.md b/.changeset/preserve-bound-event-tuples.md new file mode 100644 index 000000000..24eb72776 --- /dev/null +++ b/.changeset/preserve-bound-event-tuples.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Keep bound handler tuples reusable across non-delegated events by leaving the user-provided tuple unchanged when installing a listener. diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index 847e6fc45..46346d5b5 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -614,7 +614,7 @@ export function addEvent( name: string, handler: EventListener | EventListenerObject | (EventListenerObject & AddEventListenerOptions), delegate: boolean -): void; +): EventListener | EventListenerObject | void; export function addEvent(node, name, handler, delegate) { if (delegate) { @@ -624,8 +624,11 @@ export function addEvent(node, name, handler, delegate) { } else node[`$$${name}`] = handler; } else if (Array.isArray(handler)) { const handlerFn = handler[0]; - node.addEventListener(name, (handler[0] = e => handlerFn.call(node, handler[1], e))); + const listener = e => handlerFn.call(node, handler[1], e); + node.addEventListener(name, listener); + return listener; } else node.addEventListener(name, handler, typeof handler !== "function" && handler); + return delegate ? undefined : handler; } /** Compiler-emitted primitive; not for hand-written code. @internal */ export function style( node: Element, @@ -2042,8 +2045,9 @@ function assignProp(node, prop, value, prev, skipRef, nodeName) { node.removeEventListener(name, h); } if (delegate || value) { - addEvent(node, name, value, delegate); + const attached = addEvent(node, name, value, delegate); delegate && delegateEvents([name]); + if (!delegate) return attached; } } else if ( (hasNamespace && prop.slice(0, 5) === "prop:") || diff --git a/packages/web/test/event-handler.spec.tsx b/packages/web/test/event-handler.spec.tsx new file mode 100644 index 000000000..098e3e542 --- /dev/null +++ b/packages/web/test/event-handler.spec.tsx @@ -0,0 +1,74 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ +import { describe, expect, test } from "vitest"; +import { render } from "@solidjs/web"; +import { createSignal, flush } from "solid-js"; +import type { JSX } from "../src/index.js"; + +describe("Event handlers", () => { + test("reuses a bound handler tuple across non-delegated events", () => { + const calls: Array<[string, HTMLDivElement, Event]> = []; + const originalHandler = (data: string, event: Event) => { + calls.push([data, event.currentTarget as HTMLDivElement, event]); + }; + const handler: JSX.BoundEventHandler = [originalHandler, "shared"]; + const container = document.createElement("div"); + const dispose = render( + () => ( + <> +
+
+ + ), + container + ); + const [first, second] = Array.from(container.children) as HTMLDivElement[]; + const firstEvent = new Event("scroll"); + const secondEvent = new Event("scroll"); + + first.dispatchEvent(firstEvent); + second.dispatchEvent(secondEvent); + + expect(calls).toEqual([ + ["shared", first, firstEvent], + ["shared", second, secondEvent] + ]); + expect(handler).toEqual([originalHandler, "shared"]); + dispose(); + }); + + test("removes the previous non-delegated bound handler when a spread rebinds", () => { + type Handler = JSX.BoundEventHandler; + const calls: string[] = []; + const handlers: Handler[] = ["first", "second", "third"].map(data => [ + value => calls.push(value), + data + ]); + const [props, setProps] = createSignal<{ onScroll?: Handler }>({ + onScroll: handlers[0] + }); + const container = document.createElement("div"); + const dispose = render(() =>
, container); + const element = container.firstElementChild as HTMLDivElement; + + element.dispatchEvent(new Event("scroll")); + + setProps({ onScroll: handlers[1] }); + flush(); + element.dispatchEvent(new Event("scroll")); + + setProps({ onScroll: handlers[2] }); + flush(); + element.dispatchEvent(new Event("scroll")); + + setProps({}); + flush(); + element.dispatchEvent(new Event("scroll")); + + expect(calls).toEqual(["first", "second", "third"]); + expect(handlers.map(handler => handler[1])).toEqual(["first", "second", "third"]); + dispose(); + }); +}); From 1371998ecab80ad4b845db97d1394b9954a7ab53 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 1 Sep 2026 23:00:14 -0700 Subject: [PATCH 09/14] fix(web): retain unchanged bound event listeners Preserve the authored tuple alongside its attached wrapper so unrelated spread updates do not reorder or churn stable listeners. Co-authored-by: Cursor --- packages/web/src/client.ts | 5 ++++- packages/web/test/event-handler.spec.tsx | 28 +++++++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index 46346d5b5..65ac2e5e7 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -2041,13 +2041,16 @@ function assignProp(node, prop, value, prev, skipRef, nodeName) { const name = prop.slice(2).toLowerCase(); const delegate = DelegatedEvents.has(name); if (!delegate && prev) { + // Bound tuples keep an internal [attached listener, authored tuple] + // record so an unrelated spread rerun can preserve listener identity. + if (Array.isArray(prev) && prev[1] === value) return prev; const h = Array.isArray(prev) ? prev[0] : prev; node.removeEventListener(name, h); } if (delegate || value) { const attached = addEvent(node, name, value, delegate); delegate && delegateEvents([name]); - if (!delegate) return attached; + if (!delegate) return Array.isArray(value) ? [attached, value] : attached; } } else if ( (hasNamespace && prop.slice(0, 5) === "prop:") || diff --git a/packages/web/test/event-handler.spec.tsx b/packages/web/test/event-handler.spec.tsx index 098e3e542..255ea13b6 100644 --- a/packages/web/test/event-handler.spec.tsx +++ b/packages/web/test/event-handler.spec.tsx @@ -2,9 +2,9 @@ * @jsxImportSource @solidjs/web * @vitest-environment jsdom */ -import { describe, expect, test } from "vitest"; -import { render } from "@solidjs/web"; -import { createSignal, flush } from "solid-js"; +import { describe, expect, test, vi } from "vitest"; +import { render, spread } from "@solidjs/web"; +import { createRoot, createSignal, flush } from "solid-js"; import type { JSX } from "../src/index.js"; describe("Event handlers", () => { @@ -71,4 +71,26 @@ describe("Event handlers", () => { expect(handlers.map(handler => handler[1])).toEqual(["first", "second", "third"]); dispose(); }); + + test("does not rebind an unchanged tuple when another spread property updates", () => { + const handler: JSX.BoundEventHandler = [() => {}, "shared"]; + const [props, setProps] = createSignal({ onScroll: handler, title: "first" }); + const element = document.createElement("div"); + const add = vi.spyOn(element, "addEventListener"); + const remove = vi.spyOn(element, "removeEventListener"); + const dispose = createRoot(dispose => { + spread(element, props, true); + return dispose; + }); + + flush(); + expect(add).toHaveBeenCalledTimes(1); + + setProps({ onScroll: handler, title: "second" }); + flush(); + + expect(add).toHaveBeenCalledTimes(1); + expect(remove).not.toHaveBeenCalled(); + dispose(); + }); }); From f4e490b933854bf9c09ba8614773bc1cf8516691 Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Wed, 2 Sep 2026 11:44:23 +0700 Subject: [PATCH 10/14] fix(web): judge arguments, results and redirect targets by what they are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven defects across the server-function guards, five of them introduced by the guards themselves (#3168, #3170, #3175, #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 (#3200) and strips the whole unsafe key set (#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 (#3196, #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 (#3201). A parser strips ASCII tab and newline before it begins, so `javascript:` 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 (#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 (#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. --- .../server-function-guard-simplification.md | 13 + packages/web/server-functions/src/server.ts | 99 +++++-- packages/web/src/response.ts | 3 + packages/web/src/server.ts | 5 +- .../server-functions-content-length.spec.tsx | 191 +++++++++++++ .../server-functions-event-hook.spec.tsx | 210 ++++++++++++++ .../server-functions-proto-keys.spec.tsx | 198 +++++++++++++ .../server-functions-redirect-scheme.spec.tsx | 211 ++++++++++++++ ...rver-functions-result-descriptors.spec.tsx | 263 ++++++++++++++++++ 9 files changed, 1165 insertions(+), 28 deletions(-) create mode 100644 .changeset/server-function-guard-simplification.md create mode 100644 packages/web/test/server/server-functions-content-length.spec.tsx create mode 100644 packages/web/test/server/server-functions-event-hook.spec.tsx create mode 100644 packages/web/test/server/server-functions-proto-keys.spec.tsx create mode 100644 packages/web/test/server/server-functions-redirect-scheme.spec.tsx create mode 100644 packages/web/test/server/server-functions-result-descriptors.spec.tsx diff --git a/.changeset/server-function-guard-simplification.md b/.changeset/server-function-guard-simplification.md new file mode 100644 index 000000000..d9be0875d --- /dev/null +++ b/.changeset/server-function-guard-simplification.md @@ -0,0 +1,13 @@ +--- +"@solidjs/web": patch +--- + +Judge decoded arguments, guarded results and navigation targets by what they +are rather than by how they are spelled, and never forward a `Content-Length` +that describes a body the transport replaced. + +Seven defects, five of them introduced by the guards added in #3168, #3170, +#3175 and #3176. Each fix removes a special case rather than adding one: the +argument walk stops for no prototype, the guard shell keeps only the flag that +reaches the wire, the scheme floor asks the URL parser instead of a regex, and +the event is awaited only when it is genuinely a promise. diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 2787988f7..aaba244ce 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -1113,6 +1113,8 @@ function assertDecodeDepth(value) { * Sets; a value nested inside a revived class instance keeps its shape — * the codec owns that value's construction, not this guard. */ +const UNSAFE_ARGUMENT_KEYS = ["__proto__", "constructor", "prototype"]; + function stripOwnProtoKeys(value) { const stack = [value]; const seen = new Set(); @@ -1127,10 +1129,13 @@ function stripOwnProtoKeys(value) { } else if (v instanceof Set) { for (const member of v) stack.push(member); } else { - const proto = Object.getPrototypeOf(v); - if (proto !== Object.prototype && proto !== null) continue; - if (Object.prototype.hasOwnProperty.call(v, "__proto__")) { - delete v["__proto__"]; + // Every object is walked, whatever its prototype: this mutates in place + // and rebuilds nothing, so a non-plain prototype was never a reason to + // stop — it only hid a payload under a carrier the codec revives with + // own properties (#3200). `constructor` rides alongside `__proto__` + // because a recursive merge reaches Object.prototype through it (#3202). + 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]); } @@ -1427,7 +1432,10 @@ export function foldSetCookies(headers, setCookies) { // way response headers may merge here: never `get`/`set` folding. function mergeResponseHeaders(target, source) { source.forEach((value, key) => { - if (key !== "set-cookie") target.append(key, value); + // `content-length` describes the source's body and the caller is about to + // send a different one; forwarding a length known to be wrong truncates + // the answer at the socket (#3197, RFC 9110 §8.6). + if (key !== "set-cookie" && key !== "content-length") target.append(key, value); }); if (source.getSetCookie) { for (const cookie of source.getSetCookie()) target.append("Set-Cookie", cookie); @@ -1501,10 +1509,20 @@ const BOUNDED_COMPOSED_HEADERS = [REDIRECT_HEADER, "Location", REVALIDATE_HEADER // gives it an opt-in. The decoder enforces the same floor independently // (decodeRedirectHeaderValue), so a hostile peer cannot re-open the class // against integrations either. -function refusedTargetScheme(target) { - // explicit scheme present and not http(s) → refused - const match = /^[a-zA-Z][a-zA-Z0-9+.-]*:/.exec(target); - return match !== null && !/^https?:$/i.test(match[0]); +function refusedTargetScheme(target, base) { + // Judge the target the way a consumer will READ it, not the way its bytes + // are spelled: a URL parser strips ASCII tab and newline from anywhere and + // trims leading C0/space before it begins, so a scheme grammar over the raw + // value saw nothing where the parser sees `javascript:` (#3201). Resolving + // is what the masked and no-JS roads already did, which is why neither was + // fooled; the base keeps relative targets — the ordinary case — http(s). + let protocol; + try { + protocol = new URL(target, base).protocol; + } catch { + return true; + } + return protocol !== "http:" && protocol !== "https:"; } // The transport half of redirect()'s and initWithRevalidate's invariants: @@ -1519,7 +1537,7 @@ function refusedTargetScheme(target) { // or rewritten target is a DIFFERENT address, a dropped revalidate key is // a silently stale cache. Runs ahead of the stub fold so integration // cookies still ride the refusal (#3159). -function enforceComposedHeaderInvariants(response) { +function enforceComposedHeaderInvariants(response, base) { for (const name of BOUNDED_COMPOSED_HEADERS) { const value = response.headers.get(name); if (value === null) continue; @@ -1542,7 +1560,7 @@ function enforceComposedHeaderInvariants(response) { if (name === REVALIDATE_HEADER) continue; // REDIRECT_HEADER rides as " "; Location is the target const target = name === REDIRECT_HEADER ? value.slice(value.indexOf(" ") + 1) : value; - if (refusedTargetScheme(target)) { + if (refusedTargetScheme(target, base)) { return refuseComposedHeader( response, name, @@ -1822,13 +1840,12 @@ export function guardFailures(value, state) { // value needed no wrapping): only the rebuilt shell carries the // data property — pass the original through and the codec would // invoke the getter afresh, minting a new unguarded channel. - Object.defineProperty( - top.next, - items[i], - top.accessorRead === i - ? { enumerable: true, configurable: true, writable: true, value: guarded } - : { ...top.descriptors[items[i]], value: guarded } - ); + Object.defineProperty(top.next, items[i], { + value: guarded, + writable: true, + configurable: true, + enumerable: top.descriptors[items[i]].enumerable + }); top.changed = true; } top.i++; @@ -2029,7 +2046,17 @@ function enterGuard(value, state) { // rebuild. Getter-backed accessors are materialized by the driver (#3176): // invoked once there, rewritten as data properties on this shell. const descriptors = Object.getOwnPropertyDescriptors(value); - const next = Object.create(prototype, descriptors); + // The shell is scratch the codec reads once and the author never holds, so + // only `enumerable` has to survive — it is what the codec serializes. + // Replicating a frozen source's `writable`/`configurable` made the + // write-back below illegal (#3196) and pinning them true lost + // `enumerable: false` (#3198). + const rewritable = {}; + for (const key of Object.keys(descriptors)) { + rewritable[key] = { ...descriptors[key], writable: true, configurable: true }; + if ("get" in rewritable[key]) delete rewritable[key].writable; + } + const next = Object.create(prototype, rewritable); state.seen.set(value, next); return new Frame(OBJECT, value, next, Object.keys(descriptors), descriptors); } @@ -2784,14 +2811,32 @@ export async function handleServerFunctionRequest(request, options = {}) { } } - let event = options.createEvent ? options.createEvent(request) : { request, locals: {} }; // An async createEvent is out of contract (the type is synchronous), but - // handing a pending Promise downstream as the event is the worst failure - // available: the function runs, the caller sees 200, and every header the - // integration wrote on the real event's stub silently vanishes (#3170). - // Awaiting is strictly better than refusing — the resolved value IS the - // event the integration meant. - if (typeof (event as any)?.then === "function") event = await event; + // handing a pending Promise downstream is the worst failure available: the + // function runs, the caller sees 200, and every header the integration + // wrote on the stub vanishes (#3170). So it is awaited — but only when it + // is genuinely a Promise. The event is a datum an integration handed back, + // not something the runtime asked to be async, and awaiting anything + // wearing a `then` parked the request forever on a lazy-locals proxy and + // starved the event loop on a self-resolving one (#3199). A failure here + // is answered rather than thrown: no event exists yet, so there is no stub + // to fold and nothing downstream can report it. + let event; + try { + event = options.createEvent ? options.createEvent(request) : { request, locals: {} }; + if (event instanceof Promise) event = await event; + } catch (error) { + // tagged like every other error answer, so the transport reads it as ours + const headers = new Headers(); + headers.set( + ERROR_HEADER, + boundedErrorHeaderValue( + DEV ? String((error as any)?.message ?? error) : GENERIC_SERVER_ERROR_MESSAGE + ) + ); + const response = new Response(null, { status: 500, headers }); + return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); + } // Once an event exists, its response stub folds onto EVERY exit — the // refusals below included (#3159). A refusal that returned directly // dropped the stub silently: an integration's Set-Cookie written in @@ -3202,7 +3247,7 @@ export async function handleServerFunctionRequest(request, options = {}) { // foreign-response path at once (raw passthrough, unscripted returns and // throws, custom handleNoJS results, envelope-carried responses). const response = commitEventResponse( - enforceComposedHeaderInvariants(ownResponse(await dispatch())), + enforceComposedHeaderInvariants(ownResponse(await dispatch()), request.url), event ); return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); diff --git a/packages/web/src/response.ts b/packages/web/src/response.ts index 535f65660..77be09ee9 100644 --- a/packages/web/src/response.ts +++ b/packages/web/src/response.ts @@ -264,6 +264,9 @@ export function respond(value: T, init: ResponseHelperInit = {}) { // Carry the metadata bodiless; the server-function encoder answers the // void shapes with a real null-body response and reports value-carrying // ones legibly. + // The body below is ours, so an author-supplied length describes something + // else — and on a null-body status there is no body at all (#3197). + headers.delete("Content-Length"); if (NULL_BODY_STATUSES.has(responseInit.status)) { return new ResponseEnvelope(new Response(null, { ...responseInit, headers }), value); } diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index 37dbbcee9..ec5a9b223 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -4460,7 +4460,10 @@ const STUB_GAP_FILL_EXCLUDED = /*#__PURE__*/ new Set( SINGLE_FLIGHT_HEADER, REVALIDATE_HEADER, REDIRECT_HEADER, - "Location" + "Location", + // written before the body exists, so it can only describe a different + // one (#3197) + "Content-Length" ].map(header => header.toLowerCase()) ); diff --git a/packages/web/test/server/server-functions-content-length.spec.tsx b/packages/web/test/server/server-functions-content-length.spec.tsx new file mode 100644 index 000000000..cd5c2bb59 --- /dev/null +++ b/packages/web/test/server/server-functions-content-length.spec.tsx @@ -0,0 +1,191 @@ +/** + * A `Content-Length` an author did not compute must never describe a body + * the transport composed (#3197). + * + * Three producers merge author-supplied headers onto an answer whose body + * the runtime encodes itself — the `respond()` envelope, a returned/thrown + * `Response` on the scripted road, and the request event's response stub + * gap-fill — and a stale length is not a cosmetic mismatch: RFC 9112 §6.3 + * has a recipient with a valid `Content-Length` read exactly that many + * octets and stop, so the answer arrives truncated with no error anywhere, + * and RFC 9110 §8.6 forbids forwarding a length "known to be incorrect" at + * all. `createNoJSHandler` already reconciles this by deleting the header + * from the redirect it builds; the other producers did not. + * + * The invariant asserted here is one line wide and holds for every answer + * the handler can emit: either there is no `Content-Length`, or it equals + * the number of bytes the response body actually carries. The controls + * matter as much as the repros — a length the runtime did NOT invalidate + * (an unscripted passthrough of the author's own body) must survive, and + * the streaming road must stay length-free so it can still chunk. + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { createRequestEvent, respond } from "@solidjs/web"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const provideEvent = (_event: unknown, run: () => T): T => run(); + +type Road = "scripted" | "plain" | "form"; + +async function call(id: string, { road = "scripted" as Road, stub = null as string | null } = {}) { + const address = road === "scripted" ? `/_server/data/${id}` : `/_server/${id}`; + const form = road === "form"; + const response = await handleServerFunctionRequest( + new Request(`https://app.example${address}`, { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + ...(form + ? { + "Content-Type": "application/x-www-form-urlencoded", + "Sec-Fetch-Mode": "navigate", + Referer: "https://app.example/page" + } + : {}) + }, + ...(form ? { body: "a=1" } : {}) + }), + { + provideEvent, + createEvent: request => { + const event = createRequestEvent(request); + // a middleware parking a length on the response stub + if (stub !== null) event.response.headers.set("Content-Length", stub); + return event; + } + } + ); + const declared = response.headers.get("Content-Length"); + const bytes = response.body ? (await response.arrayBuffer()).byteLength : 0; + return { status: response.status, declared, bytes }; +} + +/** + * The whole contract in one line: a declared length is the length that was + * sent. Rendered as a row so a failure names every producer at once rather + * than only the first. + */ +function row(label: string, r: { declared: string | null; bytes: number }) { + return `${label}: Content-Length=${r.declared ?? "absent"} body=${r.bytes} bytes`; +} +function expected(label: string, r: { declared: string | null; bytes: number }) { + return `${label}: Content-Length=${r.declared === null ? "absent" : r.bytes} body=${r.bytes} bytes`; +} + +describe("Content-Length never describes a body the transport composed (#3197)", () => { + it("holds across every producer that merges author headers onto an encoded body", async () => { + // the proxy shape: `return await fetch(upstream)` — every fetch Response + // carries a Content-Length, and the author wrote none of it + registerServerFunction("cl-returned-response", async () => { + return new Response("upstream body", { + headers: { "Content-Type": "text/html", "Content-Length": "13" } + }); + }); + registerServerFunction("cl-thrown-response", async () => { + throw new Response("upstream body", { + headers: { "Content-Type": "text/html", "Content-Length": "13" } + }); + }); + registerServerFunction("cl-returned-envelope", async () => + respond({ ok: true, n: 42 }, { headers: { "Content-Length": "999" } }) + ); + registerServerFunction("cl-thrown-envelope", async () => { + throw respond({ ok: true, n: 42 }, { headers: { "Content-Length": "999" } }); + }); + registerServerFunction("cl-redirect", async () => { + throw new Response(null, { + status: 302, + headers: { Location: "/done", "Content-Length": "42" } + }); + }); + registerServerFunction("cl-string", async () => "seven!!"); + for (const status of [204, 205, 304]) { + registerServerFunction(`cl-null-${status}`, async () => + respond(undefined, { status, headers: { "Content-Length": "5" } }) + ); + } + + const cases: [string, () => Promise][] = [ + ["returned Response + Content-Length (codec road)", () => call("cl-returned-response")], + ["thrown Response + Content-Length", () => call("cl-thrown-response")], + ["returned respond() envelope (JSON road)", () => call("cl-returned-envelope")], + ["thrown respond() envelope", () => call("cl-thrown-envelope")], + ["thrown 302 carrying a Content-Length", () => call("cl-redirect")], + ["stub gap-fill: middleware sets 0", () => call("cl-string", { stub: "0" })], + ["stub gap-fill: middleware sets 999", () => call("cl-string", { stub: "999" })], + ["204 + Content-Length", () => call("cl-null-204")], + ["205 + Content-Length", () => call("cl-null-205")], + ["304 + Content-Length", () => call("cl-null-304")], + // unscripted: the same producers, plain-HTTP road + [ + "unscripted respond() envelope + Content-Length", + () => call("cl-returned-envelope", { road: "plain" }) + ], + ["unscripted stub gap-fill", () => call("cl-string", { road: "plain", stub: "999" })], + ["no-JS form post + Content-Length", () => call("cl-returned-response", { road: "form" })] + ]; + + const actual: string[] = []; + const want: string[] = []; + for (const [label, run] of cases) { + const r = await run(); + actual.push(row(label, r)); + want.push(expected(label, r)); + } + expect(actual).toEqual(want); + }); + + it("controls: nothing that was already correct changes", async () => { + registerServerFunction( + "cl-ctl-response", + async () => new Response("upstream body", { headers: { "Content-Type": "text/html" } }) + ); + registerServerFunction("cl-ctl-envelope", async () => respond({ ok: true, n: 42 })); + registerServerFunction("cl-ctl-string", async () => "seven!!"); + registerServerFunction("cl-ctl-stream", async function* () { + yield "a"; + yield "b"; + } as any); + // the author serves their OWN body on the plain road: the runtime never + // re-encodes it, so their length is the truth and must survive + registerServerFunction( + "cl-ctl-passthrough", + async () => + new Response("upstream body", { + headers: { "Content-Type": "text/html", "Content-Length": "13" } + }) + ); + + const passthrough = await call("cl-ctl-passthrough", { road: "plain" }); + expect(passthrough.declared).toBe("13"); + expect(passthrough.bytes).toBe(13); + + for (const [label, run] of [ + ["returned Response, no length", () => call("cl-ctl-response")], + ["respond() envelope, no length", () => call("cl-ctl-envelope")], + ["plain string result", () => call("cl-ctl-string")], + ["streaming result stays length-free", () => call("cl-ctl-stream")] + ] as [string, () => Promise][]) { + const r = await run(); + expect(`${label}: ${r.declared}`).toBe(`${label}: null`); + expect(r.bytes).toBeGreaterThan(0); + } + }); +}); diff --git a/packages/web/test/server/server-functions-event-hook.spec.tsx b/packages/web/test/server/server-functions-event-hook.spec.tsx new file mode 100644 index 000000000..9dc1c75a7 --- /dev/null +++ b/packages/web/test/server/server-functions-event-hook.spec.tsx @@ -0,0 +1,210 @@ +/** + * `createEvent`'s await (#3199). #3170 made the runtime tolerate an async + * `createEvent` by awaiting its return — but it duck-types the value: + * + * if (typeof event?.then === "function") event = await event; + * + * Anything carrying a `then` is treated as a promise. An event that merely + * LOOKS thenable — a lazy-locals Proxy answering any unknown key, a tracing + * wrapper — is awaited on a `then` nobody ever calls, and the request hangs + * with no response, no timeout and no log. One spelling is worse: a `then` + * that resolves with the event itself spins the promise-resolution + * procedure forever and starves the whole event loop, not just the request. + * + * The second half is independent: the call sits outside every try, so a + * rejecting `createEvent` — a session store that is down, which is exactly + * the condition the hook exists to survive — escapes `handleServerFunctionRequest` + * with no status at all. + * + * Like the other server-function specs, these run against the built + * bundles (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createRequestEvent } from "@solidjs/web"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const H = { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Format": "8", + "X-Server-Function-Instance": "server-function:test" +}; + +function scriptedPost(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: H + }); +} + +const stamp = (event: any) => { + event.response.headers.append("Set-Cookie", "sid=abc; Path=/"); + return event; +}; + +/** Answers within `ms` or reports the hang as a value, never as a timeout. */ +async function within(work: Promise, ms = 1000): Promise { + let timer: any; + const result = await Promise.race([ + work, + new Promise<"HUNG">(resolve => (timer = setTimeout(() => resolve("HUNG"), ms))) + ]); + clearTimeout(timer); + return result; +} + +describe("a thenable createEvent still dispatches (#3199)", () => { + // Every spelling of "carries a then" that an integration can arrive at by + // accident. None of them is a promise; all of them are awaited today. + const thenables: [string, (request: Request) => unknown][] = [ + [ + "an own `then` that never settles", + request => { + const event: any = stamp(createRequestEvent(request)); + event.then = () => {}; + return event; + } + ], + [ + "a non-enumerable own `then`", + request => { + const event: any = stamp(createRequestEvent(request)); + Object.defineProperty(event, "then", { value: () => {}, enumerable: false }); + return event; + } + ], + [ + "`then` behind a getter", + request => { + const event: any = stamp(createRequestEvent(request)); + Object.defineProperty(event, "then", { get: () => () => {}, configurable: true }); + return event; + } + ], + // the realistic shape: a lazy-locals / auto-stub proxy answers ANY + // unknown key with something, and `then` is an unknown key + [ + "a Proxy answering unknown keys", + request => { + const event: any = stamp(createRequestEvent(request)); + return new Proxy(event, { + get: (target, key) => + key in target ? (target as any)[key] : key === "then" ? () => {} : undefined + }); + } + ], + // NOTE: this row starves the event loop before the fix — the promise + // resolution procedure re-adopts the same thenable forever. Post-fix it + // is never awaited at all. + [ + "a `then` that resolves with the event itself", + request => { + const event: any = stamp(createRequestEvent(request)); + event.then = (resolve: (v: unknown) => void) => resolve(event); + return event; + } + ] + ]; + + test.each(thenables)("%s dispatches normally", async (label, createEvent) => { + const id = "event-thenable-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let ran = 0; + registerServerFunction(id, async () => { + ran++; + return "ok"; + }); + + const response = await within( + handleServerFunctionRequest(scriptedPost(id), { createEvent: createEvent as any }) + ); + + expect(response).not.toBe("HUNG"); + expect((response as Response).status).toBe(200); + expect(ran).toBe(1); + // the event is still the integration's event: its stub folds as always + expect((response as Response).headers.getSetCookie()).toContain("sid=abc; Path=/"); + }); +}); + +describe("a failing createEvent is answered, not escaped (#3199)", () => { + const failing: [string, (request: Request) => unknown][] = [ + [ + "a rejecting async createEvent", + async () => { + throw new Error("session store unreachable"); + } + ], + [ + "a synchronously throwing createEvent", + () => { + throw new Error("session store unreachable"); + } + ] + ]; + + test.each(failing)("%s answers a sanitized 500", async (label, createEvent) => { + const id = "event-failing-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let ran = 0; + registerServerFunction(id, async () => { + ran++; + return "unreached"; + }); + + const response = await within( + handleServerFunctionRequest(scriptedPost(id), { createEvent: createEvent as any }) + ); + + expect(response).not.toBe("HUNG"); + expect((response as Response).status).toBe(500); + // sanitized: the store's own message is not the caller's business + expect((response as Response).headers.get("X-Server-Function-Error")).toBe( + "Internal Server Error" + ); + expect(ran).toBe(0); + }); +}); + +describe("the awaited shapes #3170 added keep working (#3199 baseline)", () => { + const controls: [string, (request: Request) => unknown][] = [ + [ + "a genuinely async createEvent", + async (request: Request) => { + await Promise.resolve(); + return stamp(createRequestEvent(request)); + } + ], + ["a plain synchronous createEvent", request => stamp(createRequestEvent(request))] + ]; + + test.each(controls)("%s still lands its cookies on the wire", async (label, createEvent) => { + const id = "event-control-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let ran = 0; + registerServerFunction(id, async () => { + ran++; + return "ok"; + }); + + const response = await within( + handleServerFunctionRequest(scriptedPost(id), { createEvent: createEvent as any }) + ); + + expect(response).not.toBe("HUNG"); + expect((response as Response).status).toBe(200); + expect(ran).toBe(1); + expect((response as Response).headers.getSetCookie()).toContain("sid=abc; Path=/"); + }); +}); diff --git a/packages/web/test/server/server-functions-proto-keys.spec.tsx b/packages/web/test/server/server-functions-proto-keys.spec.tsx new file mode 100644 index 000000000..509d95b53 --- /dev/null +++ b/packages/web/test/server/server-functions-proto-keys.spec.tsx @@ -0,0 +1,198 @@ +/** + * The decode boundary strips the keys that turn an ordinary merge into + * prototype pollution — all of them, on every road (#3168, #3202). + * + * #3168 stripped `__proto__` because `Object.assign` merges by [[Set]], so + * the key fires the inherited setter and re-prototypes the merged copy. + * That reasoning covers a SHALLOW merge. A recursive merge — at least as + * common in configuration and patch handling — walks into an own + * `constructor`, finds `prototype`, and writes onto `Object.prototype` + * itself: strictly worse than the case that was fixed, because it escapes + * the copy and reaches the whole process. + * + * Half-covering the class is the failure mode this table exists to prevent: + * an author who read #3168 and concluded the boundary was handled is wrong + * for the recursive spelling. So the assertion is not "`__proto__` is + * gone" but "`Object.prototype` is untouched after a naive recursive merge + * of a decoded argument", across every key and every road that decodes one. + * + * A field named `constructorName` is the control: the strip must not eat + * ordinary data. + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { serializeString } from "@solidjs/web/server-functions/client"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; + delete (Object.prototype as any).polluted; +}); + +const provideEvent = (_event: unknown, run: () => T): T => run(); + +/** The naive recursive merge #3168's own rationale names as the sink. */ +function deepMerge(target: any, source: any) { + for (const key of Object.keys(source)) { + if (source[key] && typeof source[key] === "object") { + target[key] ??= {}; + deepMerge(target[key], source[key]); + } else target[key] = source[key]; + } + return target; +} + +/** + * A hostile peer's codec frame. The codec's OWN encoder refuses to + * serialize an object with an own `constructor` key, so no honest client + * can produce one — but the frame is just text, and nothing stops a peer + * from writing it by hand. Encode under placeholder names, rename them in + * the payload, re-length the frame header. + */ +const HOSTILE_RENAME: Record = { + ctorKey: "constructor", + protoKey: "__proto__", + prototypeKey: "prototype" +}; + +async function hostileFrame(value: unknown) { + const framed = await serializeString(value); + let json = framed.slice(framed.indexOf(";", 1) + 1); + for (const [from, to] of Object.entries(HOSTILE_RENAME)) + json = json.split(`"${from}"`).join(`"${to}"`); + const length = new TextEncoder().encode(json).byteLength; + return `;0x${length.toString(16).padStart(8, "0")};${json}`; +} + +type Road = "json-query" | "json-body" | "codec-query" | "codec-body"; + +let seq = 0; + +/** Runs one call and hands back the argument exactly as the function saw it. */ +async function decodeArgument(road: Road, payload: unknown) { + const id = `proto-keys-${seq++}`; + let seen: unknown; + registerServerFunction(id, async (first: unknown) => { + seen = first; + return "ok"; + }); + const address = `https://app.example/_server/data/${id}`; + const headers: Record = { "Sec-Fetch-Site": "same-origin" }; + let request: Request; + if (road === "json-query") { + request = new Request(`${address}?args=${encodeURIComponent(JSON.stringify([payload]))}`, { + method: "POST", + headers + }); + } else if (road === "json-body") { + request = new Request(address, { + method: "POST", + headers: { ...headers, "Content-Type": "application/json", "X-Server-Function-Format": "8" }, + body: JSON.stringify([payload]) + }); + } else if (road === "codec-query") { + request = new Request(`${address}?args=${encodeURIComponent(await hostileFrame([payload]))}`, { + method: "POST", + headers + }); + } else { + request = new Request(address, { + method: "POST", + headers: { ...headers, "Content-Type": "text/plain", "X-Server-Function-Format": "0" }, + body: await hostileFrame([payload]) + }); + } + const response = await handleServerFunctionRequest(request, { provideEvent }); + return { status: response.status, seen }; +} + +/** + * The same payload in the two spellings the two decode roads can carry: the + * JSON road takes the dangerous names literally; the codec road takes + * placeholders that `hostileFrame` renames back. + */ +const PAYLOADS: [string, unknown, unknown][] = [ + [ + "__proto__", + JSON.parse('{"__proto__":{"polluted":"viaProto"},"n":1}'), + { protoKey: { polluted: "viaProto" }, n: 1 } + ], + [ + "constructor", + JSON.parse('{"constructor":{"prototype":{"polluted":"viaCtor"}},"n":1}'), + { ctorKey: { prototypeKey: { polluted: "viaCtor" } }, n: 1 } + ], + [ + "prototype", + JSON.parse('{"prototype":{"polluted":"viaPrototype"},"n":1}'), + { prototypeKey: { polluted: "viaPrototype" }, n: 1 } + ], + [ + "constructor nested one level", + JSON.parse('{"a":{"constructor":{"prototype":{"polluted":"viaNested"}}},"n":1}'), + { a: { ctorKey: { prototypeKey: { polluted: "viaNested" } } }, n: 1 } + ], + [ + "constructor inside an array", + JSON.parse('[{"constructor":{"prototype":{"polluted":"viaArray"}}}]'), + [{ ctorKey: { prototypeKey: { polluted: "viaArray" } } }] + ] +]; + +const ROADS: Road[] = ["json-query", "json-body", "codec-query", "codec-body"]; + +describe("decoded arguments cannot reach Object.prototype (#3202)", () => { + it("no dangerous key survives any decode road into a recursive merge", async () => { + const rows: string[] = []; + for (const [name, jsonPayload, codecPayload] of PAYLOADS) { + for (const road of ROADS) { + const { status, seen } = await decodeArgument( + road, + road.startsWith("codec") ? codecPayload : jsonPayload + ); + // shallow merge — the sink #3168 closed + const shallow: any = {}; + Object.assign(shallow, seen); + const reprototyped = Object.getPrototypeOf(shallow) !== Object.prototype; + // recursive merge — the sink #3168 left open + deepMerge({}, seen); + const leaked = (Object.prototype as any).polluted; + delete (Object.prototype as any).polluted; + rows.push( + `${name} / ${road}: status=${status} reprototyped=${reprototyped} Object.prototype.polluted=${JSON.stringify( + leaked + )}` + ); + } + } + expect(rows).toEqual( + rows.map( + r => + `${r.slice(0, r.indexOf(": ") + 2)}status=200 reprototyped=false Object.prototype.polluted=undefined` + ) + ); + }); + + it("control: ordinary data with a similar name is not eaten", async () => { + const rows: string[] = []; + for (const road of ROADS) { + const { status, seen } = await decodeArgument(road, { constructorName: "Widget", n: 1 }); + rows.push(`${road}: status=${status} ${JSON.stringify(seen)}`); + } + expect(rows).toEqual( + ROADS.map(road => `${road}: status=200 {"constructorName":"Widget","n":1}`) + ); + }); +}); diff --git a/packages/web/test/server/server-functions-redirect-scheme.spec.tsx b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx new file mode 100644 index 000000000..d1ec19b20 --- /dev/null +++ b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx @@ -0,0 +1,211 @@ +/** + * The navigation-target scheme floor (#3175) must judge the target the way + * a URL parser will read it, not the way the bytes are spelled (#3201). + * + * `refusedTargetScheme` matched `/^[a-zA-Z][a-zA-Z0-9+.-]*:/` against the + * RAW header value. Every URL parser removes ASCII tab, LF and CR from a + * URL before it begins (WHATWG URL, "basic URL parser", step 2), so a TAB + * anywhere inside the scheme token made the regex see no scheme at all + * while the consumer sees `javascript:` — the floor was one character wide. + * + * Two of the roads were never fooled, because they resolve through + * `new URL()` first: the scripted mask (`maskRedirect`) and the no-JS + * handler, plus the client-side `decodeRedirectHeaderValue`. The roads that + * read a header RAW — an author's `Location` on a forwarded 3xx, and an + * author's or hook's `X-Server-Function-Redirect` — were not, which is + * exactly the road the floor was documented to backstop. + * + * The table is the point: every refused scheme, every whitespace character + * a URL parser strips, in every position it can sit, on every road. And + * the allowances the floor deliberately keeps — relative, same-origin + * absolute, CROSS-ORIGIN absolute (OAuth hand-offs), protocol-relative — + * are asserted alongside, because a fix that resolves through `new URL()` + * without a base refuses all of them. + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + REDIRECT_HEADER, + decodeRedirectHeaderValue, + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const provideEvent = (_event: unknown, run: () => T): T => run(); + +/** The four roads a navigation target can leave the transport on. */ +type Road = "location" | "redirect-header" | "masked" | "nojs"; + +let seq = 0; + +async function ship(road: Road, target: string) { + const id = `scheme-${seq++}`; + registerServerFunction(id, async () => { + // `Headers` refuses CR/LF in a value outright, so those variants never + // reach the floor at all — report that rather than hiding it as a pass + const headers = new Headers(); + if (road === "redirect-header") { + headers.set(REDIRECT_HEADER, `302 ${target}`); + return new Response(null, { status: 200, headers }); + } + headers.set("Location", target); + return new Response(null, { status: 302, headers }); + }); + const scripted = road === "masked"; + const form = road === "nojs"; + let response: Response; + try { + response = await handleServerFunctionRequest( + new Request(`https://app.example${scripted ? "/_server/data/" : "/_server/"}${id}`, { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + ...(form + ? { + "Content-Type": "application/x-www-form-urlencoded", + "Sec-Fetch-Mode": "navigate", + Referer: "https://app.example/page" + } + : {}) + }, + ...(form ? { body: "a=1" } : {}) + }), + { provideEvent } + ); + } catch { + return { shipped: null as string | null, refused: true, protocol: "n/a" }; + } + if (response.status === 500) return { shipped: null, refused: true, protocol: "n/a" }; + const carried = response.headers.get(REDIRECT_HEADER); + const shipped = carried + ? carried.slice(carried.indexOf(" ") + 1) + : response.headers.get("Location"); + if (shipped === null) return { shipped: null, refused: true, protocol: "n/a" }; + let protocol: string; + try { + protocol = new URL(shipped, "https://app.example/here").protocol; + } catch { + protocol = "unparseable"; + } + return { shipped, refused: false, protocol }; +} + +/** Every scheme the floor refuses. */ +const REFUSED_SCHEMES = ["javascript", "data", "vbscript", "file", "intent", "mailto", "myapp"]; + +/** + * Every spelling that reads back as that scheme: the URL parser strips + * ASCII TAB/LF/CR from anywhere, and leading C0-control-or-space before + * parsing, so all of these are the same target to a consumer. + */ +function spellings(scheme: string): [string, string][] { + const mid = Math.max(1, scheme.length >> 1); + const split = (c: string) => `${scheme.slice(0, mid)}${c}${scheme.slice(mid)}:x`; + return [ + ["plain", `${scheme}:x`], + ["TAB interior", split("\t")], + ["LF interior", split("\n")], + ["CR interior", split("\r")], + ["TAB before colon", `${scheme}\t:x`], + ["leading SP", ` ${scheme}:x`], + ["leading TAB", `\t${scheme}:x`], + ["leading LF", `\n${scheme}:x`] + ]; +} + +describe("non-http(s) navigation targets are refused however they are spelled (#3201)", () => { + it("every refused scheme, every stripped whitespace, every position, every road", async () => { + // The contract is not "the request fails" — the no-JS road legitimately + // answers a rejected target by redirecting BACK to the referer. It is + // that nothing carrying a non-http(s) scheme ever leaves the transport. + const rows: string[] = []; + for (const scheme of REFUSED_SCHEMES) { + for (const [name, target] of spellings(scheme)) { + for (const road of ["location", "redirect-header", "masked", "nojs"] as const) { + const r = await ship(road, target); + const safe = r.refused || r.protocol === "http:" || r.protocol === "https:"; + rows.push( + `${scheme}/${name}/${road}: ${ + safe + ? "http(s)-or-refused" + : `SHIPPED ${JSON.stringify(r.shipped)} reads as ${r.protocol}` + }` + ); + } + // the client-side decoder enforces the same floor independently + rows.push( + `${scheme}/${name}/decoder: ${ + decodeRedirectHeaderValue(`302 ${target}`) === undefined + ? "http(s)-or-refused" + : "DECODED a non-http(s) target" + }` + ); + } + } + expect(rows).toEqual(rows.map(r => `${r.slice(0, r.indexOf(":") + 1)} http(s)-or-refused`)); + }); + + it("keeps every allowance the floor deliberately grants", async () => { + const allowed: [string, string][] = [ + ["relative path", "/dashboard"], + ["relative path with query and hash", "/dashboard?next=1#top"], + ["schemeless relative segment", "dashboard"], + ["relative segment containing a colon", "./dashboard:tab"], + ["query only", "?only=query"], + ["hash only", "#only-hash"], + ["empty target", ""], + ["absolute same-origin https", "https://app.example/next"], + ["absolute same-origin http", "http://app.example/next"], + // cross-origin http(s) is DELIBERATE: the floor is a scheme floor, not + // an origin policy — OAuth hand-offs flow through it (#3175) + ["absolute cross-origin", "https://accounts.example.com/oauth"], + ["protocol-relative", "//accounts.example.com/oauth"] + ]; + const rows: string[] = []; + for (const [name, target] of allowed) { + for (const road of ["location", "masked"] as const) { + const r = await ship(road, target); + rows.push(`${name}/${road}: ${r.refused ? "REFUSED" : `shipped, reads as ${r.protocol}`}`); + } + } + expect(rows).toEqual([ + "relative path/location: shipped, reads as https:", + "relative path/masked: shipped, reads as https:", + "relative path with query and hash/location: shipped, reads as https:", + "relative path with query and hash/masked: shipped, reads as https:", + "schemeless relative segment/location: shipped, reads as https:", + "schemeless relative segment/masked: shipped, reads as https:", + "relative segment containing a colon/location: shipped, reads as https:", + "relative segment containing a colon/masked: shipped, reads as https:", + "query only/location: shipped, reads as https:", + "query only/masked: shipped, reads as https:", + "hash only/location: shipped, reads as https:", + "hash only/masked: shipped, reads as https:", + "empty target/location: shipped, reads as https:", + // an empty Location carries no navigation for a scripted caller to act + // on, so the mask emits no header at all + "empty target/masked: REFUSED", + "absolute same-origin https/location: shipped, reads as https:", + "absolute same-origin https/masked: shipped, reads as https:", + "absolute same-origin http/location: shipped, reads as http:", + "absolute same-origin http/masked: shipped, reads as http:", + "absolute cross-origin/location: shipped, reads as https:", + "absolute cross-origin/masked: shipped, reads as https:", + "protocol-relative/location: shipped, reads as https:", + "protocol-relative/masked: shipped, reads as https:" + ]); + }); +}); diff --git a/packages/web/test/server/server-functions-result-descriptors.spec.tsx b/packages/web/test/server/server-functions-result-descriptors.spec.tsx new file mode 100644 index 000000000..31ed7cfd7 --- /dev/null +++ b/packages/web/test/server/server-functions-result-descriptors.spec.tsx @@ -0,0 +1,263 @@ +/** + * The descriptor literal in guardFailures' accessor materialization + * (#3196, #3198). One line writes back every slot the walk rewrites: + * + * { value, writable: true, enumerable: true, configurable: true } + * + * Two defects come out of it, and one fix settles both. + * + * - `enumerable: true` is unconditional, so a property the author hid from + * serialization with `enumerable: false` is now materialized as an + * enumerable data property and SHIPPED (#3198). Before #3176 it never + * left the server. + * - The shell is rebuilt with `Object.create(prototype, descriptors)`, so + * a FROZEN original hands it non-configurable, non-writable slots — and + * redefining one is a TypeError. The call answers 500 after its side + * effects committed, carrying the mutation's own Set-Cookie on the same + * response (#3196). + * + * Both tables are the adjacent-shape map: the rows that fail today, and + * the rows that pass today and a careless fix must not break. + * + * Like the other server-function specs, these run against the built + * bundles (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createRequestEvent } from "@solidjs/web"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const H = { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Format": "8", + "X-Server-Function-Instance": "server-function:test" +}; + +function scriptedPost(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: H + }); +} + +/** Decodes a result response on whichever road it took. */ +async function decode(response: Response) { + if (response.headers.get("X-Server-Function-Format") === "8") return response.json(); + const { deserializeStream } = await import("@solidjs/web/server-functions/client"); + return deserializeStream(response); +} + +describe("a frozen result is not a failed call (#3196)", () => { + // Every row is a container whose walk REWRITES at least one slot — a + // channel the guard wraps, or an accessor it materializes — on the codec + // road. `Object.freeze` on a returned DTO is ordinary defensive + // authoring, and the same value without the freeze round-trips fine. + const rewriting: [string, () => unknown][] = [ + [ + "frozen object holding a promise", + () => Object.freeze({ id: 9, receipt: Promise.resolve("R-9") }) + ], + [ + "frozen object with a plain getter", + () => + Object.freeze({ + a: 1, + get computed() { + return "cheap"; + } + }) + ], + [ + "frozen object with a non-enumerable getter beside a Date", + () => { + const row: any = { name: "widget", createdAt: new Date(0) }; + Object.defineProperty(row, "hidden", { + get: () => "H", + enumerable: false, + configurable: true + }); + return Object.freeze(row); + } + ], + [ + "frozen object holding a ReadableStream", + () => + Object.freeze({ + s: new ReadableStream({ + start(c) { + c.enqueue("x"); + c.close(); + } + }) + }) + ], + [ + "frozen object holding an async iterable", + () => + Object.freeze({ + it: (async function* () { + yield 1; + })() + }) + ], + [ + "frozen object nested inside an ordinary result", + () => ({ outer: 1, inner: Object.freeze({ r: Promise.resolve("R") }) }) + ] + ]; + + // The shapes that already answer 200. A fix that reaches wider than the + // rewritten slot — freezing the shell, or refusing frozen containers — + // breaks these, so they are pinned as the baseline. + const working: [string, () => unknown][] = [ + ["sealed object holding a promise", () => Object.seal({ r: Promise.resolve("R") })], + [ + "writable:false alone", + () => { + const o: any = {}; + Object.defineProperty(o, "r", { + value: Promise.resolve("R"), + writable: false, + enumerable: true, + configurable: true + }); + return o; + } + ], + [ + "configurable:false alone", + () => { + const o: any = {}; + Object.defineProperty(o, "r", { + value: Promise.resolve("R"), + writable: true, + enumerable: true, + configurable: false + }); + return o; + } + ], + ["frozen array of a promise", () => Object.freeze([Promise.resolve("R")])], + ["frozen Map holding a promise", () => Object.freeze(new Map([["r", Promise.resolve("R")]]))], + ["frozen Set holding a promise", () => Object.freeze(new Set([Promise.resolve("R")]))], + ["frozen plain data (JSON road)", () => Object.freeze({ n: 1 })], + [ + "frozen object holding a Date (codec road, no slot rewritten)", + () => Object.freeze({ d: new Date(0) }) + ], + ["the same shape unfrozen", () => ({ id: 9, receipt: Promise.resolve("R-9") })] + ]; + + test.each([...rewriting, ...working])( + "%s answers 200 and never reports a committed call as failed", + async (label, make) => { + const id = "descriptors-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let committed = 0; + registerServerFunction(id, async () => { + committed++; + return make(); + }); + + const response = await handleServerFunctionRequest(scriptedPost(id), { + createEvent: (request: Request) => { + const event = createRequestEvent(request); + event.response.headers.append("Set-Cookie", "order=9; Path=/"); + return event; + } + }); + + expect(committed).toBe(1); + // the wire must not say "failed" and "succeeded" at once: the + // mutation's own Set-Cookie is on this response either way + expect(response.headers.getSetCookie()).toContain("order=9; Path=/"); + expect(response.headers.get("X-Server-Function-Error")).toBeNull(); + expect(response.status).toBe(200); + // and the value actually round-trips, so a 200 built by dropping the + // result would not pass either + await expect(decode(response)).resolves.toBeDefined(); + } + ); +}); + +describe("a non-enumerable accessor is not serialized (#3198)", () => { + const SECRET = "COST-SECRET-42"; + + // `enumerable: false` is the mechanism JSON.stringify honours and the one + // an author reaches for to keep a computed field server-side. Every row + // asserts the wire body against that same baseline. + const rows: [string, () => any, boolean][] = [ + // [label, factory, secret expected on the wire] + ["non-enumerable accessor beside a Date", () => hidden("accessor", new Date(0)), false], + ["non-enumerable accessor beside a Map", () => hidden("accessor", new Map([["a", 1]])), false], + ["non-enumerable accessor beside a Set", () => hidden("accessor", new Set([1])), false], + [ + "non-enumerable accessor beside a promise", + () => hidden("accessor", Promise.resolve("R")), + false + ], + ["non-enumerable accessor beside an undefined", () => hidden("accessor", undefined), false], + [ + "non-enumerable accessor nested one level down", + () => ({ wrap: hidden("accessor", new Date(0)) }), + false + ], + // controls that already pass and must keep passing + ["non-enumerable accessor alone (JSON road)", () => hidden("accessor", "plain"), false], + ["non-enumerable DATA property beside a Date", () => hidden("data", new Date(0)), false], + [ + "non-enumerable DATA property beside a promise", + () => hidden("data", Promise.resolve("R")), + false + ], + [ + "an ENUMERABLE accessor is still serialized", + () => { + const row: any = { name: "widget", extra: new Date(0) }; + Object.defineProperty(row, "visible", { + get: () => SECRET, + enumerable: true, + configurable: true + }); + return row; + }, + true + ] + ]; + + function hidden(kind: "accessor" | "data", companion: unknown) { + const row: any = { name: "widget", extra: companion }; + Object.defineProperty( + row, + "internalCostBasis", + kind === "accessor" + ? { get: () => SECRET, enumerable: false, configurable: true } + : { value: SECRET, enumerable: false, writable: true, configurable: true } + ); + return row; + } + + test.each(rows)("%s", async (label, make, onWire) => { + const id = "nonenum-" + label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + registerServerFunction(id, async () => make()); + + const response = await handleServerFunctionRequest(scriptedPost(id)); + const body = await response.text(); + + expect(response.status).toBe(200); + expect(body.includes(SECRET)).toBe(onWire); + }); +}); From 8f8116a35594595ecbc002eef45e411d21687c76 Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Wed, 2 Sep 2026 12:22:50 +0700 Subject: [PATCH 11/14] test(web): cover the carrier case, and make the tables prove the call ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gaps a mutation pass found in the tests, plus one simplification. The #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. --- packages/web/server-functions/src/server.ts | 2 +- .../server-functions-content-length.spec.tsx | 47 +++++++++++-------- .../server-functions-proto-keys.spec.tsx | 31 ++++++++++++ .../server-functions-redirect-scheme.spec.tsx | 38 ++++++++++----- 4 files changed, 84 insertions(+), 34 deletions(-) diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index aaba244ce..3a45f5ac7 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -1135,7 +1135,7 @@ function stripOwnProtoKeys(value) { // own properties (#3200). `constructor` rides alongside `__proto__` // because a recursive merge reaches Object.prototype through it (#3202). for (const key of UNSAFE_ARGUMENT_KEYS) { - if (Object.prototype.hasOwnProperty.call(v, key)) delete v[key]; + if (Object.hasOwn(v, key)) delete v[key]; } for (const key of Object.keys(v)) stack.push(v[key]); } diff --git a/packages/web/test/server/server-functions-content-length.spec.tsx b/packages/web/test/server/server-functions-content-length.spec.tsx index cd5c2bb59..62a4a7e26 100644 --- a/packages/web/test/server/server-functions-content-length.spec.tsx +++ b/packages/web/test/server/server-functions-content-length.spec.tsx @@ -82,11 +82,13 @@ async function call(id: string, { road = "scripted" as Road, stub = null as stri * sent. Rendered as a row so a failure names every producer at once rather * than only the first. */ -function row(label: string, r: { declared: string | null; bytes: number }) { - return `${label}: Content-Length=${r.declared ?? "absent"} body=${r.bytes} bytes`; +function row(label: string, r: { status: number; declared: string | null; bytes: number }) { + return `${label}: status=${r.status} Content-Length=${r.declared ?? "absent"} body=${r.bytes} bytes`; } -function expected(label: string, r: { declared: string | null; bytes: number }) { - return `${label}: Content-Length=${r.declared === null ? "absent" : r.bytes} body=${r.bytes} bytes`; +// The status is part of the contract: without it a handler that answers +// nothing at all renders as `absent / 0 bytes` and passes. +function expected(label: string, status: number, r: { declared: string | null; bytes: number }) { + return `${label}: status=${status} Content-Length=${r.declared === null ? "absent" : r.bytes} body=${r.bytes} bytes`; } describe("Content-Length never describes a body the transport composed (#3197)", () => { @@ -122,32 +124,37 @@ describe("Content-Length never describes a body the transport composed (#3197)", ); } - const cases: [string, () => Promise][] = [ - ["returned Response + Content-Length (codec road)", () => call("cl-returned-response")], - ["thrown Response + Content-Length", () => call("cl-thrown-response")], - ["returned respond() envelope (JSON road)", () => call("cl-returned-envelope")], - ["thrown respond() envelope", () => call("cl-thrown-envelope")], - ["thrown 302 carrying a Content-Length", () => call("cl-redirect")], - ["stub gap-fill: middleware sets 0", () => call("cl-string", { stub: "0" })], - ["stub gap-fill: middleware sets 999", () => call("cl-string", { stub: "999" })], - ["204 + Content-Length", () => call("cl-null-204")], - ["205 + Content-Length", () => call("cl-null-205")], - ["304 + Content-Length", () => call("cl-null-304")], + const cases: [string, () => Promise, number][] = [ + ["returned Response + Content-Length (codec road)", () => call("cl-returned-response"), 200], + ["thrown Response + Content-Length", () => call("cl-thrown-response"), 200], + ["returned respond() envelope (JSON road)", () => call("cl-returned-envelope"), 200], + ["thrown respond() envelope", () => call("cl-thrown-envelope"), 200], + ["thrown 302 carrying a Content-Length", () => call("cl-redirect"), 200], + ["stub gap-fill: middleware sets 0", () => call("cl-string", { stub: "0" }), 200], + ["stub gap-fill: middleware sets 999", () => call("cl-string", { stub: "999" }), 200], + ["204 + Content-Length", () => call("cl-null-204"), 204], + ["205 + Content-Length", () => call("cl-null-205"), 205], + ["304 + Content-Length", () => call("cl-null-304"), 304], // unscripted: the same producers, plain-HTTP road [ "unscripted respond() envelope + Content-Length", - () => call("cl-returned-envelope", { road: "plain" }) + () => call("cl-returned-envelope", { road: "plain" }), + 200 ], - ["unscripted stub gap-fill", () => call("cl-string", { road: "plain", stub: "999" })], - ["no-JS form post + Content-Length", () => call("cl-returned-response", { road: "form" })] + ["unscripted stub gap-fill", () => call("cl-string", { road: "plain", stub: "999" }), 200], + [ + "no-JS form post + Content-Length", + () => call("cl-returned-response", { road: "form" }), + 303 + ] ]; const actual: string[] = []; const want: string[] = []; - for (const [label, run] of cases) { + for (const [label, run, status] of cases) { const r = await run(); actual.push(row(label, r)); - want.push(expected(label, r)); + want.push(expected(label, status, r)); } expect(actual).toEqual(want); }); diff --git a/packages/web/test/server/server-functions-proto-keys.spec.tsx b/packages/web/test/server/server-functions-proto-keys.spec.tsx index 509d95b53..03831d0e7 100644 --- a/packages/web/test/server/server-functions-proto-keys.spec.tsx +++ b/packages/web/test/server/server-functions-proto-keys.spec.tsx @@ -185,6 +185,37 @@ describe("decoded arguments cannot reach Object.prototype (#3202)", () => { ); }); + it("a non-plain carrier does not shelter the payload underneath it (#3200)", async () => { + // The reachable shape is not an unsafe key ON a carrier — seroval drops + // that at encode — but a plain object one level UNDER one. The walk used + // to stop at any non-plain prototype, so an `Error`, or any class the + // codec revives with own properties, hid everything beneath it. Codec + // road only: the JSON road cannot express a carrier. + const rows: string[] = []; + for (const road of ["codec-query", "codec-body"] as Road[]) { + const carrier = Object.assign(new Error("validation failed"), { + payload: { ctorKey: { prototypeKey: { polluted: "viaCarrier" } }, n: 1 } + }); + const { status, seen } = await decodeArgument(road, carrier); + const payload = (seen as any)?.payload; + deepMerge({}, payload ?? {}); + const leaked = (Object.prototype as any).polluted; + delete (Object.prototype as any).polluted; + rows.push( + `${road}: status=${status} payloadKeys=${JSON.stringify( + Object.keys(payload ?? {}) + )} Object.prototype.polluted=${JSON.stringify(leaked)}` + ); + } + expect(rows).toEqual( + rows.map( + r => + `${r.slice(0, r.indexOf(": ") + 2)}status=200 payloadKeys=["n"] ` + + `Object.prototype.polluted=undefined` + ) + ); + }); + it("control: ordinary data with a similar name is not eaten", async () => { const rows: string[] = []; for (const road of ROADS) { diff --git a/packages/web/test/server/server-functions-redirect-scheme.spec.tsx b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx index d1ec19b20..31a08aaf3 100644 --- a/packages/web/test/server/server-functions-redirect-scheme.spec.tsx +++ b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx @@ -53,16 +53,24 @@ let seq = 0; async function ship(road: Road, target: string) { const id = `scheme-${seq++}`; + let ran = 0; registerServerFunction(id, async () => { + ran++; // `Headers` refuses CR/LF in a value outright, so those variants never - // reach the floor at all — report that rather than hiding it as a pass + // reach the floor — that protection comes from the platform, not from the + // code under test, so it is reported as its own outcome rather than + // counted as a refusal the floor made. const headers = new Headers(); - if (road === "redirect-header") { - headers.set(REDIRECT_HEADER, `302 ${target}`); - return new Response(null, { status: 200, headers }); + try { + if (road === "redirect-header") { + headers.set(REDIRECT_HEADER, `302 ${target}`); + return new Response(null, { status: 200, headers }); + } + headers.set("Location", target); + return new Response(null, { status: 302, headers }); + } catch { + return new Response("HEADERS-REFUSED", { status: 200 }); } - headers.set("Location", target); - return new Response(null, { status: 302, headers }); }); const scripted = road === "masked"; const form = road === "nojs"; @@ -86,21 +94,21 @@ async function ship(road: Road, target: string) { { provideEvent } ); } catch { - return { shipped: null as string | null, refused: true, protocol: "n/a" }; + return { ran, shipped: null as string | null, refused: true, protocol: "n/a" }; } - if (response.status === 500) return { shipped: null, refused: true, protocol: "n/a" }; + if (response.status === 500) return { ran, shipped: null, refused: true, protocol: "n/a" }; const carried = response.headers.get(REDIRECT_HEADER); const shipped = carried ? carried.slice(carried.indexOf(" ") + 1) : response.headers.get("Location"); - if (shipped === null) return { shipped: null, refused: true, protocol: "n/a" }; + if (shipped === null) return { ran, shipped: null, refused: true, protocol: "n/a" }; let protocol: string; try { protocol = new URL(shipped, "https://app.example/here").protocol; } catch { protocol = "unparseable"; } - return { shipped, refused: false, protocol }; + return { ran, shipped, refused: false, protocol }; } /** Every scheme the floor refuses. */ @@ -138,7 +146,7 @@ describe("non-http(s) navigation targets are refused however they are spelled (# const r = await ship(road, target); const safe = r.refused || r.protocol === "http:" || r.protocol === "https:"; rows.push( - `${scheme}/${name}/${road}: ${ + `${scheme}/${name}/${road}: ran=${r.ran} ${ safe ? "http(s)-or-refused" : `SHIPPED ${JSON.stringify(r.shipped)} reads as ${r.protocol}` @@ -147,7 +155,7 @@ describe("non-http(s) navigation targets are refused however they are spelled (# } // the client-side decoder enforces the same floor independently rows.push( - `${scheme}/${name}/decoder: ${ + `${scheme}/${name}/decoder: ran=1 ${ decodeRedirectHeaderValue(`302 ${target}`) === undefined ? "http(s)-or-refused" : "DECODED a non-http(s) target" @@ -155,7 +163,11 @@ describe("non-http(s) navigation targets are refused however they are spelled (# ); } } - expect(rows).toEqual(rows.map(r => `${r.slice(0, r.indexOf(":") + 1)} http(s)-or-refused`)); + // `ran=1` is part of the contract: a handler that dispatches nothing + // renders every row as "refused" and would otherwise pass. + expect(rows).toEqual( + rows.map(r => `${r.slice(0, r.indexOf(":") + 1)} ran=1 http(s)-or-refused`) + ); }); it("keeps every allowance the floor deliberately grants", async () => { From c1bc98e282ee239aa255ac5ef15a5b11bea54a46 Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Wed, 2 Sep 2026 12:39:33 +0700 Subject: [PATCH 12/14] refactor(web): name the framing headers once, and drop three needless params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/web/server-functions/src/server.ts | 20 ++++++------- packages/web/src/response.ts | 17 +++++++++-- packages/web/src/server.ts | 6 ++-- .../server-functions-content-length.spec.tsx | 30 ++++++++++++++++--- 4 files changed, 54 insertions(+), 19 deletions(-) diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 3a45f5ac7..9dd84de21 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -13,6 +13,7 @@ // mutation — and never what they carry. Which data a mutation invalidates, // and how an outcome reaches the UI, stay with the integration. import { + COMPOSED_BODY_FRAMING, NULL_BODY_STATUSES, RESPONSE_HEADER_VALUE_LIMIT, REVALIDATE_HEADER, @@ -1135,7 +1136,7 @@ function stripOwnProtoKeys(value) { // own properties (#3200). `constructor` rides alongside `__proto__` // because a recursive merge reaches Object.prototype through it (#3202). for (const key of UNSAFE_ARGUMENT_KEYS) { - if (Object.hasOwn(v, key)) delete v[key]; + delete v[key]; } for (const key of Object.keys(v)) stack.push(v[key]); } @@ -1432,10 +1433,7 @@ export function foldSetCookies(headers, setCookies) { // way response headers may merge here: never `get`/`set` folding. function mergeResponseHeaders(target, source) { source.forEach((value, key) => { - // `content-length` describes the source's body and the caller is about to - // send a different one; forwarding a length known to be wrong truncates - // the answer at the socket (#3197, RFC 9110 §8.6). - if (key !== "set-cookie" && key !== "content-length") target.append(key, value); + if (key !== "set-cookie" && !COMPOSED_BODY_FRAMING.has(key)) target.append(key, value); }); if (source.getSetCookie) { for (const cookie of source.getSetCookie()) target.append("Set-Cookie", cookie); @@ -1509,7 +1507,7 @@ const BOUNDED_COMPOSED_HEADERS = [REDIRECT_HEADER, "Location", REVALIDATE_HEADER // gives it an opt-in. The decoder enforces the same floor independently // (decodeRedirectHeaderValue), so a hostile peer cannot re-open the class // against integrations either. -function refusedTargetScheme(target, base) { +function refusedTargetScheme(target) { // Judge the target the way a consumer will READ it, not the way its bytes // are spelled: a URL parser strips ASCII tab and newline from anywhere and // trims leading C0/space before it begins, so a scheme grammar over the raw @@ -1518,7 +1516,9 @@ function refusedTargetScheme(target, base) { // fooled; the base keeps relative targets — the ordinary case — http(s). let protocol; try { - protocol = new URL(target, base).protocol; + // any http(s) base gives the same verdict: an absolute scheme wins over + // it, and a relative target inherits it + protocol = new URL(target, "http://base.invalid").protocol; } catch { return true; } @@ -1537,7 +1537,7 @@ function refusedTargetScheme(target, base) { // or rewritten target is a DIFFERENT address, a dropped revalidate key is // a silently stale cache. Runs ahead of the stub fold so integration // cookies still ride the refusal (#3159). -function enforceComposedHeaderInvariants(response, base) { +function enforceComposedHeaderInvariants(response) { for (const name of BOUNDED_COMPOSED_HEADERS) { const value = response.headers.get(name); if (value === null) continue; @@ -1560,7 +1560,7 @@ function enforceComposedHeaderInvariants(response, base) { if (name === REVALIDATE_HEADER) continue; // REDIRECT_HEADER rides as " "; Location is the target const target = name === REDIRECT_HEADER ? value.slice(value.indexOf(" ") + 1) : value; - if (refusedTargetScheme(target, base)) { + if (refusedTargetScheme(target)) { return refuseComposedHeader( response, name, @@ -3247,7 +3247,7 @@ export async function handleServerFunctionRequest(request, options = {}) { // foreign-response path at once (raw passthrough, unscripted returns and // throws, custom handleNoJS results, envelope-carried responses). const response = commitEventResponse( - enforceComposedHeaderInvariants(ownResponse(await dispatch()), request.url), + enforceComposedHeaderInvariants(ownResponse(await dispatch())), event ); return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); diff --git a/packages/web/src/response.ts b/packages/web/src/response.ts index 77be09ee9..26962dfdc 100644 --- a/packages/web/src/response.ts +++ b/packages/web/src/response.ts @@ -257,6 +257,19 @@ export const NULL_BODY_STATUSES: ReadonlySet = new Set([204, 205, 304]); * consumers without the client runtime (no-JS form posts, direct HTTP) * get real JSON, while integrations read `value` — no reparse. */ +/** + * Headers that describe how a body is framed on the wire. Whenever the + * transport composes a body of its own, an author-supplied value describes + * the body it replaced — a stale `Content-Length` truncates the answer at the + * socket, and a stale `Content-Encoding` tells the peer to decompress bytes + * nobody compressed (#3197, RFC 9110 §8.6). + */ +export const COMPOSED_BODY_FRAMING: ReadonlySet = /*#__PURE__*/ new Set([ + "content-length", + "content-encoding", + "transfer-encoding" +]); + export function respond(value: T, init: ResponseHelperInit = {}) { const { responseInit, headers } = initWithRevalidate(init); // A null-body status cannot carry the passthrough JSON body — building it @@ -264,9 +277,9 @@ export function respond(value: T, init: ResponseHelperInit = {}) { // Carry the metadata bodiless; the server-function encoder answers the // void shapes with a real null-body response and reports value-carrying // ones legibly. - // The body below is ours, so an author-supplied length describes something + // The body below is ours, so an author's framing headers describe something // else — and on a null-body status there is no body at all (#3197). - headers.delete("Content-Length"); + for (const header of COMPOSED_BODY_FRAMING) headers.delete(header); if (NULL_BODY_STATUSES.has(responseInit.status)) { return new ResponseEnvelope(new Response(null, { ...responseInit, headers }), value); } diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index ec5a9b223..83f72c427 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -21,7 +21,7 @@ import { // Wire-protocol header names for the commit fold's gap-fill denylist // (`commitEventResponse`): shared constants, not copies, so the fold can // never drift from what the server-function handler actually sends. -import { REVALIDATE_HEADER } from "./response.js"; +import { COMPOSED_BODY_FRAMING, REVALIDATE_HEADER } from "./response.js"; import { BODY_FORMAT_HEADER, ERROR_HEADER, @@ -4461,9 +4461,9 @@ const STUB_GAP_FILL_EXCLUDED = /*#__PURE__*/ new Set( REVALIDATE_HEADER, REDIRECT_HEADER, "Location", - // written before the body exists, so it can only describe a different + // written before the body exists, so they can only describe a different // one (#3197) - "Content-Length" + ...COMPOSED_BODY_FRAMING ].map(header => header.toLowerCase()) ); diff --git a/packages/web/test/server/server-functions-content-length.spec.tsx b/packages/web/test/server/server-functions-content-length.spec.tsx index 62a4a7e26..f0d344966 100644 --- a/packages/web/test/server/server-functions-content-length.spec.tsx +++ b/packages/web/test/server/server-functions-content-length.spec.tsx @@ -73,8 +73,9 @@ async function call(id: string, { road = "scripted" as Road, stub = null as stri } ); const declared = response.headers.get("Content-Length"); + const encoding = response.headers.get("Content-Encoding"); const bytes = response.body ? (await response.arrayBuffer()).byteLength : 0; - return { status: response.status, declared, bytes }; + return { status: response.status, declared, encoding, bytes }; } /** @@ -82,13 +83,23 @@ async function call(id: string, { road = "scripted" as Road, stub = null as stri * sent. Rendered as a row so a failure names every producer at once rather * than only the first. */ -function row(label: string, r: { status: number; declared: string | null; bytes: number }) { - return `${label}: status=${r.status} Content-Length=${r.declared ?? "absent"} body=${r.bytes} bytes`; +function row( + label: string, + r: { status: number; declared: string | null; encoding: string | null; bytes: number } +) { + return `${label}: status=${r.status} Content-Length=${r.declared ?? "absent"} Content-Encoding=${ + r.encoding ?? "absent" + } body=${r.bytes} bytes`; } // The status is part of the contract: without it a handler that answers // nothing at all renders as `absent / 0 bytes` and passes. function expected(label: string, status: number, r: { declared: string | null; bytes: number }) { - return `${label}: status=${status} Content-Length=${r.declared === null ? "absent" : r.bytes} body=${r.bytes} bytes`; + // A framing header the runtime did not compute must simply be absent: a + // stale length truncates the answer, a stale encoding tells the peer to + // decompress bytes nobody compressed. + return `${label}: status=${status} Content-Length=${ + r.declared === null ? "absent" : r.bytes + } Content-Encoding=absent body=${r.bytes} bytes`; } describe("Content-Length never describes a body the transport composed (#3197)", () => { @@ -118,6 +129,15 @@ describe("Content-Length never describes a body the transport composed (#3197)", }); }); registerServerFunction("cl-string", async () => "seven!!"); + // a Content-Encoding is the same defect wearing a different name: it + // describes a compression the transport never applied + registerServerFunction("cl-encoding-envelope", async () => + respond({ ok: true, n: 42 }, { headers: { "Content-Encoding": "gzip" } }) + ); + registerServerFunction( + "cl-encoding-response", + async () => new Response("upstream body", { headers: { "Content-Encoding": "gzip" } }) + ); for (const status of [204, 205, 304]) { registerServerFunction(`cl-null-${status}`, async () => respond(undefined, { status, headers: { "Content-Length": "5" } }) @@ -132,6 +152,8 @@ describe("Content-Length never describes a body the transport composed (#3197)", ["thrown 302 carrying a Content-Length", () => call("cl-redirect"), 200], ["stub gap-fill: middleware sets 0", () => call("cl-string", { stub: "0" }), 200], ["stub gap-fill: middleware sets 999", () => call("cl-string", { stub: "999" }), 200], + ["envelope + Content-Encoding", () => call("cl-encoding-envelope"), 200], + ["Response + Content-Encoding", () => call("cl-encoding-response"), 200], ["204 + Content-Length", () => call("cl-null-204"), 204], ["205 + Content-Length", () => call("cl-null-205"), 205], ["304 + Content-Length", () => call("cl-null-304"), 304], From 479954123b4a554240408a70f6af5eff13493287 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Wed, 2 Sep 2026 00:31:27 -0700 Subject: [PATCH 13/14] fix(web): harden server transport boundaries Close cross-realm, descriptor, argument, and SSR response gaps found while auditing the server-function transport. Co-authored-by: Cursor --- .../server-function-guard-simplification.md | 4 + packages/web/server-functions/src/server.ts | 114 +++++++++--------- packages/web/src/constants.ts | 21 +++- packages/web/src/response.ts | 15 +-- packages/web/src/server.ts | 16 ++- packages/web/test/runtime/cookies.spec.js | 36 ++++++ .../server-functions-event-hook.spec.tsx | 15 +++ .../server-functions-proto-keys.spec.tsx | 21 ++++ ...rver-functions-result-descriptors.spec.tsx | 41 +++++++ 9 files changed, 203 insertions(+), 80 deletions(-) diff --git a/.changeset/server-function-guard-simplification.md b/.changeset/server-function-guard-simplification.md index d9be0875d..112d6feb5 100644 --- a/.changeset/server-function-guard-simplification.md +++ b/.changeset/server-function-guard-simplification.md @@ -11,3 +11,7 @@ Seven defects, five of them introduced by the guards added in #3168, #3170, argument walk stops for no prototype, the guard shell keeps only the flag that reaches the wire, the scheme floor asks the URL parser instead of a regex, and the event is awaited only when it is genuinely a promise. + +Runtime-composed SSR responses now discard stale body-framing headers, and +late streaming redirects emit client script only for relative or HTTP(S) +targets. diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 9dd84de21..76c52adff 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -13,13 +13,13 @@ // mutation — and never what they carry. Which data a mutation invalidates, // and how an outcome reaches the UI, stay with the integration. import { - COMPOSED_BODY_FRAMING, NULL_BODY_STATUSES, RESPONSE_HEADER_VALUE_LIMIT, REVALIDATE_HEADER, isResponseEnvelope, isSafeError } from "../../src/response.js"; +import { COMPOSED_BODY_FRAMING, isHttpNavigationTarget } from "../../src/constants.js"; import { RequestContext, commitEventResponse, getRequestEvent } from "../../src/server.js"; import { encodeFlashCookie } from "./flash.js"; import { @@ -1096,7 +1096,7 @@ function assertDecodeDepth(value) { } /** - * Strips own `__proto__` keys from a decoded argument graph, in place. + * Strips prototype-mutating keys from a decoded argument graph, in place. * * Both decode roads preserve the key faithfully — `JSON.parse` creates it * as an ordinary own property and the codec round-trips it the same way — @@ -1111,34 +1111,28 @@ function assertDecodeDepth(value) { * The walk is iterative (the codec revives cyclic graphs, and depth is the * attack input on the JSON road) with a visited set for cycles. It reaches * plain objects and arrays, plus the values and keys of revived Maps and - * Sets; a value nested inside a revived class instance keeps its shape — - * the codec owns that value's construction, not this guard. + * Sets, and enumerable properties on revived non-plain objects. Containers + * keep their shape — the codec owns their construction, not this guard. */ const UNSAFE_ARGUMENT_KEYS = ["__proto__", "constructor", "prototype"]; -function stripOwnProtoKeys(value) { +function stripUnsafeArgumentKeys(value) { const stack = [value]; const seen = new Set(); while (stack.length) { const v = stack.pop(); if (v === null || typeof v !== "object" || seen.has(v)) continue; seen.add(v); - if (Array.isArray(v)) { - for (let i = 0; i < v.length; i++) stack.push(v[i]); - } else if (v instanceof Map) { + // Mutating in place never required a plain prototype. Strip every + // container, then walk both own metadata and collection contents. + for (const key of UNSAFE_ARGUMENT_KEYS) { + delete v[key]; + } + for (const key of Object.keys(v)) stack.push(v[key]); + if (v instanceof Map) { for (const [k, entry] of v) stack.push(k, entry); } else if (v instanceof Set) { for (const member of v) stack.push(member); - } else { - // Every object is walked, whatever its prototype: this mutates in place - // and rebuilds nothing, so a non-plain prototype was never a reason to - // stop — it only hid a payload under a carrier the codec revives with - // own properties (#3200). `constructor` rides alongside `__proto__` - // because a recursive merge reaches Object.prototype through it (#3202). - for (const key of UNSAFE_ARGUMENT_KEYS) { - delete v[key]; - } - for (const key of Object.keys(v)) stack.push(v[key]); } } return value; @@ -1201,7 +1195,7 @@ async function parseArguments(request, url, scripted, codec) { if (!Array.isArray(result)) { throw new TypeError("Server function arguments must encode an array"); } - stripOwnProtoKeys(result); + stripUnsafeArgumentKeys(result); for (const arg of result) { parsed.push(arg); } @@ -1227,7 +1221,7 @@ async function parseArguments(request, url, scripted, codec) { if (!Array.isArray(decoded)) { throw new TypeError("Server function arguments must encode an array"); } - return stripOwnProtoKeys(decoded); + return stripUnsafeArgumentKeys(decoded); } if (decoded === undefined) { // The decode switch fell through: the format tag — or its duplicate- @@ -1506,24 +1500,8 @@ const BOUNDED_COMPOSED_HEADERS = [REDIRECT_HEADER, "Location", REVALIDATE_HEADER // custom app scheme (deep links) is refused by default until that ruling // gives it an opt-in. The decoder enforces the same floor independently // (decodeRedirectHeaderValue), so a hostile peer cannot re-open the class -// against integrations either. -function refusedTargetScheme(target) { - // Judge the target the way a consumer will READ it, not the way its bytes - // are spelled: a URL parser strips ASCII tab and newline from anywhere and - // trims leading C0/space before it begins, so a scheme grammar over the raw - // value saw nothing where the parser sees `javascript:` (#3201). Resolving - // is what the masked and no-JS roads already did, which is why neither was - // fooled; the base keeps relative targets — the ordinary case — http(s). - let protocol; - try { - // any http(s) base gives the same verdict: an absolute scheme wins over - // it, and a relative target inherits it - protocol = new URL(target, "http://base.invalid").protocol; - } catch { - return true; - } - return protocol !== "http:" && protocol !== "https:"; -} +// against integrations either. The shared parser also guards late streaming +// SSR redirects before they become executable script. // The transport half of redirect()'s and initWithRevalidate's invariants: // composed-header BOUNDS (#3158 — an over-long header dies at a proxy @@ -1560,7 +1538,7 @@ function enforceComposedHeaderInvariants(response) { if (name === REVALIDATE_HEADER) continue; // REDIRECT_HEADER rides as " "; Location is the target const target = name === REDIRECT_HEADER ? value.slice(value.indexOf(" ") + 1) : value; - if (refusedTargetScheme(target)) { + if (!isHttpNavigationTarget(target)) { return refuseComposedHeader( response, name, @@ -1737,8 +1715,8 @@ function isFormPost(request) { // protection, it was a bypass: a rejecting promise behind a getter or used // as a Map key rode the wire unsanitized, was never torn down, and on the // promise path took the process down with an unhandled rejection (#3176). -// Getters are now invoked once and materialized as data properties; Map -// keys are walked like values. +// Enumerable getters are now invoked once and materialized as data +// properties; Map keys are walked like values. // // `state.gate` (optional — serializeResponseStream threads it, #3125) hooks // the two STREAMING channels into the response lifetime as they are walked: @@ -2047,18 +2025,19 @@ function enterGuard(value, state) { // invoked once there, rewritten as data properties on this shell. const descriptors = Object.getOwnPropertyDescriptors(value); // The shell is scratch the codec reads once and the author never holds, so - // only `enumerable` has to survive — it is what the codec serializes. - // Replicating a frozen source's `writable`/`configurable` made the - // write-back below illegal (#3196) and pinning them true lost - // `enumerable: false` (#3198). - const rewritable = {}; + // make its fresh descriptors rewritable in place. This preserves the one + // serialization flag (`enumerable`) while frozen slots can be replaced + // (#3196, #3198), and keeps an authored own `__proto__` descriptor as data + // instead of interpreting its name while copying through an ordinary object. for (const key of Object.keys(descriptors)) { - rewritable[key] = { ...descriptors[key], writable: true, configurable: true }; - if ("get" in rewritable[key]) delete rewritable[key].writable; + descriptors[key].configurable = true; + if ("value" in descriptors[key]) descriptors[key].writable = true; } - const next = Object.create(prototype, rewritable); + const next = Object.create(prototype, descriptors); state.seen.set(value, next); - return new Frame(OBJECT, value, next, Object.keys(descriptors), descriptors); + // The codec reads enumerable string properties; hidden accessors must stay + // hidden without being invoked merely because another slot needs guarding. + return new Frame(OBJECT, value, next, Object.keys(value), descriptors); } /** A rebuild stands if anything below changed, or if a cycle already took it. */ @@ -2537,7 +2516,19 @@ function forbiddenResponse() { headers: { "Cache-Control": "no-store" } }) ); -} /** +} + +function nativePromise(value) { + if (value instanceof Promise) return value; + try { + // `instanceof` is realm-local. The intrinsic brand check accepts a + // genuine Promise from another realm without adopting arbitrary thenables. + if (Object.prototype.toString.call(value) === "[object Promise]") + return Promise.prototype.then.call(value, value => value); + } catch {} +} + +/** * Web-standard HTTP handler for server function calls: resolves the * function id from the request, enforces the method allowlist (POST always * dispatches; GET and HEAD dispatch only to functions that declared `GET`, @@ -2824,17 +2815,20 @@ export async function handleServerFunctionRequest(request, options = {}) { let event; try { event = options.createEvent ? options.createEvent(request) : { request, locals: {} }; - if (event instanceof Promise) event = await event; + const promised = nativePromise(event); + if (promised) event = await promised; } catch (error) { - // tagged like every other error answer, so the transport reads it as ours + const safe = sanitizeServerError(error); + const message = safe instanceof Error ? safe.message : String(safe); const headers = new Headers(); - headers.set( - ERROR_HEADER, - boundedErrorHeaderValue( - DEV ? String((error as any)?.message ?? error) : GENERIC_SERVER_ERROR_MESSAGE - ) - ); - const response = new Response(null, { status: 500, headers }); + headers.set(ERROR_HEADER, boundedErrorHeaderValue(message)); + // A scripted failure is still runtime protocol, so encode it like a + // function throw rather than making the client misclassify it as an + // untagged peer 500. Plain HTTP keeps the ordinary bodiless production + // response. + const response = scripted + ? encodeResult(safe, headers, 500, codec, request.signal) + : new Response(DEV ? message : null, { status: 500 }); return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); } // Once an event exists, its response stub folds onto EVERY exit — the diff --git a/packages/web/src/constants.ts b/packages/web/src/constants.ts index a694bce63..0dfdd30e2 100644 --- a/packages/web/src/constants.ts +++ b/packages/web/src/constants.ts @@ -215,6 +215,23 @@ const DOMElements = /*#__PURE__*/ new Set( ) ); +// Headers that describe a body the runtime replaces or composes itself. +const COMPOSED_BODY_FRAMING: ReadonlySet = /*#__PURE__*/ new Set([ + "content-length", + "content-encoding", + "transfer-encoding" +]); + +// Scheme floor shared by server-function redirects and late streaming SSR. +function isHttpNavigationTarget(target: string): boolean { + try { + const protocol = new URL(target, "http://base.invalid").protocol; + return protocol === "http:" || protocol === "https:"; + } catch { + return false; + } +} + export { DOMWithState, ChildProperties, @@ -226,5 +243,7 @@ export { Namespaces, DOMElements, $$SLOT, - $$HOST + $$HOST, + COMPOSED_BODY_FRAMING, + isHttpNavigationTarget }; diff --git a/packages/web/src/response.ts b/packages/web/src/response.ts index 26962dfdc..caec24f75 100644 --- a/packages/web/src/response.ts +++ b/packages/web/src/response.ts @@ -9,6 +9,8 @@ // The revalidation keys are opaque strings here — whatever keyed cache the // integration brings assigns them meaning. +import { COMPOSED_BODY_FRAMING } from "./constants.js"; + // Identity must survive duplicated module instances (e.g. the core entry // and the server-functions entry bundled separately both carrying a copy), // so the envelope is detected by a registered-symbol brand, not instanceof. @@ -257,19 +259,6 @@ export const NULL_BODY_STATUSES: ReadonlySet = new Set([204, 205, 304]); * consumers without the client runtime (no-JS form posts, direct HTTP) * get real JSON, while integrations read `value` — no reparse. */ -/** - * Headers that describe how a body is framed on the wire. Whenever the - * transport composes a body of its own, an author-supplied value describes - * the body it replaced — a stale `Content-Length` truncates the answer at the - * socket, and a stale `Content-Encoding` tells the peer to decompress bytes - * nobody compressed (#3197, RFC 9110 §8.6). - */ -export const COMPOSED_BODY_FRAMING: ReadonlySet = /*#__PURE__*/ new Set([ - "content-length", - "content-encoding", - "transfer-encoding" -]); - export function respond(value: T, init: ResponseHelperInit = {}) { const { responseInit, headers } = initWithRevalidate(init); // A null-body status cannot carry the passthrough JSON body — building it diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index 83f72c427..086808164 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -1,5 +1,5 @@ // @ts-nocheck -import { ChildProperties } from "./constants.js"; +import { COMPOSED_BODY_FRAMING, ChildProperties, isHttpNavigationTarget } from "./constants.js"; import { sharedConfig, createRoot as root, @@ -21,7 +21,7 @@ import { // Wire-protocol header names for the commit fold's gap-fill denylist // (`commitEventResponse`): shared constants, not copies, so the fold can // never drift from what the server-function handler actually sends. -import { COMPOSED_BODY_FRAMING, REVALIDATE_HEADER } from "./response.js"; +import { REVALIDATE_HEADER } from "./response.js"; import { BODY_FORMAT_HEADER, ERROR_HEADER, @@ -4550,6 +4550,9 @@ export function commitEventResponse(response, event = getRequestEvent()) { function deriveHead(stub, responseInit = {}) { const headers = mergeStubHeaders(copyInitHeaders(responseInit.headers), stub); + // This runtime supplies the HTML body, so framing written before render + // cannot describe the bytes that will leave. + for (const header of COMPOSED_BODY_FRAMING) headers.delete(header); const status = (stub && stub.status) || responseInit.status || 200; const statusText = (stub && stub.statusText) || responseInit.statusText || undefined; return { status, statusText, headers }; @@ -4557,7 +4560,8 @@ function deriveHead(stub, responseInit = {}) { * Derives the outgoing `Response` for an SSR render result, running the * response-head lifecycle against `event.response`: commit at shell flush, * pre-flush `Location` becomes a real redirect, post-flush `Location` - * appends a client-side script redirect before the stream closes. + * appends a client-side script redirect before the stream closes when its + * target resolves to HTTP(S). * Synchronous for string results; resolves at shell flush for stream * results. */ @@ -4586,8 +4590,8 @@ export function createSSRResponse( * `Location` short-circuits to a redirect with no body (the render is * abandoned). A `Location` set after the flush * can only be honored client-side, so stream completion appends - * `` (carrying `options.nonce` for - * strict `script-src` CSPs) before closing. + * `` for relative or HTTP(S) targets + * (carrying `options.nonce` for strict `script-src` CSPs) before closing. * * `options.transformChunk(chunk)` rewrites each outgoing HTML chunk (entry * script injection, doctype prefixes, ...). The default `content-type` is @@ -4670,7 +4674,7 @@ export function createSSRResponse(result, event, options = {}) { // (a pre-flush one short-circuited above) — client-side is the only // side that can still honor it. const location = stub && stub.headers.get("Location"); - if (location) { + if (location && isHttpNavigationTarget(location)) { const attr = nonceAttr(nonce, "script"); enqueue( `window.location=${JSON.stringify(location).replace( diff --git a/packages/web/test/runtime/cookies.spec.js b/packages/web/test/runtime/cookies.spec.js index 4402c27b5..252142698 100644 --- a/packages/web/test/runtime/cookies.spec.js +++ b/packages/web/test/runtime/cookies.spec.js @@ -230,6 +230,20 @@ describe("createSSRResponse carries multiple Set-Cookie values", () => { expect(event.response.committed).toBe(true); }); + it("runtime-composed HTML drops framing headers written before render", async () => { + const event = eventWithCookies(); + event.response.headers.set("Content-Length", "1"); + event.response.headers.set("Content-Encoding", "gzip"); + event.response.headers.set("Transfer-Encoding", "chunked"); + + const response = r.createSSRResponse("hello", event); + + expect(response.headers.get("Content-Length")).toBeNull(); + expect(response.headers.get("Content-Encoding")).toBeNull(); + expect(response.headers.get("Transfer-Encoding")).toBeNull(); + expect(await response.text()).toBe("hello"); + }); + it("redirect result: cookies ride the redirect head", () => { const event = eventWithCookies(); appendCookie(event, "session", "fresh"); @@ -272,6 +286,28 @@ describe("createSSRResponse carries multiple Set-Cookie values", () => { expect(event.response.headers.get("Location")).toBe("/next"); expect(response.headers.getSetCookie()).toEqual([]); }); + + it("late stream redirects emit script only for HTTP(S) targets", async () => { + async function finish(location) { + const event = eventWithCookies(); + let sink; + const response = await r.createSSRResponse( + { + pipe(writable) { + sink = writable; + writable.write("

shell

"); + } + }, + event + ); + event.response.headers.set("Location", location); + sink.end(); + return response.text(); + } + + await expect(finish("/next")).resolves.toContain('window.location="/next"'); + await expect(finish("java\tscript:alert(1)")).resolves.not.toContain("window.location="); + }); }); describe("handleServerFunctionRequest folds the event response stub", () => { diff --git a/packages/web/test/server/server-functions-event-hook.spec.tsx b/packages/web/test/server/server-functions-event-hook.spec.tsx index 9dc1c75a7..2bc1160fa 100644 --- a/packages/web/test/server/server-functions-event-hook.spec.tsx +++ b/packages/web/test/server/server-functions-event-hook.spec.tsx @@ -20,8 +20,10 @@ * bundles (server-functions/dist/*, wired up in vite.config.server.mjs). */ import { AsyncLocalStorage } from "node:async_hooks"; +import { runInNewContext } from "node:vm"; import { afterAll, beforeAll, describe, expect, test } from "vitest"; import { + decodeResponse, handleServerFunctionRequest, registerServerFunction } from "@solidjs/web/server-functions/server"; @@ -55,6 +57,7 @@ const stamp = (event: any) => { event.response.headers.append("Set-Cookie", "sid=abc; Path=/"); return event; }; +const ForeignPromise = runInNewContext("Promise"); /** Answers within `ms` or reports the hang as a value, never as a timeout. */ async function within(work: Promise, ms = 1000): Promise { @@ -148,6 +151,10 @@ describe("a failing createEvent is answered, not escaped (#3199)", () => { throw new Error("session store unreachable"); } ], + [ + "a rejecting cross-realm createEvent", + () => ForeignPromise.reject(new Error("session store unreachable")) + ], [ "a synchronously throwing createEvent", () => { @@ -174,6 +181,10 @@ describe("a failing createEvent is answered, not escaped (#3199)", () => { expect((response as Response).headers.get("X-Server-Function-Error")).toBe( "Internal Server Error" ); + expect((response as Response).headers.get("X-Server-Function-Format")).toBe("0"); + const decoded = await decodeResponse(response as Response); + expect(decoded).toBeInstanceOf(Error); + expect((decoded as Error).message).toBe("Internal Server Error"); expect(ran).toBe(0); }); }); @@ -187,6 +198,10 @@ describe("the awaited shapes #3170 added keep working (#3199 baseline)", () => { return stamp(createRequestEvent(request)); } ], + [ + "a cross-realm Promise", + request => ForeignPromise.resolve(stamp(createRequestEvent(request))) + ], ["a plain synchronous createEvent", request => stamp(createRequestEvent(request))] ]; diff --git a/packages/web/test/server/server-functions-proto-keys.spec.tsx b/packages/web/test/server/server-functions-proto-keys.spec.tsx index 03831d0e7..e6c91e0c8 100644 --- a/packages/web/test/server/server-functions-proto-keys.spec.tsx +++ b/packages/web/test/server/server-functions-proto-keys.spec.tsx @@ -185,6 +185,27 @@ describe("decoded arguments cannot reach Object.prototype (#3202)", () => { ); }); + it("removes each dangerous own key rather than only neutralizing this merge", async () => { + for (const [name, jsonPayload, codecPayload] of PAYLOADS.slice(0, 3)) { + for (const road of ROADS) { + const { status, seen } = await decodeArgument( + road, + road.startsWith("codec") ? codecPayload : jsonPayload + ); + expect(status, `${name} / ${road}`).toBe(200); + expect(Object.prototype.hasOwnProperty.call(seen, name), `${name} / ${road}`).toBe(false); + } + } + }); + + it("refuses a non-configurable dangerous key rather than passing it through", async () => { + const payload = Object.freeze({ prototypeKey: { polluted: true }, n: 1 }); + const { status, seen } = await decodeArgument("codec-body", payload); + + expect(status).toBe(400); + expect(seen).toBeUndefined(); + }); + it("a non-plain carrier does not shelter the payload underneath it (#3200)", async () => { // The reachable shape is not an unsafe key ON a carrier — seroval drops // that at encode — but a plain object one level UNDER one. The walk used diff --git a/packages/web/test/server/server-functions-result-descriptors.spec.tsx b/packages/web/test/server/server-functions-result-descriptors.spec.tsx index 31ed7cfd7..440b23ab1 100644 --- a/packages/web/test/server/server-functions-result-descriptors.spec.tsx +++ b/packages/web/test/server/server-functions-result-descriptors.spec.tsx @@ -191,6 +191,26 @@ describe("a frozen result is not a failed call (#3196)", () => { await expect(decode(response)).resolves.toBeDefined(); } ); + + test("a guarded rebuild preserves an authored own __proto__ property", async () => { + registerServerFunction("descriptors-own-proto", async () => { + const result: any = { receipt: Promise.resolve("R") }; + Object.defineProperty(result, "__proto__", { + value: "kept", + writable: false, + enumerable: true, + configurable: false + }); + return Object.freeze(result); + }); + + const response = await handleServerFunctionRequest(scriptedPost("descriptors-own-proto")); + const result = await decode(response); + + expect(response.status).toBe(200); + expect(Object.prototype.hasOwnProperty.call(result, "__proto__")).toBe(true); + expect(result["__proto__"]).toBe("kept"); + }); }); describe("a non-enumerable accessor is not serialized (#3198)", () => { @@ -260,4 +280,25 @@ describe("a non-enumerable accessor is not serialized (#3198)", () => { expect(response.status).toBe(200); expect(body.includes(SECRET)).toBe(onWire); }); + + test("a non-enumerable getter is not invoked while another slot is guarded", async () => { + let reads = 0; + registerServerFunction("nonenum-getter-not-read", async () => { + const result: any = { receipt: Promise.resolve("R") }; + Object.defineProperty(result, "hidden", { + get() { + reads++; + throw new Error("hidden getter must stay unread"); + }, + enumerable: false, + configurable: true + }); + return result; + }); + + const response = await handleServerFunctionRequest(scriptedPost("nonenum-getter-not-read")); + + expect(response.status).toBe(200); + expect(reads).toBe(0); + }); }); From afed2e02d43dd4c7ca680ecd1b175cf1d2ca5e8b Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Wed, 2 Sep 2026 00:33:48 -0700 Subject: [PATCH 14/14] fix(web): keep transport helpers internal Avoid exposing the shared framing-header set while preserving the audited server behavior. Co-authored-by: Cursor --- .changeset/server-function-guard-simplification.md | 2 +- packages/web/src/response.ts | 13 ------------- packages/web/src/server.ts | 2 +- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/.changeset/server-function-guard-simplification.md b/.changeset/server-function-guard-simplification.md index 112d6feb5..c079dce95 100644 --- a/.changeset/server-function-guard-simplification.md +++ b/.changeset/server-function-guard-simplification.md @@ -13,5 +13,5 @@ reaches the wire, the scheme floor asks the URL parser instead of a regex, and the event is awaited only when it is genuinely a promise. Runtime-composed SSR responses now discard stale body-framing headers, and -late streaming redirects emit client script only for relative or HTTP(S) +late streaming redirects emit a client script only for relative or HTTP(S) targets. diff --git a/packages/web/src/response.ts b/packages/web/src/response.ts index 8dd0f9c10..caec24f75 100644 --- a/packages/web/src/response.ts +++ b/packages/web/src/response.ts @@ -259,19 +259,6 @@ export const NULL_BODY_STATUSES: ReadonlySet = new Set([204, 205, 304]); * consumers without the client runtime (no-JS form posts, direct HTTP) * get real JSON, while integrations read `value` — no reparse. */ -/** - * Headers that describe how a body is framed on the wire. Whenever the - * transport composes a body of its own, an author-supplied value describes - * the body it replaced — a stale `Content-Length` truncates the answer at the - * socket, and a stale `Content-Encoding` tells the peer to decompress bytes - * nobody compressed (#3197, RFC 9110 §8.6). - */ -export const COMPOSED_BODY_FRAMING: ReadonlySet = /*#__PURE__*/ new Set([ - "content-length", - "content-encoding", - "transfer-encoding" -]); - export function respond(value: T, init: ResponseHelperInit = {}) { const { responseInit, headers } = initWithRevalidate(init); // A null-body status cannot carry the passthrough JSON body — building it diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index 8cdcbe59d..086808164 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -21,7 +21,7 @@ import { // Wire-protocol header names for the commit fold's gap-fill denylist // (`commitEventResponse`): shared constants, not copies, so the fold can // never drift from what the server-function handler actually sends. -import { COMPOSED_BODY_FRAMING, REVALIDATE_HEADER } from "./response.js"; +import { REVALIDATE_HEADER } from "./response.js"; import { BODY_FORMAT_HEADER, ERROR_HEADER,