Skip to content

A body-format tag the client cannot read resolves the call as a void success #3245

Description

@frenzzy
## Summary

A server function call can resolve as a successful `undefined` on a response the runtime never wrote, including a 500. The client's two "did our runtime write this answer?" guards test whether `X-Server-Function-Format` is *present*; `extractBody` then matches that header by *value* and falls through to `return undefined` for any value it has no case for. So a response carrying a format tag this build cannot read passes both guards, decodes to nothing, and resolves — indistinguishable from a function that returned nothing. That is the phantom void success #3173 closed, reopened one layer down by a header value rather than a missing header.

This merges two symptoms that would otherwise be filed separately, so a search for either lands here:

- **version skew** — a peer running a newer runtime writes a `BodyFormat` tag past `Void` (`"10"`), and every client from the previous build reads it, recognises nothing, and resolves `undefined`;
- **a duplicated `X-Server-Function-Format` header** — an intermediary appends its own copy rather than replacing, and `Headers.get` joins the two into the single string `"8, 9"`, which is neither tag.

Neither needs a hostile peer.

## Reproduction

`packages/web/server-functions/dist/client.js` only; `fetch` is stubbed so nothing is served. Node 20+, run from `packages/web`:

```js
// format-tag.mjs
import { createServerReference } from "@solidjs/web/server-functions/client";

const H = "X-Server-Function-Format";

const cases = [
  ["CONTROL  200, no tag at all", () => new Response('{"total":3}', { status: 200 })],
  ["CONTROL  500, no tag at all", () => new Response(null, { status: 500 })],
  ["CONTROL  200, tag 8 (Json)", () =>
    new Response('{"total":3}', { status: 200, headers: { [H]: "8", "content-type": "application/json" } })],
  ["         200, tag 10 (next runtime's)", () =>
    new Response('{"total":3}', { status: 200, headers: { [H]: "10", "content-type": "application/json" } })],
  ["         500, tag 10 (next runtime's)", () =>
    new Response(null, { status: 500, headers: { [H]: "10" } })],
  ["         502, tag 'nonsense'", () =>
    new Response("<html>bad gateway</html>", { status: 502, headers: { [H]: "nonsense", "content-type": "text/html" } })],
  ["         200, tag sent TWICE (8 then 9)", () => {
    const h = new Headers(); h.append(H, "8"); h.append(H, "9");
    return new Response('{"total":3}', { status: 200, headers: h });
  }],
  ["         500, tag sent TWICE (8 then 9)", () => {
    const h = new Headers(); h.append(H, "8"); h.append(H, "9");
    return new Response(null, { status: 500, headers: h });
  }]
];

for (const [label, answer] of cases) {
  globalThis.fetch = () => Promise.resolve(answer());
  const read = answer().headers.get(H);
  let outcome;
  try {
    const value = await createServerReference("getCart")();
    outcome = `RESOLVED ${JSON.stringify(value) ?? "undefined"}`;
  } catch (error) {
    outcome = `rejected  ${error.message || error.name}`;
  }
  console.log(`${label.padEnd(42)} get(${H}) = ${JSON.stringify(read).padEnd(9)} -> ${outcome}`);
}

Measured on next at f0f7531b:

CONTROL  200, no tag at all                get(X-Server-Function-Format) = null      -> rejected  Server function response carries no recognized encoding (status 200, content-type text/plain;charset=UTF-8): answered by something other than the server function runtime
CONTROL  500, no tag at all                get(X-Server-Function-Format) = null      -> rejected  Server function call failed with status 500
CONTROL  200, tag 8 (Json)                 get(X-Server-Function-Format) = "8"       -> RESOLVED {"total":3}
         200, tag 10 (next runtime's)      get(X-Server-Function-Format) = "10"      -> RESOLVED undefined
         500, tag 10 (next runtime's)      get(X-Server-Function-Format) = "10"      -> RESOLVED undefined
         502, tag 'nonsense'               get(X-Server-Function-Format) = "nonsense" -> RESOLVED undefined
         200, tag sent TWICE (8 then 9)    get(X-Server-Function-Format) = "8, 9"    -> RESOLVED undefined
         500, tag sent TWICE (8 then 9)    get(X-Server-Function-Format) = "8, 9"    -> RESOLVED undefined

The three CONTROL rows are the contrast. Remove the header entirely and the same responses fail loudly — a 200 rejects with "carries no recognized encoding", a 500 rejects with its status. Adding a header the client cannot read turns both failures into a success. The third control shows a tag this build does know still decoding, so the difference is recognition, not presence.

For reference, the same script with hasReadableBodyFormat swapped into the two guards:

CONTROL  200, no tag at all                get(X-Server-Function-Format) = null      -> rejected  Server function response carries no recognized encoding (status 200, content-type text/plain;charset=UTF-8): answered by something other than the server function runtime
CONTROL  500, no tag at all                get(X-Server-Function-Format) = null      -> rejected  Server function call failed with status 500
CONTROL  200, tag 8 (Json)                 get(X-Server-Function-Format) = "8"       -> RESOLVED {"total":3}
         200, tag 10 (next runtime's)      get(X-Server-Function-Format) = "10"      -> rejected  Server function response carries no recognized encoding (status 200, content-type application/json): answered by something other than the server function runtime
         500, tag 10 (next runtime's)      get(X-Server-Function-Format) = "10"      -> rejected  Server function call failed with status 500
         502, tag 'nonsense'               get(X-Server-Function-Format) = "nonsense" -> rejected  Server function call failed with status 502
         200, tag sent TWICE (8 then 9)    get(X-Server-Function-Format) = "8, 9"    -> rejected  Server function response carries no recognized encoding (status 200, content-type text/plain;charset=UTF-8): answered by something other than the server function runtime
         500, tag sent TWICE (8 then 9)    get(X-Server-Function-Format) = "8, 9"    -> rejected  Server function call failed with status 500

BodyFormat.Void ("9") still resolves undefined on its own, as it should — it is an encoding this build reads.

Where

Line numbers against next at f0f7531b.

Why it matters

The realistic path is a deployment where something sits between the client bundle and the origin, or where the two halves of the runtime are not the same build.

  • Duplicated header. Appending rather than replacing a response header is ordinary proxy, CDN and service-mesh behaviour, and both values here are tags the runtime legitimately writes. One hop is enough. The client then reads "8, 9", and a mutation that returned a real result — or a 500 that failed — is handed to the caller as a resolved undefined. For a mutation this is the bad direction: an error boundary sees nothing, useSubmission reports success, and the UI commits.

  • Version skew. This one is latent rather than live: today Void: "9" is the top tag, so no shipped Solid build writes a value another shipped build cannot read. It becomes real the day a tenth BodyFormat lands, at which point every client on the previous version silently resolves undefined against a server on the new one. Partial rollouts and cached client bundles make that a normal state, not an exotic one. A stale server-function id is an undistinguishable 404, so version skew cannot be recovered #3110 already decided this class of problem deserves an honest answer, for an unknown function id; an unknown encoding is the same situation one field over.

Honest limits on reachability, since they matter to how urgently this should be weighed:

  • The repro synthesizes the responses. I have not observed a proxy in the wild appending this specific header — X-Server-Function-Format is not a header intermediaries have any reason to know about, so the duplication case needs a hop that appends response headers broadly (a header-injection rule, a mesh sidecar policy, an edge worker), not a proxy behaving typically.
  • The skew case cannot be triggered today at all against a matched pair of Solid builds. It requires either a future BodyFormat addition or a non-Solid peer answering the endpoint by hand.
  • Nothing here is a security boundary. There is no way for an attacker who cannot already write response headers to induce it, and one who can write response headers has stronger options.

What makes it worth fixing anyway is the direction of the failure: this is the one class of wrong answer the two guards exist specifically to prevent, and the fix that closed it for a missing header left it open for an unreadable one.

Options

  1. Document only. State that the format tag must reach the client exactly once and unmodified, and that deployers must not let intermediaries touch it. Costs nothing, changes no behaviour. But it leaves the guarantee Revisit #3087: a 2xx the client cannot decode resolves as undefined, and a captive portal is indistinguishable from a void result #3173 established true only for the header-absent case, and a deployment gets no signal when it is violated — the failure is silent by construction, so documentation is the one remedy that cannot be verified at runtime.

  2. Derive a readability predicate from BodyFormat and swap it into the two existing guards. const READABLE_BODY_FORMATS = new Set(Object.values(BodyFormat)) plus a one-line hasReadableBodyFormat(source), used at client.ts:686 and client.ts:765. Adds no new decision point — the two guards keep their existing shape and placement, and the set cannot drift from the enum, so a tag added to BodyFormat is readable the same day. It is a behaviour change: a response that today resolves undefined will reject. That only affects a peer writing a tag outside BodyFormat and relying on extractBody's fall-through, which was never a documented contract.

  3. Make extractBody throw on an unrecognised tag instead of falling through to undefined. One site rather than two, and it covers the request leg as well. Against it: the failure then happens after the two guards, so a 4xx/5xx from a foreign peer would have its body decoded before the call fails — exactly what fix(web): fail a call on a response the runtime did not write #3088's comment says must not happen, since that body is someone else's. decodeResponse is also public surface that integrations call, so this changes their behaviour too, not just the transport's.

  4. Normalise instead of refusing — split a comma-joined value and accept the first token, so a duplicated header still decodes. Rescues the proxy case without any rejection, but it invents a wire rule (which copy wins, the origin's or the hop's?) with no basis in the protocol, and does nothing at all for version skew.

  5. Version the protocol explicitly — a runtime-version header, with the client refusing a peer it does not understand. More general and catches more than the format tag, but it is a new wire field and a new negotiation step, which is a lot of new surface for a failure mode the existing tag can already express.

Recommendation: option 2. In Solid's minimalism terms it is the cheapest of the five: no new concept, no new wire field, no new branch — one predicate, derived from data that already exists, substituted into two conditions that already exist. It also reads as the guards' intended meaning rather than a new rule; the 2xx guard's error message has said "carries no recognized encoding" since #3173, and this is the first implementation that matches it.

Two calls I think belong to a maintainer rather than to me:

  • Reject vs. tolerate a duplicated header. Refusing is consistent (an encoding this build cannot read fails like no encoding at all), but one could argue a joined "8, 9" is a transport artefact an adapter should normalise away before the runtime ever sees it, and that the runtime should not be the layer deciding what a hop did. That is a runtime-vs-adapter placement question, not a correctness one.
  • Whether the request leg gets the same treatment. isFormPost at packages/web/server-functions/src/server.ts:2071 also tests the header's presence, to decide whether a POST is a plain form navigation. A tag the server cannot read there produces a plain response rather than a phantom success, so it is not the same defect and I have not touched it — but it is the same predicate, and whether the two legs should agree is worth a decision either way.

Regression test

packages/web/test/server/server-functions-format-tag-recognition.spec.tsx. Three tests, all red against f0f7531b ("promise resolved 'undefined' instead of rejecting"), all green with the predicate swapped in. Reverting only that swap and running the full server suite reddens these three and nothing else (734 passed | 3 failed of 739).

/**
 * 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", "<html>bad gateway</html>", "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 });
    }
  });
});

It goes red against any build whose transport guards ask whether X-Server-Function-Format is present rather than whether its value is one this build can decode.

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