diff --git a/.changeset/bound-body-by-bytes-received.md b/.changeset/bound-body-by-bytes-received.md new file mode 100644 index 000000000..5069070fb --- /dev/null +++ b/.changeset/bound-body-by-bytes-received.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Measure `bodySizeLimit` on the bytes a server-function call actually sends, and settle an aborted upload whichever way its length was framed. Dispatch decided how to pay for the body from the declaration alone — `if (!(declared > 0))` — so any positive digit string skipped the counting read: `Content-Length: 10` on a 2 MiB body dispatched the whole 2 MiB past a 1 MiB cap, believing the same header, from the same untrusted producer, whose `-1` #3153 had already established must not be believed. That gate also carried the upload lifecycle, so the signal/reader coupling of #3218 was installed only for bodies that declared no length: an ordinary browser POST abandoned mid-upload never settled its handler and never cancelled its upload source (#3217/#3219). A declaration is now only ever grounds for refusing a payload the peer has itself announced as oversized, before a byte is read; every body a capped route accepts goes through the bounded read. diff --git a/.changeset/bound-the-no-js-flash-cookie.md b/.changeset/bound-the-no-js-flash-cookie.md new file mode 100644 index 000000000..e6837f251 --- /dev/null +++ b/.changeset/bound-the-no-js-flash-cookie.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Close the no-JS flash cookie's four unbounded edges. The cookie is now named `__Host-flash` and written with `SameSite=Lax` and a 60-second lifetime through a single writer shared with `clearFlashCookie`, so a sibling subdomain cannot toss an outcome at the app, a cross-site post's outcome is never stored, an unread submission no longer rides every request to the origin for the life of the browser, and the clear is a cookie the browser accepts for that name. A falsy outcome is an outcome: the decode no longer discards a well-formed cookie whose result is `""`/`0`/`false`/`null` (or a thrown `Error("")`, flags and all), and the handler flashes a function that simply returns. The size ladder now bounds the call's `url` as its last rung, so a form action carrying long state can no longer produce a cookie past the browser's ceiling — discarded whole, silently, after the mutation committed — while claiming it degraded to fit. The flash decode applies the same prototype-key strip as the argument decode, and refuses a payload whose `url` is not a string rather than handing the render one. Finally, a browser form navigation refused BEFORE dispatch — a stale id after a deploy, a malformed multipart body, an upload past `bodySizeLimit`, an origin the gate cannot vouch for — is bounced back to the form with the refusal flashed instead of being stranded on `/_server/`; nothing has committed at those exits, and every other caller keeps the status it always got. diff --git a/.changeset/contain-and-own-the-single-flight-fold.md b/.changeset/contain-and-own-the-single-flight-fold.md new file mode 100644 index 000000000..13b1ff793 --- /dev/null +++ b/.changeset/contain-and-own-the-single-flight-fold.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Hold the fold to its own containment rule, and to the response it hands back. A collector that returned a value the wire cannot carry escaped the try/catch that covers a collector which throws, because the slice is encoded later — destroying the committed mutation's own result and every other cache's slice, and answering `200` with no error header, so the caller retries a write that already happened. The fold also appended this request's `Set-Cookie` and gap-filled headers into whatever `transformFlightResult` returned without owning it first, though the thrown tail already owns it for exactly the reason named in its comment; an integration that memoizes its shell served one caller's session and redirect target to the next. The caller-supplied source list is now read as the set the protocol defines rather than a multiset, so a repeated id can no longer buy a revalidation pass per repetition, and a redirect's own `Location` is the flight target whether or not the request carried a `Referer` — `Referrer-Policy: no-referrer` no longer switches single-flight off for every redirecting mutation. diff --git a/.changeset/end-the-call-the-transport-opened.md b/.changeset/end-the-call-the-transport-opened.md new file mode 100644 index 000000000..1ca2a4079 --- /dev/null +++ b/.changeset/end-the-call-the-transport-opened.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Let the client transport finish a call it started, and read the body once. Each call minted an `AbortController` that was wired only into the async-iterator wrapper, so an ordinary result never fired it and never cancelled the reader — while `ChunkReader` held the body lock, leaving the application unable to reclaim it either; a peer that answers correctly and then holds the body open wedges six calls' worth of an origin's HTTP/1.1 budget while every call reports success. Cancelling one branch of a tee does not cancel the fetch, so the teardown only became possible once the transport stopped cloning: `extractBody` now consumes the response it is given. Separately, both "did the runtime write this answer?" guards tested the format header's presence while the decoder matches it by value, so a duplicated header or a tag from a newer build resolved any status — `500` included — as a void success; the guards now derive their predicate from `BodyFormat` itself. diff --git a/.changeset/hold-the-invocation-seams.md b/.changeset/hold-the-invocation-seams.md new file mode 100644 index 000000000..35c7f6fd4 --- /dev/null +++ b/.changeset/hold-the-invocation-seams.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Close the roads that reached a server function body around the hooks an application hangs its policy on. A `GET()` grant was stored as a bare word against an id rather than against the function it was granted to, so a rebind in one interleaving handed GET — and with it the origin-gate exemption — to a function that never declared it, and `withMeta` could report a revocation the wire had not performed. A `wrapInvocation` option of `null` — the other spelling of absent, and the one `provideEvent` already treats as absent — removed the configured gate instead of falling back to it; a per-handler wrap never reached the calls a dispatched body made in process; `provideEvent`'s exactly-once contract was enforced on the HTTP tail and nowhere else, so a defective adapter was a clean 500 on the wire and a silent double-commit on the direct road; and `new fn()` entered the body past the apply trap entirely, with no request scope and no guard. `transformResult` now runs for a plain thrown value as its own documentation promises, so an error-mapping or audit policy sees the failures that happen to an application and not only the ones its author shaped by hand. diff --git a/.changeset/keep-negative-zero-off-the-fast-path.md b/.changeset/keep-negative-zero-off-the-fast-path.md new file mode 100644 index 000000000..e84c7de21 --- /dev/null +++ b/.changeset/keep-negative-zero-off-the-fast-path.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Refuse `-0` on the JSON fast path. `isJSONSafe` turns away every number `JSON.stringify` cannot spell faithfully except this one: `JSON.stringify(-0)` is `"0"`, so a result of `-0` arrived as `+0` while the codec, which encodes it correctly, was never reached. The same quiet corruption the neighbouring branches already refuse. diff --git a/.changeset/own-decoded-promises.md b/.changeset/own-decoded-promises.md new file mode 100644 index 000000000..3fa0ee85e --- /dev/null +++ b/.changeset/own-decoded-promises.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Take ownership of every promise the decoder mints from a peer's bytes. The encode side has always kept a fallback owner on the promises it creates (`guardFailures`); the decode side owned none of them, and seroval's atomic promise node settles synchronously inside `fromCrossJSON`, so `createJSONDeserializer.abort`'s sweep — which already defused the constructor spelling — never saw it. A 115-byte argument body encoding a rejected promise therefore answered `200`, ran the function, and then ended the process on Node's default unhandled-rejection policy, from any client that can send a raw request. Ownership is now taken where promises are minted, which covers both spellings because both park the bare promise in the deserializer's `refs` map; `abort`'s half-guard is deleted as the second half of the same guard rather than kept as a second one. diff --git a/.changeset/scope-deferred-bodies-in-containers.md b/.changeset/scope-deferred-bodies-in-containers.md new file mode 100644 index 000000000..0d6fb19f3 --- /dev/null +++ b/.changeset/scope-deferred-bodies-in-containers.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Scope a deferred body wherever it sits in a directly-called result, not only at the top level. `scopeDeferredResult` looked at the returned value itself, so `return { rows: cursor() }` from an SSR-time call ran its generator under the render's ambient event: two concurrent direct calls read and wrote each other's `locals`, and the render's own, which is the failure the per-call `locals` copy exists to prevent. The descent now covers containers, so a generator or stream one level down carries the call's own request event like a top-level one. diff --git a/.changeset/strip-at-the-decode-boundary.md b/.changeset/strip-at-the-decode-boundary.md new file mode 100644 index 000000000..34d102e77 --- /dev/null +++ b/.changeset/strip-at-the-decode-boundary.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Apply the prototype-key strip at the decode boundary instead of at one of its callers. `__proto__` / `constructor` / `prototype` were removed from decoded POST arguments by a strip attached to the argument path in the server, so every other graph the same decoder produced — above all every response the client decodes, and the no-JS flash cookie — handed the key through as an own property. No hostile server is required: a function returning `JSON.parse` of stored user text emits it through the JSON fast path, and any recursive merge on the client then writes through to `Object.prototype`. The guard now lives in `extractBody`, the boundary all three roads share, so the argument leg's existing guarantee and the two that were missing are one implementation rather than copies to keep in sync. diff --git a/.changeset/widen-the-result-guard-walk.md b/.changeset/widen-the-result-guard-walk.md new file mode 100644 index 000000000..8cb96fa83 --- /dev/null +++ b/.changeset/widen-the-result-guard-walk.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Let the result guard see the slots a value actually holds. Reachability on the result path was decided against a narrower idea of "holds" than the codec and the consumer use — plain prototype only, enumerable keys only — so a failure channel parked just outside it was sanitized by nobody while the identical channel one step inside was fully guarded. A rejecting promise under an `Object.assign(new Error(...), …)` carrier shipped its raw message and own properties to the client under a `200`; one on a non-enumerable own data property was never owned and ended the process behind a delivered response, and a stream there was never torn down. The walk now reaches both without invoking hidden accessors, which is the hazard the narrowing was introduced to remove. diff --git a/packages/web/serialization/src/serializer-decode.ts b/packages/web/serialization/src/serializer-decode.ts index e369f2fb0..7b1eae4e6 100644 --- a/packages/web/serialization/src/serializer-decode.ts +++ b/packages/web/serialization/src/serializer-decode.ts @@ -300,38 +300,112 @@ export function createJSONDeserializer(options?: JSONCodecOptions): (node: Se * chunks resolve through a map shared across calls, so all chunks from one * stream must go through the same deserializer instance. */ +/** + * Whether a ref in a deserializer's shared map is a value only a LATER chunk + * can settle. That map holds seroval's in-progress state between chunks: an + * open stream under `__SEROVAL_STREAM__`, and a pending promise as the + * `{p, s, f}` resolver triple (the promise under one id, its resolver under + * the special-reference id next to it). One predicate for both readers of + * that state — the sweep that settles them and the check that asks whether + * any exist — so "still waiting" is one definition, not two. + */ +function awaitsLaterChunk(value) { + if (value === null || typeof value !== "object") return false; + return ( + !!value.__SEROVAL_STREAM__ || + (typeof value.s === "function" && typeof value.f === "function" && value.p instanceof Promise) + ); +} + export function createJSONDeserializer(options) { const refs = new Map(); const resolved = resolveCodecOptions(options); + // How many `refs` entries `ownDecodedPromises` has already claimed. + // seroval never reassigns a ref id (a second write throws "Conflicted ref + // id"), so the map only ever grows and iterates in insertion order: + // skipping the entries already claimed makes the sweep amortized O(1) per + // decoded node rather than O(refs) on every chunk of a long stream. + let owned = 0; + /** + * Takes ownership of every promise this decoder has just minted. + * + * A decoded payload is a PEER's bytes, and a promise it decodes to is a + * rejection nobody is holding: the value goes on to be a server function + * argument, or a slot in a decoded result, and ordinary code does not + * await a slot it never expected to be a promise. Under Node's default + * policy that ends the process — so the decoder keeps a fallback owner on + * what it mints, exactly as the encode side does for the promises IT + * mints (`guardFailures`, server.js: "Keep a fallback owner on the + * promise WE minted"). This changes nothing for a real consumer: `p` + * still rejects for whoever awaits it; only the "nobody at all" case is + * covered. + * + * It runs at the MINT, not at `abort`, because seroval has two promise + * spellings and only one of them is still pending when a stream ends. The + * constructor pair (`{p, s, f}` under the special-reference id, the bare + * promise under its own) settles from a later chunk, so `abort` can still + * reach it. The ATOMIC promise node (seroval type 12) settles + * SYNCHRONOUSLY inside the `fromCrossJSON` call that reads it — by the + * time any later hook runs, the microtask queue has drained and Node has + * already reported the rejection. Both spellings put the bare promise in + * `refs`, so claiming promises here covers the pair as well and there is + * one guard rather than one per spelling. + */ + function ownDecodedPromises() { + if (refs.size === owned) return; + let index = 0; + for (const value of refs.values()) { + if (index++ < owned) continue; + if (value instanceof Promise) value.then(undefined, () => {}); + } + owned = refs.size; + } function deserializeJSONChunk(node) { - return fromCrossJSON(node, { refs, ...resolved }); + try { + return fromCrossJSON(node, { refs, ...resolved }); + } finally { + // `finally`: a chunk that throws part-way through (a malformed node, a + // depth-limit refusal) has already minted whatever it minted. + ownDecodedPromises(); + } } /** - * Fails every value still waiting on chunks that will never arrive. The - * shared refs map holds seroval's in-progress state between chunks: open - * streams (`__SEROVAL_STREAM__`) and pending-promise resolvers (`{p, s, f}` - * — the promise under one id, its resolver under the special-reference id - * next to it). Both settle idempotently — throwing into a completed stream - * and rejecting a resolved promise are no-ops — so the sweep is safe to - * run on normal end-of-stream too. The defusing handler on `p` keeps a - * rejection nobody awaited (a pending promise the app never touched) from - * surfacing as an unhandled rejection. + * Fails every value still waiting on chunks that will never arrive. Both + * kinds settle idempotently — throwing into a completed stream and + * rejecting a resolved promise are no-ops — so the sweep is safe to run on + * normal end-of-stream too. Settling is all this does: the rejection it + * induces already has a fallback owner, put there by + * `ownDecodedPromises` when the chunk that minted the promise was read. */ deserializeJSONChunk.abort = function abort(error) { for (const value of refs.values()) { - if (value === null || typeof value !== "object") continue; + if (!awaitsLaterChunk(value)) continue; if (value.__SEROVAL_STREAM__) { value.throw(error); - } else if ( - typeof value.s === "function" && - typeof value.f === "function" && - value.p instanceof Promise - ) { - value.p.then(undefined, () => {}); + } else { value.f(error); } } }; + /** + * Whether anything decoded so far can still be changed by a later chunk — + * the question a reader asks before it stops reading. Answered from the + * same refs the sweep above settles, so the two can never disagree about + * what "still waiting" means. + * + * Conservative by construction: a resolver stays in the map after it + * settles, so a stream or promise that is already done still answers + * true. That is the safe direction. Saying "nothing is waiting" is a + * licence to stop reading, and stopping early would strand a value that + * was still on its way; saying it late only means the reader waits for + * the peer's own end of body, which is what every reader did before. + */ + deserializeJSONChunk.pending = function pending() { + for (const value of refs.values()) { + if (awaitsLaterChunk(value)) return true; + } + return false; + }; return deserializeJSONChunk; } export function createJSONDataTable(options?: JSONCodecOptions): JSONDataTable; diff --git a/packages/web/serialization/src/serializer.ts b/packages/web/serialization/src/serializer.ts index d3258c6b5..7308f1d29 100644 --- a/packages/web/serialization/src/serializer.ts +++ b/packages/web/serialization/src/serializer.ts @@ -4,6 +4,7 @@ import { Feature, Serializer, getCrossReferenceHeader, + toCrossJSON, toCrossJSONStream, createPlugin as createPluginImpl, OpaqueReference as OpaqueReferenceImpl @@ -229,6 +230,45 @@ export function getLocalHeaderScript(id) { */ export function serializeJSON(value: unknown, options: JSONSerializeOptions): () => void; +/** + * Whether the codec has a node for `value` — the encoder's own answer, + * asked before anything is committed to a stream. + * + * `serializeJSON` reports an unsupported type through `onError`, which for + * a transport arrives after the head is on the wire: too late to answer + * differently. A caller that can still choose (single-flight's fold drops + * a slice the wire cannot carry rather than losing the whole envelope with + * it) asks here instead of re-deriving the supported set, which plugins + * make unknowable from outside anyway. + * + * Asked through the SYNCHRONOUS parser, so an async channel — a promise, + * stream or async iterable — parses to a node without being drained and + * the real encode still has it. Enumerable accessors are read, exactly as + * the encode reads them; a caller that must not mint values early walks + * its own containers and asks only about leaves. + * @internal + */ +export function canSerializeJSON(value: unknown, options?: JSONCodecOptions): boolean; + +/** + * Whether the codec has a node for `value`, answered by the synchronous + * parser under the same plugins and feature policy `serializeJSON` uses. + */ +export function canSerializeJSON(value, options) { + const resolved = resolveCodecOptions(options); + try { + toCrossJSON(value, { + refs: new Map(), + ...resolved, + disabledFeatures: + resolved.disabledFeatures | serializeOnlyDisabledFeatures(resolved.serializeErrorStacks) + }); + return true; + } catch { + return false; + } +} + // ---- JSON codec (server function transports) ---- // // Unlike hydration output (executable JS targeting a global), the JSON codec diff --git a/packages/web/server-functions/src/client.ts b/packages/web/server-functions/src/client.ts index 876c09f3e..c74746867 100644 --- a/packages/web/server-functions/src/client.ts +++ b/packages/web/server-functions/src/client.ts @@ -21,17 +21,20 @@ import { UNKNOWN_HEADER, configureServerFunctionsCodec, decodeResponse, + declareMeta, + extractBody, getFlightDataConsumer, getFlightDataSourceIds, getHeadersAndBody, getServerFunctionMetadata, + getServerFunctionsCodec, + hasReadableBodyFormat, isJSONSafe, isServerFunction, parseServerFunctionAddress, provideServerFunctionRPC, serverFunctionAddress, - serverFunctionDataAddress, - withMeta + serverFunctionDataAddress } from "./shared.js"; // The flash cookie's name, detection and clearing are re-exported here so @@ -677,13 +680,16 @@ async function fetchServerFunction(base, id, options, args, meta, callArgs = arg if (handled !== undefined) return handled; } - // Every response the runtime encodes carries the body format — a void one - // and a thrown one included — so at 400 and up its absence means the peer - // refused. Answered before the passthrough beneath, because a refusal can - // carry a `Location` of its own and the passthrough would hand it back as - // control flow; and undecoded, because its body is someone else's, not a - // payload for the caller. - if (response.status >= 400 && !response.headers.has(BODY_FORMAT_HEADER)) { + // Every response the runtime encodes carries a body format this build can + // read — a void one and a thrown one included — so at 400 and up anything + // else means the peer refused. READ, not merely present: a tag with no + // case behind it is no evidence the runtime wrote the answer, and it + // decodes to `undefined`, so honouring its presence turned a 500 into a + // silent void success (see `hasReadableBodyFormat`). Answered before the + // passthrough beneath, because a refusal can carry a `Location` of its own + // and the passthrough would hand it back as control flow; and undecoded, + // because its body is someone else's, not a payload for the caller. + if (response.status >= 400 && !hasReadableBodyFormat(response)) { throw serverFunctionFailure(response, undefined); } @@ -762,7 +768,7 @@ async function fetchServerFunction(base, id, options, args, meta, callArgs = arg // a conditional read, not a payload, and decodes to nothing at any peer. if ( response.status < 300 && - !response.headers.has(BODY_FORMAT_HEADER) && + !hasReadableBodyFormat(response) && !response.headers.has("X-Content-Raw") ) { throw serverFunctionFailure( @@ -778,7 +784,14 @@ async function fetchServerFunction(base, id, options, args, meta, callArgs = arg ); } - const result = await decodeResponse(response.clone()); + // Decoded from the response ITSELF, not a clone. The transport owns this + // body — every road that hands it to somebody else has already returned + // above — so a clone would only tee it into a branch nobody reads, which + // queues the whole payload for the life of the read. Owning it is also + // what lets the decoder END the call: cancelling one branch of a tee + // leaves the fetch running, cancelling the real body does not. + // `decodeResponse` keeps its clone for integrations, who still own theirs. + const result = response.body ? await extractBody(response, getServerFunctionsCodec()) : undefined; if (failed) { throw serverFunctionFailure(response, result); } @@ -791,7 +804,24 @@ async function fetchServerFunction(base, id, options, args, meta, callArgs = arg [Symbol.asyncIterator]() { const it = result[Symbol.asyncIterator](); return { - next: () => it.next(), + // Both ways a consumer can be done with the stream end the CALL. + // `for await` invokes `return()` on an early exit but not on a + // natural end, so the teardown that already existed on the + // abandoned leg is mirrored onto the finished one: a stream + // drained to its last item has strictly less left to say than one + // that was walked away from, and it must not be the leg that keeps + // the connection. `done` is decided by the payload's own closing + // frame, so this fires whether or not the peer ends the body. + next: async () => { + try { + const step = await it.next(); + if (step.done) controller.abort(); + return step; + } catch (error) { + controller.abort(); + throw error; + } + }, return: value => (controller.abort(), Promise.resolve({ done: true, value })) }; } @@ -988,8 +1018,11 @@ export function GET(fn) { get: () => serverFunctionAddress(config.endpoint, id), configurable: true }); - // the declaration itself is a metadata write like any other - return withMeta(wrapped, { method: "GET" }); + // the declaration records itself on the metadata channel — through the + // declaration writer, not `withMeta`: the GET wire is baked into `run` + // above, so a metadata write is a REPORT of this declaration and cannot + // be a way to make (or unmake) one + return declareMeta(wrapped, { method: "GET" }); } /** * A live reference: calling it opens an iteration and hands back the * reconnecting iterable ITSELF, synchronously — not a promise of one (the diff --git a/packages/web/server-functions/src/flash.ts b/packages/web/server-functions/src/flash.ts index 2dae31f3a..d9795153a 100644 --- a/packages/web/server-functions/src/flash.ts +++ b/packages/web/server-functions/src/flash.ts @@ -17,7 +17,8 @@ // The payload is plain JSON rather than the wire codec: it has to survive a // 4 KB cookie, and both halves here are synchronous while the codec is not. -import { FLASH_COOKIE, parseCookieHeader, serializeCookie } from "../../src/cookies.js"; +import { FLASH_COOKIE, parseCookieHeader, writeFlashCookie } from "../../src/cookies.js"; +import { stripUnsafeKeys } from "./shared.js"; /** * The outcome of a call made without the client runtime, as it rides the @@ -108,27 +109,46 @@ export function encodeFlashCookie(url, result, input, thrown) { // second write. Ladder: drop the input echo (usually the bulk), then // bound the value itself — a string keeps the longest prefix that fits // (halving, because percent-encoding inflates unevenly), anything - // structured has no partial JSON and reduces to the outcome flag `true`. - // `url` and the error/thrown flags always survive: what happened, and to - // which submission, is the part that must not be lost. + // structured has no partial JSON and reduces to the outcome flag `true`, + // and only then is `url` itself bounded. The error/thrown flags always + // survive: THAT it happened is the part that must not be lost. payload.truncated = true; payload.input = []; if (!fitsCookie(payload)) { if (typeof payload.result === "string") { - let prefix = payload.result; - while (prefix.length > 0 && !fitsCookie({ ...payload, result: prefix })) { - prefix = prefix.slice(0, prefix.length >> 1); - } + const prefix = boundedPrefix(payload, "result"); payload.result = prefix.length > 0 ? prefix : true; } else { payload.result = true; } } + // Last rung, and the one the ladder above forgot it needed: `url` is + // `pathname + search` of a request the CALLER chose, and a form whose + // action carries state (`
0 && !fitsCookie({ ...payload, [field]: prefix })) { + prefix = prefix.slice(0, prefix.length >> 1); + } + return prefix; +} + function flashCookie(payload) { - return serializeCookie(FLASH_COOKIE, JSON.stringify(payload), { secure: true, httpOnly: true }); + return writeFlashCookie(JSON.stringify(payload)); } // The browser ceiling is 4096 bytes of `name=value` (RFC 6265bis §5.6); @@ -159,7 +179,24 @@ export function decodeFlashCookie(cookieHeader) { if (!match) return; try { const payload = JSON.parse(match); - if (!payload || !payload.result) return; + // What makes a payload READABLE is the field the ladder promises always + // survives, not the optional one: a truthiness test on `result` + // discarded a well-formed cookie whose outcome was `""`, `0`, `false` + // or `null` — and a thrown `Error("")` with it, flags and all. The + // encoder wrote that cookie, the browser stored it, and the render + // showed nothing: the same "nothing happened" the size ladder degrades + // to avoid, on the commonest results there are. `url` is also what + // every integration reads to decide whether the outcome belongs to the + // page it is rendering (`url.startsWith("/")`), so a payload carrying + // anything else there is malformed — caught here, where a malformed + // cookie is answered with undefined, rather than in the render this + // module promises it can never take down. + if (!payload || typeof payload !== "object" || typeof payload.url !== "string") return; + // Same guard the argument road applies, on the same grounds: what a + // decoded value meets downstream is a merge, and `JSON.parse` makes + // `__proto__` an own property (#3168/#3202). Reaching this road costs a + // cookie on the origin — narrower than the argument road, not nil. + stripUnsafeKeys(payload); const result = payload.error ? new Error(payload.result) : payload.result; const submission = { input: Array.isArray(payload.input) ? payload.input.map(decodeInputValue) : [], diff --git a/packages/web/server-functions/src/registry.ts b/packages/web/server-functions/src/registry.ts index a02be61c3..a4e3ae973 100644 --- a/packages/web/server-functions/src/registry.ts +++ b/packages/web/server-functions/src/registry.ts @@ -49,7 +49,9 @@ export function isServerFunction(fn) { * (client proxy or server-registered callable) and returns the reference. * Writes ride the same channel `GET` uses: later writes shallow-merge over * earlier ones, and `getServerFunctionMetadata(fn)` reads the merged bag — - * so `withMeta` composes with `GET` in either order. + * so `withMeta` composes with `GET` in either order. Metadata only, never + * behavior: `method` is a declaration, not a user write, and is refused + * here (see below). * * The pattern is declare-on-function, react-in-hook: metadata declared * here is what `prepareRequest` receives as `context.meta`, letting @@ -80,10 +82,35 @@ export function withMeta(fn, meta) { if (!metadata) { throw new Error("withMeta expects a server function reference"); } + // `method` is a DECLARATION, and a declaration is more than its metadata: + // `GET(fn)` also records the grant the server's dispatch reads (which is + // what skips the CSRF origin gate — #3114), and the client's `GET` + // reference bakes the query encoding into its own call. A write here + // reaches neither, so it could only ever REPORT a grant, or a revocation, + // that the wire never performed — `getServerFunctionMetadata(fn).method` + // reading "POST" while a cross-site GET still executes the function. + // Refused for the reason `invoke` refuses it, in the same words. + if (meta && "method" in meta) { + throw new Error("`method` is not user-written metadata. " + INVOKE_OPTION_REDIRECTS.method); + } Object.assign(metadata, meta); return fn; } +/** + * The declaration channel's own write: `GET` (and any future + * declaration-static capability) records what it declared on the metadata + * bag, alongside the behavior the declaration carries — the server's grant, + * the client's GET wire. Internal on purpose: `withMeta` is the user write, + * and a user write must not be able to forge a declaration, or revoke one, + * by writing its metadata alone. + * @internal + */ +export function declareMeta(fn, meta) { + Object.assign(getServerFunctionMetadata(fn), meta); + return fn; +} + // The invocation channel. References (and declaration wrappers — GET, live) // carry their per-call invoker under a registered symbol, and `invoke` // dispatches to it. Core's wrappers forward it mechanically because they diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index ff9cdf88f..46bb54ec3 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -47,9 +47,10 @@ import { parseServerFunctionAddress, provideServerFunctionRPC, serverFunctionAddress, + stripUnsafeKeys, createChunk, - encodeErrorTrailer, - withMeta + declareMeta, + encodeErrorTrailer } from "./shared.js"; export { @@ -439,16 +440,25 @@ export interface HandleServerFunctionOptions { /** * Overrides the configured per-invocation wrap for this handler — same * contract as the `wrapInvocation` config option (see - * `WrapInvocationHook`), except it only applies to HTTP dispatch (a - * per-request option can't see direct SSR calls). + * `WrapInvocationHook`). It owns every server-function body entered while + * handling this request: the call the wire addressed, and the calls that + * call makes in-process (`context.direct` is true for those). It cannot + * see a direct call made outside a request it owns — a call during a + * document render belongs to no handler invocation — which is what the + * configured hook is for. */ wrapInvocation?: WrapInvocationHook; /** * Observes or replaces the function's result before encoding — the * extension point for response metadata policies (headers, statuses, - * substituted results). Runs for returned and thrown results alike - * (`context.thrown` distinguishes); `context.instance` is null for no-JS - * calls. The context carries the call's identity — the function `id` and + * substituted results). Runs for returned and thrown results alike — + * every thrown value, a plain `Error` included (`context.thrown` + * distinguishes), so an error-mapping or audit policy sees the failures + * that happen to the app, not only the ones its author shaped by hand. + * Returning a `Response`/envelope for a thrown error maps it onto the + * wire; anything else stays a thrown value and is sanitized as usual + * (see `markSafeError`). `context.instance` is null for no-JS calls. The + * context carries the call's identity — the function `id` and * the parsed `args` the implementation was invoked with — matching the * direct-call mirror (`transformDirectResult`), so a policy keying state * by the call works over either dispatch path. Return the result @@ -659,12 +669,157 @@ function provideEvent(event, fn) { ); } +// The two ways a hand-written provideEvent breaks the call it is scoping, +// in the words both dispatch legs answer with. +const PROVIDE_EVENT_TWICE = + "provideEvent invoked the server function callback more than once: a second invocation " + + "would commit the call's side effects twice. The hook must call fn exactly once and return " + + "its result."; +const PROVIDE_EVENT_NEVER = + "provideEvent returned without invoking the server function callback: the call would have " + + "answered as a void success without running the function. The hook must call fn exactly once " + + "and return its result."; + +// provideEvent's contract — run the callback, once, with `event` visible to +// getRequestEvent() — enforced rather than assumed (#3172): every way a +// hand-written hook gets it wrong otherwise answers as an ordinary success. +// The hook is one object an adapter installs once, so a hook broken in +// either direction is broken for every call the process makes — which is +// why the guard belongs to the hook contract and not to one dispatch leg: +// HTTP dispatch and the direct SSR call both enter through here. +// +// A second invocation is refused BEFORE the body runs again (a retry +// wrapper or a misplaced await double-committed a mutation, silently), and +// the count is re-checked once the hook has returned, so swallowing the +// in-flight refusal cannot turn it back into a success. Zero invocations +// is the other violation: a void result a caller cannot tell from a +// function that returned nothing. +// +// The re-check waits for a promised result and is otherwise synchronous — +// that is what keeps the direct leg transparent (a synchronous function +// called during a render still returns its value, not a promise) while +// still catching a hook that only defers its invocation. +function provideEventOnce(provide, event, run) { + let invocations = 0; + const settle = () => { + if (invocations !== 1) + throw new Error(invocations === 0 ? PROVIDE_EVENT_NEVER : PROVIDE_EVENT_TWICE); + }; + const result = provide(event, () => { + // thrown synchronously, never as a rejected promise: the second + // execution must not START, and a hook that ignores the return must not + // mint an unobserved rejection + if (++invocations > 1) throw new Error(PROVIDE_EVENT_TWICE); + return run(); + }); + const promised = nativePromise(result); + if (promised) { + return promised.then(value => { + settle(); + return value; + }); + } + settle(); + return result; +} + // Calling a generator only allocates it; calling a stream's reader is what // runs its pull. A request scope around the function CALL therefore does not // own either body. Bind each deferred operation to the event explicitly so // direct SSR calls keep their per-call event after the proxy has returned. // Non-deferred values pass through by identity and synchronously. +// +// A body one CONTAINER down is driven by the consumer exactly the same way: +// `return { rows: cursor() }` is the shape the codec road's guard walk was +// taught to descend into for this very reason (#3125), and until this road +// descended it too, #3222's harm survived intact for it — the body ran under +// the RENDER's ambient event instead of the per-call copy #3156 made for it, +// so two concurrent direct calls read and wrote each other's request state, +// and the render's own locals were mutated by a call that should not have +// been able to reach them. +// +// The descent is IN PLACE. The caller holds the value the author returned, +// so only the deferred slots are swapped for their bound wrappers and the +// carrier keeps its identity and its shape — the same call the argument +// road's own walk makes (`stripUnsafeKeys`), for the same reason. +// It is iterative because a legal result nests arbitrarily deep and a +// recursive walk overflows on one (#3160). function scopeDeferredResult(value, scope) { + const bound = bindDeferredBody(value, scope); + return bound === value ? scopeDeferredContents(value, scope) : bound; +} + +/** The containers a consumer reaches a deferred body through — the same set + * the codec road's walk descends. */ +function isDeferredContainer(value) { + if ( + Array.isArray(value) || + value instanceof Map || + value instanceof Set || + value instanceof Error + ) { + return true; + } + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} + +function scopeDeferredContents(value, scope) { + if (value === null || typeof value !== "object" || !isDeferredContainer(value)) return value; + const stack = [value]; + const seen = new Set(); + while (stack.length) { + const node = stack.pop(); + if (node === null || typeof node !== "object" || seen.has(node) || !isDeferredContainer(node)) { + continue; + } + seen.add(node); + if (Array.isArray(node)) { + for (let i = 0; i < node.length; i++) { + const bound = bindDeferredBody(node[i], scope); + if (bound === node[i]) stack.push(bound); + else node[i] = bound; + } + } else if (node instanceof Map) { + // A Map VALUE is rebindable in place; a key is its own identity in the + // table, so replacing one would rehash the entry — keys are descended + // into, not rebound. + for (const [key, entry] of node) { + const bound = bindDeferredBody(entry, scope); + if (bound === entry) stack.push(entry); + else node.set(key, bound); + stack.push(key); + } + } else if (node instanceof Set) { + for (const member of node) stack.push(member); + } else { + const descriptors = Object.getOwnPropertyDescriptors(node); + for (const key of Object.keys(descriptors)) { + const descriptor = descriptors[key]; + // Own DATA properties, at any enumerability — what the consumer can + // read is not what a `for...in` would list. An ACCESSOR is left to + // the consumer to invoke: reading it here would run authored code + // for a value nobody has asked for yet. (The codec road invokes + // enumerable ones because the codec reads them itself; nothing reads + // them for the caller of a direct call but the caller.) + if (!("value" in descriptor)) continue; + const bound = bindDeferredBody(descriptor.value, scope); + if (bound === descriptor.value) stack.push(bound); + else if (descriptor.writable) node[key] = bound; + else if (descriptor.configurable) { + Object.defineProperty(node, key, { ...descriptor, value: bound }); + } + // A sealed, non-writable slot keeps the body the author put there: + // binding it needs a substitution the slot refuses, and rebuilding + // the carrier around it would hand the caller a value they never + // returned. Unbound is where this road already was. + } + } + } + return value; +} + +function bindDeferredBody(value, scope) { if (value === null || (typeof value !== "object" && typeof value !== "function")) return value; const promised = scope(() => nativePromise(value)); @@ -734,7 +889,11 @@ const REGISTRATIONS = new Map(); // Declared-method bookkeeping keyed by function id (internal, not public // API): the server half of `GET` records entries here so the HTTP handler // can gate GET dispatch — a GET request to a function that never declared -// it answers 405. Declaring GET grants GET without revoking POST. +// it answers 405. Declaring GET grants GET without revoking POST. The +// entry is the BINDING the grant was made to, not the bare word "GET": +// what a declaration asserts is safe is a function, and dispatch is +// reached by an id, so the grant carries the one thing that can tell them +// apart later (see `declaresRead` and `GET`). const METHODS = new Map(); // In-flight invocation state, keyed by the request event the call runs // under — the derived event a direct SSR call creates, or the handler's own @@ -743,6 +902,29 @@ const METHODS = new Map(); // (#3156) makes writes call-local, which is the wrong lifetime for state // the wrapper established before the copy existed. const INVOCATIONS = new WeakMap(); +// The wrap a request runs under, keyed by that request's event — the same +// keying and the same lifetime as INVOCATIONS, and for the same reason. +// `config.wrapInvocation` is ambient, so a direct in-process call finds it +// by itself; a per-handler `wrapInvocation` option is only in scope inside +// the handler, and a call the dispatched body makes must not be able to +// step outside the policy the request was given (see the apply trap). +const REQUEST_WRAPS = new WeakMap(); +// Which registered function a reference NAMES, so a declaration made about +// the reference can be recorded against the binding it was made about +// rather than against its id alone (see `GET`). +const REFERENCE_BINDINGS = new WeakMap(); + +// Whether the id's CURRENT binding is the function a `GET()` grant was made +// to — the question both dispatch gates actually ask. A grant made about a +// function must never govern another one, in either interleaving: the id +// rebound after the declaration (#3129, dropped eagerly at rebind below) or +// already rebound before it (the grant then names a binding this id does +// not have). Neither is a declared read, so such a call is gated exactly +// like a function that never declared GET — 405, origin gate on. +function declaresRead(id) { + const granted = METHODS.get(id); + return granted !== undefined && granted === REGISTRATIONS.get(id); +} // Server mirror of the client transport's late-bound RPC registration (see // client.js provideRPC and registry.js): the server half's `GET` records the @@ -927,17 +1109,25 @@ export function createServerReference({ id, fn, name }) { // this call's provideEvent scope and evaporates with the derived event. INVOCATIONS.set(evt, { id }); evt.serverOnly = true; + // Which wrap owns this call: the one that owns the request it is made + // under. The CONFIGURED hook covers every call the process makes, so + // per-function middleware built on it can't be bypassed by calling + // the function during a render. A per-handler `wrapInvocation` option + // is not ambient — it rides the request's event (REQUEST_WRAPS), and + // through this derived one, so a call the dispatched body makes + // in-process is gated by the same policy as the call that reached the + // wire, however deep the chain goes. A call made during a document + // render belongs to no such request, and there the configured hook is + // the only policy there can be. + const requestWrap = REQUEST_WRAPS.get(ogEvt); + if (requestWrap) REQUEST_WRAPS.set(evt, requestWrap); + const wrap = requestWrap || config.wrapInvocation; const scope = run => provideEvent(evt, run); - let result = provideEvent(evt, () => { + let result = provideEventOnce(provideEvent, evt, () => { const run = () => fn.apply(thisArg, args); - // Per-invocation wrap (see configureServerFunctionsServer): direct - // SSR calls run through the same policy as HTTP dispatch, so - // per-function middleware built on it can't be bypassed by calling - // the function during a render. The wrapper must return run()'s - // value (this path stays synchronous for synchronous functions). - return config.wrapInvocation - ? config.wrapInvocation(run, { id, args, event: evt, direct: true }) - : run(); + // The wrapper must return run()'s value (this path stays + // synchronous for synchronous functions). + return wrap ? wrap(run, { id, args, event: evt, direct: true }) : run(); }); // A generator or stream body runs when the caller pulls it, after the // call-time scope above has gone. Bind the WRAPPER'S result (not merely @@ -957,8 +1147,25 @@ export function createServerReference({ id, fn, name }) { return transform ? scopeDeferredResult(transform(result, { id, args, event: evt }), scope) : result; + }, + // `new fn()` is a call too, and there is one road into the body. The + // whole server-side contract lives in the apply trap above — the + // outside-a-request guard, the derived event, the invocation identity, + // the wrap — and a Proxy without this trap forwards construction + // straight to the target, past all of it: a generic caller (a DI + // container, a serializer reviving a value, `Reflect.construct`) would + // run the body with no event in scope, where policy could not have been + // evaluated even in principle. Refused rather than routed through + // apply: `new` demands an object back, which a server function's result + // is under no obligation to be. + construct() { + throw new Error( + "Cannot construct a server function: server functions are called, not constructed." + ); } }); + // the reference names this binding (see `GET`) + REFERENCE_BINDINGS.set(proxy, fn); return proxy; } /** * Declares a server function callable over HTTP GET. The server half is @@ -1017,7 +1224,8 @@ export function GET( * The declaration is about the FUNCTION, not the id: registering a * different function under the same id revokes it (#3129), and the new * function's own `GET()` — which module order runs right after the - * re-registration — is what re-grants it. + * re-registration — is what re-grants it. A declaration that names a + * binding the id no longer has grants nothing, for the same reason. * * Wrap the reference at its declaration; the compiler round-trips the call * in both builds: @@ -1033,9 +1241,23 @@ export function GET(fn) { if (!isServerFunction(fn) || typeof fn.id !== "string") { throw new Error("GET expects a server function reference"); } - METHODS.set(fn.id, "GET"); - // the declaration itself is a metadata write like any other - return withMeta(fn, { method: "GET" }); + // The grant is recorded as the BINDING it was granted to (see METHODS): + // a declaration is made ABOUT a function, so it may reach dispatch only + // while the id still names that function. #3129 covers the id rebound + // AFTER the declaration; recording the binding covers the other + // interleaving too — the id already rebound BEFORE `GET()` ran (an id + // collision between integrations, a module re-evaluated in a live process + // after an edit dropped the wrapper), where an unconditional write handed + // GET, and with it the origin-gate exemption, to a function that never + // signed it. A reference this module did not build names no binding we + // can verify; the grant then falls back to the live registration, as + // before. + METHODS.set(fn.id, REFERENCE_BINDINGS.get(fn) || REGISTRATIONS.get(fn.id)); + // the declaration records itself on the metadata channel — through the + // declaration writer, not `withMeta`: the grant above IS the declaration, + // so the metadata is its report and must not be a second way to make one, + // or to unmake one + return declareMeta(fn, { method: "GET" }); } /** * Declares a value-shaped live source: a server function returning an async * iterable whose yields are successive VALUES of one logical query, with @@ -1178,52 +1400,10 @@ function assertDecodeDepth(value) { } /** - * 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 — - * and core itself is unharmed: `Object.prototype` is never touched. What - * the key subverts is the handler's most ordinary downstream move: - * `Object.assign` merges by [[Set]], so merging a decoded argument into a - * fresh object re-prototypes the copy with attacker-supplied data (#3168). - * This boundary already makes decisions of exactly this class — the decode - * depth cap, the RegExp exclusion, the argument-count bound — so the key - * is stripped here at the seam rather than documented away. - * - * 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, 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 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); - // 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); - } - } - return value; -} - -/** - * Buffers a POST body that declared no length (chunked transfer), refusing - * once it runs past the limit — a declared length is enforced by the HTTP - * server's own framing and is checked against the limit before this runs. + * Buffers a POST body, refusing once it runs past the limit. Every body a + * capped route accepts is read through here — a declaration is checked + * against the limit before this runs, but a declaration under it is not + * evidence of anything, so the count is taken on the bytes that arrive. * The original body is read, not a clone: cancellation must tear down the * upload source rather than one branch of a tee (#3219). On success the * consumed body is replaced so the ordinary decoder can still read it. @@ -1298,7 +1478,9 @@ async function parseArguments(request, url, scripted, codec) { if (!Array.isArray(result)) { throw new TypeError("Server function arguments must encode an array"); } - stripUnsafeArgumentKeys(result); + // The url road decodes here rather than through `extractBody`, so it + // is the one place the strip is still applied by hand (#3168/#3200). + stripUnsafeKeys(result); for (const arg of result) { parsed.push(arg); } @@ -1324,7 +1506,9 @@ async function parseArguments(request, url, scripted, codec) { if (!Array.isArray(decoded)) { throw new TypeError("Server function arguments must encode an array"); } - return stripUnsafeArgumentKeys(decoded); + // Already stripped: `extractBody` guards both structured roads, for + // arguments and results alike. + return decoded; } if (decoded === undefined) { // Node hosts commonly construct a web Request from the incoming socket @@ -1349,6 +1533,63 @@ async function parseArguments(request, url, scripted, codec) { return parsed; } +/** + * Whether a collected slice can ride the wire, asked before it joins the + * envelope and without side effects on anything the encode still needs. + * + * Two roads carry a payload (see encodeResult), so the question is asked of + * both. `isJSONSafe` answers for the fast road, and answers for the shape + * the fold's own comment calls THE single-flight response — plain data — + * so the common slice never reaches the codec twice. Everything past it is + * the codec's call and not ours to re-derive: plugins alone make the + * supported set unknowable from outside, and a table of built-ins would + * drift into dropping data the wire can carry. + * + * So the codec is asked, about LEAVES only. Plain containers are walked + * here, through descriptors and never `v[k]`, because reading an accessor + * MINTS a value nobody guards — a rejecting promise from a getter took the + * process down before the codec ever saw the object (#3176) — and the + * codec's own read happens moments later under `guardFailures`. An + * accessor is therefore not verifiable from here and does not condemn a + * slice; nor does a repeat or a cycle, which the codec carries. + */ +async function sliceIsCarriable(slice, codec) { + if (isJSONSafe(slice)) return true; + const { canSerializeJSON } = await import("../../serialization/src/serializer.js"); + const stack = [slice]; + const seen = new Set(); + while (stack.length) { + const value = stack.pop(); + const type = typeof value; + if (value !== null && type === "object") { + if (seen.has(value)) continue; + seen.add(value); + if (Array.isArray(value)) { + // index iteration, like isJSONSafe: a hole reads undefined, which + // for-in would skip + for (let i = 0; i < value.length; i++) stack.push(value[i]); + continue; + } + const proto = Object.getPrototypeOf(value); + if (proto === Object.prototype || proto === null) { + for (const key in value) { + const descriptor = Object.getOwnPropertyDescriptor(value, key); + if (descriptor && "value" in descriptor) stack.push(descriptor.value); + } + continue; + } + // a foreign prototype — a Date, a Map, a class instance, a live + // handle: which of those the codec has a node for is its own answer + } else if (type !== "function" && type !== "symbol") { + // every other primitive has a node; asking per string would make the + // walk cost a full second serialization of the slice + continue; + } + if (!canSerializeJSON(value, codec)) return false; + } + return true; +} + /** * Runs the single-flight hooks and standardizes their contribution: each * `[source, hook]` pair that returns data is folded into the body's @@ -1379,7 +1620,23 @@ async function foldFlightData(hooks, event, headers, outcome, context = {}) { // means that cache revalidates the normal way. try { const slice = await hook(event, outcome); - if (slice !== undefined) folded.push([source, slice]); + if (slice === undefined) continue; + // A collector fails in two ways and only one of them is a throw: the + // other is handing back a value the wire cannot carry — a cache entry + // still holding a function, a class instance, a live DB handle. That + // failure used to land far outside this try, in the encode, where the + // slices are no longer separable: it took the mutation's own return + // value and every sibling cache's slice with it, in band, at status + // 200 with no error tag, over a mutation that had already committed. + // Asked HERE the answer still costs only this slice, which is what + // the rule above says a failing collector may cost. + if (!(await sliceIsCarriable(slice, context.codec))) { + console.error( + `Flight data for source "${source}" cannot be encoded and was dropped from the response` + ); + continue; + } + folded.push([source, slice]); } catch (error) { console.error(`Error collecting flight data for source "${source}"`, error); } @@ -1398,15 +1655,24 @@ async function foldFlightData(hooks, event, headers, outcome, context = {}) { context ); if (transformed !== undefined) { + // Ownership BEFORE the first write (#3155): nothing in the hook's + // contract says the Response is freshly built, and a policy that + // memoizes its rendered shell hands the same object to every caller — + // so the appends below would stamp this request's session cookie onto + // it permanently, and serve it to the next tenant. The thrown path's + // tail copies for exactly this reason, but by then these writes have + // already landed on the shared object, and copying contamination only + // carries it forward. + const owned = ownResponse(transformed); // Headers accumulated during the call (the mutation's cookies, an // envelope's metadata) belong on whatever body carries the outcome. - for (const cookie of headers.getSetCookie()) transformed.headers.append("Set-Cookie", cookie); + for (const cookie of headers.getSetCookie()) owned.headers.append("Set-Cookie", cookie); headers.forEach((value, key) => { - if (key !== "set-cookie" && !transformed.headers.has(key)) { - transformed.headers.set(key, value); + if (key !== "set-cookie" && !owned.headers.has(key)) { + owned.headers.set(key, value); } }); - return transformed; + return owned; } } // A void mutation's envelope omits the `value` key rather than carrying @@ -1440,14 +1706,19 @@ function digestOutcome(event, outcome) { ...(response?.headers?.getSetCookie() ?? []) ]); try { + // Only the SECOND case needs a referer. A redirect's destination is + // derived from the outcome the server itself just produced, so gating it + // on the referring page made `Referrer-Policy: no-referrer` — a routine + // security header — silently switch single-flight off for every + // redirecting mutation, which is the shape single-flight exists for. + const location = response?.headers.get("Location"); const referrer = request.headers.get("referer"); - if (referrer) { - const location = response?.headers.get("Location"); + if (location || referrer) { const target = location ? new URL(location, request.url) : new URL(referrer); if (target.origin === new URL(request.url).origin) outcome.targetUrl = target.toString(); } } catch { - // unparseable referer — same as no referer + // unparseable Location or referer — same as none } } @@ -1766,9 +2037,16 @@ export function createNoJSHandler({ base = "" } = {}) { } else { headers = new Headers({ Location: back }); } - // Responses carry their meaning in their metadata; anything else flashes - // the outcome for the next render to read. - if (result && !(result instanceof Response)) { + // Responses carry their meaning in their metadata; anything else + // flashes the outcome for the next render to read — anything, not just + // anything truthy. `async () => { await db.save(draft); }` is the + // commonest action shape there is and the one with no value to be + // truthy: skipping the cookie for it left the next render unable to + // tell a committed submission from one that never happened, which is + // the read that makes a user submit again (#3137). The scripted leg + // has no such gap — a call returning `undefined` resolves — and this + // leg exists to show the outcome exactly as that one would. + if (!(result instanceof Response)) { headers.append( "Set-Cookie", encodeFlashCookie(url.pathname + url.search, result, args, thrown) @@ -1821,13 +2099,19 @@ function isFormPost(request) { // // Left alone deliberately, so a channel behind one is unguarded: class // instances, whose own properties are not ours to rebuild (private fields, -// getters, invariants). Plain-object accessors and Map keys used to sit in -// that list too — but the codec pumps both, so "not ours to invoke" was not -// 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, invariants) — and which the codec cannot encode either, so a +// channel under one costs a failed call, not a leak. Plain-object +// accessors, Map keys, Error carriers and non-enumerable own data +// properties all used to sit in that list too, and the codec pumps every +// one of them, so "not ours to touch" was never 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); the same promise under +// an Error carrier (#3200) or on a hidden own property did the same. // Enumerable getters are now invoked once and materialized as data -// properties; Map keys are walked like values. +// properties; Map keys are walked like values; and enterGuard's object leg +// no longer decides what to visit by prototype or by enumerability, only by +// whether reading the slot would run authored code. // // `state.gate` (optional — serializeResponseStream threads it, #3125) hooks // the two STREAMING channels into the response lifetime as they are walked: @@ -2164,8 +2448,21 @@ function enterGuard(value, state) { return new Frame(SET, value, next, [...value], null); } + // What the walk descends is what the CODEC encodes out of a value's own + // properties, because a slot the codec reads is a slot that can carry a + // channel onto the wire. That is plain and null-prototype objects — and + // Errors, which seroval encodes natively (t:13/t:14) WITH their own + // properties, so `Object.assign(new Error(...), { rows })`, the ordinary + // shape of a domain failure carrying its context, is as reachable as any + // plain result (#3200 on the result road; the argument road's walk reached + // the same carriers by dropping its own plain-prototype test). Anything + // else is left alone deliberately: seroval either encodes it from internal + // state and ignores own properties (Date, RegExp, typed arrays — whose + // indices this walk has no business enumerating), or cannot encode it at + // all, and the call fails closed with a sanitized body long before a + // channel underneath could reach a caller. const prototype = Object.getPrototypeOf(value); - if (prototype !== Object.prototype && prototype !== null) { + if (prototype !== Object.prototype && prototype !== null && !(value instanceof Error)) { state.seen.set(value, value); return value; } @@ -2179,15 +2476,29 @@ function enterGuard(value, state) { // 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. + // + // The same pass picks the slots to walk. ENUMERABILITY is not the question + // — hiding a property hides it from the wire, never from the codec's pump + // (an Error's options are read by own NAME, and a channel the codec skips + // is worse, not better: it belongs to nobody, so a rejection there reaches + // no handler and takes the process down after the 200 is already + // delivered, and a stream there is never registered with the response + // teardown). INVOCATION is: walking a data property costs nothing, while + // reading an accessor runs authored code, so hidden accessors stay unread + // (47995412) and enumerable ones are materialized as before, because the + // codec reads those itself. + const keys = []; for (const key of Object.keys(descriptors)) { - descriptors[key].configurable = true; - if ("value" in descriptors[key]) descriptors[key].writable = true; + const descriptor = descriptors[key]; + descriptor.configurable = true; + if ("value" in descriptor) { + descriptor.writable = true; + keys.push(key); + } else if (descriptor.enumerable) keys.push(key); } const next = Object.create(prototype, descriptors); state.seen.set(value, next); - // 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); + return new Frame(OBJECT, value, next, keys, descriptors); } /** A rebuild stands if anything below changed, or if a cycle already took it. */ @@ -2670,6 +2981,21 @@ function forbiddenResponse() { ); } +// What the next render is told when a form navigation is refused before it +// ever dispatched. The status is the whole of it — the reason text is +// DEV-only on the wire, production sends none — except for the one refusal +// the user can act on: an id the deployment no longer has is version skew +// (#3110), the page in front of them is stale, and reloading is the +// recovery. Flashed as a THROWN outcome, so an integration branches on it +// exactly as it branches on a function that threw. +function refusalError(response) { + return new Error( + response.headers.get(UNKNOWN_HEADER) + ? "This page is out of date: the server function it submitted to is no longer deployed." + : `The submission was refused before it ran (${response.status}).` + ); +} + function nativePromise(value) { if (value instanceof Promise) return value; try { @@ -2769,9 +3095,11 @@ export function handleServerFunctionRequest( * (`getServerFunctionInvocation()` answers before, during and after * `run()`); the context carries `{ id, args, event, request, direct }`. * Must return (or resolve to) `run()`'s result — replacing it replaces - * the function's result. The configured hook (see - * `configureServerFunctionsServer`) also wraps direct SSR calls, where - * `context.direct` is `true` and `request` is absent. + * the function's result. It also wraps the in-process calls the + * dispatched body makes (`context.direct` is `true` and `request` is + * absent for those); the configured hook (see + * `configureServerFunctionsServer`) wraps those AND direct SSR calls made + * outside any request, e.g. during a document render. * - `transformResult(event, result, context)`: observes/replaces the result * before encoding — the extension point for response metadata policies. * The context carries the call's identity (`id`, parsed `args`) alongside @@ -2827,9 +3155,7 @@ export async function handleServerFunctionRequest(request, options = {}) { // CDNs that ignore Vary, poisons) the shared-cache entries the GET helper // exists to enable (#3071). State-changing dispatch (POST) stays gated. const declaredRead = - (method === "GET" || method === "HEAD") && - functionId !== null && - METHODS.get(functionId) === "GET"; + (method === "GET" || method === "HEAD") && functionId !== null && declaresRead(functionId); const csrf = options.csrf !== undefined ? options.csrf : config.csrf; // The skip is `GET()`'s safety contract at work (see its notes and // #3114); `protectDeclaredReads` is the opt-in for deployments that @@ -2837,6 +3163,71 @@ export async function handleServerFunctionRequest(request, options = {}) { const protectsRequest = csrf !== false && (!declaredRead || (typeof csrf === "object" && csrf.protectDeclaredReads === true)); + // Which of the two answer shapes this call gets — codec encodings for the + // client transport, plain HTTP for everyone else — is decided by the + // ADDRESS: the data address IS the scripted protocol, the bare address is + // plain HTTP. On the url, not a header, because shared caches key on the + // url and store one answer per key — a header-driven shape means one + // caller kind's cached answer can be replayed to the other (#3094). The + // instance header does not shape the answer; it still identifies the + // call (invocation context, no-JS gating). A meaningless path has no + // address at all, and nothing scripted addresses one. + const scripted = !!address && address.data; + + // The no-JS convention is decided HERE, before the gates, because its + // promise — "the browser is never left on the endpoint" + // (`createNoJSHandler`) — is about the BROWSER, not about whether the + // call was accepted. Deciding it after them left every pre-dispatch + // refusal answering a form navigation with a status and no `Location`: + // a stale id after a deploy, a malformed multipart body, an upload past + // `bodySizeLimit`, an origin the gate cannot vouch for — each one a + // blank page at `/_server/` with the back button as the only way + // out and everything the user typed gone. Only the DECISION moves; the + // refusal for a form-shaped scripted fetch stays where it was, below, + // with the gates it belongs behind. + // + // Which caller kind this is was decided by the ABSENCE of a header + // (#3139, the shape-on-the-url doctrine's one leftover): a same-origin + // page script posting a form-encoded body — fetch(url, { body: new + // URLSearchParams(...) }) — is form-shaped too, and routing IT into the + // convention lands it on the referrer's HTML with `response.ok === true` + // while its answer disappears into its own cookie jar. The browser's own + // word tells the two apart: a real form navigation sends `Sec-Fetch-Mode: + // navigate` (or nothing, on older browsers), a script's fetch never does. + const formShaped = !scripted && isFormPost(request); + const fetchMode = formShaped ? request.headers.get("Sec-Fetch-Mode") : null; + const formNavigation = formShaped && (fetchMode === null || fetchMode === "navigate"); + // Same fallback pattern as the hooks below, then the built-in + // convention: an unconfigured app still gets working progressive + // enhancement for real form posts, while direct HTTP calls keep the + // plain response. + let handleNoJS = options.handleNoJS !== undefined ? options.handleNoJS : config.handleNoJS; + const conventional = handleNoJS === undefined; + if (conventional && formNavigation) { + handleNoJS = defaultNoJSHandler || (defaultNoJSHandler = createNoJSHandler()); + } + let event; + // Every refusal answers through here, dispatched or not. A browser form + // navigation is bounced back to the form with the refusal flashed, the + // way a function that threw would be; every other caller keeps the plain + // status it always got. Nothing has COMMITTED at any of these exits — + // that is what makes the bounce safe, and what makes this a progressive- + // enhancement hole rather than a correctness one: there is no outcome to + // double-submit, only a form to return to. + // + // The event's response stub folds onto the answer once an event exists + // (#3159), bounce included: an integration's Set-Cookie written in + // `createEvent` (a rotated session, a fresh CSRF token) must still reach + // the browser on exactly the requests where something went wrong. + const refuse = async (response, vary = protectsRequest) => { + let answer = + formNavigation && handleNoJS + ? await handleNoJS(refusalError(response), request, [], true) + : response; + if (event) answer = commitEventResponse(answer, event); + if (vary) answer = withCSRFVary(answer); + return finalizeTransportResponse(answer, method); + }; // Labelled (#3110): the address is well-formed but its id is not part of // this deployment — the wire shape of version skew (a tab holding the // previous build's ids) or a genuinely removed function. Without the @@ -2863,35 +3254,25 @@ export async function handleServerFunctionRequest(request, options = {}) { } catch { // no Vary: the answer does not depend on origin proof, so it must // not fragment shared-cache entries on it - return finalizeTransportResponse( + return refuse( new Response(DEV ? `Unknown server function: ${functionId}` : null, { status: 404, headers: { [UNKNOWN_HEADER]: "true" } }), - method + false ); } } if (protectsRequest && !(await allowsServerFunctionRequest(request, csrf === true ? {} : csrf))) { - return finalizeTransportResponse(forbiddenResponse(), method); + return refuse(forbiddenResponse(), false); } const instance = request.headers.get(INSTANCE_HEADER); if (!functionId) { const response = new Response(DEV ? "Server function not found" : null, { status: 404 }); - return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); + return refuse(response); } - // Which of the two answer shapes this call gets — codec encodings for the - // client transport, plain HTTP for everyone else — is decided by the - // ADDRESS: the data address IS the scripted protocol, the bare address is - // plain HTTP. On the url, not a header, because shared caches key on the - // url and store one answer per key — a header-driven shape means one - // caller kind's cached answer can be replayed to the other (#3094). The - // instance header does not shape the answer; it still identifies the - // call (invocation context, no-JS gating). - const scripted = address.data; - // Method allowlist: POST always dispatches (the default transport); // GET and HEAD dispatch only to functions that declared GET (the server // half of `GET` records them) — no crafted read URLs against functions @@ -2906,19 +3287,18 @@ export async function handleServerFunctionRequest(request, options = {}) { DEV ? `Method not allowed for server function: ${functionId}` : null, { status: 405, - headers: { Allow: METHODS.get(functionId) === "GET" ? "POST, GET, HEAD" : "POST" } + headers: { Allow: declaresRead(functionId) ? "POST, GET, HEAD" : "POST" } } ); - return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); + return refuse(response); } // The argument payload is buffered and decoded before dispatch, so its // cost is paid before application code can decline it — bound it before - // paying (#3115). A CONFORMING declared Content-Length is trusted (the - // HTTP server's framing enforces it); a body without one — or with a - // declaration that isn't a plain digit string (#3153) — is buffered under - // the cap. The `?args=` encoding is the same payload on a different road, - // so it gets the same ceiling. + // paying (#3115). A declared Content-Length is evidence, never proof: an + // over-declaration is refused before a byte is read, and the bound itself + // is measured on the bytes that ARRIVE. The `?args=` encoding is the same + // payload on a different road, so it gets the same ceiling. const bodySizeLimit = options.bodySizeLimit !== undefined ? options.bodySizeLimit : config.bodySizeLimit; const argsEncoding = url.searchParams.get("args"); @@ -2927,17 +3307,34 @@ export async function handleServerFunctionRequest(request, options = {}) { DEV ? "Server function arguments exceed the configured bodySizeLimit" : null, { status: 413 } ); - return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); + return refuse(response); } if (method === "POST" && request.body !== null && bodySizeLimit !== Infinity) { - // Trust only a CONFORMING declaration — digits, per RFC 9110 §8.6. The - // bare Number() parse lost that information: Number("-1") is -1, which - // is neither `> limit` nor falsy, so a negative declaration satisfied - // NEITHER guard and the body streamed into the decoder uncapped - // (#3153). A stock node:http parser refuses it first, but an adapter - // that builds the Request itself, or a rewriting proxy, delivers it - // here — anything non-conforming now routes through the bounded buffer - // alongside the undeclared bodies. + // The one thing a declaration is good for: a CONFORMING one — digits, + // per RFC 9110 §8.6 — that is already over the limit says the peer + // intends to send too much, so refuse before paying for a byte. That + // is the whole of the trust. Believing a declaration in the other + // direction, to SKIP the counting read, believed the same header from + // the same untrusted producer that #3153 established must not be + // believed when it reads `-1`: `Content-Length: 10` on a 2 MiB body + // dispatched the whole 2 MiB, so the cap was not late, it was off. A + // stock node:http parser frames the body BY the declaration and would + // truncate it first, but an adapter that builds the Request itself, or + // a rewriting proxy, delivers it here. So every body is read through + // the counting read and the cap bounds what it advertises: the bytes + // that arrive. (A capped route pays for the whole body either way: the + // arguments are decoded before dispatch, and a body that declared a + // length was materialized by its sender before it was sent — nothing + // can still be arriving on a stream framed by a Content-Length. So the + // single road costs one copy, not a capability, and + // `bodySizeLimit: Infinity` remains the way to opt a route out of + // buffering entirely.) + // + // One road is also the only way the upload lifecycle stays honest: the + // signal/reader coupling that settles an abandoned request and tears + // down its source (#3217/#3218/#3219) lives in that read, so it now + // covers the conforming-declaration POST every browser sends, and not + // just chunked uploads. const raw = request.headers.get("content-length"); const declared = raw !== null && /^\d+$/.test(raw) ? Number(raw) : NaN; if (declared > bodySizeLimit) { @@ -2945,36 +3342,28 @@ export async function handleServerFunctionRequest(request, options = {}) { DEV ? "Server function request body exceeds the configured bodySizeLimit" : null, { status: 413 } ); - return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); + return refuse(response); } - if (!(declared > 0)) { - let bounded; - try { - bounded = await bufferBodyWithin(request, bodySizeLimit); - } catch { - // A failed or aborted upload is an incomplete argument encoding, - // not a handler failure. Match the decoder's malformed-body answer - // instead of rejecting out of dispatch (#3217). - const response = new Response(DEV ? "Malformed server function arguments" : null, { - status: 400 - }); - return finalizeTransportResponse( - protectsRequest ? withCSRFVary(response) : response, - method - ); - } - if (bounded === null) { - const response = new Response( - DEV ? "Server function request body exceeds the configured bodySizeLimit" : null, - { status: 413 } - ); - return finalizeTransportResponse( - protectsRequest ? withCSRFVary(response) : response, - method - ); - } - request = bounded; + let bounded; + try { + bounded = await bufferBodyWithin(request, bodySizeLimit); + } catch { + // A failed or aborted upload is an incomplete argument encoding, not + // a handler failure. Match the decoder's malformed-body answer + // instead of rejecting out of dispatch (#3217). + const response = new Response(DEV ? "Malformed server function arguments" : null, { + status: 400 + }); + return refuse(response); } + if (bounded === null) { + const response = new Response( + DEV ? "Server function request body exceeds the configured bodySizeLimit" : null, + { status: 413 } + ); + return refuse(response); + } + request = bounded; } // An async createEvent is out of contract (the type is synchronous), but @@ -2987,7 +3376,6 @@ export async function handleServerFunctionRequest(request, options = {}) { // 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); @@ -3004,7 +3392,10 @@ export async function handleServerFunctionRequest(request, options = {}) { const response = scripted ? encodeResult(safe, headers, 500, codec, request.signal) : new Response(DEV ? message : null, { status: 500 }); - return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); + // Nothing to fold: `event` never became one, and a rejected promise + // must not be mistaken for one on the way out. + event = undefined; + return refuse(response); } // Once an event exists, its response stub folds onto EVERY exit — the // refusals below included (#3159). A refusal that returned directly @@ -3013,11 +3404,9 @@ export async function handleServerFunctionRequest(request, options = {}) { // browser on exactly the requests where something already went wrong, and // the next request carried stale credentials with the failure pointing at // the wrong place. Committing here also arms the stub's late-write - // instrumentation, same as the dispatch tail. - const refuseCommitted = raw => { - const response = commitEventResponse(raw, event); - return finalizeTransportResponse(protectsRequest ? withCSRFVary(response) : response, method); - }; + // instrumentation, same as the dispatch tail. `refuse` above does the + // fold now that it can see the event, so the refusals below are the same + // exit as the ones before it. const provide = options.provideEvent || provideEvent; const scope = run => provide(event, run); const flightHook = @@ -3027,47 +3416,45 @@ export async function handleServerFunctionRequest(request, options = {}) { // configured transform (frames installs itself here once, server-wide). const transformResult = options.transformResult !== undefined ? options.transformResult : config.transformResult; + // The gate resolves differently from the transforms above, deliberately: + // an option value that is not a hook cannot REMOVE policy. `!== undefined` + // reads `null` — the other spelling of "nothing to override", the one a + // JS adapter or a computed `perRoute.wrap ?? null` produces, and the one + // `provideEvent` already treats as absent — as "run this call with no + // wrap at all", which for the seam an app hangs authorization on means + // one request served past the gate, with a 200. Overriding the configured + // wrap takes a wrap; anything else leaves the configured one owning the + // call. const wrapInvocation = - options.wrapInvocation !== undefined ? options.wrapInvocation : config.wrapInvocation; + typeof options.wrapInvocation === "function" ? options.wrapInvocation : config.wrapInvocation; + // And whichever wrap owns this request owns the calls its body makes + // in-process, not just the one the wire addressed: recorded on the event + // for the apply trap to find (see REQUEST_WRAPS), since a per-handler + // option — unlike the configured hook — is in scope nowhere else. + if (wrapInvocation) REQUEST_WRAPS.set(event, wrapInvocation); const transformFlightResult = options.transformFlightResult !== undefined ? options.transformFlightResult : config.transformFlightResult; - // Same fallback, then the built-in convention: an unconfigured app still - // gets working progressive enhancement for real form posts, while direct - // HTTP calls keep the plain response. - // - // The convention is for form NAVIGATIONS — the browser follows the 303 - // and the flash cookie carries the outcome to the next render. Which - // caller kind this is was decided by the ABSENCE of a header (#3139, - // the shape-on-the-url doctrine's one leftover): a same-origin page - // script posting a form-encoded body — fetch(url, { body: new - // URLSearchParams(...) }) — is form-shaped too, and routing IT into the - // convention lands it on the referrer's HTML with `response.ok === true` - // while its answer disappears into its own cookie jar. The browser's own - // word tells the two apart: a real form navigation sends `Sec-Fetch-Mode: - // navigate` (or nothing, on older browsers), a script's fetch never does. - // The script's call is refused as malformed — BEFORE dispatch, because - // the old behavior's real harm was running the mutation and then hiding - // the outcome — pointing at the two spellings that work. Answering it - // plain instead would put a second, header-decided shape on the bare - // address, which is the exact thing #3094 moved onto the url. - let handleNoJS = options.handleNoJS !== undefined ? options.handleNoJS : config.handleNoJS; - if (handleNoJS === undefined && !scripted && isFormPost(request)) { - const fetchMode = request.headers.get("Sec-Fetch-Mode"); - if (fetchMode === null || fetchMode === "navigate") { - handleNoJS = defaultNoJSHandler || (defaultNoJSHandler = createNoJSHandler()); - } else { - const response = new Response( - DEV - ? "The bare server-function address answers form navigations with the " + - "no-JS redirect convention. Scripted callers use the data address " + - `(…/data/${functionId}) or send the ${BODY_FORMAT_HEADER} tag.` - : null, - { status: 400 } - ); - return refuseCommitted(response); - } + // The other half of the convention decided above: a form-SHAPED call + // that is not a form NAVIGATION is a page script's fetch, and it is + // refused as malformed — BEFORE dispatch, because the old behavior's + // real harm was running the mutation and then hiding the outcome — + // pointing at the two spellings that work. Answering it plain instead + // would put a second, header-decided shape on the bare address, which is + // the exact thing #3094 moved onto the url. It stays HERE, behind the + // gates: it is a refusal of the CALL, not of the browser, so it has + // nothing to bounce and no reason to outrank a stale id or a bad origin. + if (conventional && formShaped && !formNavigation) { + const response = new Response( + DEV + ? "The bare server-function address answers form navigations with the " + + "no-JS redirect convention. Scripted callers use the data address " + + `(…/data/${functionId}) or send the ${BODY_FORMAT_HEADER} tag.` + : null, + { status: 400 } + ); + return refuse(response); } // single-flight is scripted-client opt-in: the caller sends the request // header naming the sources it can consume ("true" is the unnamed hook's @@ -3086,8 +3473,16 @@ export async function handleServerFunctionRequest(request, options = {}) { // honoring it from anyone else hands a curl one shared-cache poisoning. const flightHeader = scripted && method === "POST" ? request.headers.get(SINGLE_FLIGHT_HEADER) : null; + // The list is a SET, and deduping it is not tidiness: the header is + // caller-supplied, one entry runs one collector, and a collector is the + // most expensive per-request work here (it re-runs the invalidated reads + // inside a request-event scope). A repeat cannot even contribute a second + // slice — the envelope is built with Object.fromEntries and the client + // holds one consumer per source id — so every run past the first is work + // whose result is discarded, and honoring the multiplicity hands a + // same-origin caller an amplifier it chooses the factor for. const flightHooks = flightHeader - ? flightHeader.split(",").flatMap(source => { + ? [...new Set(flightHeader.split(","))].flatMap(source => { const hook = source === "true" ? flightHook : flightSources.get(source); return hook ? [[source, hook]] : []; }) @@ -3104,7 +3499,7 @@ export async function handleServerFunctionRequest(request, options = {}) { const response = new Response(DEV ? "Malformed server function arguments" : null, { status: 400 }); - return refuseCommitted(response); + return refuse(response); } // The decoded array is spread into the call, so an unbounded argument @@ -3117,7 +3512,7 @@ export async function handleServerFunctionRequest(request, options = {}) { DEV ? "Server function call exceeds the configured maxArguments" : null, { status: 400 } ); - return refuseCommitted(response); + return refuse(response); } // What the fold needs to build a body itself, and what a result transform @@ -3143,20 +3538,11 @@ export async function handleServerFunctionRequest(request, options = {}) { // `event.response.headers` during a server function reach the wire. const dispatch = async () => { try { - // provideEvent's contract — run the callback, once, with `event` - // visible to getRequestEvent() — is enforced rather than assumed - // (#3172): every way a hand-written hook gets it wrong used to answer - // a successful-looking 200. The two data-integrity violations are - // counted here at the seam: a second invocation is refused BEFORE the - // function body runs again (a retry wrapper or a misplaced await - // double-committed a mutation under a 200, silently), and a hook that - // never invoked the callback must not resolve as a void success a - // caller cannot distinguish from a function that returned nothing. - // Both land on dispatch's catch — a sanitized 500 in production, the - // hook named in development — and the count is re-checked after the - // hook returns, so swallowing the in-flight refusal does not turn it - // back into a 200. - let invocations = 0; + // provideEvent's invocation contract is enforced, not assumed + // (#3172) — by the same guard the direct SSR call enters through, so + // a hook broken in either direction fails on whichever leg meets it + // (see provideEventOnce). Both violations land on dispatch's catch: a + // sanitized 500 in production, the hook named in development. const invokeOnce = async () => { // Identity is established BEFORE the wrapper runs, so // getServerFunctionInvocation() answers throughout the wrap — code @@ -3167,30 +3553,7 @@ export async function handleServerFunctionRequest(request, options = {}) { ? wrapInvocation(run, { id: functionId, args: parsed, event, request, direct: false }) : run(); }; - let result = await provide(event, () => { - if (++invocations > 1) { - // thrown synchronously, never as a rejected promise: the second - // execution must not START, and a hook that ignores the return - // must not mint an unobserved rejection - throw new Error( - "provideEvent invoked the server function callback more than once: a second " + - "invocation would commit the call's side effects twice. The hook must call " + - "fn exactly once and return its result." - ); - } - return invokeOnce(); - }); - if (invocations !== 1) { - throw new Error( - invocations === 0 - ? "provideEvent returned without invoking the server function callback: the call " + - "would have answered as a void success without running the function. The hook " + - "must call fn exactly once and return its result." - : "provideEvent invoked the server function callback more than once: a second " + - "invocation would commit the call's side effects twice. The hook must call " + - "fn exactly once and return its result." - ); - } + let result = await provideEventOnce(provide, event, invokeOnce); if (transformResult) { result = await transformResult(event, result, flightContext); @@ -3317,19 +3680,32 @@ export async function handleServerFunctionRequest(request, options = {}) { // thrown envelopes keep the author's status above. return encodeResult(safe, headers, 500, codec, request.signal, scope); }; - if (x instanceof Response || isResponseEnvelope(x)) { - if (transformResult) { - try { - x = await transformResult(event, x, { ...flightContext, thrown: true }); - } catch (hookError) { - // Same hook, same failure, same containment as the return path - // (#3171): there a throwing transformResult lands in this catch - // as a plain error and answers a sanitized 500. Uncontained here, - // it escaped the handler entirely — no status, no event stub, - // the host adapter left to improvise. - return respondThrown(hookError); - } + // The result policy sees EVERY outcome, which is what `context.thrown` + // is for: a thrown Error or a thrown string is the failure shape an + // error-mapping or audit policy is written for, and it used to be the + // one shape the hook never saw — the hook ran inside the branch below, + // so it met every success and every failure an author had already + // shaped by hand, and none of the failures that happen TO the app. + // Hoisted here it runs once for the whole thrown path, and the tail is + // chosen by the shape the policy settled on, exactly as on the return + // path: mapping an internal error to a wire shape (the mapping the + // sanitization notes describe) works by returning one. What the hook + // hands back is not privileged — a plain value still answers through + // `respondThrown`, sanitized unless branded safe, so meeting the raw + // error is not a road for it onto the wire. + if (transformResult) { + try { + x = await transformResult(event, x, { ...flightContext, thrown: true }); + } catch (hookError) { + // Same hook, same failure, same containment as the return path + // (#3171): there a throwing transformResult lands in this catch + // as a plain error and answers a sanitized 500. Uncontained here, + // it escaped the handler entirely — no status, no event stub, + // the host adapter left to improvise. + return respondThrown(hookError); } + } + if (x instanceof Response || isResponseEnvelope(x)) { let status = 200; let metadata; if (isResponseEnvelope(x)) { diff --git a/packages/web/server-functions/src/shared.ts b/packages/web/server-functions/src/shared.ts index 57a9318b9..4d4c74934 100644 --- a/packages/web/server-functions/src/shared.ts +++ b/packages/web/server-functions/src/shared.ts @@ -40,6 +40,7 @@ export { LIVE_SOURCE, SERVER_FUNCTION_INVOKE, SERVER_FUNCTION_METADATA, + declareMeta, getServerFunctionMetadata, getServerFunctionRPC, invoke, @@ -160,7 +161,11 @@ export type ServerFunctionInvoker = ( * over earlier ones. */ export interface ServerFunctionMetadata { - /** The declared HTTP method. Undeclared references call over POST. */ + /** + * The declared HTTP method. Undeclared references call over POST. + * Written by the `GET(fn)` declaration alone — `withMeta` refuses it, + * because the grant it reports lives on the wire, not in this bag. + */ readonly method?: "GET" | "POST"; /** * A human-readable label for the function, seeded by development builds @@ -713,6 +718,35 @@ export const BodyFormat = { Void: "9" }; +// Every tag `extractBody` has a case for. Derived from `BodyFormat` itself +// so the two can never drift: a tag added there is readable here the same +// day, and a tag from anywhere else is not. +const READABLE_BODY_FORMATS = new Set(Object.values(BodyFormat)); + +/** + * Whether a message carries a body-format tag THIS build can decode. + * + * The transport's "did the runtime write this answer?" guards ask this, not + * whether the header is present. Presence is not readability: `extractBody` + * matches the tag by value and falls through to `undefined` for anything it + * has no case for, so a tag the reader cannot place used to pass the guards + * and resolve the call as a void success — the phantom success #3173 closed, + * reopened one layer down by a header the runtime never wrote. Two ordinary + * ways in, neither hostile: version skew (a tag past `Void` from a newer + * peer) and a duplicated header, which `Headers.get` joins into the single + * unreadable value `"8, 9"`. An encoding this build cannot read must fail + * as loudly as no encoding at all — the honesty #3110 gave an unknown id. + * + * Transport building block; not meant for hand-written code. + * @internal + */ +export function hasReadableBodyFormat(source: Request | Response): boolean; + +/** Whether a message carries a body-format tag this build can decode. */ +export function hasReadableBodyFormat(source) { + return READABLE_BODY_FORMATS.has(source.headers.get(BODY_FORMAT_HEADER)); +} + // Nesting deeper than this is not JSON-safe. The guard itself walks an // explicit stack so any depth is CHECKABLE, but claiming safety means // JSON.stringify must then deliver. Stringify is recursive and the cliff @@ -729,23 +763,24 @@ const JSON_SAFE_DEPTH_LIMIT = 4096; // never collide with user data. const EXIT = {}; /** * Whether a value survives a `JSON.stringify` round trip faithfully: JSON - * primitives (finite numbers only), arrays, and plain objects. Anything - * else — Dates, Maps, typed arrays, undefined (bare or as a property), - * NaN, class instances, cyclic structures — needs the codec. Never throws: - * cycles and pathological depth answer `false`. Both peers negotiate the - * wire format with this guard: the client for argument lists, the server - * for results. + * primitives (numbers `JSON.stringify` spells faithfully only), arrays, + * and plain objects. Anything else — Dates, Maps, typed arrays, undefined + * (bare or as a property), NaN, `-0`, class instances, cyclic structures — + * needs the codec. Never throws: cycles and pathological depth answer + * `false`. Both peers negotiate the wire format with this guard: the + * client for argument lists, the server for results. */ export function isJSONSafe(value: unknown): boolean; /** * Whether a value survives a `JSON.stringify` round trip faithfully: JSON - * primitives (finite numbers only), arrays, and plain objects. Anything - * else — Dates, Maps, typed arrays, undefined (bare or as a property), - * NaN, class instances, cyclic structures — needs the codec. Both peers - * negotiate with this same guard: the client for argument lists (see - * client.js), the server for results (see server.js encodeResult) — so - * the codec rides the wire exactly when a value actually needs it. + * primitives (numbers `JSON.stringify` spells faithfully only), arrays, + * and plain objects. Anything else — Dates, Maps, typed arrays, undefined + * (bare or as a property), NaN, `-0`, class instances, cyclic structures — + * needs the codec. Both peers negotiate with this same guard: the client + * for argument lists (see client.js), the server for results (see + * server.js encodeResult) — so the codec rides the wire exactly when a + * value actually needs it. * * Traversal is iterative on an explicit stack with an ancestor set: the * old recursive walk overflowed on cycles (forever) and on deep nesting @@ -769,7 +804,14 @@ export function isJSONSafe(value) { const t = typeof v; if (t === "string" || t === "boolean") continue; if (t === "number") { - if (!Number.isFinite(v)) return false; + // A signed zero belongs with NaN and the infinities, not with the + // finite numbers: `JSON.stringify(-0)` is `"0"`, so the fast path + // would carry it and silently flatten the sign — the same class of + // quiet corruption the branches below refuse (`undefined` read back + // as `null`, a sparse hole read back as `null`). The codec spells it + // exactly (seroval has a constant for it), so `-0` rides the codec + // like every other number stringify cannot spell. + if (!Number.isFinite(v) || Object.is(v, -0)) return false; continue; } if (t !== "object") return false; @@ -789,7 +831,18 @@ export function isJSONSafe(value) { // silently losing the stream (async generators dodge this branch only // by prototype). Such values must ride the codec. if (Symbol.asyncIterator in v || Symbol.iterator in v) return false; - for (const k in v) { + // Own names, at any enumerability. `for (const k in v)` asked which + // slots stringify would EMIT; the question this guard answers is which + // slots the value HOLDS, and they are not the same question for a + // channel: `enumerable: false` hides a promise or a stream from both + // roads alike, so hiding it changes nothing about the wire and + // everything about which road the value takes — and this road is the + // one where guardFailures never runs. Nobody then owns the rejection + // (it reaches no handler and takes the process down AFTER the 200 has + // been delivered) and nobody closes the stream when the caller leaves. + // Answering "not safe" hands the value to the codec, where the walk + // reaches the slot on the same terms. + for (const k of Object.getOwnPropertyNames(v)) { // Through the descriptor, never `v[k]`: reading a getter here MINTS // a value nobody guards — a rejecting promise from a getter took // the process down as an unhandled rejection before the codec ever @@ -878,12 +931,85 @@ export function getHeadersAndBody(body) { default: return undefined; } +} /** + * Strips the keys that turn a decoded graph into prototype pollution, in + * place, and hands the same value back. + * + * Both decode roads preserve such a key faithfully — `JSON.parse` creates + * it as an ordinary own property and the codec round-trips it the same way + * — and core itself is unharmed: `Object.prototype` is never touched. What + * the key subverts is the most ordinary thing either side does with a + * decoded value: `Object.assign` merges by [[Set]], so merging one into a + * fresh object re-prototypes the copy with data the peer chose (#3168), + * and a recursive merge walks `constructor.prototype` onto + * `Object.prototype` itself (#3202). + * + * It lives at the decode boundary rather than on one leg because neither + * leg's payload was authored by the side that decodes it, and both arrive + * through this one function. Arguments were the reported half (#3200); a + * result is the same graph travelling the other way — the most ordinary + * handler there is, `async raw => JSON.parse(raw)` over a document its + * user wrote, puts `__proto__` on the wire without a hostile server + * anywhere — and on that leg the pollution lands on the page's single + * shared `Object.prototype`, which every framework internal and + * third-party script on it reads through. One guard, both directions. + * + * This seam already makes decisions of exactly this class — the decode + * depth cap, the argument-count bound, the RegExp exclusion — so the key + * is removed here rather than documented away. + * + * 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, and enumerable properties on revived non-plain objects. Containers + * keep their shape — the codec owns their construction, not this guard. + * Only the value a decode resolves with is walked: a graph that settles + * later, through an async placeholder inside a frame stream, reaches the + * caller as that side's own promise rather than as an own key on the + * delivered value. + * + * Transport building block; the decoders below apply it for you. + * @internal + */ +export function stripUnsafeKeys(value: T): T; + +const UNSAFE_DECODED_KEYS = ["__proto__", "constructor", "prototype"]; + +/** Strips prototype-mutating keys from a decoded graph, in place. */ +export function stripUnsafeKeys(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); + // Mutating in place never required a plain prototype. Strip every + // container, then walk both own metadata and collection contents. + for (const key of UNSAFE_DECODED_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); + } + } + return value; } /** * Decodes a Request/Response body according to its `BODY_FORMAT_HEADER` * tag (falling back to content-type sniffing for form posts that never saw * the client runtime). The inverse of `getHeadersAndBody` + the serialized * stream. Resolves undefined for bodies without a recognized encoding. * + * CONSUMES `source`: the body is read where it lies, not from a clone. The + * decoder holding the real body is what lets it END the message — a framed + * body whose payload is complete cancels its own reader instead of sitting + * on the connection until the peer closes (see `deserializeStream`), and a + * tee branch nobody reads is not left queueing the payload behind it. The + * caller that still needs its own copy hands over one: that is exactly what + * `decodeResponse`, the integration-facing entry, does. + * * Transport building block; use `decodeResponse` from integration code. * @internal */ @@ -900,31 +1026,35 @@ export function extractBody( export async function extractBody(source, codecOptions) { const contentType = source.headers.get("content-type"); const format = source.headers.get(BODY_FORMAT_HEADER); - const clone = source.clone(); switch (true) { + // The two roads that decode to a structured graph are the two roads a + // dangerous key can ride; the rest are bytes, text or a natural HTTP + // encoding with no own keys to strip. Applied here so BOTH legs are + // covered by construction: an argument body and a response body are + // the same two encodings through the same function. case format === BodyFormat.Serialized: - return await deserializeStream(clone, codecOptions); + return stripUnsafeKeys(await deserializeStream(source, codecOptions)); case format === BodyFormat.Json: - return JSON.parse(await clone.text()); + return stripUnsafeKeys(JSON.parse(await source.text())); case format === BodyFormat.String: - return await clone.text(); + return await source.text(); case format === BodyFormat.File: { - const formData = await clone.formData(); + const formData = await source.formData(); return formData.get(FILE_FORM_KEY); } case format === BodyFormat.FormData: case contentType && contentType.startsWith("multipart/form-data"): - return await clone.formData(); + return await source.formData(); case format === BodyFormat.URLSearchParams: case contentType && contentType.startsWith("application/x-www-form-urlencoded"): - return new URLSearchParams(await clone.text()); + return new URLSearchParams(await source.text()); case format === BodyFormat.Blob: - return await clone.blob(); + return await source.blob(); case format === BodyFormat.ArrayBuffer: - return await clone.arrayBuffer(); + return await source.arrayBuffer(); case format === BodyFormat.Uint8Array: - return new Uint8Array(await clone.arrayBuffer()); + return new Uint8Array(await source.arrayBuffer()); } return undefined; @@ -1047,6 +1177,18 @@ export class ChunkReader { interpret(result.value); } } + + /** + * Stops reading and releases the body. On a response body that is a live + * connection this is what ENDS the call rather than abandoning it: the + * body is the fetch, and cancelling it is the only way a reader holding + * the lock can give the socket back. Rejections are swallowed — a body + * already errored is a body already released. + */ + cancel() { + this.done = true; + return this.reader.cancel().catch(() => {}); + } } // A codec frame's payload is `JSON.stringify` of a SerovalNode — it always @@ -1205,6 +1347,33 @@ export async function deserializeStream(source, codecOptions) { return deserializeChunk(JSON.parse(chunk)); } + // The head chunk is interpreted before the drain is wired, because what + // it decodes to is what decides whether there is anything left to read. + let value; + try { + value = interpretChunk(result.value); + } catch (error) { + // The head did not decode, so the answer is already final — no later + // chunk has anything to say to a value that was never delivered. + reader.cancel(); + throw error; + } + + // A head chunk that left NOTHING waiting on a later one is the whole + // answer, and reading on would only be waiting for an end of body the + // peer owes us. Ending the read here is what makes a finished call give + // its connection back: a peer that holds the socket after the payload — + // a proxy, a CDN, a hung origin — otherwise keeps one connection per + // SUCCESSFUL call, and six of those wedge a browser's per-origin pool + // while every one of them reported success. The decision is the + // payload's, never the peer's, which is the same standard the streaming + // result is held to. Cancelling reaches the caller's body because + // `extractBody` reads the real one rather than a clone. + if (!deserializeChunk.pending()) { + reader.cancel(); + return value; + } + // Failure wiring for the drain: a network drop or malformed frame must // fail every value still waiting on later chunks — otherwise their // promises hang forever and open streams never terminate (and the drain @@ -1218,7 +1387,7 @@ export async function deserializeStream(source, codecOptions) { error => deserializeChunk.abort(error) ); - return interpretChunk(result.value); + return value; } return undefined; } /** @@ -1261,7 +1430,14 @@ export function decodeResponse( */ export async function decodeResponse(response, codecOptions) { if (!response.body) return undefined; - return await extractBody(response, codecOptions === undefined ? codecConfig.codec : codecOptions); + // The clone lives HERE, at the entry whose contract promises it: an + // integration hands over a response it still owns. The transport, which + // owns the body it opened, calls `extractBody` directly — a clone there + // buys nothing and costs the whole payload in a tee branch nobody reads. + return await extractBody( + response.clone(), + codecOptions === undefined ? codecConfig.codec : codecOptions + ); } /** * `decodeResponse` plus the single-flight envelope split: when the response * carries the single-flight header the decoded `{ value, data }` payload is diff --git a/packages/web/src/cookies.ts b/packages/web/src/cookies.ts index cda8911c9..5948d7586 100644 --- a/packages/web/src/cookies.ts +++ b/packages/web/src/cookies.ts @@ -165,10 +165,44 @@ function assertServableCookie(name: string, options: CookieOptions): void { // the transport + codec, which a router-only app never ships. The codec // that fills and decodes the cookie is server-only and stays behind the // server-functions server entry (server-functions/flash.js). -export const FLASH_COOKIE = "flash"; +// +// The name carries the `__Host-` prefix because the cookie carries the +// SUBMISSION — a no-JS login form's password rides it as plaintext JSON +// (see the codec's own note on why it is plain JSON) — and the prefix is +// what bounds who can produce one: browsers refuse a `__Host-` cookie that +// names a `Domain`, so a sibling subdomain cannot toss a second `flash` +// entry at the app. It has to be the NAME, not a check at the read: a +// Cookie header is one string and `parseCookieHeader` cannot tell a +// sibling's entry from the app's own, so last-wins hands the render the +// attacker's outcome. The prefix's price is `Secure`, which the encoder +// already required: a no-JS outcome never reaches a plain-http origin, and +// localhost is potentially-trustworthy, so development is unaffected. +export const FLASH_COOKIE = "__Host-flash"; const FLASH_MATCHER = new RegExp(`(?:^|;\\s*)${FLASH_COOKIE}=([^;]+)`); +// Written ONCE, for both directions. The prefix rules bind the set and the +// clear together — a `__Host-` deletion cookie without `Secure` is +// rejected on arrival exactly like the cookie it meant to delete (#3138), +// silently, and the outcome then haunts every later request — so the two +// must not be spelled twice and allowed to drift. `SameSite=Lax` keeps a +// cross-site post's outcome out of the jar entirely. +// +// Spelled out rather than run through `serializeCookie` for the same +// reason the matcher above is a regex and not `parseCookieHeader`: an +// integration that only clears the cookie must not drag the pair codec +// into its client bundle. Nothing here is caller-supplied, so there is no +// combination for that function's dev-time prefix check to catch. +const FLASH_ATTRIBUTES = "Path=/; HttpOnly; Secure; SameSite=Lax"; + +// One redirect's worth of life. Clearing is the integration's (it consumes +// the cookie eagerly per request, see below), but no integration is +// REQUIRED to exist, and without a lifetime an unread outcome is a session +// cookie at `Path=/`: the submission, in the clear, attached to every +// subsequent request to the origin — assets included — and into every +// access and CDN log, for as long as the browser lives. +const FLASH_MAX_AGE = 60; + /** Whether a Cookie header carries a flash cookie (readable or not). */ export function hasFlashCookie(cookieHeader: string | null): boolean { return !!cookieHeader && FLASH_MATCHER.test(cookieHeader); @@ -180,7 +214,15 @@ export function matchFlashCookie(cookieHeader: string | null): string | undefine return match ? match[1] : undefined; } +/** + * The `Set-Cookie` value that carries `value` as the flash cookie: the one + * writer, shared with the clear below so the attributes cannot drift apart. + */ +export function writeFlashCookie(value: string): string { + return `${FLASH_COOKIE}=${encodeURIComponent(value)}; ${FLASH_ATTRIBUTES}; Max-Age=${FLASH_MAX_AGE}`; +} + /** The Set-Cookie value clearing the flash cookie after it has been read. */ export function clearFlashCookie(): string { - return `${FLASH_COOKIE}=; Max-Age=0; Path=/`; + return `${FLASH_COOKIE}=; ${FLASH_ATTRIBUTES}; Max-Age=0`; } diff --git a/packages/web/test/server/server-functions-abort-conforming-length.spec.tsx b/packages/web/test/server/server-functions-abort-conforming-length.spec.tsx new file mode 100644 index 000000000..3d92a6efb --- /dev/null +++ b/packages/web/test/server/server-functions-abort-conforming-length.spec.tsx @@ -0,0 +1,180 @@ +/** + * A client that hangs up mid-upload must settle the request and tear down + * the upload source — on the road that carries the traffic. + * + * Fetch does not couple a `Request`'s signal to its body stream, so nothing + * wakes a pending `read()` when the host abandons the request: the handler + * never settles and the source is never cancelled (#3217/#3219). The fix in + * e220cfae wires `request.signal` to the reader — but it wires it inside + * `bufferBodyWithin`, which dispatch only reaches under the + * `if (!(declared > 0))` gate. So the coupling is installed exactly on the + * bodies that declared no length (chunked uploads), and skipped for a + * conforming `Content-Length` — which is what every ordinary browser POST, + * `fetch` with a string/FormData body, and the shipped client stub sends. + * The leak was fixed on the side road and left open on the main one. + * + * Observed on HEAD with the abort fired 60 ms into a stalled upload and + * 800 ms of grace: + * + * no content-length : settled=400 source cancelled, reason AbortError + * content-length=200: settled=PENDING source live, never cancelled + * + * A never-settling handler is not a slow one: the connection's request task, + * its buffered chunks and the upload source all stay resident for as long as + * the process lives, and a peer that can abort can open another. That is the + * whole point of #3218 — it just has to hold for both declarations, since + * the abort has nothing to do with how the body's length was framed. + * + * These pin the coupling, not a buffering strategy: whatever a fix decides + * to do with the body, it must install the coupling on the road that + * declared a length too. What it may not buy the coupling with is the + * declaration check itself — "refuses a declared length past the limit + * without reading the body" in server-functions-request-bounds pins that a + * peer announcing an oversized payload is still answered before a byte is + * read, and the sibling spec (server-functions-body-cap-declaration-trust) + * pins from the other side that the bound is measured on the bytes that + * arrive. + * + * The settled answer is spelled 400 to match the undeclared road: an upload + * that ended early is an incomplete argument encoding, not a handler + * failure (#3217). + * + * 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, vi } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); +const BODY_FORMAT_HEADER = "X-Server-Function-Format"; +const JSON_FORMAT = "8"; + +const dispatched = vi.fn(async () => "reached"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); + registerServerFunction("abort-coupling-sink", dispatched); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const PENDING = Symbol("pending"); + +async function within(promise: Promise, ms: number) { + let timer!: ReturnType; + const outcome = await Promise.race([ + promise, + new Promise(resolve => { + timer = setTimeout(() => resolve(PENDING), ms); + }) + ]); + clearTimeout(timer); + return outcome; +} + +/** + * Starts an upload that enqueues one byte and then stalls forever, aborts + * the request once dispatch is actually reading it, and reports what the + * runtime did with the abort. + */ +async function abortMidUpload(declaration: string | null) { + dispatched.mockClear(); + const abort = new AbortController(); + let sourceController!: ReadableStreamDefaultController; + let cancelled = false; + let cancelReason: any = null; + let signalPullStarted!: () => void; + const pullStarted = new Promise(resolve => (signalPullStarted = resolve)); + const body = new ReadableStream({ + start(controller) { + sourceController = controller; + controller.enqueue(new Uint8Array([91])); // "[" + }, + pull() { + // the upload the client is still sending when it disappears + signalPullStarted(); + return new Promise(() => {}); + }, + cancel(reason) { + cancelled = true; + cancelReason = reason; + } + }); + const headers: Record = { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + [BODY_FORMAT_HEADER]: JSON_FORMAT + }; + // A conforming declaration is the ONLY difference between the two rows. + if (declaration !== null) headers["content-length"] = declaration; + + const pending = handleServerFunctionRequest( + new Request("https://app.example/_server/data/abort-coupling-sink", { + method: "POST", + body, + duplex: "half", + signal: abort.signal, + headers + } as RequestInit) + ).then( + response => `status=${response.status}` as const, + error => `threw=${error?.name ?? error}` as const + ); + + await pullStarted; + await new Promise(resolve => setTimeout(resolve, 60)); + abort.abort(new DOMException("client gone", "AbortError")); + + const outcome = await within(pending, 800); + // Release the stalled source so a failing row cannot leave the reader (or + // the test run) parked, and so the assertions below describe the state at + // the deadline rather than after cleanup. + if (outcome === PENDING) { + sourceController.error(new Error("test cleanup")); + await within(pending, 1000); + } + return { + settled: outcome === PENDING ? "PENDING" : outcome, + ran: dispatched.mock.calls.length, + cancelled, + reason: cancelReason?.name ?? (cancelReason === null ? "none" : String(cancelReason)) + }; +} + +function row( + declaration: string | null, + r: { settled: string; ran: number; cancelled: boolean; reason: string } +) { + return `content-length ${declaration ?? "(absent)"}: settled=${r.settled} ran=${ + r.ran + } sourceCancelled=${r.cancelled} cancelReason=${r.reason}`; +} + +describe("an aborted upload", () => { + it("settles the request and cancels the source whether or not the body declared a length", async () => { + const undeclared = await abortMidUpload(null); + const declared = await abortMidUpload("200"); + // Rendered as a pair so the failure names the asymmetry itself: the + // undeclared row is the behaviour #3218 already secured, the declared + // row is the same request on the road the browsers use. + expect([row(null, undeclared), row("200", declared)]).toEqual([ + "content-length (absent): settled=status=400 ran=0 sourceCancelled=true cancelReason=AbortError", + "content-length 200: settled=status=400 ran=0 sourceCancelled=true cancelReason=AbortError" + ]); + }); + + it("does not park the handler forever when the body declared a length", async () => { + // The half of the invariant that costs a process: even setting the + // cancellation aside, the response promise must resolve. On HEAD this + // one never does. + const declared = await abortMidUpload("200"); + expect(row("200", declared)).not.toContain("settled=PENDING"); + expect(declared.settled).toBe("status=400"); + }); +}); diff --git a/packages/web/test/server/server-functions-body-cap-declaration-trust.spec.tsx b/packages/web/test/server/server-functions-body-cap-declaration-trust.spec.tsx new file mode 100644 index 000000000..122d138dd --- /dev/null +++ b/packages/web/test/server/server-functions-body-cap-declaration-trust.spec.tsx @@ -0,0 +1,132 @@ +/** + * The body cap must bound the bytes the runtime actually buffers, not the + * bytes a peer SAYS it will send. + * + * `bodySizeLimit` exists because the argument payload is buffered and + * decoded before dispatch, so its cost is paid before application code can + * decline it (#3115). An over-declaration is refused before a byte is read + * — that much a declaration is good for — and the bound itself is taken by + * the counting read in `bufferBodyWithin`, which stops at the limit. It was + * the declaration that decided which of those happened: the gate read + * `if (!(declared > 0))`, so ANY positive digit string was trusted and the + * counting read skipped entirely, and `Content-Length: 10` on a 2 MiB body + * against a 1 MiB cap dispatched the whole 2 MiB into the function. + * + * The defence is internally inconsistent about the same header from the + * same untrusted producer: #3153 already established that a `-1` must not + * be believed — "an adapter that builds the Request itself, or a rewriting + * proxy, delivers it here" — and routed it through the cap. A `10` on a + * 2 MiB body is the same lie by the same producer, and is believed. + * + * This is defence in depth, not a live bypass: stock `node:http` frames the + * body BY the declaration and rejects `Content-Length` + `Transfer-Encoding` + * together, so llhttp truncates such a request at 10 bytes long before this + * code sees it. The exposure is every producer that builds the `Request` + * itself and is not llhttp — adapters, proxies, test harnesses, WinterCG + * runtimes — which is exactly the population #3153 named when it decided a + * non-conforming declaration is not evidence. The invariant pinned here is + * the one the cap advertises and the one a reader assumes: no more than + * `bodySizeLimit` bytes reach the decoder, whatever the header claims. + * + * The refusal is spelled 413 because that is what the counting read already + * answers for an undeclared body of the same size; the substantive half of + * each assertion is that the oversized payload never reached the function. + * + * 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"; + +const RequestContext = Symbol.for("solid.RequestContext"); +const BODY_FORMAT_HEADER = "X-Server-Function-Format"; +const JSON_FORMAT = "8"; +const LIMIT = 1024 * 1024; + +let received: number | null = null; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); + registerServerFunction("cap-declaration-sink", async (payload: unknown) => { + received = typeof payload === "string" ? payload.length : -1; + return "reached"; + }); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +// A 2 MiB argument string — twice the cap the calls below configure — and +// its well-behaved counterpart, an ordinary 4 KiB POST. +const oversized = JSON.stringify(["x".repeat(2 * LIMIT)]); +const modest = JSON.stringify(["y".repeat(4096)]); + +async function post(body: string, declaration: string | null) { + received = null; + const headers: Record = { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + [BODY_FORMAT_HEADER]: JSON_FORMAT + }; + // undici only computes Content-Length at fetch time, so a Request built + // here declares exactly what this line declares, and nothing when it + // declares nothing — the two roads through the gate, side by side. + if (declaration !== null) headers["content-length"] = declaration; + const response = await handleServerFunctionRequest( + new Request("https://app.example/_server/data/cap-declaration-sink", { + method: "POST", + body, + headers + }), + { bodySizeLimit: LIMIT } + ); + return { status: response.status, reachedFunction: received }; +} + +// One row per declaration so a failure names which declarations were +// believed rather than only the first. +const label = (declaration: string | null) => `Content-Length: ${declaration ?? "(absent)"}`; + +function row(declaration: string | null, r: { status: number; reachedFunction: number | null }) { + return `${label(declaration)} -> status=${r.status} bytesReachingFunction=${ + r.reachedFunction === null ? "none" : r.reachedFunction + }`; +} + +describe("the body cap against an under-declared Content-Length", () => { + it("bounds the bytes it buffers by what arrives, not by what the declaration claims", async () => { + // One table, because the controls are the point as much as the repros: + // an honest declaration under the cap must still dispatch, intact, and + // an honest over-declaration must still be refused before a byte is + // read. Closing the hole may not cost either. + const cases: Array<[string | null, string, string]> = [ + // declaration body expected row + [null, oversized, "status=413 bytesReachingFunction=none"], + ["0", oversized, "status=413 bytesReachingFunction=none"], + ["10", oversized, "status=413 bytesReachingFunction=none"], + ["1024", oversized, "status=413 bytesReachingFunction=none"], + [String(LIMIT), oversized, "status=413 bytesReachingFunction=none"], + ["999999999999", oversized, "status=413 bytesReachingFunction=none"], + // an honest declaration of the oversized body: refused before the read + [String(Buffer.byteLength(oversized)), oversized, "status=413 bytesReachingFunction=none"], + // the control: an ordinary browser POST under the cap, delivered whole + [String(Buffer.byteLength(modest)), modest, "status=200 bytesReachingFunction=4096"] + ]; + const rows: string[] = []; + for (const [declaration, body] of cases) { + rows.push(row(declaration, await post(body, declaration))); + } + // Observed on HEAD: every positive declaration smaller than the body is + // believed, so `Content-Length: 10` answers 200 with the whole + // 2097152-byte argument delivered to the function — the cap is not + // merely late, it is off. + expect(rows).toEqual( + cases.map(([declaration, , expected]) => `${label(declaration)} -> ${expected}`) + ); + }); +}); diff --git a/packages/web/test/server/server-functions-connection-teardown.spec.tsx b/packages/web/test/server/server-functions-connection-teardown.spec.tsx new file mode 100644 index 000000000..c1eb6aaea --- /dev/null +++ b/packages/web/test/server/server-functions-connection-teardown.spec.tsx @@ -0,0 +1,213 @@ +/** + * A call that is over must END its connection, and it must decide that from + * the payload rather than from the peer closing the body. + * + * The transport mints an AbortController per call precisely so a streaming + * result can be ENDED and not merely abandoned — aborting the fetch closes + * the response body here and fires `request.signal` on the server. That + * controller is wired into exactly one place: the `return()` of the + * async-iterator wrapper, the leg a `break` in a `for await` walks. Every + * other way a call finishes leaves it unfired, so nothing ever cancels the + * reader; and because the codec's ChunkReader holds the body lock, the + * application cannot cancel it either. The connection stays open for as + * long as the peer keeps it open. + * + * That is not a leak while the peer behaves — a server that ends its body + * ends the connection. The trigger is any peer that does not: a hung + * origin, a proxy, a CDN holding the socket after the payload. In a browser + * over HTTP/1.1 the six-connections-per-origin cap turns six such calls + * into a wedged origin while every one of them reported success. + * + * Two ends are pinned here, both cases where NOTHING is outstanding: + * + * - a streamed result drained to completion. `for await` calls `return()` + * on a `break` but not on a natural end, so the guard that exists on the + * abandoned leg was never mirrored onto the finished one — the same call, + * consumed to the last item, keeps its connection. + * - a result that is not an async iterable at all. Once the head chunk has + * been interpreted and the value holds no unsettled references, there is + * nothing left for a later chunk to say. + * + * A result still awaiting values — a promise inside it that has not + * resolved — is deliberately NOT pinned: that call is not over, and its + * connection is load-bearing. + * + * The transport only owns the signal when the caller brought none; a + * caller-supplied signal already owns the wire and cancellation stays + * theirs, which is the escape hatch these calls do not have. + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createServerReference } 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]; +}); + +type Connections = { + /** Bodies handed to the transport. */ + opened: number; + /** Bodies the transport cancelled, or that `init.signal` closed. */ + cancelled: number; + /** Bodies the peer itself finished. */ + endedByPeer: number; + readonly open: number; +}; + +const disconnects: (() => void)[] = []; +afterEach(() => { + while (disconnects.length) disconnects.pop()!(); +}); + +/** + * A transport that behaves like a connection rather than a buffer: the + * response body is a socket, `init.signal` closes it the way a browser's + * fetch does, and `hold` models the peer that never sends the terminating + * frame — the payload arrives complete and the socket stays open behind it. + */ +function connectTransport({ hold = false }: { hold?: boolean } = {}): Connections { + const original = globalThis.fetch; + const counts: Connections = { + opened: 0, + cancelled: 0, + endedByPeer: 0, + get open() { + return this.opened - this.cancelled - this.endedByPeer; + } + }; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const address = input instanceof Request ? input.url : input.toString(); + const request = new Request( + new URL(address, "http://localhost"), + input instanceof Request ? input : init + ); + request.headers.set("Sec-Fetch-Site", "same-origin"); + const upstream = await handleServerFunctionRequest(request); + if (!upstream.body) return upstream; + counts.opened++; + const reader = upstream.body.getReader(); + let settled = false; + const close = () => { + if (settled) return; + settled = true; + counts.cancelled++; + reader.cancel().catch(() => {}); + }; + init?.signal?.addEventListener("abort", close); + const body = new ReadableStream({ + async pull(controller) { + const { done, value } = await reader.read(); + if (done) { + // the peer holding on: the payload is all there, the socket is not + // closed, and only the reader's own cancel can reclaim it + if (hold) return new Promise(() => {}); + if (!settled) { + settled = true; + counts.endedByPeer++; + } + controller.close(); + return; + } + controller.enqueue(value); + }, + cancel: close + }); + return new Response(body, { status: upstream.status, headers: upstream.headers }); + }) as typeof fetch; + disconnects.push(() => { + globalThis.fetch = original; + }); + return counts; +} + +/** Lets the transport's own teardown, if any, run before the count is read. */ +const settle = () => new Promise(resolve => setTimeout(resolve, 50)); + +describe("server-function connection teardown", () => { + it("ends the connection when a streamed result is drained to its last item", async () => { + registerServerFunction("teardown-stream", async function* () { + yield 1; + yield 2; + yield 3; + }); + + // The leg that works, kept here because it is what makes the other one a + // defect rather than a design: abandoning the stream fires the + // controller through the wrapper's `return()`. + const abandoned = connectTransport({ hold: true }); + for await (const value of (await createServerReference( + "teardown-stream" + )()) as AsyncIterable) { + expect(value).toBe(1); + break; + } + await settle(); + expect({ ...abandoned, open: abandoned.open }).toMatchObject({ + opened: 1, + cancelled: 1, + open: 0 + }); + + // The same call consumed to the end. `for await` calls `return()` only + // on an early exit, so the finished stream — which has strictly less + // left to say than the abandoned one — keeps its connection forever. + const drained = connectTransport({ hold: true }); + const seen: number[] = []; + for await (const value of (await createServerReference( + "teardown-stream" + )()) as AsyncIterable) { + seen.push(value); + } + expect(seen).toEqual([1, 2, 3]); + await settle(); + expect({ ...drained, open: drained.open }).toMatchObject({ + opened: 1, + cancelled: 1, + open: 0 + }); + }); + + it("ends the connection when the result is not an async iterable", async () => { + // A `Date` needs the streaming codec (JSON cannot carry it), so the call + // reads a framed body — but the value holds no unsettled reference, so + // the head chunk is the whole answer and no later chunk can add to it. + registerServerFunction("teardown-value", async () => new Date(0)); + + const held = connectTransport({ hold: true }); + const value = await createServerReference("teardown-value")(); + expect(value).toBeInstanceOf(Date); + expect((value as Date).getTime()).toBe(0); + await settle(); + expect({ ...held, open: held.open }).toMatchObject({ opened: 1, cancelled: 1, open: 0 }); + }); + + it("does not wedge a browser's per-origin connection pool", async () => { + // HTTP/1.1 allows six connections per origin. Six successful calls + // against a peer that holds its sockets must not be able to stop the + // seventh — every one of these resolved, so nothing in the application + // has any reason to suspect the origin is now unreachable. + registerServerFunction("teardown-pool", async () => new Date(0)); + const pool = connectTransport({ hold: true }); + for (let call = 0; call < 8; call++) { + expect(await createServerReference("teardown-pool")()).toBeInstanceOf(Date); + } + await settle(); + expect( + pool.open, + `${pool.open} of ${pool.opened} connections still open after 8 resolved calls` + ).toBe(0); + }); +}); diff --git a/packages/web/test/server/server-functions-construct-trap.spec.tsx b/packages/web/test/server/server-functions-construct-trap.spec.tsx new file mode 100644 index 000000000..2c18a141e --- /dev/null +++ b/packages/web/test/server/server-functions-construct-trap.spec.tsx @@ -0,0 +1,134 @@ +/** + * `new fn()` is a call, and every road into a server function body has to + * be one road. + * + * `createServerReference` returns a Proxy over the user's function with a + * `get` trap (identity, metadata, the invoke channel) and an `apply` trap. + * The apply trap is where the whole server-side contract lives: the "cannot + * call a server function outside of a request" guard, the derived event + * with its copied locals, the invocation identity, `wrapInvocation`, and + * `transformDirectResult`. + * + * A Proxy with no `construct` trap forwards construction straight to the + * target. So `new fn()` — or `Reflect.construct(fn, args)`, which is what a + * generic dispatcher, a DI container, or a serializer reviving a value + * reaches for — runs the body having consulted none of it: no request + * scope, no event, and no authorization hook. It is the one entry that + * skips even the outside-a-request guard, so it does not merely bypass + * policy, it runs the body somewhere policy could not have been evaluated + * in the first place. + * + * The invariant: the body is reachable through the apply trap or not at + * all. Whether construction is routed through it or refused outright, it + * cannot be a second, unguarded entrance. + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + configureServerFunctionsServer, + createServerReference as createServerSideReference, + registerServerReference +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +/** + * `configure` ignores `undefined` — that is its spelling of "not + * overriding" — so undoing a hook between tests takes a value. `null` is + * the falsy "nothing configured" every read site tests for; the cast is + * only because the option type describes hooks, not their absence. + */ +const NO_HOOK = null as any; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterEach(() => { + configureServerFunctionsServer({ wrapInvocation: NO_HOOK }); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +/** Runs `fn` under an established request scope, as a render would. */ +function underRender(fn: () => T): T { + const storage = (globalThis as any)[RequestContext] as AsyncLocalStorage; + return storage.run({ request: new Request("https://app.example/page"), locals: {} }, fn); +} + +describe("construction is not a second entrance to the body", () => { + it("refuses `new fn()` outside a request, as calling it does", () => { + let bodyRan = 0; + const call = createServerSideReference( + registerServerReference("construct-outside", function () { + bodyRan++; + }) + ); + + // the road next to it is guarded; this one is not. The message is left + // to the fix — routing construction through the apply trap raises the + // existing "outside of a request" guard, refusing it outright says so + // in its own words — what is pinned is that the body is not reached. + let threw = false; + try { + new (call as any)(); + } catch { + threw = true; + } + // today: { threw: false, bodyRan: 1 } — the body ran with no request + // event in scope + expect({ threw, bodyRan }).toStrictEqual({ threw: true, bodyRan: 0 }); + }); + + it("does not let `new fn()` walk around the authorization gate inside a request", () => { + let bodyRan = 0; + let gateRan = 0; + configureServerFunctionsServer({ + wrapInvocation: () => { + gateRan++; + throw new Error("policy denied"); + } + }); + const call = createServerSideReference( + registerServerReference("construct-gated", function (this: any) { + bodyRan++; + this.secret = "the secret"; + }) + ); + + // whichever way construction is answered — routed through the apply + // trap, or refused as not-a-call — it must not reach the body + let threw = false; + try { + underRender(() => new (call as any)()); + } catch { + threw = true; + } + // today: { threw: false, bodyRan: 1 }, and the gate never saw the call. + // `gateRan` is deliberately not asserted: 1 if construction routes + // through the apply trap, 0 if it is refused before one exists. + expect({ threw, bodyRan, gateRan }).toStrictEqual({ threw: true, bodyRan: 0, gateRan }); + }); + + it("refuses Reflect.construct too — the spelling a generic caller uses", () => { + let bodyRan = 0; + const call = createServerSideReference( + registerServerReference("construct-reflect", function () { + bodyRan++; + }) + ); + + let threw = false; + try { + Reflect.construct(call as any, []); + } catch { + threw = true; + } + expect({ threw, bodyRan }).toStrictEqual({ threw: true, bodyRan: 0 }); + }); +}); diff --git a/packages/web/test/server/server-functions-direct-provide-event-once.spec.tsx b/packages/web/test/server/server-functions-direct-provide-event-once.spec.tsx new file mode 100644 index 000000000..30a80ec1f --- /dev/null +++ b/packages/web/test/server/server-functions-direct-provide-event-once.spec.tsx @@ -0,0 +1,131 @@ +/** + * `provideEvent` must call the function exactly once — on BOTH dispatch + * legs, not only over HTTP. + * + * The HTTP tail counts invocations at the seam and refuses a second one + * before the body re-enters, "a second invocation would commit the call's + * side effects twice", and refuses zero, because a hook that never invoked + * the callback would answer as a void success indistinguishable from a + * function that returned nothing. Both land on dispatch's catch as a + * sanitized 500. + * + * `createServerReference`'s apply trap — the direct SSR leg — calls the + * same host-supplied hook with no such count. The hook is one object + * installed once by the adapter, so a hook broken in either direction is + * broken for every call the process makes; the leg that catches it is the + * only difference. Today that means the same defective adapter is a clean + * 500 on the wire and a silent double-commit during a render — the leg + * with no client, no status line and no log to notice it by — and a hook + * that skips the callback hands the render `undefined` as a successful + * value, which then flows into the page as if the function had returned + * nothing. + * + * The invariant: the exactly-once guard belongs to the hook contract, so it + * has to hold wherever the hook is honored. Also pinned: the guard must not + * cost the direct leg its transparency — a synchronous function called + * during a render still returns its value, not a promise (see + * `server-functions-invocation-wrap.spec.tsx`). + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + configureServerFunctionsServer, + createServerReference as createServerSideReference, + registerServerReference +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +/** + * `configure` ignores `undefined` — that is its spelling of "not + * overriding" — so undoing a hook between tests takes a value. `null` is + * the falsy "nothing configured" every read site tests for; the cast is + * only because the option type describes hooks, not their absence. + */ +const NO_HOOK = null as any; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterEach(() => { + configureServerFunctionsServer({ provideEvent: NO_HOOK }); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +/** Runs `fn` under an established request scope, as a render would. */ +function underRender(fn: () => T): T { + const storage = (globalThis as any)[RequestContext] as AsyncLocalStorage; + return storage.run({ request: new Request("https://app.example/page"), locals: {} }, fn); +} + +describe("the exactly-once contract on the direct SSR leg", () => { + it("refuses a hook that invokes the function twice, before the body runs again", () => { + let bodyRan = 0; + configureServerFunctionsServer({ + // the shape a retry wrapper or a misplaced await produces + provideEvent: (event, fn) => { + fn(); + return fn(); + } + }); + const call = createServerSideReference( + registerServerReference("provide-twice-direct", () => { + bodyRan++; + return bodyRan; + }) + ); + + // today: no throw at all — the body commits twice and the render is + // handed the second run's value as an ordinary success. The message is + // the HTTP tail's, which already names the hook and says why the second + // invocation is refused; a render has even less context than a request + // log, so it needs it more. + expect(() => underRender(() => (call as any)())).toThrow(/more than once/); + expect(bodyRan).toBe(1); + }); + + it("refuses a hook that never invokes the function, instead of answering undefined", () => { + let bodyRan = 0; + configureServerFunctionsServer({ + // off-contract on purpose, so the cast is the test's subject + provideEvent: (() => undefined) as any + }); + const call = createServerSideReference( + registerServerReference("provide-never-direct", () => { + bodyRan++; + return "the value"; + }) + ); + + // today: returns undefined, which a render cannot tell from a function + // that returned nothing + expect(() => underRender(() => (call as any)())).toThrow(/without invoking/); + expect(bodyRan).toBe(0); + }); + + it("still returns a synchronous function's value synchronously under a correct hook", () => { + let bodyRan = 0; + configureServerFunctionsServer({ + provideEvent: (event, fn) => fn() + }); + const call = createServerSideReference( + registerServerReference("provide-once-direct", (n: number) => { + bodyRan++; + return n * 2; + }) + ); + + // the guard must not turn the direct leg async or wrap its result: + // paired with the negatives above so a guard that refuses everything + // cannot go green + expect(underRender(() => (call as any)(21))).toBe(42); + expect(bodyRan).toBe(1); + }); +}); diff --git a/packages/web/test/server/server-functions-flash-cookie-attributes.spec.tsx b/packages/web/test/server/server-functions-flash-cookie-attributes.spec.tsx new file mode 100644 index 000000000..0667c62a0 --- /dev/null +++ b/packages/web/test/server/server-functions-flash-cookie-attributes.spec.tsx @@ -0,0 +1,111 @@ +/** + * The attributes the flash cookie is written with — the half of the no-JS + * leg the browser enforces, and the half nothing in this package can + * observe once it is wrong. + * + * What rides this cookie is not a status line: it is the SUBMISSION. A + * no-JS login form flashes `input` verbatim, so the user's password sits in + * the value as plaintext JSON. The codec is deliberate about that being + * plaintext (flash.ts: "The payload is plain JSON rather than the wire + * codec") and confidentiality is the caller's, but plaintext raises the bar + * on the three attributes that decide WHO the browser hands it back to and + * FOR HOW LONG, and today the encoder sets none of them: + * + * - No `SameSite`, so a cross-site form post's outcome is stored and + * replayed by the ordinary defaults an app never sees. + * - No `Max-Age` and no `Expires`, so it is a SESSION cookie at `Path=/`: + * it is attached to every subsequent request to the origin — scripts, + * images, fonts — and lands in access and CDN logs, for as long as the + * browser lives. Clearing it is delegated to the integration + * (`clearFlashCookie`, cookies.ts), and no integration is required to + * exist; an app that reads the cookie by hand keeps it forever. + * - No `__Host-` prefix, though this codebase knows the prefix well + * enough to refuse cookies that break it (cookies.ts + * `assertServableCookie`). Without host-locking, a sibling subdomain + * can toss a `Domain`-scoped `flash` at the app; `parseCookieHeader` is + * last-wins, so the tossed one can displace the real outcome and the + * app renders an attacker's "result" as its own. `clearFlashCookie` + * carries no `Domain`, so a tossed cookie is never cleared either — the + * prefix closes both, which is why it is the fix rather than a + * domain-guessing clear. + * + * The prefix is not free: `__Host-` requires `Secure`, so the flash never + * reaches a plain-http origin other than localhost. The encoder already + * hardcodes `secure: true`, so that is the status quo, not a regression. + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { describe, expect, it } from "vitest"; +import { + FLASH_COOKIE, + clearFlashCookie, + decodeFlashCookie, + encodeFlashCookie +} from "@solidjs/web/server-functions/server"; + +/** The attribute list of a Set-Cookie value, lowercased for matching. */ +function attributesOf(setCookie: string) { + return setCookie + .split(";") + .slice(1) + .map(part => part.trim().toLowerCase()); +} + +function hasAttribute(setCookie: string, name: string) { + return attributesOf(setCookie).some(part => part === name || part.startsWith(`${name}=`)); +} + +/** A no-JS login post, as the encoder receives it. */ +function loginOutcome() { + const form = new FormData(); + form.set("email", "ada@example.com"); + form.set("password", "correct-horse-battery-staple"); + return encodeFlashCookie("/_server/log-in", { welcome: "Ada" }, [form]); +} + +describe("the flash cookie is written so the browser can bound it", () => { + it("names a SameSite, so a cross-site post's outcome is not stored unasked", () => { + const cookie = loginOutcome(); + expect(attributesOf(cookie)).toContain("httponly"); // the one it does set + expect(hasAttribute(cookie, "samesite")).toBe(true); + }); + + it("carries a lifetime, so an unread outcome does not ride the whole session", () => { + // the value the browser would keep sending: it is the submission, in + // the clear, on every request to the origin including assets + const cookie = loginOutcome(); + expect(decodeURIComponent(cookie)).toContain("correct-horse-battery-staple"); + expect(hasAttribute(cookie, "max-age") || hasAttribute(cookie, "expires")).toBe(true); + }); + + it("is host-locked, so a sibling subdomain cannot toss one at the app", () => { + // evil.app.example sets `flash=...; Domain=app.example; Path=/` and the + // app receives TWO entries of the same name. Last-wins is not the bug — + // it is the RFC's own reading and cannot be legislated away in the + // parser, which sees one header and cannot tell the entries apart: + const shadowed = decodeFlashCookie( + `${FLASH_COOKIE}=${encodeURIComponent(JSON.stringify({ url: "/x", result: "ours" }))}; ` + + `${FLASH_COOKIE}=${encodeURIComponent(JSON.stringify({ url: "/x", result: "theirs" }))}` + ); + expect(shadowed?.result).toBe("theirs"); + + // so the collision has to be made impossible upstream, and the platform + // already has the mechanism: `__Host-` forbids `Domain`, which locks a + // sibling's cookie to the sibling and leaves exactly one entry here. + // The same prefix is what makes `clearFlashCookie`'s Domain-less clear + // complete rather than partial. + expect(FLASH_COOKIE.toLowerCase().startsWith("__host-")).toBe(true); + }); + + it("is cleared by a cookie the browser will actually accept for that name", () => { + // the prefix rules apply to the DELETION Set-Cookie too: a `__Host-` + // name without `Secure` is rejected on arrival with no trace (#3138), + // so the clear silently fails and the outcome haunts the next request + const clear = clearFlashCookie(); + expect(clear.startsWith(`${FLASH_COOKIE}=`)).toBe(true); + expect(hasAttribute(clear, "path")).toBe(true); + expect(hasAttribute(clear, "max-age")).toBe(true); + expect(hasAttribute(clear, "secure")).toBe(true); + }); +}); diff --git a/packages/web/test/server/server-functions-flash-decode-hygiene.spec.tsx b/packages/web/test/server/server-functions-flash-decode-hygiene.spec.tsx new file mode 100644 index 000000000..4261807c6 --- /dev/null +++ b/packages/web/test/server/server-functions-flash-decode-hygiene.spec.tsx @@ -0,0 +1,94 @@ +/** + * The flash cookie is a decode boundary, and it is missing the two guards + * the other decode boundary has. + * + * The argument road strips the prototype-mutating keys at the seam + * (`stripUnsafeArgumentKeys`, server.ts ~1199; the reasoning and the table + * of roads live in server-functions-proto-keys.spec.tsx, #3168/#3202) on + * the grounds that a decoded value's most ordinary downstream move is a + * merge, and a merge by [[Set]] fires the inherited setter. `JSON.parse` + * creates `__proto__` as an ordinary own property, so the flash road + * produces exactly the same graph — and hands it straight to the + * integration, unstripped, on BOTH fields it carries: `input` (the echo an + * integration re-populates the form from) and `result` (the value it + * renders). + * + * Reachability is narrower than the argument road's: setting a cookie on + * the origin is the price of entry. It is not zero — the flash cookie is + * unsigned, and it is not host-locked, so a sibling subdomain can toss one + * (see server-functions-flash-cookie-attributes.spec.tsx). A guard that + * exists on one leg of a decode boundary and not the other is a guard the + * next reader will assume covers both. + * + * The same asymmetry shows in the field the decoder copies without looking: + * `FlashSubmission.url` is typed `string` and every integration reads it as + * one (`url.startsWith("/")`, `new URL(url, base)`), but the decoder passes + * `payload.url` through whatever it is. An object there does not fail the + * decode — it fails the RENDER, which is the one place flash.ts's own + * header promises a malformed cookie can never reach ("a malformed cookie + * never takes down the render"). + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { afterEach, describe, expect, it } from "vitest"; +import { FLASH_COOKIE, decodeFlashCookie } from "@solidjs/web/server-functions/server"; + +afterEach(() => { + delete (Object.prototype as any).polluted; + delete (Object.prototype as any).isAdmin; +}); + +/** A cookie header carrying a payload no honest encoder would write. */ +function cookieHeader(payload: string) { + return `${FLASH_COOKIE}=${encodeURIComponent(payload)}`; +} + +/** 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; +} + +describe("the flash decode strips what the argument decode strips", () => { + it("carries no live __proto__ out of the submission echo", () => { + const submission = decodeFlashCookie( + cookieHeader( + '{"url":"/_server/save","result":"ok","error":false,"thrown":false,' + + '"input":[{"name":"Ada","__proto__":{"polluted":"yes"}}]}' + ) + )!; + + // the shallow merge #3168 fixed for the argument road + expect(Object.getPrototypeOf(Object.assign({}, submission.input[0]))).toBe(Object.prototype); + expect(Object.keys(submission.input[0])).toEqual(["name"]); + }); + + it("carries no live constructor out of the result an integration renders", () => { + const submission = decodeFlashCookie( + cookieHeader( + '{"url":"/_server/save","error":false,"thrown":false,"input":[],' + + '"result":{"ok":true,"constructor":{"prototype":{"isAdmin":true}}}}' + ) + )!; + + deepMerge({}, submission.result); + expect(({} as any).isAdmin, "Object.prototype was written through the result").toBeUndefined(); + }); + + it("never hands the render a url that is not a string", () => { + const submission = decodeFlashCookie( + cookieHeader('{"url":{"href":"/x"},"result":"ok","error":false,"thrown":false,"input":[]}') + ); + + // the read every integration makes to decide whether the outcome belongs + // to the page it is rendering + expect(typeof submission?.url).not.toBe("object"); + expect(() => submission?.url.startsWith("/")).not.toThrow(); + }); +}); diff --git a/packages/web/test/server/server-functions-flash-falsy-outcomes.spec.tsx b/packages/web/test/server/server-functions-flash-falsy-outcomes.spec.tsx new file mode 100644 index 000000000..4dd132f94 --- /dev/null +++ b/packages/web/test/server/server-functions-flash-falsy-outcomes.spec.tsx @@ -0,0 +1,115 @@ +/** + * A committed mutation must report itself, whatever its value LOOKS like. + * + * The whole reason the flash cookie degrades instead of vanishing (#3137, + * pinned by server-functions-flash-bounds.spec.tsx) is that a missing + * confirmation reads as "nothing happened", and the natural response to + * that is to submit again — which for a non-idempotent handler is the + * second write. The ladder states the invariant in flash.ts itself: "`url` + * and the error/thrown flags always survive: what happened, and to which + * submission, is the part that must not be lost." + * + * Two truthiness tests sit on that road and lose the outcome for free: + * + * - flash.ts, decode: `if (!payload || !payload.result) return;` — a + * WELL-FORMED cookie whose result is falsy is discarded. The encoder + * wrote it, the browser stored it, the render decodes nothing. `""` is + * an ordinary return (a form that saves and answers with an empty + * message), `false` and `0` are ordinary answers, and a thrown + * `Error("")` loses the ERROR flag too — the one thing the ladder above + * promises always survives. + * - server.ts, `createNoJSHandler`: `if (result && !(result instanceof + * Response))` — a function that simply returns emits no cookie at all, + * so the most ordinary action of the lot (`async () => { await + * db.save(...) }`) is exactly the one whose commit is invisible. + * + * The scripted leg has no such gap: a call returning `undefined` resolves + * and the submission reports success. The no-JS leg is meant to show the + * outcome "exactly as it would for a scripted call" (flash.ts header). + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { describe, expect, it } from "vitest"; +import { + FLASH_COOKIE, + createNoJSHandler, + decodeFlashCookie, + encodeFlashCookie +} from "@solidjs/web/server-functions/server"; + +/** What the browser stores: the name=value pair, before the attributes. */ +function pairOf(setCookie: string) { + const end = setCookie.indexOf("; "); + return end < 0 ? setCookie : setCookie.slice(0, end); +} + +function roundTrip(setCookie: string) { + return decodeFlashCookie(pairOf(setCookie)); +} + +/** A browser form post, as the no-JS leg receives it. */ +function formPost() { + return new Request("https://app.example/_server/save-draft", { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + Referer: "https://app.example/drafts/9" + }, + body: "body=hello" + }); +} + +describe("a falsy result is still an outcome", () => { + it("delivers an empty-string result rather than discarding the cookie", () => { + const cookie = encodeFlashCookie("/_server/save-draft", "", ["hello"]); + const submission = roundTrip(cookie); + + // the control: absence really is absence, and stays undefined + expect(decodeFlashCookie(null)).toBeUndefined(); + + expect(submission).toBeDefined(); + expect(submission?.url).toBe("/_server/save-draft"); + expect(submission?.result).toBe(""); + }); + + it("delivers 0, false and null the same way", () => { + for (const result of [0, false, null]) { + const submission = roundTrip(encodeFlashCookie("/_server/vote", result, [])); + expect(submission, `result ${JSON.stringify(result)} was discarded`).toBeDefined(); + expect(submission?.url).toBe("/_server/vote"); + expect(submission?.result).toBe(result); + } + }); + + it("keeps the error flag on a thrown outcome whose message is empty", () => { + // the flag, not the text, is what the next render branches on: losing it + // turns a failed charge into a page that looks like nothing was posted + const cookie = encodeFlashCookie("/_server/charge", new Error(""), [], true); + const submission = roundTrip(cookie); + + expect(submission).toBeDefined(); + expect(submission?.error).toBeInstanceOf(Error); + expect((submission?.error as Error).message).toBe(""); + expect(submission?.result).toBeUndefined(); + }); + + it("flashes that the submission happened when the function simply returns", () => { + // `async () => { await db.save(draft); }` — the commonest action shape + // there is, and the one with no value to be truthy + const response = createNoJSHandler()(undefined, formPost(), ["hello"]); + + expect(response.status).toBe(303); + const flash = response.headers + .getSetCookie() + .find(entry => entry.startsWith(`${FLASH_COOKIE}=`)); + expect( + flash, + "no outcome cookie at all — the next render cannot tell it committed" + ).toBeDefined(); + + const submission = roundTrip(flash!); + expect(submission?.url).toBe("/_server/save-draft"); + expect(submission?.input).toEqual(["hello"]); + }); +}); diff --git a/packages/web/test/server/server-functions-flash-url-bound.spec.tsx b/packages/web/test/server/server-functions-flash-url-bound.spec.tsx new file mode 100644 index 000000000..7d3fe91fb --- /dev/null +++ b/packages/web/test/server/server-functions-flash-url-bound.spec.tsx @@ -0,0 +1,134 @@ +/** + * The degrade ladder has to bound EVERY field it writes, or it does not + * bound the cookie. + * + * flash.ts's ladder (#3137, and see server-functions-flash-bounds.spec.tsx) + * exists because a cookie past the browser's ~4096-byte ceiling is + * discarded WHOLE, with no signal in the response, the console, or + * server-side: the page after the redirect is indistinguishable from one + * where nothing was submitted, the mutation has already committed, and the + * retry writes twice. + * + * The ladder bounds two of the payload's three variable-length fields. It + * drops `input`, it bounds `result` — and it never looks at `url`, which is + * `pathname + search` of a request the caller chose. A form whose action + * carries state (` { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +/** RFC 6265bis §5.6: what the browser measures is the name=value pair. */ +const COOKIE_CEILING = 4096; + +function pairOf(setCookie: string) { + const end = setCookie.indexOf("; "); + return end < 0 ? setCookie : setCookie.slice(0, end); +} + +describe("the flash cookie's url is bounded like everything else in it", () => { + it("keeps the pair storable when the url alone overruns the ceiling", () => { + const url = "/_server/publish?return=" + encodeURIComponent("/catalog/" + "a".repeat(4200)); + const cookie = encodeFlashCookie(url, { published: true }, []); + const pair = pairOf(cookie); + + expect( + pair.length, + `${pair.length} bytes — the browser discards the whole cookie` + ).toBeLessThanOrEqual(COOKIE_CEILING); + // and having fit, it still says what happened + expect(decodeFlashCookie(pair)?.truncated).toBe(true); + }); + + it("never reports truncated success for a payload that cannot be stored", () => { + // the ladder has already spent input and result here; `truncated: true` + // is the encoder's own claim that what it returned fits + const url = "/_server/import?" + "q=" + "b".repeat(40000); + const pair = pairOf(encodeFlashCookie(url, { rows: 12000 }, [{ big: "c".repeat(50000) }])); + const payload = JSON.parse(decodeURIComponent(pair.slice(FLASH_COOKIE.length + 1))); + + expect(payload.truncated).toBe(true); + expect( + pair.length, + "the payload claims it was degraded to fit, and did not" + ).toBeLessThanOrEqual(COOKIE_CEILING); + }); + + it("still tells the next render that a long-url submission FAILED", () => { + const url = "/_server/charge?receipt=" + "d".repeat(4200); + const pair = pairOf(encodeFlashCookie(url, new Error("card declined"), [], true)); + + expect(pair.length).toBeLessThanOrEqual(COOKIE_CEILING); + const submission = decodeFlashCookie(pair); + expect(submission?.error).toBeInstanceOf(Error); + expect(submission?.result).toBeUndefined(); + }); + + it("holds through the handler, where the url is not the encoder's to choose", async () => { + let ran = 0; + registerServerFunction("flash-url-bound-publish", async () => { + ran++; + return { published: true }; + }); + + const back = "/catalog/" + "e".repeat(4200); + const response = await handleServerFunctionRequest( + new Request( + "https://app.example/_server/flash-url-bound-publish?return=" + encodeURIComponent(back), + { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + "Content-Type": "application/x-www-form-urlencoded", + Referer: "https://app.example/catalog" + }, + body: "qty=1" + } + ) + ); + + expect(ran).toBe(1); // the mutation COMMITTED — this is the #3137 case + expect(response.status).toBe(303); + const flash = response.headers + .getSetCookie() + .find(entry => entry.startsWith(`${FLASH_COOKIE}=`))!; + const pair = pairOf(flash); + expect( + pair.length, + `${pair.length} bytes reach the browser and none come back` + ).toBeLessThanOrEqual(COOKIE_CEILING); + }); +}); diff --git a/packages/web/test/server/server-functions-flight-redirect-target.spec.tsx b/packages/web/test/server/server-functions-flight-redirect-target.spec.tsx new file mode 100644 index 000000000..f25bbb471 --- /dev/null +++ b/packages/web/test/server/server-functions-flight-redirect-target.spec.tsx @@ -0,0 +1,152 @@ +/** + * A redirecting mutation knows where the client is going without being told + * by a `Referer`. + * + * `digestOutcome` computes `targetUrl` — "the URL the client will show after + * the mutation" — and its own comment gives the rule as two cases: "the + * redirect `Location` when the outcome carries one (resolved against the + * request URL, as a browser would), the referring page otherwise". Only the + * second case actually needs a referer; the first is derived from the + * outcome the server itself produced. Both sit inside one `if (referrer)`. + * + * So a site that sends `Referrer-Policy: no-referrer` — a routine security + * header, and the default under several CSP/hardening presets — silently + * loses single-flight on every redirecting mutation: `targetUrl` is + * undefined, the router's collector has no destination to produce data for, + * nothing folds, and the mutation costs a second round trip. Nothing warns; + * the feature just stops paying. + * + * The `otherwise` half must keep needing a referer (a non-browser caller + * has no page to produce data for), and the cross-origin guard must keep + * holding on both halves — a redirect leaving the app's origin produces no + * target either way. These specs pin the rule as the comment states it. + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + SINGLE_FLIGHT_HEADER, + decodeResponse, + handleServerFunctionRequest, + registerFlightDataSource, + 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 unregisters: (() => void)[] = []; +afterEach(() => { + while (unregisters.length) unregisters.pop()!(); +}); + +/** + * Registers a mutation that throws the given redirect and a router-shaped + * collector that only produces data when it knows the destination. Returns + * a caller and the digested outcomes the collector saw. + */ +function redirectingMutation(id: string, location: string) { + const outcomes: any[] = []; + registerServerFunction(id, async () => { + throw new Response(null, { status: 303, headers: { Location: location } }); + }); + unregisters.push( + registerFlightDataSource("router", (_event: any, outcome: any) => { + outcomes.push(outcome); + return outcome.targetUrl ? { [outcome.targetUrl]: ["destination data"] } : undefined; + }) + ); + const call = (referer?: string) => + handleServerFunctionRequest( + new Request(`http://localhost/_server/data/${id}`, { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + [SINGLE_FLIGHT_HEADER]: "router", + ...(referer ? { referer } : {}) + } + }) + ); + return { call, outcomes }; +} + +describe("single-flight target url for a redirecting mutation", () => { + it("derives the target from the redirect Location when the caller sends no Referer", async () => { + const sameOrigin = redirectingMutation("sf-target-noreferer", "/orders/42"); + await sameOrigin.call(); + const derived = sameOrigin.outcomes.at(-1); + + expect( + derived.targetUrl, + `Location was ${derived.response?.headers?.get("Location")} but targetUrl was ` + + `${derived.targetUrl}` + ).toBe("http://localhost/orders/42"); + + // The guards the referer gate was incidentally providing have to keep + // holding once the Location half no longer sits behind it: a redirect + // leaving the app's origin has no page of ours to produce data for, and + // a non-redirecting mutation still falls back to the referring page — + // which a caller that sends none does not have. + const crossOrigin = redirectingMutation( + "sf-target-crossorigin", + "https://elsewhere.example/orders/42" + ); + await crossOrigin.call(); + expect( + crossOrigin.outcomes.at(-1).targetUrl, + "a redirect leaving the origin must still produce no target" + ).toBeUndefined(); + + registerServerFunction("sf-target-plain", async () => "committed"); + const plain: any[] = []; + unregisters.push( + registerFlightDataSource("router", (_event: any, outcome: any) => { + plain.push(outcome); + return undefined; + }) + ); + await handleServerFunctionRequest( + new Request("http://localhost/_server/data/sf-target-plain", { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + [SINGLE_FLIGHT_HEADER]: "router" + } + }) + ); + expect( + plain.at(-1).targetUrl, + "a non-redirecting mutation without a Referer must still produce no target" + ).toBeUndefined(); + }); + + it("still folds destination data for a mutation sent under Referrer-Policy: no-referrer", async () => { + // The wire-visible cost of the gate: the response is byte-identical to + // a call with no hooks at all, so the client has to go back for the + // destination it was already redirected to. + const { call } = redirectingMutation("sf-target-fold", "/orders/42"); + + const response = await call(); + const body = await response.clone().text(); + + expect( + response.headers.get(SINGLE_FLIGHT_HEADER), + `nothing folded — the mutation costs a second round trip; body was ${body}` + ).toBe("router"); + expect(await decodeResponse(response)).toEqual({ + value: null, + data: { router: { "http://localhost/orders/42": ["destination data"] } } + }); + }); +}); diff --git a/packages/web/test/server/server-functions-flight-result-ownership.spec.tsx b/packages/web/test/server/server-functions-flight-result-ownership.spec.tsx new file mode 100644 index 000000000..62cc5dc72 --- /dev/null +++ b/packages/web/test/server/server-functions-flight-result-ownership.spec.tsx @@ -0,0 +1,180 @@ +/** + * The fold must own a `transformFlightResult` Response before stamping it. + * + * `transformFlightResult` is the seam where an integration builds the + * single-flight body itself, and its contract is "return a Response" — + * nothing in it says the Response must be freshly constructed on every + * call. `foldFlightData` then appends the mutation's `Set-Cookie`s onto + * whatever came back and gap-fills the accumulated headers into it, writing + * through to an object the integration may still hold. + * + * That the runtime knows this is in-contract is visible in the thrown + * path's own fold tail, which copies before ITS write with the comment "the + * fold may hand back a Response an integration hook caches (see + * ownResponse)" — the same argument, applied on one leg and not the other. + * Copying there does not help: the fold's writes already landed on the + * shared object before the copy is taken. + * + * What leaks is the worst thing that can: session cookies. A transform that + * memoizes its rendered shell hands tenant A's `Set-Cookie` to tenant B and + * then hands both to tenant C, with no error anywhere — the same class of + * defect `ownResponse` was introduced for (#3155). + * + * Reachability, stated honestly: stock Solid is not affected. The frames + * policy (`frameTransformFlightResult`) constructs a fresh Response per + * call, so nothing in-tree caches one. These specs pin the contract for the + * seam as documented, which any integration may implement. + * + * 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 { + SINGLE_FLIGHT_HEADER, + handleServerFunctionRequest, + registerFlightDataSource, + registerServerFunction +} from "@solidjs/web/server-functions/server"; + +// not re-exported from the server entry; the wire name is the contract here +const BODY_FORMAT_HEADER = "X-Server-Function-Format"; +const JSON_BODY_FORMAT = "8"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +/** + * A transform that keeps the Response it built — the shape the contract + * permits and this spec is about. `retained` is the object the integration + * still holds after the request is over. + */ +function memoizingTransform() { + let retained: Response | undefined; + const transform = async () => { + if (!retained) { + retained = new Response("region", { + status: 200, + headers: { "X-Content-Raw": "1", "Content-Type": "text/html" } + }); + } + return retained; + }; + return { + transform, + get retained() { + return retained!; + } + }; +} + +/** One tenant's mutation: it sets that tenant's session cookie. */ +async function callAs(id: string, tenant: string, transformFlightResult: any) { + return handleServerFunctionRequest( + new Request(`http://localhost/_server/data/${id}`, { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + [BODY_FORMAT_HEADER]: JSON_BODY_FORMAT, + referer: `http://localhost/tenant/${tenant}/cart`, + [SINGLE_FLIGHT_HEADER]: "router" + }, + body: JSON.stringify([tenant]) + }), + { transformFlightResult } + ); +} + +describe("single-flight fold owns the transformed response", () => { + it("does not stamp one tenant's session cookie onto the next tenant's response (returned outcome)", async () => { + registerServerFunction( + "sf-own-returned", + async (tenant: string) => + new Response(null, { + status: 303, + headers: { Location: `/tenant/${tenant}/orders`, "Set-Cookie": `session=${tenant}` } + }) + ); + registerFlightDataSource("router", (_event: any, outcome: any) => ({ + [outcome.targetUrl ?? "/"]: ["x"] + })); + const memo = memoizingTransform(); + + const cookies: Record = {}; + for (const tenant of ["ALICE", "BOB", "CAROL"]) { + const response = await callAs("sf-own-returned", tenant, memo.transform); + cookies[tenant] = response.headers.getSetCookie(); + } + + expect(cookies.ALICE).toEqual(["session=ALICE"]); + expect(cookies.BOB, `BOB's response carried ${JSON.stringify(cookies.BOB)}`).toEqual([ + "session=BOB" + ]); + expect(cookies.CAROL, `CAROL's response carried ${JSON.stringify(cookies.CAROL)}`).toEqual([ + "session=CAROL" + ]); + }); + + it("leaves the Response the integration retains unwritten (returned outcome)", async () => { + registerServerFunction( + "sf-own-retained", + async (tenant: string) => + new Response(null, { + status: 303, + headers: { Location: `/tenant/${tenant}/orders`, "Set-Cookie": `session=${tenant}` } + }) + ); + registerFlightDataSource("router", () => ({ "/": ["x"] })); + const memo = memoizingTransform(); + + await callAs("sf-own-retained", "ALICE", memo.transform); + + const stamped = memo.retained.headers.getSetCookie(); + expect( + stamped, + `the fold wrote ${JSON.stringify(stamped)} onto the integration's own object` + ).toEqual([]); + expect( + memo.retained.headers.get(SINGLE_FLIGHT_HEADER), + "the fold stamped its protocol header onto the integration's own object" + ).toBe(null); + }); + + it("does not stamp one tenant's session cookie onto the next tenant's response (thrown outcome)", async () => { + // The thrown leg is the common single-flight shape (a mutation that + // throws a redirect) and the one whose tail already knows to copy — its + // copy is simply taken after the fold has written. + registerServerFunction("sf-own-thrown", async (tenant: string) => { + throw new Response(null, { + status: 303, + headers: { Location: `/tenant/${tenant}/orders`, "Set-Cookie": `session=${tenant}` } + }); + }); + registerFlightDataSource("router", (_event: any, outcome: any) => ({ + [outcome.targetUrl ?? "/"]: ["x"] + })); + const memo = memoizingTransform(); + + const cookies: Record = {}; + for (const tenant of ["ALICE", "BOB", "CAROL"]) { + const response = await callAs("sf-own-thrown", tenant, memo.transform); + cookies[tenant] = response.headers.getSetCookie(); + } + + expect(cookies.ALICE).toEqual(["session=ALICE"]); + expect(cookies.BOB, `BOB's response carried ${JSON.stringify(cookies.BOB)}`).toEqual([ + "session=BOB" + ]); + expect(cookies.CAROL, `CAROL's response carried ${JSON.stringify(cookies.CAROL)}`).toEqual([ + "session=CAROL" + ]); + }); +}); diff --git a/packages/web/test/server/server-functions-flight-slice-encoding.spec.tsx b/packages/web/test/server/server-functions-flight-slice-encoding.spec.tsx new file mode 100644 index 000000000..c8e99ffe4 --- /dev/null +++ b/packages/web/test/server/server-functions-flight-slice-encoding.spec.tsx @@ -0,0 +1,164 @@ +/** + * A flight slice that cannot be encoded must cost only that slice. + * + * `foldFlightData` already states the containment rule in its own comment — + * "one cache's collector failing must not cost the mutation's outcome or + * the other caches' slices" — and implements it around the CALL to each + * hook. But a collector has two ways to fail, and only one of them is a + * throw: the other is returning a value the wire cannot carry. A query + * cache handing back an entry that still holds a function, a class instance + * or a live DB handle encodes nothing, and that failure lands far outside + * the per-source try — in `encodeResult`, once the envelope is one object + * and the individual slices are no longer separable. + * + * The cost is paid by a mutation that ALREADY COMMITTED. The charge went + * through; the client's decode throws; the mutation's own return value and + * every sibling cache's slice are destroyed with it; and the answer is a + * 200 carrying no failure tag, so no CDN, load balancer or log sees a + * failure either. The user retries a charge that already succeeded. + * + * The sibling case pins the other edge of the same rule: the slice that + * survives carries a `Date`, so containment cannot be bought by narrowing + * the fold to what JSON alone can carry. The codec exists precisely so a + * result needn't be JSON, and cache entries built from ORM rows are the + * common case, not the exotic one. + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + SINGLE_FLIGHT_HEADER, + decodeResponse, + handleServerFunctionRequest, + registerFlightDataSource, + 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 unregisters: (() => void)[] = []; +afterEach(() => { + while (unregisters.length) unregisters.pop()!(); +}); + +/** A scripted mutation that advertised `sources` on the request leg. */ +function flightRequest(id: string, sources: string) { + return new Request(`http://localhost/_server/data/${id}`, { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + [SINGLE_FLIGHT_HEADER]: sources + } + }); +} + +/** + * The whole response as the client sees it: what the transport's own decode + * makes of the body, plus the wire facts a failure would have to announce + * itself through. + */ +async function readAsClient(response: Response) { + const raw = await response.clone().text(); + let payload: any; + let decodeError: unknown; + try { + payload = await decodeResponse(response); + } catch (error) { + decodeError = error; + } + return { + status: response.status, + errorTag: response.headers.get("X-Server-Function-Error"), + foldedSources: response.headers.get(SINGLE_FLIGHT_HEADER), + decodeError: decodeError === undefined ? null : String(decodeError), + payload, + raw + }; +} + +describe("single-flight slice encoding is contained per source", () => { + it("does not destroy a committed mutation's result when the only slice cannot be encoded", async () => { + let mutationRan = 0; + registerServerFunction("sf-encode-solo", async () => { + mutationRan++; + return { orderId: "o-1", charged: true }; + }); + // A cache entry that still holds a live handle. Nothing about it is + // exotic — a function, a class instance or a DB connection does it. + unregisters.push(registerFlightDataSource("badCache", () => ({ handler: function () {} }))); + + const seen = await readAsClient( + await handleServerFunctionRequest(flightRequest("sf-encode-solo", "badCache")) + ); + + expect(mutationRan, "the mutation must have run — that is the whole point").toBe(1); + // The client's own decode is the ground truth for "the caller saw a + // failure": it throws on the codec's error trailer. + expect(seen.decodeError, `decode threw for a mutation that committed: ${seen.raw}`).toBe(null); + expect(seen.payload?.value ?? seen.payload, `the mutation's return value: ${seen.raw}`).toEqual( + { + orderId: "o-1", + charged: true + } + ); + }); + + it("does not take a healthy cache's slice down with an un-encodable sibling", async () => { + let mutationRan = 0; + registerServerFunction("sf-encode-pair", async () => { + mutationRan++; + return "committed"; + }); + // The healthy slice carries a Date on purpose: it is exactly the shape + // that separates "the wire cannot carry this" from "JSON cannot carry + // this". A cache entry off an ORM row is full of them, so a containment + // rule that answered the second question would take single-flight away + // from most applications to close the first. + unregisters.push( + registerFlightDataSource("goodCache", () => ({ "/orders": [{ at: new Date(0) }] })) + ); + unregisters.push(registerFlightDataSource("badCache", () => ({ handler: function () {} }))); + + const seen = await readAsClient( + await handleServerFunctionRequest(flightRequest("sf-encode-pair", "goodCache,badCache")) + ); + + expect(mutationRan).toBe(1); + expect(seen.decodeError, `decode threw; wire body was: ${seen.raw}`).toBe(null); + expect(seen.payload?.value, `payload: ${seen.raw}`).toBe("committed"); + expect( + seen.payload?.data?.goodCache, + `the healthy cache's slice was destroyed by its sibling: ${seen.raw}` + ).toEqual({ "/orders": [{ at: new Date(0) }] }); + }); + + it("names in the response header only the sources whose slices the payload carries", async () => { + // The client routes slices to consumers by this header. Advertising a + // source that is not in the envelope points a consumer at nothing. + registerServerFunction("sf-encode-header", async () => "committed"); + unregisters.push(registerFlightDataSource("goodCache", () => ({ "/orders": ["fresh"] }))); + unregisters.push(registerFlightDataSource("badCache", () => ({ handler: function () {} }))); + + const seen = await readAsClient( + await handleServerFunctionRequest(flightRequest("sf-encode-header", "goodCache,badCache")) + ); + + const named = seen.foldedSources ? seen.foldedSources.split(",") : []; + const carried = Object.keys(seen.payload?.data ?? {}); + expect( + named, + `header named [${named}] but the payload carries [${carried}]; body: ${seen.raw}` + ).toEqual(carried); + }); +}); diff --git a/packages/web/test/server/server-functions-flight-source-dedupe.spec.tsx b/packages/web/test/server/server-functions-flight-source-dedupe.spec.tsx new file mode 100644 index 000000000..1eb450b3f --- /dev/null +++ b/packages/web/test/server/server-functions-flight-source-dedupe.spec.tsx @@ -0,0 +1,132 @@ +/** + * The request-leg source list is attacker-controlled input, and a collector + * is the most expensive per-request work single-flight does. + * + * `X-Single-Flight` names the sources the caller can consume, and the + * handler resolves that list to hooks by splitting on commas — one entry, + * one hook run. The list is a SET on the client (one consumer per source + * id, later registrations replace), and the envelope is built with + * `Object.fromEntries`, so a repeated id cannot contribute a second slice: + * every run past the first is work whose result is thrown away. + * + * Nothing dedupes it, so a same-origin authenticated caller who repeats one + * id N times pays for one request and gets N collections — each a re-run of + * the invalidated reads inside a request-event scope — plus an echoed + * response header N ids long. That is amplification with a multiplier the + * caller chooses, on the single most expensive thing in the request. + * + * Deduping is also the only reading consistent with the protocol as + * documented: the id list is "the ids its registered consumers can actually + * use", and the response header "names the folded sources". + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + SINGLE_FLIGHT_HEADER, + decodeResponse, + handleServerFunctionRequest, + registerFlightDataSource, + 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 unregisters: (() => void)[] = []; +afterEach(() => { + while (unregisters.length) unregisters.pop()!(); +}); + +function flightRequest(id: string, sources: string) { + return new Request(`http://localhost/_server/data/${id}`, { + method: "POST", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + [SINGLE_FLIGHT_HEADER]: sources + } + }); +} + +describe("single-flight source list is a set, not a multiset", () => { + it("runs a repeated source's collector exactly once", async () => { + let collectorRuns = 0; + registerServerFunction("sf-dedupe-runs", async () => "committed"); + unregisters.push( + registerFlightDataSource("expensive", () => { + collectorRuns++; + return { "/orders": ["fresh"] }; + }) + ); + + const repeats = 2000; + await handleServerFunctionRequest( + flightRequest("sf-dedupe-runs", Array(repeats).fill("expensive").join(",")) + ); + + expect( + collectorRuns, + `${repeats} repetitions of one id ran the collector ${collectorRuns} times` + ).toBe(1); + }); + + it("echoes a repeated source once in the response header", async () => { + registerServerFunction("sf-dedupe-header", async () => "committed"); + unregisters.push(registerFlightDataSource("expensive", () => ({ "/orders": ["fresh"] }))); + + const repeats = 2000; + const response = await handleServerFunctionRequest( + flightRequest("sf-dedupe-header", Array(repeats).fill("expensive").join(",")) + ); + + // Compared by shape, not by string: the failing value is the whole + // echoed header, and printing 20 KB of it helps nobody. + const folded = response.headers.get(SINGLE_FLIGHT_HEADER) ?? ""; + const ids = folded ? folded.split(",") : []; + expect( + { ids: ids.length, bytes: folded.length }, + `${SINGLE_FLIGHT_HEADER} began "${folded.slice(0, 40)}…"` + ).toEqual({ ids: 1, bytes: "expensive".length }); + expect(ids[0]).toBe("expensive"); + }); + + it("still folds each distinct source once when the list repeats several", async () => { + // Deduping must not cost a caller its second cache: the guard is on + // repetition, not on multiple sources. + const runs: Record = { a: 0, b: 0 }; + registerServerFunction("sf-dedupe-distinct", async () => "committed"); + unregisters.push( + registerFlightDataSource("cacheA", () => { + runs.a++; + return { "/a": ["fresh"] }; + }) + ); + unregisters.push( + registerFlightDataSource("cacheB", () => { + runs.b++; + return { "/b": ["fresh"] }; + }) + ); + + const response = await handleServerFunctionRequest( + flightRequest("sf-dedupe-distinct", "cacheA,cacheB,cacheA,cacheB,cacheA") + ); + + expect(runs, `collector runs were ${JSON.stringify(runs)}`).toEqual({ a: 1, b: 1 }); + expect(response.headers.get(SINGLE_FLIGHT_HEADER)).toBe("cacheA,cacheB"); + expect(await decodeResponse(response)).toEqual({ + value: "committed", + data: { cacheA: { "/a": ["fresh"] }, cacheB: { "/b": ["fresh"] } } + }); + }); +}); diff --git a/packages/web/test/server/server-functions-format-tag-recognition.spec.tsx b/packages/web/test/server/server-functions-format-tag-recognition.spec.tsx new file mode 100644 index 000000000..fba0cc9d3 --- /dev/null +++ b/packages/web/test/server/server-functions-format-tag-recognition.spec.tsx @@ -0,0 +1,168 @@ +/** + * A body-format tag the client does not RECOGNISE is not the same thing as + * a body-format tag being present (#3173, one layer down). + * + * The transport's two "is this ours?" guards test the header's PRESENCE: a + * 4xx/5xx without `X-Server-Function-Format` is the peer refusing, and a + * 2xx without it (or without `X-Content-Raw`) is infrastructure answering + * in the origin's place. Both then hand the response to `extractBody`, + * which matches the tag by exact VALUE and falls through to `undefined` for + * anything it has no case for. So a tag that is present but unrecognised + * passes the presence guards, decodes to nothing, and RESOLVES the call — + * the phantom void result #3173 closed, reopened by a header the runtime + * never wrote. + * + * Two ways in, neither hostile: + * + * - a duplicated header. Intermediaries append rather than replace, and + * `Headers.get` joins the duplicates with a comma, so two perfectly + * valid tags read back as the single unrecognised value `"8, 9"`. + * - version skew. The tag is a small integer that grows with the runtime; + * the day a `BodyFormat` past `Void` ships, every client from the + * previous build reads the new tag, recognises nothing, and resolves + * `undefined` where the truth is "this build cannot read that answer". + * #3110 made an unknown *id* legible for exactly this reason; an unknown + * *encoding* deserves the same honesty, and silence is the one answer it + * must not give. + * + * The contrast that makes this a defect rather than a design: the same + * responses with NO tag at all fail loudly (status 500 rejects with the + * status; 200 rejects with "no recognized encoding"). Adding a header the + * client cannot read must not turn a failure into a success. + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createServerReference } from "@solidjs/web/server-functions/client"; + +const RequestContext = Symbol.for("solid.RequestContext"); +const BODY_FORMAT_HEADER = "X-Server-Function-Format"; +const JSON_FORMAT = "8"; +const VOID_FORMAT = "9"; +/** The next tag the runtime ships — today's clients have no case for it. */ +const FUTURE_FORMAT = "10"; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const disconnects: (() => void)[] = []; +afterEach(() => { + while (disconnects.length) disconnects.pop()!(); +}); + +/** + * Routes the client stub's fetch into the built handler, or — with + * `answer` — into a response that came from somewhere else on the way. + */ +function connectTransport(answer?: () => Response) { + const original = globalThis.fetch; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + if (answer) return Promise.resolve(answer()); + const address = input instanceof Request ? input.url : input.toString(); + const request = new Request( + new URL(address, "http://localhost"), + input instanceof Request ? input : init + ); + request.headers.set("Sec-Fetch-Site", "same-origin"); + return handleServerFunctionRequest(request); + }) as typeof fetch; + disconnects.push(() => { + globalThis.fetch = original; + }); +} + +/** A response tagged with one format value, whatever the client makes of it. */ +const tagged = (status: number, tag: string, body: BodyInit | null, type?: string) => () => + new Response(body, { + status, + headers: { [BODY_FORMAT_HEADER]: tag, ...(type ? { "content-type": type } : {}) } + }); + +/** + * The shape an intermediary produces: the origin's tag plus one appended by + * something on the path. `Headers.get` joins them, and the join is the value + * the client actually reads. + */ +const doubleTagged = (status: number, body: BodyInit | null) => () => { + const headers = new Headers(); + headers.append(BODY_FORMAT_HEADER, JSON_FORMAT); + headers.append(BODY_FORMAT_HEADER, VOID_FORMAT); + return new Response(body, { status, headers }); +}; + +describe("unrecognised body-format tags", () => { + it("fails a success answer whose format tag it cannot read", async () => { + registerServerFunction("tag-success", async () => "ok"); + for (const answer of [ + tagged(200, FUTURE_FORMAT, '{"total":3}', "application/json"), + tagged(200, "nonsense", '{"total":3}', "application/json"), + tagged(201, FUTURE_FORMAT, null) + ]) { + connectTransport(answer); + const status = answer().status; + const tag = answer().headers.get(BODY_FORMAT_HEADER); + await expect( + createServerReference("tag-success")(), + `status ${status} tagged ${tag} must not resolve` + ).rejects.toBeInstanceOf(Error); + connectTransport(answer); + await expect(createServerReference("tag-success")()).rejects.toMatchObject({ status }); + } + // the control the tags above are read against: the two tags this build + // does know still decode, so the pin is on recognition, not presence + connectTransport(tagged(200, JSON_FORMAT, '{"total":3}', "application/json")); + expect(await createServerReference("tag-success")()).toEqual({ total: 3 }); + connectTransport(tagged(200, VOID_FORMAT, null)); + expect(await createServerReference("tag-success")()).toBeUndefined(); + }); + + it("fails a refusal whose format tag it cannot read", async () => { + registerServerFunction("tag-refusal", async () => "ok"); + // The presence guard at 400-and-up exists to tell a peer's refusal from + // an authored status; a tag nothing in this build can read is no + // evidence the runtime wrote the answer, and a 500 that resolves + // `undefined` is the worst outcome available. + for (const answer of [ + tagged(500, FUTURE_FORMAT, null), + tagged(502, "nonsense", "bad gateway", "text/html"), + tagged(403, FUTURE_FORMAT, null) + ]) { + connectTransport(answer); + const status = answer().status; + await expect( + createServerReference("tag-refusal")(), + `status ${status} tagged ${answer().headers.get(BODY_FORMAT_HEADER)} must not resolve` + ).rejects.toMatchObject({ status }); + } + // the control: the same statuses with a tag this build reads are the + // author's own answer and keep resolving (#3097) + connectTransport(tagged(500, JSON_FORMAT, '{"field":"required"}', "application/json")); + expect(await createServerReference("tag-refusal")()).toEqual({ field: "required" }); + }); + + it("fails a call whose format header arrived twice", async () => { + registerServerFunction("tag-doubled", async () => "ok"); + // Nothing here is malformed on the wire: both values are tags the + // runtime writes. `Headers.get` hands the client `"8, 9"`, which is + // neither, and a proxy that appends its own copy of a header is + // ordinary — this needs no hostile peer, only a hop. + for (const status of [200, 500]) { + connectTransport(doubleTagged(status, null)); + await expect( + createServerReference("tag-doubled")(), + `duplicated format header at status ${status} must not resolve` + ).rejects.toMatchObject({ status }); + } + }); +}); diff --git a/packages/web/test/server/server-functions-get-grant-binding.spec.tsx b/packages/web/test/server/server-functions-get-grant-binding.spec.tsx new file mode 100644 index 000000000..f89503c90 --- /dev/null +++ b/packages/web/test/server/server-functions-get-grant-binding.spec.tsx @@ -0,0 +1,210 @@ +/** + * A `GET()` grant is made ABOUT a function, and dispatch must never honor + * it for anything else. + * + * `GET(fn)` writes two places: the id-keyed `METHODS` map, which alone + * governs dispatch AND the CSRF origin-gate exemption, and the reference's + * metadata channel, which is what tools and hooks read back + * (`getServerFunctionMetadata(fn).method`). The grant is a safety + * assertion, not a transport preference — a GET-declared function is + * executable from any origin, with caller-chosen arguments, carrying the + * user's ambient cookies (#3114) — so the two writes disagreeing is not a + * cosmetic drift: whatever `METHODS` says is what a cross-site + * `` can reach. + * + * #3129 closed one way for them to disagree: re-registering an id drops the + * grant, so `register -> GET -> register` cannot leave a mutation reachable + * over an un-gated GET. Two more remain, and both are the same shape — a + * write to one channel that the other never hears: + * + * - `withMeta(fn, { method: "POST" })` writes only the metadata. The + * reference reads back as a POST function while the wire still executes + * it on a cross-site GET. Whether `withMeta` should revoke or refuse the + * write, it cannot be allowed to report a revocation it did not perform. + * + * - `GET(fn)` writes `METHODS` for `fn.id` unconditionally, so the + * symmetric interleaving `register -> register -> GET` re-arms the grant + * against the binding that is live NOW, which is not the function the + * declaration was made about. #3129's fix verifies the binding at REBIND + * time; this is the same check owed at GRANT time. + * + * The two share a fix if the fix is "a grant is verified against the + * binding it names", which is why they are pinned together. + * + * Reachability is asserted the way an attacker gets it: an unscripted + * cross-site GET, the request a hostile page can cause a browser to send + * with the victim's cookies. A function that never declared GET answers + * that request with 403 (the origin gate) and never runs its body — that + * is the shape every test below expects. + * + * 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 } from "@solidjs/web"; +import { + GET, + createServerReference as createServerSideReference, + getServerFunctionMetadata, + handleServerFunctionRequest, + registerServerFunction, + registerServerReference, + withMeta +} 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 createEvent = (request: Request) => createRequestEvent(request); + +/** The request a hostile page can cause: cross-site, no client runtime. */ +function crossSiteGet(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "GET", + headers: { "Sec-Fetch-Site": "cross-site" } + }); +} + +describe("the grant tracks the function it was granted about", () => { + it("lets a cross-site GET through for a function that declared it", async () => { + let ran = 0; + GET( + createServerSideReference( + registerServerReference("grant-control", () => { + ran++; + return "a read"; + }) + ) + ); + + const response = await handleServerFunctionRequest(crossSiteGet("grant-control"), { + createEvent + }); + + // the control: without this passing, every expectation below is green + // for the wrong reason + expect({ status: response.status, ran }).toStrictEqual({ status: 200, ran: 1 }); + }); + + it("refuses a cross-site GET for a function that never declared it", async () => { + let ran = 0; + registerServerFunction("grant-undeclared", () => { + ran++; + return "a mutation"; + }); + + const response = await handleServerFunctionRequest(crossSiteGet("grant-undeclared"), { + createEvent + }); + + expect({ status: response.status, ran }).toStrictEqual({ status: 403, ran: 0 }); + }); + + it("drops the grant when the id is rebound after the declaration (#3129)", async () => { + let mutationRan = 0; + GET(createServerSideReference(registerServerReference("grant-rebound-after", () => "a read"))); + registerServerFunction("grant-rebound-after", () => { + mutationRan++; + return "a mutation"; + }); + + const response = await handleServerFunctionRequest(crossSiteGet("grant-rebound-after"), { + createEvent + }); + + expect({ status: response.status, mutationRan }).toStrictEqual({ status: 403, mutationRan: 0 }); + }); + + it("does not grant to a binding installed before the declaration ran", async () => { + let mutationRan = 0; + // the declaration is made about THIS function... + const declared = createServerSideReference( + registerServerReference("grant-rebound-before", () => "a read") + ); + // ...but by the time it runs, the id belongs to another one — an id + // collision between integrations, or a module re-evaluated in a live + // process after an edit dropped the wrapper + registerServerFunction("grant-rebound-before", () => { + mutationRan++; + return "a mutation"; + }); + // refusing the grant loudly at declaration time is an equally good + // answer to a declaration that names a binding that is gone; what is + // not an answer is granting it + let declarationThrew = false; + try { + GET(declared); + } catch { + declarationThrew = true; + } + + const response = await handleServerFunctionRequest(crossSiteGet("grant-rebound-before"), { + createEvent + }); + + // today: declarationThrew false, { status: 200, mutationRan: 1 } — the + // mutation ran from a cross-site GET, with no origin gate, under the + // victim's cookies + expect({ status: response.status, mutationRan, declarationThrew }).toStrictEqual({ + status: 403, + mutationRan: 0, + declarationThrew + }); + }); +}); + +describe("what the metadata channel reports about the grant", () => { + it("does not let withMeta report a revocation the wire did not perform", async () => { + let ran = 0; + const fn = createServerSideReference( + registerServerReference("grant-withmeta-revoke", () => { + ran++; + return "a read"; + }) + ); + GET(fn); + // an app narrowing a declaration back down — the only spelling the + // metadata channel offers + let withMetaThrew = false; + try { + withMeta(fn, { method: "POST" }); + } catch { + withMetaThrew = true; + } + + const response = await handleServerFunctionRequest(crossSiteGet("grant-withmeta-revoke"), { + createEvent + }); + const outcome = { + withMetaThrew, + reportedMethod: getServerFunctionMetadata(fn)!.method, + status: response.status, + ran + }; + + // Two honest answers, and the choice between them is a design call the + // fix makes, not something this spec should decide: either `method` is + // declaration-scoped and `withMeta` refuses to write it (pointing at + // `GET(fn)`, the way the invoke channel already redirects `method`), or + // the write is a revocation and dispatch stops honoring the grant. + // Today's behavior is neither: the write is accepted, the reference + // reports POST, and the wire still executes the body for a cross-site + // GET. + const refusedTheWrite = { withMetaThrew: true, reportedMethod: "GET", status: 200, ran: 1 }; + const performedTheRevocation = { + withMetaThrew: false, + reportedMethod: "POST", + status: 403, + ran: 0 + }; + expect([refusedTheWrite, performedTheRevocation]).toContainEqual(outcome); + }); +}); diff --git a/packages/web/test/server/server-functions-guard-walk-reach.spec.tsx b/packages/web/test/server/server-functions-guard-walk-reach.spec.tsx new file mode 100644 index 000000000..cd8ec5445 --- /dev/null +++ b/packages/web/test/server/server-functions-guard-walk-reach.spec.tsx @@ -0,0 +1,386 @@ +/** + * WHAT THE GUARD WALK REFUSES TO LOOK AT. + * + * `guardFailures` walks a result before the codec encodes it and wraps + * every failure channel it finds — a rejecting promise, a throwing async + * iterable, an erroring stream — so the failure is sanitized before it + * rides a 200 whose head is already committed, and so the response + * teardown can close the source when the caller leaves. + * + * A walk protects only what it VISITS, and a result reaches it only if the + * format gate sent it that way. There are two places where that decision + * goes against the channel: + * + * - A carrier whose prototype is not `Object.prototype` is handed back + * untouched, channels and all. `Object.assign(new Error(...), { ... })` + * is the ordinary shape of a domain failure carrying its context, and the + * codec does not share the walk's opinion of it: seroval encodes an + * Error's own enumerable properties like any other object's. So the + * payload underneath is serialized having never been guarded — the #3200 + * shape ("a non-plain carrier does not shelter the payload"), which was + * closed on the ARGUMENT road only. The control in every row is the same + * channel under a plain object, which is sanitized today. + * + * - Own properties are picked by ENUMERABILITY, which correctly refuses to + * invoke hidden ACCESSORS but also drops non-enumerable DATA properties, + * which carry no invocation hazard. Hiding a channel changes nothing + * about the wire — every encoder drops it — and everything about whether + * anything owns it. Both gates read the same way: `isJSONSafe` calls such + * a result JSON-safe and sends it down the fast path, where the walk + * never runs at all, and the walk itself would have skipped the slot had + * it run. So a rejecting promise there belongs to nobody — it reaches no + * handler and takes the process down AFTER the 200 has been delivered — + * and a stream there is never registered with the response teardown. + * + * The two halves of that second one must hold AT ONCE, which is why the + * hidden-slot tests park a throwing non-enumerable getter beside the hidden + * data property: plainly restoring `Object.keys(descriptors)` passes the + * data half by reintroducing the accessor hazard that commit removed — the + * hazard `server-functions-result-descriptors.spec.tsx` pins from the other + * side ("a non-enumerable getter is not invoked while another slot is + * guarded"). + * + * Reaching the payload by rebuilding the carrier through a PLAIN shell is + * not a fix either: the codec would encode an ordinary object, and the + * caller would be handed `{ message, code }` where the author returned an + * Error. Every case here asserts the carrier's own message, its extra own + * data and its identity survive ALONGSIDE the sanitization, so a fix has to + * keep the carrier itself — its own properties walked in place, or copied + * through a shell that still carries its prototype. + * + * Like the other server-function specs, these run against the built bundles + * (server-functions/dist/*, wired up in vite.config.server.mjs), which are + * the production variant — `DEV` is false, so the sanitizer is live. + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createServerReference } 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]; +}); + +const H = { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Format": "8", + "X-Server-Function-Instance": "server-function:test" +}; + +/** The secret a driver error carries; it must never reach the wire. */ +const SECRET = "postgres://app:hunter2@10.0.0.5:5432"; +/** What the sanitizer puts on the wire in its place. */ +const SANITIZED = "Internal Server Error"; +/** The carrier's own message — the author's value, which must survive. */ +const CARRIER = "checkout failed"; + +class ValidationError extends Error {} + +/** A driver error as one actually arrives: secrets in message and own props. */ +function databaseError() { + return Object.assign(new Error(`connect ECONNREFUSED ${SECRET}`), { + connectionString: SECRET + }); +} + +function scriptedPost(id: string, signal?: AbortSignal) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: H, + signal + }); +} + +async function wireBody(id: string, fn: () => unknown) { + registerServerFunction(id, fn); + const response = await handleServerFunctionRequest(scriptedPost(id)); + return { status: response.status, body: await response.text() }; +} + +/** Routes the client stub's fetch straight into the handler. */ +function connectTransport() { + const original = globalThis.fetch; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const address = input instanceof Request ? input.url : input.toString(); + const request = new Request( + new URL(address, "http://localhost"), + input instanceof Request ? input : init + ); + request.headers.set("Sec-Fetch-Site", "same-origin"); + return handleServerFunctionRequest(request); + }) as typeof fetch; + return () => { + globalThis.fetch = original; + }; +} + +const tick = async (turns: number) => { + for (let turn = 0; turn < turns; turn++) await new Promise(resolve => setImmediate(resolve)); +}; + +describe("a non-plain carrier does not shelter a failure channel (#3200, result road)", () => { + // Every row is the SAME channel under a different carrier, and every row + // runs the plain-object control first: a row that fails while its control + // passes is the walk declining to look, not a broken harness. + const carriers: [string, () => object][] = [ + [ + "an Error carrying the channel as an own property", + () => Object.assign(new Error(CARRIER), { chan: Promise.reject(databaseError()) }) + ], + [ + "an Error subclass carrying the channel", + () => Object.assign(new ValidationError(CARRIER), { chan: Promise.reject(databaseError()) }) + ], + [ + "an AggregateError carrying the channel", + () => + Object.assign(new AggregateError([], CARRIER), { chan: Promise.reject(databaseError()) }) + ], + [ + "an Error nested one level inside a plain result", + () => ({ + outcome: Object.assign(new Error(CARRIER), { chan: Promise.reject(databaseError()) }) + }) + ], + [ + "an Error carrying an erroring stream", + () => + Object.assign(new Error(CARRIER), { + chan: new ReadableStream({ + start(controller) { + controller.enqueue("first"); + queueMicrotask(() => controller.error(databaseError())); + } + }) + }) + ], + [ + "an Error carrying an async iterable that throws", + () => + Object.assign(new Error(CARRIER), { + chan: (async function* () { + yield { page: 1 }; + throw databaseError(); + })() + }) + ] + ]; + + test.each(carriers)("%s is still sanitized", async (label, make) => { + const slug = label.replace(/[^a-z0-9]+/gi, "-").toLowerCase(); + let ran = 0; + + const control = await wireBody(`carrier-control-${slug}`, () => { + ran++; + return { chan: Promise.reject(databaseError()) }; + }); + // the control is the working road: the same channel under a plain + // object. It has to pass, or this row proves nothing. + expect({ + where: "control", + status: control.status, + leaked: control.body.includes(SECRET) + }).toEqual({ where: "control", status: 200, leaked: false }); + + const carried = await wireBody(`carrier-${slug}`, () => { + ran++; + return make(); + }); + + expect(ran).toBe(2); + expect({ + carrier: label, + status: carried.status, + secretOnWire: carried.body.includes(SECRET), + sanitizedMarker: carried.body.includes(SANITIZED), + carrierMessageKept: carried.body.includes(CARRIER) + }).toEqual({ + carrier: label, + status: 200, + secretOnWire: false, + sanitizedMarker: true, + // the carrier is the author's returned value: sanitizing what it + // holds must not cost it its own message + carrierMessageKept: true + }); + }); + + test("the carrier reaches the client as the Error the author returned", async () => { + registerServerFunction("carrier-roundtrip", async () => + Object.assign(new Error(CARRIER), { + code: "E_CHECKOUT", + chan: Promise.reject(databaseError()) + }) + ); + + const restore = connectTransport(); + let carrier: any; + try { + carrier = await (createServerReference("carrier-roundtrip") as () => Promise)(); + } finally { + restore(); + } + + // shape first: a fix that rebuilds the carrier through a plain shell + // changes the value the author returned, and fails right here + expect({ + isError: carrier instanceof Error, + message: carrier?.message, + code: carrier?.code + }).toEqual({ isError: true, message: CARRIER, code: "E_CHECKOUT" }); + + // and the channel it carries arrives sanitized + const settled = await carrier.chan.then( + (value: unknown) => ({ rejected: false, message: String(value) }), + (error: any) => ({ rejected: true, message: error?.message }) + ); + expect(settled).toEqual({ rejected: true, message: SANITIZED }); + }); + + test("an endless generator carried by an Error is torn down when the caller leaves", async () => { + let finallyRan = false; + let produced = 0; + registerServerFunction("carrier-teardown", async () => + Object.assign(new Error(CARRIER), { + feed: (async function* () { + try { + for (;;) { + produced++; + yield { n: produced }; + await new Promise(resolve => setTimeout(resolve, 5)); + } + } finally { + finallyRan = true; + } + })() + }) + ); + + const response = await handleServerFunctionRequest(scriptedPost("carrier-teardown")); + expect(response.status).toBe(200); + const reader = response.body!.getReader(); + await reader.read(); + await reader.cancel(); + await new Promise(resolve => setTimeout(resolve, 50)); + + // an unguarded producer keeps pumping into a response nobody reads: + // the generator's `finally` (a DB cursor's close, in the shape this + // exists for) never runs + expect({ finallyRan, producedAfterCancel: produced > 0 }).toEqual({ + finallyRan: true, + producedAfterCancel: true + }); + }); +}); + +describe("a channel on a non-enumerable own property is still the walk's (47995412)", () => { + /** A hidden own DATA property — no accessor, so nothing to invoke. */ + function hide(target: T, key: string, value: unknown) { + Object.defineProperty(target, key, { + value, + enumerable: false, + writable: true, + configurable: true + }); + return target; + } + + test("a rejecting promise there is sanitized instead of killing the process after the 200", async () => { + const unhandled: unknown[] = []; + const onUnhandled = (error: unknown) => unhandled.push(error); + process.on("unhandledRejection", onUnhandled); + // The other half of the same line: a hidden ACCESSOR must stay unread. + // A fix that walks all descriptors invokes this and fails the call. + let accessorReads = 0; + try { + registerServerFunction("hidden-slot-promise", async () => { + const result: any = hide({ ok: 1 }, "audit", Promise.reject(databaseError())); + Object.defineProperty(result, "costBasis", { + get() { + accessorReads++; + throw new Error("a hidden accessor must not be invoked to guard a sibling"); + }, + enumerable: false, + configurable: true + }); + return result; + }); + + const response = await handleServerFunctionRequest(scriptedPost("hidden-slot-promise")); + const body = await response.text(); + await tick(20); + + expect({ + status: response.status, + accessorReads, + // the hidden slot stays hidden: guarding it is not a licence to + // serialize what `enumerable: false` kept off the wire + hiddenSlotOnWire: body.includes("audit"), + secretOnWire: body.includes(SECRET), + // nobody owns an unguarded rejection in a slot the codec skips too + unhandled: unhandled.map((error: any) => error?.message) + }).toEqual({ + status: 200, + accessorReads: 0, + hiddenSlotOnWire: false, + secretOnWire: false, + unhandled: [] + }); + } finally { + process.off("unhandledRejection", onUnhandled); + } + }); + + test("a stream there is registered with the response teardown, so an abort closes it", async () => { + let hiddenCancelled = false; + let visibleTornDown = false; + registerServerFunction("hidden-slot-stream", async () => { + const hiddenFeed = new ReadableStream({ + pull(controller) { + controller.enqueue("x"); + }, + cancel() { + hiddenCancelled = true; + } + }); + const visible = (async function* () { + try { + for (let n = 0; ; n++) { + yield { n }; + await new Promise(resolve => setImmediate(resolve)); + } + } finally { + visibleTornDown = true; + } + })(); + return hide({ items: visible }, "hiddenFeed", hiddenFeed); + }); + + const controller = new AbortController(); + const response = await handleServerFunctionRequest( + scriptedPost("hidden-slot-stream", controller.signal) + ); + expect(response.status).toBe(200); + const reader = response.body!.getReader(); + await reader.read(); + controller.abort(); + await tick(60); + + // the visible source is the control — teardown reaches everything the + // walk visited, and only that + expect({ visibleTornDown, hiddenCancelled }).toEqual({ + visibleTornDown: true, + hiddenCancelled: true + }); + }); +}); diff --git a/packages/web/test/server/server-functions-hook-option-absence.spec.tsx b/packages/web/test/server/server-functions-hook-option-absence.spec.tsx new file mode 100644 index 000000000..f02d35bd3 --- /dev/null +++ b/packages/web/test/server/server-functions-hook-option-absence.spec.tsx @@ -0,0 +1,150 @@ +/** + * How a handler option spells "I am not overriding this" — and why the + * answer has to be the same for every hook. + * + * `handleServerFunctionRequest` resolves its hooks against the server-wide + * configuration two different ways: + * + * const provide = options.provideEvent || provideEvent; // null falls back + * const wrapInvocation = options.wrapInvocation !== undefined // null DISABLES + * ? options.wrapInvocation : config.wrapInvocation; + * + * The same absent-looking value therefore lands on opposite sides of the + * safety line, and the unsafe side is the security hook: `wrapInvocation` + * is the per-invocation seam an app hangs authorization on (see + * `server-functions-invocation-wrap.spec.tsx`), so a `null` here silently + * takes the app's gate off exactly one request and dispatch runs the body + * unguarded, with a 200. + * + * TypeScript callers are protected — `null` is not assignable to + * `WrapInvocationHook` — which is precisely why this is worth pinning: the + * callers that CAN produce it are the ones with no compiler watching. An + * options object assembled in a JS adapter, or computed as + * `{ wrapInvocation: perRoute.wrap ?? null }`, reads as "nothing to + * override" everywhere else in this file and disables the gate here. + * + * The invariant: an option value that is not a hook cannot REMOVE policy. + * Overriding the configured wrap requires supplying a wrap; absence — in + * whatever spelling — means the configured one still owns the call. + * (`transformResult` and the flight hooks resolve the same way, but they + * are result policy, not the gate; only the security hook is pinned here.) + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { createRequestEvent } from "@solidjs/web"; +import { + configureServerFunctionsServer, + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +/** + * `configure` ignores `undefined` — that is its spelling of "not + * overriding" — so undoing a hook between tests takes a value. `null` is + * the falsy "nothing configured" every read site tests for; the cast is + * only because the option type describes hooks, not their absence. + */ +const NO_HOOK = null as any; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterEach(() => { + configureServerFunctionsServer({ wrapInvocation: NO_HOOK }); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const createEvent = (request: Request) => createRequestEvent(request); + +/** A scripted POST call, the shape the client transport produces. */ +function post(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: { + "Sec-Fetch-Site": "same-origin", + "content-type": "application/json", + "X-Server-Function-Format": "8", + "X-Server-Function-Instance": "server-function:test" + } + }); +} + +/** + * The app's configured gate: it refuses every call with a 403 and never + * lets the body run. Each call reports what actually happened, so a + * failure names the leak instead of an opaque status. + */ +function withConfiguredGate(id: string) { + let gateRan = 0; + let bodyRan = 0; + configureServerFunctionsServer({ + wrapInvocation: () => { + gateRan++; + throw new Response(null, { status: 403 }); + } + }); + registerServerFunction(id, async () => { + bodyRan++; + return "the secret"; + }); + return async (options: Record) => { + gateRan = 0; + bodyRan = 0; + const response = await handleServerFunctionRequest(post(id), { createEvent, ...options }); + return { + status: response.status, + gateRan, + bodyRan, + leaked: (await response.text()).includes("the secret") + }; + }; +} + +describe("a handler option cannot take the configured authorization gate off", () => { + it("runs the configured wrap when no option is supplied at all", async () => { + const call = await withConfiguredGate("absence-baseline"); + expect(await call({})).toStrictEqual({ status: 403, gateRan: 1, bodyRan: 0, leaked: false }); + }); + + it("runs the configured wrap when the option is explicitly undefined", async () => { + const call = await withConfiguredGate("absence-undefined"); + expect(await call({ wrapInvocation: undefined })).toStrictEqual({ + status: 403, + gateRan: 1, + bodyRan: 0, + leaked: false + }); + }); + + it("runs the configured wrap when the option is null, the other spelling of absent", async () => { + const call = await withConfiguredGate("absence-null"); + // today: { status: 200, gateRan: 0, bodyRan: 1, leaked: true } — the + // gate is skipped for this one request and the body answers the caller + expect(await call({ wrapInvocation: null })).toStrictEqual({ + status: 403, + gateRan: 1, + bodyRan: 0, + leaked: false + }); + }); + + it("already treats a null provideEvent as absent, which is the direction to match", async () => { + const call = await withConfiguredGate("absence-provide-event"); + expect(await call({ provideEvent: null })).toStrictEqual({ + status: 403, + gateRan: 1, + bodyRan: 0, + leaked: false + }); + }); +}); diff --git a/packages/web/test/server/server-functions-negative-zero.spec.tsx b/packages/web/test/server/server-functions-negative-zero.spec.tsx new file mode 100644 index 000000000..5967d0201 --- /dev/null +++ b/packages/web/test/server/server-functions-negative-zero.spec.tsx @@ -0,0 +1,162 @@ +/** + * The JSON fast path is a negotiation, not a coercion: a value takes it + * only when `JSON.stringify` carries it FAITHFULLY. That is the whole + * contract of `isJSONSafe` (server-functions/shared.js), which both peers + * consult — the client for argument lists, the server for results — so the + * codec rides the wire exactly when a value actually needs it. + * + * `-0` is the one number that breaks the contract quietly. `JSON.stringify(-0)` + * is `"0"`, but the guard admits any `Number.isFinite(v)`, so a signed zero + * takes the fast path and arrives as `+0`. Status 200, the function runs, + * the sign is simply gone — the failure mode the guard's own siblings were + * written to prevent (`undefined` corrupted to `null`, a sparse hole read + * back as `null`). + * + * `NaN` and `Infinity`, the other numbers stringify cannot carry, are + * already refused and ride the codec instead — and the codec encodes `-0` + * exactly, as its own constant. So this is not "JSON cannot carry it"; it + * is the fast path claiming a value that belongs to the codec, and it is + * the one leg of the finite-number guard nobody mirrored. Each test below + * pairs its case with the `NaN` control that already behaves, so a reader + * can see the two halves disagree rather than take the claim on faith. + * + * The sign is load-bearing wherever a signed zero is the value: a delta + * that decreased to nothing, a coordinate approached from the negative + * side, `1 / x` read back as a direction. + * + * 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 { + configureServerFunctionsClient, + createServerReference, + getServerFunctionsCodec, + serializeString +} from "@solidjs/web/server-functions/client"; + +const RequestContext = Symbol.for("solid.RequestContext"); +const BODY_FORMAT_HEADER = "X-Server-Function-Format"; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); + // What `enableRichArguments()` installs, spelled out here because the + // rich-args entry has no alias in this config. It matters only for the + // ARGUMENT direction: with the codec available for arguments, nothing + // forces the fast path's hand, so the question these tests ask is purely + // "which encoding does the guard choose for -0" and not "does the client + // have anywhere else to put it". Results never needed the opt-in: the + // handler always holds both halves of the codec. + configureServerFunctionsClient({ + serializeArgs: args => serializeString(args, getServerFunctionsCodec()) + }); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +// The client transport's fetch dispatches into the built server handler, so +// each test is a full round trip through both published bundles, and every +// request is captured to show which encoding negotiation actually picked. +function connectTransport() { + const original = globalThis.fetch; + const requests: Request[] = []; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const request = + input instanceof Request + ? input + : new Request(new URL(input.toString(), "https://app.example"), init); + request.headers.set("Sec-Fetch-Site", "same-origin"); + requests.push(request.clone()); + return handleServerFunctionRequest(request); + }) as typeof fetch; + return { + requests, + restore() { + globalThis.fetch = original; + } + }; +} + +describe("a signed zero on the wire", () => { + it("reaches the server function as -0, the way NaN already does", async () => { + const seen: Record = {}; + registerServerFunction("negative-zero-arg", async (n: number) => { + seen.arg = n; + return "ok"; + }); + registerServerFunction("nan-arg", async (n: number) => { + seen.control = n; + return "ok"; + }); + const transport = connectTransport(); + try { + // the control: NaN is refused by the fast path, rides the codec, and + // arrives intact — the behaviour -0 is measured against + await createServerReference("nan-arg")(NaN); + expect(Number.isNaN(seen.control), `NaN control arrived as ${String(seen.control)}`).toBe( + true + ); + + await createServerReference("negative-zero-arg")(-0); + expect( + Object.is(seen.arg, -0), + `the function was handed ${Object.is(seen.arg, -0) ? "-0" : String(seen.arg)}` + + ` (1/x = ${1 / (seen.arg as number)}), sent as ${await transport.requests[1].clone().text()}` + + ` under format ${transport.requests[1].headers.get(BODY_FORMAT_HEADER)}` + ).toBe(true); + } finally { + transport.restore(); + } + }); + + it("comes back from the server function as -0, the way NaN already does", async () => { + registerServerFunction("negative-zero-result", async () => -0); + registerServerFunction("nan-result", async () => NaN); + const transport = connectTransport(); + try { + const control = await createServerReference("nan-result")(); + expect(Number.isNaN(control), `NaN control came back as ${String(control)}`).toBe(true); + + const result = await createServerReference("negative-zero-result")(); + expect( + Object.is(result, -0), + `the call resolved with ${Object.is(result, -0) ? "-0" : String(result)}` + + ` (1/x = ${1 / (result as number)})` + ).toBe(true); + } finally { + transport.restore(); + } + }); + + it("keeps its sign inside an otherwise JSON-safe object result", async () => { + // the whole object rides one encoding, so a single unsafe value drags + // the rest onto the codec: with NaN alongside it the -0 survives today, + // and without it the same field is flattened. Same data, same shape — + // only the company it keeps decides whether the sign lives. + registerServerFunction("negative-zero-field", async () => ({ delta: -0 })); + registerServerFunction("negative-zero-field-with-nan", async () => ({ delta: -0, other: NaN })); + const transport = connectTransport(); + try { + const dragged: any = await createServerReference("negative-zero-field-with-nan")(); + expect( + Object.is(dragged.delta, -0), + `the codec road lost the sign too: delta=${String(dragged.delta)}` + ).toBe(true); + + const plain: any = await createServerReference("negative-zero-field")(); + expect( + Object.is(plain.delta, -0), + `the same field, alone, came back as ${String(plain.delta)}` + ).toBe(true); + } finally { + transport.restore(); + } + }); +}); diff --git a/packages/web/test/server/server-functions-nested-deferred-scope.spec.tsx b/packages/web/test/server/server-functions-nested-deferred-scope.spec.tsx new file mode 100644 index 000000000..40f4dad2e --- /dev/null +++ b/packages/web/test/server/server-functions-nested-deferred-scope.spec.tsx @@ -0,0 +1,177 @@ +/** + * #3222 ON THE DIRECT SSR ROAD, ONE CONTAINER DOWN. + * + * Calling a generator only allocates it; calling a stream's reader is what + * runs its pull. So the request scope around the CALL does not own either + * body — the consumer drives it later, from whatever async context it + * happens to be in, which during SSR is the render's ambient event. + * `scopeDeferredResult` exists to bind those deferred operations to the + * per-call event instead, and `server-functions-request-event-scope.spec.tsx` + * pins it for a body the function RETURNS DIRECTLY. + * + * It only ever looks at that top level. A generator or stream handed back + * inside an object or an array — `return { rows: cursor() }`, the shape the + * codec road's guard walk was taught to descend into for exactly this + * reason — is left bound to nothing, and #3222's harm comes straight back: + * + * - the body reads and WRITES the render's `locals` rather than the + * per-call copy #3156 made for it, so two concurrent direct calls see + * each other's request state (call A reading call B's tenant, auth, + * DB handle); + * - the render's own `locals` is mutated by a call that was supposed to + * be unable to reach it. + * + * Each test runs two concurrent calls whose bodies interleave by design + * (A sleeps longer than B, so B's write lands while A is parked). Correctly + * scoped, each call reads back its OWN write — [["A:A"], ["B:B"]]; sharing + * the ambient event gives A whatever B wrote last — [["A:B"], ["B:B"]]. + * The call counter proves both bodies actually ran, so an assertion cannot + * pass on a result nobody produced. + * + * 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, getRequestEvent } from "@solidjs/web"; +import { createServerReference } from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); +const requestContext = new AsyncLocalStorage(); + +beforeAll(() => { + (globalThis as any)[RequestContext] = requestContext; +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + +/** A's body parks long enough for B's whole call to land inside it. */ +const pause = (who: string) => delay(who === "A" ? 30 : 5); + +function renderEvent() { + const event = createRequestEvent(new Request("https://app.example/page")); + event.locals.writer = "none"; + return event; +} + +async function drain(source: AsyncIterable) { + const values: T[] = []; + for await (const value of source) values.push(value); + return values; +} + +async function readAll(stream: ReadableStream) { + const reader = stream.getReader(); + const values: T[] = []; + for (;;) { + const step = await reader.read(); + if (step.done) return values; + values.push(step.value); + } +} + +/** The body every case shares: write, park, read back what I wrote. */ +async function* witness(who: string) { + getRequestEvent()!.locals.writer = who; + await pause(who); + yield `${who}:${getRequestEvent()!.locals.writer}`; +} + +describe("a deferred body nested in the result of a direct SSR call (#3222)", () => { + it("isolates concurrent calls whose generator is nested in an object", async () => { + let calls = 0; + const reference = createServerReference({ + id: "nested-scope-object", + name: "nestedScopeObject", + fn: async (who: string) => { + calls++; + return { rows: witness(who) }; + } + } as any) as (who: string) => Promise<{ rows: AsyncIterable }>; + const render = renderEvent(); + + const values = await requestContext.run(render, () => + Promise.all([ + reference("A").then(result => drain(result.rows)), + reference("B").then(result => drain(result.rows)) + ]) + ); + + expect(calls).toBe(2); + expect({ values, renderWriter: render.locals.writer }).toEqual({ + values: [["A:A"], ["B:B"]], + // #3156's per-call copy is only per-call while the body runs under + // the derived event; unscoped, the nested generator writes through + // to the render itself + renderWriter: "none" + }); + }); + + it("isolates concurrent calls whose generator is nested in an array", async () => { + let calls = 0; + const reference = createServerReference({ + id: "nested-scope-array", + name: "nestedScopeArray", + fn: async (who: string) => { + calls++; + return [witness(who)]; + } + } as any) as (who: string) => Promise[]>; + const render = renderEvent(); + + const values = await requestContext.run(render, () => + Promise.all([ + reference("A").then(result => drain(result[0])), + reference("B").then(result => drain(result[0])) + ]) + ); + + expect(calls).toBe(2); + expect({ values, renderWriter: render.locals.writer }).toEqual({ + values: [["A:A"], ["B:B"]], + renderWriter: "none" + }); + }); + + it("isolates concurrent calls whose stream is nested in an object", async () => { + let calls = 0; + const reference = createServerReference({ + id: "nested-scope-stream", + name: "nestedScopeStream", + fn: async (who: string) => { + calls++; + return { + feed: new ReadableStream( + { + async pull(controller) { + getRequestEvent()!.locals.writer = who; + await pause(who); + controller.enqueue(`${who}:${getRequestEvent()!.locals.writer}`); + controller.close(); + } + }, + { highWaterMark: 0 } + ) + }; + } + } as any) as (who: string) => Promise<{ feed: ReadableStream }>; + const render = renderEvent(); + + const values = await requestContext.run(render, () => + Promise.all([ + reference("A").then(result => readAll(result.feed)), + reference("B").then(result => readAll(result.feed)) + ]) + ); + + expect(calls).toBe(2); + expect({ values, renderWriter: render.locals.writer }).toEqual({ + values: [["A:A"], ["B:B"]], + renderWriter: "none" + }); + }); +}); diff --git a/packages/web/test/server/server-functions-nojs-refusal-destination.spec.tsx b/packages/web/test/server/server-functions-nojs-refusal-destination.spec.tsx new file mode 100644 index 000000000..4d28e7894 --- /dev/null +++ b/packages/web/test/server/server-functions-nojs-refusal-destination.spec.tsx @@ -0,0 +1,156 @@ +/** + * The no-JS convention's promise covers the refusals too. + * + * `createNoJSHandler`'s contract is stated as an absolute — "the browser is + * never left on the endpoint" (server.ts ~1705, and the describe name in + * server-functions-nojs-destination.spec.tsx): a form post has no channel + * to receive a value, so the answer is a redirect and the outcome rides the + * flash cookie. The handler honours that for everything it is HANDED. + * + * It is handed nothing when the request is refused before dispatch, because + * the convention is chosen at server.ts ~3055 — AFTER the gates. A stale id + * from before a deploy, a malformed multipart body, an upload past + * `bodySizeLimit`, a request the origin check cannot vouch for: each + * answers 404 / 400 / 413 / 403 with no `Location` and, in production, + * no body. What the user sees is a blank page at `/_server/` with the + * back button as the only way out and everything they typed gone. + * + * This is a progressive-enhancement hole, not a correctness one: the + * mutation has NOT committed in any of these cases — each test proves that + * with a counter — so there is nothing to double-submit. That is exactly + * why the answer can be the ordinary bounce back to the form. + * + * The origin refusal is pinned for destination only. Where the outcome + * should be legible to the next render is pinned on the deploy-skew case: + * the id in the shipped HTML is stale, the user is not at fault, and a + * silent bounce back to an unchanged form is the failure the flash cookie + * exists to avoid. + * + * 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 { + FLASH_COOKIE, + decodeFlashCookie, + 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 ORIGIN = "https://app.example"; +const FORM_PAGE = `${ORIGIN}/settings`; + +let ran = 0; +registerServerFunction("nojs-refusal-save", async (...args: unknown[]) => { + ran++; + return { saved: args.length }; +}); + +/** + * A real browser form navigation: no `X-Server-Function-Instance`, a form + * content type, and no `Sec-Fetch-Mode` (which dispatch reads as navigate, + * the older-browser spelling — #3139). + */ +function formNavigation( + id: string, + { + contentType = "application/x-www-form-urlencoded", + body = "name=Ada", + referer = FORM_PAGE as string | null + } = {} +) { + const headers: Record = { + "Sec-Fetch-Site": "same-origin", + "Content-Type": contentType + }; + if (referer) headers.Referer = referer; + return new Request(`${ORIGIN}/_server/${id}`, { method: "POST", headers, body }); +} + +/** The destination assertion the convention promises, whatever went wrong. */ +function expectsBounceBack(response: Response, message: string) { + const location = response.headers.get("Location"); + expect(response.status, `${message} — status ${response.status}`).toBe(303); + expect(location, `${message} — no Location, the browser stays on the endpoint`).not.toBeNull(); + expect(new URL(location!, ORIGIN).origin).toBe(ORIGIN); +} + +function flashed(response: Response) { + const cookie = response.headers + .getSetCookie() + .find(entry => entry.startsWith(`${FLASH_COOKIE}=`)); + return cookie ? decodeFlashCookie(cookie.split(";")[0]) : undefined; +} + +describe("a refused form navigation is bounced back, not stranded", () => { + it("returns to the form when the id is stale after a deploy", async () => { + const before = ran; + const response = await handleServerFunctionRequest(formNavigation("nojs-refusal-retired-id")); + + expect(ran - before).toBe(0); // nothing registered at that id could have run + expectsBounceBack(response, "unknown server function"); + }); + + it("tells the next render that the stale-id submission failed", async () => { + const response = await handleServerFunctionRequest(formNavigation("nojs-refusal-retired-id-2")); + + // a silent bounce back to the same form reads as "nothing happened", + // which is the read that makes a user submit again + const submission = flashed(response); + expect(submission, "no outcome cookie — the form re-renders as if untouched").toBeDefined(); + expect(submission?.error).toBeInstanceOf(Error); + expect(submission?.result).toBeUndefined(); + }); + + it("returns to the form when the multipart body is malformed", async () => { + const before = ran; + const response = await handleServerFunctionRequest( + formNavigation("nojs-refusal-save", { + contentType: "multipart/form-data; boundary=----SolidBoundary", + body: "this is not a multipart body" + }) + ); + + expect(ran - before).toBe(0); + expectsBounceBack(response, "malformed arguments"); + }); + + it("returns to the form when the upload runs past bodySizeLimit", async () => { + const before = ran; + const response = await handleServerFunctionRequest( + formNavigation("nojs-refusal-save", { body: "note=" + "A".repeat(5000) }), + { bodySizeLimit: 100 } + ); + + expect(ran - before).toBe(0); + expectsBounceBack(response, "body over the limit"); + }); + + it("returns to the app when the origin check cannot vouch for the post", async () => { + // a privacy extension, a `Referrer-Policy: no-referrer` page, an + // embedded webview: no fetch metadata, so the gate refuses — and there + // is no referer to return to either, which is what `base` is for + const before = ran; + const response = await handleServerFunctionRequest( + new Request(`${ORIGIN}/_server/nojs-refusal-save`, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body: "name=Ada" + }) + ); + + expect(ran - before).toBe(0); + expectsBounceBack(response, "origin check refused"); + }); +}); diff --git a/packages/web/test/server/server-functions-rejected-promise-arguments.spec.tsx b/packages/web/test/server/server-functions-rejected-promise-arguments.spec.tsx new file mode 100644 index 000000000..08b3afef8 --- /dev/null +++ b/packages/web/test/server/server-functions-rejected-promise-arguments.spec.tsx @@ -0,0 +1,178 @@ +/** + * A decoded argument must never hand the process a rejection nobody holds. + * + * seroval's cross-JSON has two promise spellings, and the decode boundary + * only ever defused one of them. The spelling an honest encoder emits is + * the CONSTRUCTOR pair — a pending promise under one id, its resolver under + * the special reference next to it — settled by a later chunk; that pair is + * exactly what `createJSONDeserializer.abort` sweeps (`{p, s, f}`), and the + * comment there explains why: "a rejection nobody awaited ... surfacing as + * an unhandled rejection". The decoder ALSO accepts an atomic promise node + * (seroval type 12), which no encoder in this codebase writes. It settles + * synchronously while the first chunk is still decoding and stores the bare + * promise, so the sweep never sees it and the guard covers one leg only. + * + * Handed to a server function as an argument, an already-rejected promise + * is a rejection nobody awaits: an ordinary function does not await an + * argument it never expected to be a promise. Node's default policy for an + * unhandled rejection is to kill the process — so one 115-byte request ends + * every in-flight request on the pod, on every tenant it serves. The + * response is 200: the function ran, and the process dies behind it. The + * request needs nothing privileged. `Sec-Fetch-Site` is forbidden to page + * script but is one header to curl, and function ids ship in the client + * bundle the compiler emits. + * + * These tests take over the `unhandledRejection` event for the length of + * one call. Under the runner vitest owns that event and would report the + * escape as a file-level error; the process-level answer IS the finding, so + * it is asserted here by name rather than left to the runner's channel. + * + * The resolved-flag control in the first test is the argument that this is + * the decode boundary's problem and not the payload's: the two frames are + * the same bytes but for `"s"`. + * + * 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"; + +const RequestContext = Symbol.for("solid.RequestContext"); +const BODY_FORMAT_HEADER = "X-Server-Function-Format"; +const SERIALIZED_FORMAT = "0"; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const provideEvent = (_event: unknown, run: () => T): T => run(); + +/** Wraps a payload in the codec's length-prefixed frame. */ +function frame(payload: string) { + const length = new TextEncoder().encode(payload).byteLength; + return `;0x${length.toString(16).padStart(8, "0")};${payload}`; +} + +/** An argument array whose single element is the given node. */ +const argumentArray = (node: string) => `{"t":9,"i":0,"a":[${node}],"o":0}`; + +/** + * The atomic promise node: `s` is the settlement flag (0 rejected, 1 + * fulfilled) and `f` the settled value. Handwritten because the encoder + * never produces this shape — but the wire is text, and a peer writes + * whatever it likes. + */ +const REJECTED = `{"t":12,"i":1,"s":0,"f":{"t":13,"i":2,"s":0,"m":"pwned","p":{"k":[],"v":[]}}}`; +const FULFILLED = `{"t":12,"i":1,"s":1,"f":{"t":1,"s":"fine"}}`; +/** The same rejected promise, one level down inside a plain object. */ +const NESTED_REJECTED = + `{"t":10,"i":1,"p":{"k":["deep"],"v":[` + + `{"t":12,"i":2,"s":0,"f":{"t":13,"i":3,"s":0,"m":"pwned","p":{"k":[],"v":[]}}}` + + `],"s":1}}`; + +/** + * Runs one dispatch owning `unhandledRejection`, and reports what escaped. + * Two macrotask turns: Node emits the event after the microtask queue + * drains, and the argument graph settles inside the dispatch's own await. + */ +async function watchRejections(run: () => Promise) { + const previous = process.listeners("unhandledRejection"); + process.removeAllListeners("unhandledRejection"); + const escaped: string[] = []; + const capture = (reason: unknown) => escaped.push(String(reason)); + process.on("unhandledRejection", capture); + try { + const response = await run(); + await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise(resolve => setTimeout(resolve, 0)); + return { escaped, status: response.status }; + } finally { + process.off("unhandledRejection", capture); + for (const listener of previous) process.on("unhandledRejection", listener as any); + } +} + +let seq = 0; + +/** Registers a function that records the argument it was handed. */ +function registerProbe() { + const id = `rejected-promise-argument-${seq++}`; + const probe = { id, ran: 0, seen: undefined as unknown }; + registerServerFunction(id, async (first: unknown) => { + probe.ran++; + probe.seen = first; + return "ok"; + }); + return probe; +} + +const codecBody = (id: string, node: string) => + new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + headers: { + "Content-Type": "text/plain", + [BODY_FORMAT_HEADER]: SERIALIZED_FORMAT, + "Sec-Fetch-Site": "same-origin" + }, + body: frame(argumentArray(node)) + }); + +const codecQuery = (id: string, node: string) => + new Request( + `https://app.example/_server/data/${id}?args=${encodeURIComponent(frame(argumentArray(node)))}`, + { method: "POST", headers: { "Sec-Fetch-Site": "same-origin" } } + ); + +describe("a decoded argument that is an already-rejected promise", () => { + it("does not escape as an unhandled rejection when it arrives in the body", async () => { + // the control first: the same node with the fulfilled flag, which is + // the only byte that differs, and which nothing about this boundary + // should treat specially + const control = registerProbe(); + const fulfilled = await watchRejections(() => + handleServerFunctionRequest(codecBody(control.id, FULFILLED), { provideEvent }) + ); + expect( + fulfilled.escaped, + `the fulfilled control escaped: ${fulfilled.escaped.join(", ")}` + ).toEqual([]); + + const probe = registerProbe(); + const { escaped, status } = await watchRejections(() => + handleServerFunctionRequest(codecBody(probe.id, REJECTED), { provideEvent }) + ); + expect( + escaped, + `status=${status} ran=${probe.ran} argumentIsPromise=${probe.seen instanceof Promise}` + + ` — an unhandled rejection here is process death under Node's default policy` + ).toEqual([]); + }); + + it("does not escape when the codec frame rides the url's args instead", async () => { + const probe = registerProbe(); + const { escaped, status } = await watchRejections(() => + handleServerFunctionRequest(codecQuery(probe.id, REJECTED), { provideEvent }) + ); + expect(escaped, `status=${status} ran=${probe.ran}`).toEqual([]); + }); + + it("does not escape when it sits inside an ordinary object argument", async () => { + const probe = registerProbe(); + const { escaped, status } = await watchRejections(() => + handleServerFunctionRequest(codecBody(probe.id, NESTED_REJECTED), { provideEvent }) + ); + expect( + escaped, + `status=${status} ran=${probe.ran} — the guard must cover the whole argument` + + ` graph, the way stripUnsafeArgumentKeys already walks it` + ).toEqual([]); + }); +}); diff --git a/packages/web/test/server/server-functions-response-buffering.spec.tsx b/packages/web/test/server/server-functions-response-buffering.spec.tsx new file mode 100644 index 000000000..049eae748 --- /dev/null +++ b/packages/web/test/server/server-functions-response-buffering.spec.tsx @@ -0,0 +1,137 @@ +/** + * The transport must not tee a response body it never reads. + * + * `Response.clone()` is not free and it is not a copy: it tees the body, and + * the branch nobody drains queues every byte that passes through the branch + * somebody does. One unread clone therefore costs the whole payload in + * memory, for as long as the read branch runs. + * + * The client makes two of them per call. It decodes `response.clone()`, and + * `extractBody` clones AGAIN before reading. Nothing ever reads the outer + * response, and nothing ever reads the intermediate clone, so a streamed + * result is buffered twice over on top of the copy the caller asked for. + * Measured peak heap+external over the baseline for one streamed result: + * 16 MiB -> 47.9, 64 MiB -> 162.6, 128 MiB -> 310.9 — around 2.4x the + * payload, against ~0.4x with the redundant tees gone and the decoded + * frames byte-identical. + * + * The invariant pinned here is the one that survives whichever clone turns + * out to be load-bearing: every clone the transport makes on the way to a + * result must be READ. Exactly one of the two has a reason to exist — + * `decodeResponse` is the integration-facing entry, where the caller still + * owns the response it handed over and may read it again, and its contract + * says so out loud — so one clone per call is allowed and zero is allowed; + * two, with the first abandoned, is the waste. + * + * Deliberately structural rather than a heap measurement: the tee is the + * mechanism, and a byte count taken inside a worker pool measures the other + * tests as much as this one. + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createServerReference } 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]; +}); + +const disconnects: (() => void)[] = []; +afterEach(() => { + while (disconnects.length) disconnects.pop()!(); +}); + +/** Routes the client stub's fetch straight into the built handler. */ +function connectTransport() { + const original = globalThis.fetch; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const address = input instanceof Request ? input.url : input.toString(); + const request = new Request( + new URL(address, "http://localhost"), + input instanceof Request ? input : init + ); + request.headers.set("Sec-Fetch-Site", "same-origin"); + return handleServerFunctionRequest(request); + }) as typeof fetch; + disconnects.push(() => { + globalThis.fetch = original; + }); +} + +/** + * Records every body tee that happens while `run` is in flight. A clone + * left with `bodyUsed === false` is a queue that filled for nobody. + */ +async function tees(run: () => Promise): Promise<{ value: T; clones: Response[] }> { + const original = Response.prototype.clone; + const clones: Response[] = []; + Response.prototype.clone = function clone(this: Response) { + const teed = original.call(this); + if (this.body) clones.push(teed); + return teed; + }; + try { + const value = await run(); + return { value, clones }; + } finally { + Response.prototype.clone = original; + } +} + +const abandoned = (clones: Response[]) => clones.filter(clone => !clone.bodyUsed).length; + +describe("server-function response buffering", () => { + it("reads every body it tees, on each encoding a result can ride", async () => { + // one per format the response side negotiates: the JSON fast path, the + // streaming codec, and a value with a natural HTTP encoding of its own + registerServerFunction("buffer-json", async () => ({ items: [1, 2, 3] })); + registerServerFunction("buffer-serialized", async () => new Date(0)); + registerServerFunction("buffer-native", async () => "here"); + + for (const [id, expected] of [ + ["buffer-json", { items: [1, 2, 3] }], + ["buffer-serialized", new Date(0)], + ["buffer-native", "here"] + ] as const) { + connectTransport(); + const { value, clones } = await tees(() => createServerReference(id)()); + // the result itself is the control: whatever the fix does to the + // clones, the decoded value may not move + expect(value).toEqual(expected); + expect( + abandoned(clones), + `${id}: ${abandoned(clones)} of ${clones.length} teed bodies were never read` + ).toBe(0); + expect(clones.length, `${id} teed its body ${clones.length} times`).toBeLessThanOrEqual(1); + } + }); + + it("does not tee a streamed result once per layer it passes through", async () => { + // The shape the cost is paid on: a body that arrives over many frames, + // where each abandoned tee queues the whole stream rather than a header. + registerServerFunction("buffer-stream", async function* () { + for (let index = 0; index < 64; index++) yield "x".repeat(1024); + }); + connectTransport(); + const { value, clones } = await tees(() => createServerReference("buffer-stream")()); + let bytes = 0; + for await (const frame of value as AsyncIterable) bytes += frame.length; + expect(bytes).toBe(64 * 1024); + expect( + abandoned(clones), + `${abandoned(clones)} of ${clones.length} teed bodies queued the whole stream for nobody` + ).toBe(0); + }); +}); diff --git a/packages/web/test/server/server-functions-response-proto-keys.spec.tsx b/packages/web/test/server/server-functions-response-proto-keys.spec.tsx new file mode 100644 index 000000000..f551cf333 --- /dev/null +++ b/packages/web/test/server/server-functions-response-proto-keys.spec.tsx @@ -0,0 +1,365 @@ +/** + * The strip belongs to the decode BOUNDARY, not to the request leg. + * + * #3168/#3200/#3202 taught the argument decoder to delete `__proto__`, + * `constructor` and `prototype` from every decoded argument graph, on the + * stated grounds that the handler's most ordinary downstream move — + * merging a decoded object — turns the key into prototype pollution. The + * response leg decodes with the same primitives (`decodeResponse` -> + * `extractBody` -> `JSON.parse` for the fast path, `deserializeStream` for + * a codec frame) and stripped nothing: the walk sat in server.ts behind + * the argument decoder, and the client bundle carried no mirror of it. A + * guard that exists on one leg and was never mirrored on the other is the + * whole finding; these specs pin the missing half, now held by a single + * strip at the shared decode boundary — `extractBody`, the one function an + * argument body and a response body both pass through. + * + * The peer supplying the key is NOT a hostile server. It is the most + * ordinary server function there is — + * + * const loadProfile = async (raw: string) => JSON.parse(raw); + * + * — handing back a document the user wrote. `JSON.parse` makes + * `"__proto__"` an ordinary own property, the encoder puts it on the wire + * verbatim on both roads, and the client's decoder hands it to the caller + * with the own descriptor intact. The sink is the same one #3168 named, + * and it now fires in the BROWSER, on the page's single shared + * `Object.prototype`, which every framework internal and every third-party + * script on that page reads through. The blast radius grew; the guard did + * not follow. + * + * So the assertions are not "the client happens to be safe today" but + * "`Object.prototype` is untouched after a naive merge of a decoded + * RESULT" and "the response decoder removes exactly the keys the argument + * decoder removes" — across both wire roads, the collections the codec + * revives, the single-flight envelope routers seed caches from, and + * `decodeResponse` itself, which is the decoder integrations call by hand. + * + * `constructorName` rides along in the key-parity table as 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), + * so they check the artifacts the package actually publishes. + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { + createServerReference, + decodeResponse, + serializeString, + subscribeFlightData +} from "@solidjs/web/server-functions/client"; + +const RequestContext = Symbol.for("solid.RequestContext"); +const BODY_FORMAT_HEADER = "X-Server-Function-Format"; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +// Nothing here may leave the shared prototype dirty for the next file in +// the worker — a leaked key would make an unrelated spec fail somewhere +// else entirely. +afterEach(() => { + delete (Object.prototype as any).polluted; +}); + +/** 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; +} + +/** Reads and clears the pollution flag in one move. */ +function takePollution() { + const leaked = (Object.prototype as any).polluted; + delete (Object.prototype as any).polluted; + return leaked; +} + +/** + * The two roads a successful result can travel. `json` is the fast path + * (format 8, `JSON.stringify` on the way out and `JSON.parse` on the way + * back); `codec` is a serialized frame (format 0), which an honest result + * takes as soon as it carries anything JSON cannot spell — here a `Date` + * sibling, the most banal reason a real payload leaves the fast path. + */ +type Road = "json" | "codec"; + +let seq = 0; +/** Body format observed on the wire, per road, so the roads are provably distinct. */ +const observedFormat: Record = {}; +/** Proves the registered function body ran rather than the call short-circuiting. */ +let invocations = 0; + +/** + * One real round trip: the built client stub calls out through `fetch`, + * the built server handler answers, and the value comes back through + * `decodeResponse` exactly as an application would receive it. + */ +async function callServerFunction(road: Road, result: unknown) { + const id = `response-proto-${seq++}`; + registerServerFunction(id, async () => { + invocations++; + // The codec road needs one non-JSON value somewhere in the graph; the + // payload itself is identical on both roads. + return road === "codec" ? { payload: result, at: new Date(0) } : result; + }); + const original = globalThis.fetch; + let status = 0; + globalThis.fetch = (async (url: string, init?: RequestInit) => { + const request = new Request(new URL(url, "http://localhost"), init); + request.headers.set("Sec-Fetch-Site", "same-origin"); + const response = await handleServerFunctionRequest(request); + status = response.status; + observedFormat[road] = response.headers.get(BODY_FORMAT_HEADER); + return response; + }) as typeof fetch; + try { + const decoded: any = await createServerReference(id)(); + return { status, value: road === "codec" ? decoded.payload : decoded }; + } finally { + globalThis.fetch = original; + } +} + +const ROADS: Road[] = ["json", "codec"]; + +/** + * User documents, parsed by an ordinary handler, each paired with the + * roads it can honestly travel. Written through `JSON.parse` rather than + * as literals: an object literal's `__proto__` sets the prototype instead + * of creating an own key, which is precisely the difference that makes the + * parsed form dangerous. + * + * `constructor` is json-only, and not because the codec road is safe: the + * codec's ENCODE half refuses an object with an own `constructor` (the + * call answers 500 before anything is decoded), so no honest server can + * put that key in a frame. A peer writing the frame by hand still can — + * that cell is covered by the `decodeResponse` case below, the same + * hand-built-frame technique the argument-leg spec uses. + */ +const PAYLOADS: [string, unknown, Road[]][] = [ + [ + "__proto__", + JSON.parse('{"displayName":"ada","__proto__":{"polluted":"viaProto"},"n":1}'), + ["json", "codec"] + ], + [ + "constructor", + JSON.parse('{"displayName":"ada","constructor":{"prototype":{"polluted":"viaCtor"}},"n":1}'), + ["json"] + ], + [ + "constructor nested one level", + JSON.parse('{"a":{"constructor":{"prototype":{"polluted":"viaNested"}}},"n":1}'), + ["json"] + ], + [ + "__proto__ inside an array", + JSON.parse('[{"__proto__":{"polluted":"viaArray"}}]'), + ["json", "codec"] + ] +]; + +describe("a decoded RESULT cannot reach Object.prototype either", () => { + it("no dangerous key survives a response into a recursive merge, on either road", async () => { + const before = invocations; + let calls = 0; + const rows: string[] = []; + for (const [name, payload, roads] of PAYLOADS) { + for (const road of roads) { + calls++; + const { status, value } = await callServerFunction(road, payload); + deepMerge({}, value); + rows.push( + `${name} / ${road}: status=${status} format=${observedFormat[road]} ` + + `Object.prototype.polluted=${JSON.stringify(takePollution())}` + ); + } + } + // the roads really were different wires, and every function body ran + expect(observedFormat).toEqual({ json: "8", codec: "0" }); + expect(invocations - before).toBe(calls); + expect(rows).toEqual( + rows.map( + row => + `${row.slice(0, row.indexOf(": ") + 2)}status=200 ` + + `format=${row.includes("/ json:") ? "8" : "0"} Object.prototype.polluted=undefined` + ) + ); + }); + + it("removes each dangerous own key from a result while leaving lookalike data intact", async () => { + // The invariant is key REMOVAL, not "this particular merge stayed + // clean": an author who neutralizes one sink leaves every other merge + // helper in the ecosystem holding the same key. `constructorName` is + // the control in the same table, so a strip that over-reaches fails + // here rather than in someone's application. The codec row carries one + // key fewer for the encode-side reason noted above, not because the + // decoder treats it differently. + const CARRIERS: [Road, string][] = [ + [ + "json", + '{"__proto__":{"polluted":"a"},"constructor":{"polluted":"b"},' + + '"prototype":{"polluted":"c"},"constructorName":"Widget","n":1}' + ], + [ + "codec", + '{"__proto__":{"polluted":"a"},"prototype":{"polluted":"c"},' + + '"constructorName":"Widget","n":1}' + ] + ]; + const rows: string[] = []; + for (const [road, document] of CARRIERS) { + const { status, value } = await callServerFunction(road, JSON.parse(document)); + rows.push(`${road}: status=${status} ownKeys=${JSON.stringify(Object.keys(value))}`); + } + expect(rows).toEqual( + CARRIERS.map(([road]) => `${road}: status=200 ownKeys=["constructorName","n"]`) + ); + }); + + it("a shallow merge of a decoded result cannot re-prototype the copy (#3168, response leg)", async () => { + // Verbatim #3168, one leg over: `Object.assign` merges by [[Set]], so + // an own `__proto__` on the source re-prototypes the destination with + // attacker-supplied data. This is the exact case the argument decoder + // was taught to close. + const rows: string[] = []; + for (const road of ROADS) { + const { status, value } = await callServerFunction( + road, + JSON.parse('{"displayName":"ada","__proto__":{"isAdmin":true},"n":1}') + ); + const copy: any = Object.assign({}, value); + rows.push( + `${road}: status=${status} reprototyped=${Object.getPrototypeOf(copy) !== Object.prototype} ` + + `isAdmin=${JSON.stringify(copy.isAdmin)}` + ); + } + expect(rows).toEqual( + ROADS.map(road => `${road}: status=200 reprototyped=false isAdmin=undefined`) + ); + }); + + it("reaches into the Maps and Sets the codec revived, not only plain objects", async () => { + // The argument-side walk deliberately covers revived collections, and + // a result is where collections actually show up: returning a `Map` + // keyed by id is ordinary, and the values inside it are the same + // user-parsed documents. + const inMap = await callServerFunction( + "codec", + new Map([["ada", JSON.parse('{"__proto__":{"polluted":"inMap"},"n":1}')]]) + ); + deepMerge({}, (inMap.value as Map).get("ada")); + const fromMap = takePollution(); + + const inSet = await callServerFunction( + "codec", + new Set([JSON.parse('{"__proto__":{"polluted":"inSet"},"n":1}')]) + ); + deepMerge({}, [...(inSet.value as Set)][0]); + const fromSet = takePollution(); + + expect({ fromMap, fromSet }).toEqual({ fromMap: undefined, fromSet: undefined }); + }); + + it("hands the single-flight data slice to its consumer with the key already gone", async () => { + // The worst destination on this leg: a slice does not merely reach the + // caller, it is written into the router's cache before the caller's + // await resolves, from where every later reader picks it up. + registerServerFunction("response-proto-flight", async () => { + invocations++; + return "mutated"; + }); + const original = globalThis.fetch; + globalThis.fetch = (async (url: string, init?: RequestInit) => { + const request = new Request(new URL(url, "http://localhost"), init); + request.headers.set("Sec-Fetch-Site", "same-origin"); + return handleServerFunctionRequest(request, { + collectFlightData: () => + JSON.parse('{"/notes":{"__proto__":{"polluted":"viaFlight"},"n":1}}') + }); + }) as typeof fetch; + const delivered: any[] = []; + const unsubscribe = subscribeFlightData(data => { + delivered.push(data); + }); + try { + const value = await createServerReference("response-proto-flight")(); + expect(value).toBe("mutated"); + expect(delivered).toHaveLength(1); + deepMerge({}, delivered[0]["/notes"]); + expect({ + sliceKeys: Object.keys(delivered[0]["/notes"]), + polluted: takePollution() + }).toEqual({ sliceKeys: ["n"], polluted: undefined }); + } finally { + unsubscribe(); + globalThis.fetch = original; + } + }); + + it("strips a hand-built frame decoded through decodeResponse, the integration-facing decoder", async () => { + // Routers call `decodeResponse` themselves on the responses the + // transport hands over whole (redirects, revalidation, single-flight + // payloads without a consumer), so the guarantee has to hold at that + // entry point too — not only inside the stub. The codec's own encoder + // refuses an own `constructor`, so this frame is written by hand under + // placeholder names and renamed, the same technique the argument-leg + // spec uses: a frame is just text. + const framed = await serializeString({ + ctorKey: { prototypeKey: { polluted: "viaFrame" } }, + n: 1 + }); + let json = framed.slice(framed.indexOf(";", 1) + 1); + json = json + .split('"ctorKey"') + .join('"constructor"') + .split('"prototypeKey"') + .join('"prototype"'); + const length = new TextEncoder().encode(json).byteLength; + const body = `;0x${length.toString(16).padStart(8, "0")};${json}`; + + const decoded: any = await decodeResponse( + new Response(body, { headers: { [BODY_FORMAT_HEADER]: "0" } }) + ); + deepMerge({}, decoded); + expect({ ownKeys: Object.keys(decoded), polluted: takePollution() }).toEqual({ + ownKeys: ["n"], + polluted: undefined + }); + }); + + it("a `prototype` key in a result cannot re-prototype the class it is merged onto", async () => { + // `prototype` is the third key the argument leg strips and the one + // whose sink is a merge onto a constructor rather than onto a plain + // object — settings folded onto a class, a plugin patching a widget. + // The key reaches every instance ever made, so parity with the + // argument leg is not decoration. + const { status, value } = await callServerFunction( + "json", + JSON.parse('{"prototype":{"polluted":"onClass"},"n":1}') + ); + function Widget(this: any) {} + deepMerge(Widget, value); + expect({ status, seenByInstance: (new (Widget as any)() as any).polluted }).toEqual({ + status: 200, + seenByInstance: undefined + }); + }); +}); diff --git a/packages/web/test/server/server-functions-transform-result-thrown.spec.tsx b/packages/web/test/server/server-functions-transform-result-thrown.spec.tsx new file mode 100644 index 000000000..f65f4a433 --- /dev/null +++ b/packages/web/test/server/server-functions-transform-result-thrown.spec.tsx @@ -0,0 +1,136 @@ +/** + * `transformResult` and the failures it never sees. + * + * The hook documents itself as running "for returned and thrown results + * alike (`context.thrown` distinguishes)" — and `context.thrown` exists for + * no other reason. It is the seam an app hangs result policy on: mapping + * internal errors to a wire shape, tagging a response, writing the audit + * record that says this call failed. + * + * Dispatch honors that on the return path and on one half of the throw + * path: a thrown `Response` or `ResponseEnvelope` is offered to the hook, + * because the tail that handles those calls it. The plain-thrown tail — + * `respondThrown`, where a thrown `Error` or a thrown string goes — never + * does. So the hook sees every SUCCESS and every failure an author already + * shaped by hand, and none of the failures that happen to the app: a driver + * error, a null dereference, an assertion, the ones a failure policy is + * written for. Silently: nothing logs, and the 500 looks the same as it + * would with no hook at all. + * + * Either the code is wrong or the doc is. This spec takes the position that + * the code is: a hook whose whole purpose is result policy cannot be + * exempt from the majority of results, and `thrown` is already carried + * into the context of the half that works. If the fix goes the other way — + * narrowing the doc to "returned results and thrown HTTP shapes" — these + * tests go with it. + * + * The pairing matters as much as the pinning: giving the hook the raw error + * must not become a way to leak it. The sanitized generic 500 is asserted + * alongside, so a fix that hands the error to the hook and then forgets to + * sanitize what it returns cannot go green here. + * + * 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 } from "@solidjs/web"; +import { + ERROR_HEADER, + GENERIC_SERVER_ERROR_MESSAGE, + 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 createEvent = (request: Request) => createRequestEvent(request); + +function post(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: { + "Sec-Fetch-Site": "same-origin", + "content-type": "application/json", + "X-Server-Function-Format": "8", + "X-Server-Function-Instance": "server-function:test" + } + }); +} + +/** Dispatches `id` with a recording, pass-through `transformResult`. */ +async function dispatchWithHook(id: string) { + const seen: { thrown: boolean; result: string }[] = []; + const response = await handleServerFunctionRequest(post(id), { + createEvent, + transformResult: (event, result, context) => { + seen.push({ + thrown: context.thrown === true, + result: result instanceof Error ? `Error: ${result.message}` : String(result) + }); + return result; + } + }); + return { seen, response }; +} + +describe("the result policy hook sees every outcome it says it sees", () => { + it("sees a returned value", async () => { + registerServerFunction("transform-returned", async () => "a value"); + + const { seen, response } = await dispatchWithHook("transform-returned"); + + expect(response.status).toBe(200); + expect(seen).toStrictEqual([{ thrown: false, result: "a value" }]); + }); + + it("sees a thrown Response — the half that already works", async () => { + registerServerFunction("transform-thrown-response", async () => { + throw new Response(null, { status: 418 }); + }); + + const { seen, response } = await dispatchWithHook("transform-thrown-response"); + + expect(response.status).toBe(418); + expect(seen.map(entry => entry.thrown)).toStrictEqual([true]); + }); + + it("sees a thrown Error, the failure shape a failure policy is written for", async () => { + registerServerFunction("transform-thrown-error", async () => { + throw new Error("connection to shard 7 refused"); + }); + + const { seen, response } = await dispatchWithHook("transform-thrown-error"); + + // today: seen is [] — the audit record is never written and the error + // mapping never runs + expect(seen).toStrictEqual([{ thrown: true, result: "Error: connection to shard 7 refused" }]); + // and the answer stays sanitized: the hook getting the real error is + // not a road for it onto the wire + expect(response.status).toBe(500); + expect(response.headers.get(ERROR_HEADER)).toBe(GENERIC_SERVER_ERROR_MESSAGE); + expect(await response.text()).not.toContain("shard 7"); + }); + + it("sees a thrown non-Error value too", async () => { + registerServerFunction("transform-thrown-string", async () => { + throw "shard 7 refused"; + }); + + const { seen, response } = await dispatchWithHook("transform-thrown-string"); + + // today: seen is [] + expect(seen).toStrictEqual([{ thrown: true, result: "shard 7 refused" }]); + expect(response.status).toBe(500); + expect(await response.text()).not.toContain("shard 7"); + }); +}); diff --git a/packages/web/test/server/server-functions-wrap-nested-direct.spec.tsx b/packages/web/test/server/server-functions-wrap-nested-direct.spec.tsx new file mode 100644 index 000000000..450536a0b --- /dev/null +++ b/packages/web/test/server/server-functions-wrap-nested-direct.spec.tsx @@ -0,0 +1,159 @@ +/** + * The per-handler `wrapInvocation` and the in-process call made UNDER it. + * + * A server function's body may call another server function directly — the + * reference is in scope, and on the server calling it runs the original + * in-process rather than going back out over HTTP (that is what + * `createServerReference` is for). Both calls belong to one request. + * + * The configured wrap covers both, on purpose: `createServerReference`'s + * apply trap reads `config.wrapInvocation`, so "per-function middleware + * built on it can't be bypassed by calling the function during a render". + * The per-handler OPTION reads nothing — it is threaded through the HTTP + * dispatch tail only. For a document render that is a fact of scope: a + * per-request option cannot exist for a call that is not a request, and + * `HandleServerFunctionRequestOptions` says so ("it only applies to HTTP + * dispatch"). Inside the handler it is not: the nested call happens within + * the option's own dynamic extent, under the option's own event, and the + * option is the only policy an adapter that wires per-request (per-tenant + * policy derived from the request, a per-route gate) has. + * + * So an adapter that gates with the option gates the function the wire + * addressed and nothing that function reaches in-process — the shape a + * hop-by-hop authorization check exists to prevent. The invariant pinned + * here: whichever wrap owns a request owns every server-function body + * entered while handling it, not just the entry point. + * + * Only the nested leg is pinned. The render-time leg — a direct call made + * during document SSR, outside any `handleServerFunctionRequest` — has no + * per-request option to consult and cannot be fixed in code; there the + * source comment above the apply trap, which advertises the guarantee + * without naming which of the two hooks earns it, is what is wrong. + * + * 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, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { createRequestEvent } from "@solidjs/web"; +import { + configureServerFunctionsServer, + createServerReference as createServerSideReference, + handleServerFunctionRequest, + registerServerFunction, + registerServerReference +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +/** + * `configure` ignores `undefined` — that is its spelling of "not + * overriding" — so undoing a hook between tests takes a value. `null` is + * the falsy "nothing configured" every read site tests for; the cast is + * only because the option type describes hooks, not their absence. + */ +const NO_HOOK = null as any; + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterEach(() => { + configureServerFunctionsServer({ wrapInvocation: NO_HOOK }); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const createEvent = (request: Request) => createRequestEvent(request); + +function post(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: { + "Sec-Fetch-Site": "same-origin", + "content-type": "application/json", + "X-Server-Function-Format": "8", + "X-Server-Function-Instance": "server-function:test" + } + }); +} + +/** An entry function whose body calls a second server function in-process. */ +function registerPair(prefix: string) { + let innerRan = 0; + const inner = createServerSideReference( + registerServerReference(`${prefix}-inner`, () => { + innerRan++; + return "the inner secret"; + }) + ); + registerServerFunction(`${prefix}-outer`, async () => (inner as any)()); + return { + ranInner: () => innerRan, + reset: () => (innerRan = 0) + }; +} + +describe("the wrap that owns a request owns the calls made under it", () => { + it("names both the wire call and the call its body makes, when configured", async () => { + const seen: string[] = []; + const pair = registerPair("nested-configured"); + configureServerFunctionsServer({ + wrapInvocation: (run, context) => { + seen.push(`${context.id}:${context.direct ? "direct" : "http"}`); + return run(); + } + }); + + const response = await handleServerFunctionRequest(post("nested-configured-outer"), { + createEvent + }); + + expect(response.status).toBe(200); + expect(pair.ranInner()).toBe(1); + // the configured hook is a real hop-by-hop seam: it sees the entry and + // the in-process call the entry made + expect(seen).toStrictEqual(["nested-configured-outer:http", "nested-configured-inner:direct"]); + }); + + it("names both when the wrap arrives as a per-handler option instead", async () => { + const seen: string[] = []; + const pair = registerPair("nested-option"); + + const response = await handleServerFunctionRequest(post("nested-option-outer"), { + createEvent, + wrapInvocation: (run, context) => { + seen.push(`${context.id}:${context.direct ? "direct" : "http"}`); + return run(); + } + }); + + expect(response.status).toBe(200); + expect(pair.ranInner()).toBe(1); + // today: only ["nested-option-outer:http"] — the nested body ran with + // no policy at all, inside the very request the option was given for + expect(seen).toStrictEqual(["nested-option-outer:http", "nested-option-inner:direct"]); + }); + + it("stops the nested body when the request's wrap declines, not only the entry", async () => { + const pair = registerPair("nested-deny"); + + const response = await handleServerFunctionRequest(post("nested-deny-outer"), { + createEvent, + wrapInvocation: (run, context) => { + // an authorization check that clears the entry point and refuses + // what it reaches — the reason a gate runs per invocation + if (context.id === "nested-deny-inner") throw new Response(null, { status: 403 }); + return run(); + } + }); + + // today: 200, and `ranInner()` is 1 — the refusal never had the chance + // to fire because the nested invocation never consulted the option + expect(pair.ranInner()).toBe(0); + expect(response.status).toBe(403); + }); +});