diff --git a/.changeset/remove-capturing-event-listeners.md b/.changeset/remove-capturing-event-listeners.md new file mode 100644 index 000000000..93f5d0c9e --- /dev/null +++ b/.changeset/remove-capturing-event-listeners.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Remove replaced non-delegated event listener objects with their original capture option. diff --git a/.changeset/server-function-guard-simplification.md b/.changeset/server-function-guard-simplification.md new file mode 100644 index 000000000..c079dce95 --- /dev/null +++ b/.changeset/server-function-guard-simplification.md @@ -0,0 +1,17 @@ +--- +"@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. + +Runtime-composed SSR responses now discard stale body-framing headers, and +late streaming redirects emit a client script only for relative or HTTP(S) +targets. diff --git a/.changeset/support-delegated-event-listener-objects.md b/.changeset/support-delegated-event-listener-objects.md new file mode 100644 index 000000000..3d62e6a99 --- /dev/null +++ b/.changeset/support-delegated-event-listener-objects.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Dispatch delegated events to EventListenerObject handlers through their handleEvent method, including after replacing a bound tuple. diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 2787988f7..76c52adff 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -19,6 +19,7 @@ import { 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 { @@ -1095,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 — @@ -1110,29 +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. */ -function stripOwnProtoKeys(value) { +const UNSAFE_ARGUMENT_KEYS = ["__proto__", "constructor", "prototype"]; + +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 { - const proto = Object.getPrototypeOf(v); - if (proto !== Object.prototype && proto !== null) continue; - if (Object.prototype.hasOwnProperty.call(v, "__proto__")) { - delete v["__proto__"]; - } - for (const key of Object.keys(v)) stack.push(v[key]); } } return value; @@ -1195,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); } @@ -1221,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- @@ -1427,7 +1427,7 @@ 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); + 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); @@ -1500,12 +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) { - // 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]); -} +// 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 @@ -1542,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, @@ -1719,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: @@ -1822,13 +1818,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,9 +2024,20 @@ 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); + // The shell is scratch the codec reads once and the author never holds, so + // 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)) { + descriptors[key].configurable = true; + if ("value" in descriptors[key]) descriptors[key].writable = true; + } 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. */ @@ -2510,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`, @@ -2784,14 +2802,35 @@ 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: {} }; + const promised = nativePromise(event); + if (promised) event = await promised; + } catch (error) { + const safe = sanitizeServerError(error); + const message = safe instanceof Error ? safe.message : String(safe); + const headers = new 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 // refusals below included (#3159). A refusal that returned directly // dropped the stub silently: an integration's Set-Cookie written in diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index 65ac2e5e7..9b885fb42 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -618,10 +618,14 @@ export function addEvent( export function addEvent(node, name, handler, delegate) { if (delegate) { + const key = `$$${name}`; if (Array.isArray(handler)) { - node[`$$${name}`] = handler[0]; - node[`$$${name}Data`] = handler[1]; - } else node[`$$${name}`] = handler; + node[key] = handler[0]; + node[`${key}Data`] = handler[1]; + } else { + node[key] = handler; + node[`${key}Data`] = undefined; + } } else if (Array.isArray(handler)) { const handlerFn = handler[0]; const listener = e => handlerFn.call(node, handler[1], e); @@ -2045,7 +2049,7 @@ function assignProp(node, prop, value, prev, skipRef, nodeName) { // 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); + node.removeEventListener(name, h, typeof h !== "function" && h); } if (delegate || value) { const attached = addEvent(node, name, value, delegate); @@ -2126,7 +2130,11 @@ function eventHandler(e, container, state) { } if (handler && !node.disabled) { const data = node[`${key}Data`]; - data !== undefined ? handler.call(node, data, e) : handler.call(node, e); + data !== undefined + ? handler.call(node, data, e) + : typeof handler === "function" + ? handler.call(node, e) + : handler.handleEvent(e); if (e.cancelBubble) return; } node.host && 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 535f65660..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. @@ -264,6 +266,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's framing headers describe something + // else — and on a null-body status there is no body at all (#3197). + 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 37dbbcee9..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, @@ -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 they can only describe a different + // one (#3197) + ...COMPOSED_BODY_FRAMING ].map(header => header.toLowerCase()) ); @@ -4547,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 }; @@ -4554,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. */ @@ -4583,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 @@ -4667,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/capturing-event-listener.spec.ts b/packages/web/test/capturing-event-listener.spec.ts new file mode 100644 index 000000000..33b746215 --- /dev/null +++ b/packages/web/test/capturing-event-listener.spec.ts @@ -0,0 +1,25 @@ +/** + * @vitest-environment jsdom + */ +import { describe, expect, test, vi } from "vitest"; +import { assign } from "../src/client.js"; + +describe("capturing event listeners", () => { + test("removes the previous non-delegated EventListenerObject when props change", () => { + const previous = vi.fn(); + const current = vi.fn(); + const previousListener = { capture: true, handleEvent: previous }; + const currentListener = { capture: true, handleEvent: current }; + const parent = document.createElement("div"); + const child = document.createElement("span"); + const prevProps = {}; + parent.append(child); + + assign(parent, { onScroll: previousListener }, true, prevProps); + assign(parent, { onScroll: currentListener }, true, prevProps); + child.dispatchEvent(new Event("scroll", { bubbles: true })); + + expect(previous).not.toHaveBeenCalled(); + expect(current).toHaveBeenCalledOnce(); + }); +}); diff --git a/packages/web/test/delegated-event-listener-object.spec.ts b/packages/web/test/delegated-event-listener-object.spec.ts new file mode 100644 index 000000000..692febc8f --- /dev/null +++ b/packages/web/test/delegated-event-listener-object.spec.ts @@ -0,0 +1,51 @@ +/** + * @vitest-environment jsdom + */ +import { describe, expect, test, vi } from "vitest"; +import { addEvent, delegateEvents, render } from "../src/client.js"; + +describe("delegated EventListenerObject handlers", () => { + test("dispatches delegated events through handleEvent", () => { + const handleEvent = vi.fn(); + const listener = { handleEvent }; + const element = document.createElement("button"); + const container = document.createElement("div"); + document.body.append(container); + const dispose = render(() => element, container); + + try { + addEvent(element, "click", listener, true); + delegateEvents(["click"]); + element.click(); + + expect(handleEvent).toHaveBeenCalledOnce(); + expect(handleEvent.mock.instances[0]).toBe(listener); + expect(handleEvent.mock.calls[0][0]).toBeInstanceOf(MouseEvent); + } finally { + dispose(); + container.remove(); + } + }); + + test("replaces a bound tuple without retaining its data slot", () => { + const previous = vi.fn(); + const handleEvent = vi.fn(); + const element = document.createElement("button"); + const container = document.createElement("div"); + document.body.append(container); + const dispose = render(() => element, container); + + try { + addEvent(element, "click", [previous, "stale"] as any, true); + addEvent(element, "click", { handleEvent }, true); + delegateEvents(["click"]); + element.click(); + + expect(previous).not.toHaveBeenCalled(); + expect(handleEvent).toHaveBeenCalledOnce(); + } finally { + dispose(); + container.remove(); + } + }); +}); 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-content-length.spec.tsx b/packages/web/test/server/server-functions-content-length.spec.tsx new file mode 100644 index 000000000..f0d344966 --- /dev/null +++ b/packages/web/test/server/server-functions-content-length.spec.tsx @@ -0,0 +1,220 @@ +/** + * 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 encoding = response.headers.get("Content-Encoding"); + const bytes = response.body ? (await response.arrayBuffer()).byteLength : 0; + return { status: response.status, declared, encoding, 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: { 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 }) { + // 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)", () => { + 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!!"); + // 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" } }) + ); + } + + 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], + ["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], + // unscripted: the same producers, plain-HTTP road + [ + "unscripted respond() envelope + Content-Length", + () => call("cl-returned-envelope", { road: "plain" }), + 200 + ], + ["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, status] of cases) { + const r = await run(); + actual.push(row(label, r)); + want.push(expected(label, status, 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..2bc1160fa --- /dev/null +++ b/packages/web/test/server/server-functions-event-hook.spec.tsx @@ -0,0 +1,225 @@ +/** + * `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 { runInNewContext } from "node:vm"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + decodeResponse, + 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; +}; +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 { + 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 rejecting cross-realm createEvent", + () => ForeignPromise.reject(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((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); + }); +}); + +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 cross-realm Promise", + request => ForeignPromise.resolve(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..e6c91e0c8 --- /dev/null +++ b/packages/web/test/server/server-functions-proto-keys.spec.tsx @@ -0,0 +1,250 @@ +/** + * 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("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 + // 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) { + 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..31a08aaf3 --- /dev/null +++ b/packages/web/test/server/server-functions-redirect-scheme.spec.tsx @@ -0,0 +1,223 @@ +/** + * 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++}`; + let ran = 0; + registerServerFunction(id, async () => { + ran++; + // `Headers` refuses CR/LF in a value outright, so those variants never + // 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(); + 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 }); + } + }); + 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 { ran, shipped: null as string | 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 { 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 { ran, 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}: ran=${r.ran} ${ + 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: ran=1 ${ + decodeRedirectHeaderValue(`302 ${target}`) === undefined + ? "http(s)-or-refused" + : "DECODED a non-http(s) target" + }` + ); + } + } + // `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 () => { + 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..440b23ab1 --- /dev/null +++ b/packages/web/test/server/server-functions-result-descriptors.spec.tsx @@ -0,0 +1,304 @@ +/** + * 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(); + } + ); + + 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)", () => { + 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); + }); + + 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); + }); +}); diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index f3a6a7667..d6b28a674 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -42,6 +42,11 @@ const framesEsbuildConfig = config => ({ // delta is the separately reviewed server-function transport hardening. // Limits below are the measured Linux CI/local artifacts rounded up to the // next 0.01 kB; this is a ratchet reconciliation, not additional headroom. +// +// Delegated EventListenerObject parity (#3206): scenarios retaining the +// delegated dispatcher pay for object-form invocation and for clearing a +// replaced bound tuple's data slot. Linux CI/local measurements are rounded +// to the next 0.01 kB at the affected limits below. module.exports = [ { name: "signals: core floor (createSignal/Memo/Effect/Root/flush)", @@ -343,7 +348,7 @@ module.exports = [ // 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", + limit: "17.70 KB", modifyEsbuildConfig }, { @@ -414,7 +419,7 @@ module.exports = [ // retains every store family, so it pays the whole module. Ruled // correctness-over-size in the #3164 thread; conscious bump. path: "hydrating-store-app.js", - limit: "26.94 KB", + limit: "27.00 KB", modifyEsbuildConfig }, { @@ -444,7 +449,7 @@ module.exports = [ // scheduler-resident ledger (nothing to shake), so it pays only the // hook call site's second argument plus brotli layout drift. path: "csr-app.js", - limit: "13.09 KB", + limit: "13.11 KB", modifyEsbuildConfig }, { @@ -494,7 +499,7 @@ module.exports = [ // Fold relocation pass (2026-09-01): 16.94 -> 16.91 KB, measured at // 16.90 — retained-ledger shake, same as the hydrating no-store note. path: "csr-app-patch-lists.js", - limit: "17.04 KB", + limit: "17.07 KB", modifyEsbuildConfig }, {