Skip to content

The single-flight source list is honored as a multiset, so a caller picks its own amplification factor #3251

Description

@frenzzy

Summary

X-Single-Flight is caller-supplied, and the handler resolves it to collectors by splitting on commas with one hook run per entry. Nothing dedupes the list, so a same-origin caller who repeats one source id N times pays for one request and gets N collector runs — each a re-run of the reads the mutation invalidated, inside a request-event scope, sequentially, on the connection. Every run past the first is discarded work: the envelope is built with Object.fromEntries and the client holds one consumer per source id, so a repeated id cannot contribute a second slice. The multiplier is chosen by the caller, on the single most expensive thing single-flight does; a ~3.5 KB request header holds one request open for 500 sequential collector runs.

This is scoped narrowly to multiplicity in the request-leg source list. It is not the fold's slice-containment or response-ownership behaviour (different value, different point in the request — separate issues), and it deliberately does not touch trimming or normalising ids.

Reproduction

packages/web/repro-flight-dedupe.mjs, run with plain node from packages/web. "before" is the released line: the built module with the one expression rewritten back, written next to the shipped one so its imports resolve identically.

// What a repeated single-flight source id costs. `node repro-flight-dedupe.mjs`
// from packages/web.
import { AsyncLocalStorage } from "node:async_hooks";
import { readFileSync, writeFileSync } from "node:fs";
import { setTimeout as sleep } from "node:timers/promises";

globalThis[Symbol.for("solid.RequestContext")] = new AsyncLocalStorage();
const AFTER = new URL("./server-functions/dist/server.js", import.meta.url);
const BEFORE = new URL("server.before.js", AFTER);
const after = readFileSync(AFTER, "utf8");
const before = after.replace('[...new Set(flightHeader.split(","))]', 'flightHeader.split(",")');
if (before === after) throw new Error("patch site not found");
writeFileSync(BEFORE, before);
const builds = { before: await import(BEFORE.href), after: await import(AFTER.href) };
let seq = 0;

// queryMs stands in for what a collector actually does: re-run the reads the
// mutation invalidated. Collectors run sequentially, inside the request.
async function row(label, build, sources, queryMs = 0, unnamed = false) {
  const mod = builds[build];
  const id = `fn-${seq++}`;
  const runs = { orders: 0, session: 0, unnamed: 0 };
  const collect = key => async () => (runs[key]++, queryMs && (await sleep(queryMs)), { "/": [1] });
  mod.registerServerFunction(id, async () => "committed");
  mod.registerFlightDataSource("orders", collect("orders"));
  mod.registerFlightDataSource("session", collect("session"));
  const started = process.hrtime.bigint();
  const response = await mod.handleServerFunctionRequest(
    new Request(`https://app.example/_server/data/${id}`, {
      method: "POST",
      headers: {
        "Sec-Fetch-Site": "same-origin",
        "X-Server-Function-Instance": "server-function:test",
        [mod.SINGLE_FLIGHT_HEADER]: sources
      }
    }),
    unnamed ? { collectFlightData: collect("unnamed") } : {}
  );
  const ms = Number(process.hrtime.bigint() - started) / 1e6;
  const echoed = response.headers.get(mod.SINGLE_FLIGHT_HEADER) ?? "";
  console.log(
    [
      label.padEnd(31),
      `reqBytes=${String(Buffer.byteLength(sources)).padStart(5)}`,
      `collectorRuns=${`${runs.orders}+${runs.session}+${runs.unnamed}`.padEnd(11)}`,
      `echoedIds=${String(echoed ? echoed.split(",").length : 0).padStart(4)}`,
      `echoedBytes=${String(Buffer.byteLength(echoed)).padStart(5)}`,
      `ms=${ms.toFixed(1).padStart(6)}`
    ].join("  ")
  );
}

await row("(warmup, not reported)", "before", "orders");
console.log("collectorRuns = orders+session+unnamed; collector costs nothing\n");
await row("CONTROL before  orders", "before", "orders");
await row("CONTROL after   orders", "after", "orders");
await row("CONTROL after   A,B,A,B,A", "after", "orders,session,orders,session,orders");
await row("SUBJECT before  A,B,A,B,A", "before", "orders,session,orders,session,orders");
await row("SUBJECT before  orders x2000", "before", Array(2000).fill("orders").join(","));
await row("SUBJECT after   orders x2000", "after", Array(2000).fill("orders").join(","));
console.log("\nsame, collector = one 1 ms query\n");
await row("CONTROL before  orders", "before", "orders", 1);
await row("SUBJECT before  orders x500", "before", Array(500).fill("orders").join(","), 1);
await row("SUBJECT after   orders x500", "after", Array(500).fill("orders").join(","), 1);
console.log('\nno named source needed: "true" is the unnamed hook a router registers\n');
await row("SUBJECT before  true x1000", "before", Array(1000).fill("true").join(","), 0, true);
await row("SUBJECT after   true x1000", "after", Array(1000).fill("true").join(","), 0, true);

Measured output:

(warmup, not reported)           reqBytes=    6  collectorRuns=1+0+0        echoedIds=   1  echoedBytes=    6  ms=  13.5
collectorRuns = orders+session+unnamed; collector costs nothing

CONTROL before  orders           reqBytes=    6  collectorRuns=1+0+0        echoedIds=   1  echoedBytes=    6  ms=   0.4
CONTROL after   orders           reqBytes=    6  collectorRuns=1+0+0        echoedIds=   1  echoedBytes=    6  ms=   1.1
CONTROL after   A,B,A,B,A        reqBytes=   36  collectorRuns=1+1+0        echoedIds=   2  echoedBytes=   14  ms=   0.2
SUBJECT before  A,B,A,B,A        reqBytes=   36  collectorRuns=3+2+0        echoedIds=   5  echoedBytes=   36  ms=   0.2
SUBJECT before  orders x2000     reqBytes=13999  collectorRuns=2000+0+0     echoedIds=2000  echoedBytes=13999  ms=   3.1
SUBJECT after   orders x2000     reqBytes=13999  collectorRuns=1+0+0        echoedIds=   1  echoedBytes=    6  ms=   0.3

same, collector = one 1 ms query

CONTROL before  orders           reqBytes=    6  collectorRuns=1+0+0        echoedIds=   1  echoedBytes=    6  ms=   0.4
SUBJECT before  orders x500      reqBytes= 3499  collectorRuns=500+0+0      echoedIds= 500  echoedBytes= 3499  ms= 573.6
SUBJECT after   orders x500      reqBytes= 3499  collectorRuns=1+0+0        echoedIds=   1  echoedBytes=    6  ms=   1.4

no named source needed: "true" is the unnamed hook a router registers

SUBJECT before  true x1000       reqBytes= 4999  collectorRuns=0+0+1000     echoedIds=1000  echoedBytes= 4999  ms=   1.1
SUBJECT after   true x1000       reqBytes= 4999  collectorRuns=0+0+1        echoedIds=   1  echoedBytes=    4  ms=   0.1

before: response X-Single-Flight for "orders,orders,orders" = "orders,orders,orders"

Reading it:

  • The CONTROL rows are the correct behaviour, and they are correct on both lines: one id runs its collector once, and a list of several distinct ids (orders,session,orders,session,orders on the fixed line) runs each of them exactly once and echoes orders,session. Nothing about deduping costs a caller its second cache.
  • SUBJECT before orders x2000 is the defect: 13,999 request-header bytes buy 2,000 collector runs and a 13,999-byte echoed response header, for one slice.
  • The 1 ms-query section is the same call with a collector that does the smallest realistic amount of work. 3,499 request bytes hold the request open for 573.6 ms against 0.4 ms for the control, because collectors run sequentially inside the request.
  • The last section shows no named source is required. "true" is the reserved id of the unnamed hook — the one a plain Solid Router app registers — so the amplifier exists in the default single-integration setup, not only in multi-source apps.

The mutation itself runs once in every row; only the collection is multiplied.

Where

  • packages/web/server-functions/src/server.ts:3090 at f0f7531bflightHeader.split(",").flatMap(...), the resolution of the request-leg list to hooks (block spans 3089-3094). Fixed at packages/web/server-functions/src/server.ts:3475-3486 in the integrated tree.
  • packages/web/server-functions/src/server.ts:1389 at f0f7531bheaders.set(SINGLE_FLIGHT_HEADER, folded.map(([source]) => source).join(",")) in foldFlightData. This is not a second bug; it is where the multiplicity becomes visible on the response leg, and it is why any fix has to preserve first-seen order rather than sort or re-key.
  • packages/web/server-functions/src/server.ts:1388Object.fromEntries(folded), which is the proof that the extra runs are discarded: a repeated id collapses to one key in the envelope no matter how many times its collector ran.

Provenance: 653dd41e ("feat(web): multi-source single-flight — named flight-data sources", 2026-08-28). That commit introduced both the split and the per-entry hook run. Before it the header carried no list at all — the opt-in was presence-only (request.headers.has(SINGLE_FLIGHT_HEADER) at 653dd41e^:packages/web/server-functions/src/server.ts:1842, with the response leg a literal "true"), so there was nothing to repeat. The multiplicity is original to the feature, not something a later fix created.

Why it matters

The realistic path is an authenticated same-origin caller. Any account that can invoke one mutation can, with a single hand-written fetch from the app's own origin, make the server run that mutation's collectors a few hundred to a few thousand times and hold the connection for the duration. Collectors are where the router or query cache re-runs the reads the mutation invalidated, so in a real app each run is database work with the caller's own credentials — the measured 573.6 ms for 500 runs of a 1 ms query is the shape of it. Repeat across a handful of connections and the request cost per unit of attacker effort is well out of line with everything else in the handler, which is otherwise carefully bounded (bodySizeLimit, maxArguments).

The honest limits:

  • It is not a cross-site attack. A POST to the data address is gated by allowsServerFunctionRequest: Sec-Fetch-Site: cross-site/same-site/none is refused outright, and a cross-origin Origin/Referer fails matchesOrigin. A hostile page cannot drive this from a victim's browser.
  • A plain non-browser client does not reach it either, unless the deployment sets allowRequestsWithoutOriginCheck: true or a matching csrf.origin. Node's fetch and curl send none of the three headers the gate reads, so by default they get a 403 before any of this. Deployments that do opt into that (server-to-server callers, monitors) are the case where an unauthenticated client reaches it.
  • The shipped client never sends a repeat. Consumers live in a Map keyed by source id and getFlightDataSourceIds() returns [...map.keys()], so the header the transport builds is a set by construction. Reaching this requires deliberately crafting the request.
  • The practical multiplier is capped by header limits, not by the handler. Front a Solid app with anything that caps request headers at 8 KB and the ceiling is roughly 8192 / (len(id) + 1) — order 4,000 for a one-character id, order 1,000 for a typical one. That is a real cap, and it is also still three orders of magnitude of amplification.
  • The echoed response header being N ids long is wasted bytes on the response and, on the client leg, N sequential deliveries of the same slice to the same consumer — but that only lands on the caller who crafted the request, so it is cost, not a second victim.

So: not remotely exploitable, but a self-service amplifier available to every logged-in user of an app that has single-flight turned on at all.

Options

  1. Dedupe the split list — [...new Set(...)] at the resolution site. One expression, at the point where caller-supplied text becomes work. Set preserves first-seen insertion order, so the hook order and the echoed header are byte-identical to today for any list that was already a set — which is every list the shipped client sends. Cost: it changes observable behaviour for a caller that sends repeats, which is the point.
  2. Dedupe on the response leg instead — make foldFlightData collapse folded before building the header. Fixes the visible symptom (the echoed header, the response bytes) and leaves the expensive half — N collector runs — exactly as it is. That is the wrong half of the problem.
  3. Reject a list containing a repeat with 400. Arguably the most honest reading of "the list is a set", and it makes a broken client loud rather than silently corrected. Cost: it is a new refusal on a header that has shipped, it is one more failure mode for integrations to reason about, and the failure it catches has no legitimate cause — nothing in the protocol produces a repeat by accident.
  4. Cap the list length (a maxFlightSources config, alongside maxArguments). Bounds the damage without deciding what a repeat means, and would also bound a caller who sends 2,000 distinct unregistered ids. But that list already costs nothing — unregistered ids resolve to no hook and are dropped — so the cap would be a knob that exists for a case the dedupe already closes, and Solid does not add knobs for cases a two-word fix closes.
  5. Document it and leave the runtime alone, on the grounds that the request-leg header is transport-internal and a caller who repeats an id is asking for what it gets. Defensible if the header is considered a private contract between the shipped client and the handler. It does mean the amplifier stays reachable by any authenticated user, which is a call about how much a same-origin authenticated caller is trusted — the maintainer's to make.

Recommendation: (1). In Solid's terms it is the minimal change that puts the guarantee where the invariant already is — the protocol documents the list as "the ids its registered consumers can actually use", the client builds it from Map keys, and the envelope is Object.fromEntries, so the set-ness is asserted at three points already and only the server's resolution reads it as a multiset. It adds no configuration, no new refusal, no second code path, and it is inert for every conforming caller. Whether to also refuse a repeat rather than quietly collapse it (option 3) is a separate judgement about how loud the protocol should be; the dedupe does not foreclose adding that later.

Applied fix, with the reasoning kept next to it:

// 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
  ? [...new Set(flightHeader.split(","))].flatMap(source => {
      const hook = source === "true" ? flightHook : flightSources.get(source);
      return hook ? [[source, hook]] : [];
    })
  : [];

Regression test

packages/web/test/server/server-functions-flight-source-dedupe.spec.tsx:

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<string, number> = { 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"] } }
    });
  });
});

Like the other server-function specs it runs against the built bundles. Against the released line all three fail — 2000 repetitions of one id ran the collector 2000 times: expected 2000 to be 1, expected { ids: 2000, bytes: 19999 } to deeply equal { ids: 1, bytes: 9 }, and collector runs were {"a":3,"b":2} — and the third is the one that keeps a future fix from over-reaching, since it goes red on any change that dedupes by collapsing the list to a single source instead of to distinct sources.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions