Skip to content

A falsy outcome is no outcome: a committed submission reports nothing to the next render #3248

Description

@frenzzy

Summary

A no-JS form submission that commits and returns nothing tells the next render nothing, so the page after the redirect is indistinguishable from one where the form was never posted — and the natural response to that is to submit again, which for a non-idempotent handler is the second write. Two truthiness tests on the same road cause it: createNoJSHandler writes no flash cookie unless the result is truthy (packages/web/server-functions/src/server.ts:1771), and decodeFlashCookie discards a well-formed cookie unless payload.result is truthy (packages/web/server-functions/src/flash.ts:162) — so "", 0, false, null and a thrown Error("") all arrive as "nothing happened", the last of them losing the error flag the size ladder directly above it promises always survives. The decode's readability test is also the only place payload.url could be typed, and it is not: FlashSubmission.url is declared string, every integration reads it as one, and a payload carrying an object there reaches the render and throws a TypeError — in the one module whose header promises "a malformed cookie never takes down the render".

This merges three symptoms that would otherwise be filed apart, so a reader searching for any of them lands here: (1) no flash cookie at all for an action that simply returns; (2) a flash cookie discarded when the result is falsy, thrown errors with empty messages included; (3) decodeFlashCookie handing the render a url that is not a string. They are one edited line on the decode side — keying readability on url is simultaneously what stops discarding falsy results and what refuses a malformed payload — plus the same sentence said on the writing side.

Reproduction

packages/web/flash-repro.mjs, run against the built bundle (npm run build -w @solidjs/web first). The dev bundle is used so the thrown error keeps its own message; see the note below for the production difference.

import {
  FLASH_COOKIE, configureServerFunctionsServer, decodeFlashCookie, encodeFlashCookie,
  handleServerFunctionRequest, registerServerReference
} from "./server-functions/dist/server.dev.js";

configureServerFunctionsServer({ provideEvent: (event, fn) => fn() });

const db = [];
registerServerReference("save", async () => { db.push("draft"); });        // returns nothing
registerServerReference("rate", async () => 0);                           // returns 0
registerServerReference("charge", async () => { throw new Error(""); });  // throws, empty message
registerServerReference("publish", async () => "published");              // CONTROL: truthy result

/** A browser form navigation: no client runtime, no fetch. */
function formNavigation(id) {
  return new Request(`https://app.example/_server/${id}`, {
    method: "POST",
    headers: {
      "Content-Type": "application/x-www-form-urlencoded",
      Origin: "https://app.example",
      Referer: "https://app.example/drafts/9",
      "Sec-Fetch-Site": "same-origin",
      "Sec-Fetch-Mode": "navigate"
    },
    body: "title=hello"
  });
}

const show = s => s === undefined ? "nothing" : JSON.stringify({
  url: s.url, result: s.result, error: s.error && `Error(${JSON.stringify(s.error.message)})`
});

for (const id of ["save", "rate", "charge", "publish"]) {
  const response = await handleServerFunctionRequest(formNavigation(id));
  const setCookie = response.headers.getSetCookie().find(c => c.startsWith(FLASH_COOKIE + "="));
  const stored = setCookie ? setCookie.split("; ")[0] : null;   // what the browser keeps
  console.log(id, response.status, setCookie ? "cookie" : "NO COOKIE",
              "next render sees:", show(stored ? decodeFlashCookie(stored) : undefined));
}

// the exported codec on its own — what a custom handleNoJS and its render use
for (const [label, value, thrown] of [['returned ""', "", false], ["returned 0", 0, false],
  ["returned false", false, false], ["returned null", null, false],
  ['threw Error("")', new Error(""), true], ['returned "ok"  [CONTROL]', "ok", false]]) {
  const stored = encodeFlashCookie("/_server/save", value, [], thrown).split("; ")[0];
  console.log(label, "->", show(decodeFlashCookie(stored)));
}

// a payload whose url is not a string
const bad = FLASH_COOKIE + "=" + encodeURIComponent(
  '{"url":{"href":"/x"},"result":"ok","error":false,"thrown":false,"input":[]}');
const submission = decodeFlashCookie(bad);
try { console.log("url.startsWith('/') ->", submission?.url.startsWith("/")); }
catch (error) { console.log("url.startsWith('/') THREW:", `${error.name}: ${error.message}`); }

Measured on f0f7531b, as built:

-- form post -> 303 + flash cookie -> the render that follows --
async () => { await db.save() }          303  cookie: NONE  next render sees: nothing
async () => 0                            303  cookie: NONE  next render sees: nothing
async () => { throw new Error("") }      303  cookie: yes   next render sees: nothing
async () => "published"  [CONTROL]       303  cookie: yes   next render sees: {"url":"/_server/publish","result":"published"}
rows actually written by those four submissions: 1

-- the exported codec on its own (what a custom handleNoJS uses) --
returned ""                -> nothing
returned 0                 -> nothing
returned false             -> nothing
returned null              -> nothing
threw Error("")            -> nothing
returned "ok"  [CONTROL]   -> {"url":"/_server/save","result":"ok"}
no cookie at all  [CONTROL] -> undefined

-- a payload whose url is not a string --
decoded: {"input":[],"url":{"href":"/x"},"result":"ok"}
render's own test, url.startsWith('/') THREW: TypeError: submission?.url.startsWith is not a function

The CONTROL rows are the contrast: a truthy result rides the cookie and arrives intact, and a genuinely absent cookie decodes to undefined, exactly as it should. Note row 3 of the first block: the cookie was written and stored — the browser has it — and the decode still reports nothing. Note also the last line of the first block: the save row committed its write. The render just cannot say so.

Same script, after the two lines in Where are changed and nothing else (rebuilt from the same checkout):

-- form post -> 303 + flash cookie -> the render that follows --
async () => { await db.save() }          303  cookie: yes   next render sees: {"url":"/_server/save"}
async () => 0                            303  cookie: yes   next render sees: {"url":"/_server/rate","result":0}
async () => { throw new Error("") }      303  cookie: yes   next render sees: {"url":"/_server/charge","error":"Error(\"\")"}
async () => "published"  [CONTROL]       303  cookie: yes   next render sees: {"url":"/_server/publish","result":"published"}
rows actually written by those four submissions: 1

-- the exported codec on its own (what a custom handleNoJS uses) --
returned ""                -> {"url":"/_server/save","result":""}
returned 0                 -> {"url":"/_server/save","result":0}
returned false             -> {"url":"/_server/save","result":false}
returned null              -> {"url":"/_server/save","result":null}
threw Error("")            -> {"url":"/_server/save","error":"Error(\"\")"}
returned "ok"  [CONTROL]   -> {"url":"/_server/save","result":"ok"}
no cookie at all  [CONTROL] -> undefined

-- a payload whose url is not a string --
decoded: undefined (refused)
render's own test, url.startsWith('/') -> undefined

Production-bundle difference, measured: with server-functions/dist/server.js, sanitizeServerError replaces the thrown error's message with Internal Server Error before it reaches the flash encoder, so the charge row's cookie is readable there (error: Error("Internal Server Error")). The two returning rows — save and rate — print cookie: NONE in the production bundle exactly as above. So the writer half is the same in dev and prod; the decode half's empty-Error case reaches dispatch in dev, and reaches everyone through the exported encodeFlashCookie/decodeFlashCookie pair, which is what a custom handleNoJS is built on.

Where

  • packages/web/server-functions/src/flash.ts:162if (!payload || !payload.result) return;. The readability test keys on the optional field instead of the mandatory one. Four lines below, flash.ts:166 copies url: payload.url with no type test — the same line is the only place either could be checked.
  • packages/web/server-functions/src/server.ts:1771if (result && !(result instanceof Response)) { inside createNoJSHandler. The comment above it already reads "anything else flashes the outcome for the next render to read"; the result && says "anything else truthy".

Provenance: both lines arrived together in 89a0531 ("Absorb expressions into Solid and collapse the rxcore seam", 2026-08-25), which landed the server-functions runtime at packages/web/src/server-functions/flash.js; 7182195 ("Migrate the absorbed DOM runtime to TypeScript and flatten it into feature folders", same day) moved them to their current paths unchanged. Found with:

git log --all --oneline -S 'if (!payload || !payload.result) return;'
git log --all --oneline -S 'if (result && !(result instanceof Response))'

Worth flagging for the reviewer of the neighbouring fix: ecfee20 ("fix(web): bound the flash cookie and validate unservable cookie shapes (#3137, #3138)", 2026-08-31) did not introduce either line, but it built the size ladder directly above the decode and wrote the invariant the decode contradicts — flash.ts:112, "url and the error/thrown flags always survive: what happened, and to which submission, is the part that must not be lost." That ladder spends real effort keeping a too-large outcome from vanishing; the truthiness test five lines further down drops an ordinary one for free.

Why it matters

The realistic path is short and needs no attacker: an app with progressive enhancement (or a user with JS off, or a failed bundle load) posts a form; the action writes a row and returns nothing; the browser follows the 303 back to the page; the page shows no confirmation because it received no submission; the user submits again. Whether that is a duplicated order or a harmless second idempotent write is the application's business, but the convention exists precisely so the app can tell the difference, and flash.ts's own header states the goal — show the outcome "exactly as it would for a scripted call". The scripted leg has no gap here: a call returning undefined resolves and the submission reports success.

Honest limits:

  • It is confined to the no-JS leg. A scripted call — the default for anyone shipping the client runtime — is unaffected. Dispatch routes to handleNoJS only for a form-shaped POST that is an actual navigation (Sec-Fetch-Mode: navigate, At the bare address the no-JS answer shape is decided by the absence of a header #3139), or wherever an integration wired createNoJSHandler explicitly.
  • It only bites apps that render the flash outcome. An app whose no-JS story is "redirect back and re-read the data" never notices.
  • "Submits again" is a plausible user behaviour, not a guaranteed one; the defect makes a committed write invisible, it does not itself perform a second write.
  • The non-string-url half is not reachable from Solid's own encoderencodeFlashCookie always writes a string. It needs a cookie that some other writer put there: a hand-set cookie, a third-party encoder, or a tampered one. The flash cookie is unsigned, which is what makes that non-zero rather than impossible, but I am not claiming a practical attack: this half is filed as type hygiene on a decode boundary, folded in here because it is the same edited line, not as a security finding.

Options

The decode's readability test (flash.ts:162):

  1. Key on urlif (!payload || typeof payload !== "object" || typeof payload.url !== "string") return;. One line; tests the field the ladder guarantees, which is also the field every integration reads, and types it in the same breath. Behaviour change: a payload with a non-string url now decodes to undefined rather than to a half-typed submission. That is a change from "crashes the render" to "reports nothing", which seems the right direction given the module's stated promise, but it is a behaviour change on malformed input and the call is yours.
  2. Just drop the result testif (!payload || typeof payload !== "object") return;. Smallest possible edit, fixes every falsy case, and leaves url untyped: the measured TypeError above still reaches the render. Cheaper, and it leaves standing the one road flash.ts explicitly says cannot exist.
  3. Keep truthiness and widen it — e.g. if (!payload || (!("result" in payload) && !payload.error)) return;. Restores the falsy cases, costs more surface than option 1, and still says nothing about url. It also re-encodes the same mistake: it asks about the optional field.
  4. Coerce rather than refuse — keep the payload but write url: String(payload.url). Never returns undefined for a stored cookie, but manufactures "[object Object]" as a url, which an integration will then compare against real paths. Refusing looks more honest, but coerce-vs-refuse on malformed input is a maintainer judgement.

The writer (server.ts:1771):

  1. Drop result && — flash every non-Response outcome. Matches the scripted leg, where an action returning undefined still resolves and still reports. Cost, measured: one extra Set-Cookie of 141 bytes on a no-JS submission that previously sent none (flash={"url":"/_server/save","error":false,"thrown":false,"input":[]} plus attributes) — paid once, cleared on the next render.
  2. Flash only when result !== undefined or thrown — preserves a "no value, no cookie" rule, at the price of keeping the bug for async () => { await db.save(draft); }, which is the shape the report is about. Not viable on its own.
  3. Change nothing; document that no-JS actions must return a value. No code change, no extra cookie. It pushes a constraint into every action an app writes, makes the two legs disagree about what undefined means, and leaves the defect as a documented sharp edge rather than a fixed one. Behaviour change vs documentation is yours to weigh; I would only note that the constraint is unenforceable — nothing warns the author who forgets.

Recommendation: decode option 1 plus writer option 1. In Solid's minimalism terms it is the cheapest shape available: one edited condition and one deleted result &&, no new API, no option, no dual path, nothing to configure, and no code that exists only to preserve the old behaviour. Both edits make the code say what the comments beside them already claim — "anything else flashes the outcome", and "url and the error/thrown flags always survive" — so the invariant stops living only in prose. Folding the url typing into the same line means the decode boundary gets its type guard for free rather than growing a second test for it.

Regression test

packages/web/test/server/server-functions-flash-falsy-outcomes.spec.tsx:

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"]);
  });
});

Plus one test in packages/web/test/server/server-functions-flash-decode-hygiene.spec.tsx for the url half (that file's other two tests belong to the separate prototype-key issue):

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();
});

Against f0f7531b all four falsy-outcome tests fail (expected undefined to be defined on the first three, no outcome cookie at all on the fourth) and the url test fails with expected 'object' not to be 'object'; with the two lines above changed and nothing else rebuilt, all five pass — the two prototype-key tests in the hygiene file stay red, as they should, since they are a different fix.

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