Skip to content

fix(query): follow redirects carried in X-Server-Function-Redirect (#603) - #606

Open
Stephen-Psaradellis wants to merge 1 commit into
solidjs:nextfrom
Stephen-Psaradellis:fix/query-server-function-redirect-603
Open

Stephen-Psaradellis wants to merge 1 commit into
solidjs:nextfrom
Stephen-Psaradellis:fix/query-server-function-redirect-603

Conversation

@Stephen-Psaradellis

Copy link
Copy Markdown

Summary

Fixes #603.

A redirect() thrown or returned inside a "use server" function wrapped in query() redirected on a full page request but not on a client-side navigation. For scripted callers the server-function transport masks the 3xx to a 200, deletes Location, and carries the target in X-Server-Function-Redirect (<status> <absolute-url>). The client transport hands that Response over whole. query's handleResponse only checked Location, so it fell through, and the Response became the query's value. The navigation then completed as if the guard had passed.

handleResponse now decodes the carrier when Location is absent, on the client only, using the runtime's own decodeRedirectHeaderValue, the same reader action() uses. A same-origin target becomes pathname + search + hash and goes through the existing branch: soft navigate(..., { replace: true }), X-Revalidate keys invalidated and swept, and the read held pending. A cross-origin target keeps its absolute href and navigates the document, as action() does.

query.ts deliberately does not import @solidjs/web/server-functions statically; see its header comment on keeping the transport and codec out of plain-fetch apps. So the decoder is loaded with a dynamic import() that only runs when the header is present. A carrier can only come from the transport, which means the module is already loaded wherever that happens. The header name is a local constant next to LocationHeader, with a comment pointing at REDIRECT_HEADER.

One behaviour change to weigh: the header name now lives in query.ts as a literal ("X-Server-Function-Redirect"), because importing REDIRECT_HEADER would bring back the static import. If core ever renames the header, this check drifts silently; the router's test mocks already mirror the same literal. The redirect path also awaits one dynamic import before it navigates, and only when the carrier is present. A seam-level decodeRedirect on ServerFunctionRPC would remove both points, but that is a core change, so I left it out.

How did you test this change?

Against next at f7602bf, with solid-js/@solidjs/web 2.0.0-rc.8 from the lockfile. On Windows I had to install with pnpm install --frozen-lockfile --config.node-linker=hoisted: the default pnpm layout put vitest's chunk path past 260 characters, and vitest then failed at startup with ERR_PACKAGE_IMPORT_NOT_DEFINED. That was local, not a repo problem.

New tests in test/query-redirect.spec.tsx, under a new describe("redirects carried by the server-function transport (#603)"). They build the masked shape by hand: a 200 with no Location and X-Server-Function-Redirect: 302 <absolute-url>, returned from the query the way the client transport returns it.

  • a masked redirect navigates and never reaches consumers: expects the /sign-in route, with no value observed by consumers and nothing caught.
  • X-Revalidate keys on a masked redirect invalidate and revalidate: expects the /login route, 2 session fetches, and user:anonymous.

npx vitest run test/query-redirect.spec.tsx

  • Unpatched: 2 failed, 3 passed (5).
    • AssertionError: expected '<div><span>account:</span></div>' to contain 'sign-in-page'
    • AssertionError: expected '<section><header>user:expired</header>…' to contain 'login-page', where the received HTML includes <span>files:[object Response]</span>.
  • Patched: 5 passed (5).

Full suites:

Command Base Patched
npx vitest run 399 passed, 2 failed (401) 401 passed, 2 failed (403)
npx vitest run --config vitest.config.server.ts 35 passed (35) 35 passed (35)
npx tsc --project tsconfig.test.json (test:types) exit 0 exit 0
npx tsc --noEmit not run exit 0

The same two jsdom tests fail before and after, both on the 5000 ms timeout:

  • exact-optional-types.spec.ts › accepts trees with present-but-undefined optional properties takes 8.9–12 s on this machine.
  • navigate-commit.spec.tsx › rapid-fire navigation bursts always land on the last target passes when its file runs on its own.

Neither touches query.ts.

Bundle: pnpm build exits 0, and the import stays a lazy await import('@solidjs/web/server-functions') in both dist/data/query.js and the flat dist/index.js. node .shake-check/check.mjs (patched build) exits 0 with every pass ok. In the split pass, which mirrors a Vite app, the eager chunk still reports query.ts excluded and action.ts excluded, and the codec markers stay excluded. The split-pass eager size is 62.9 KB unminified. I didn't build the base for a size comparison.

Prettier: the new test file passes prettier --check. src/data/query.ts already fails --check at base, in lines this PR does not touch, so I didn't reformat it; none of the added lines show up in prettier's diff. The PR also adds a patch changeset (.changeset/query-decodes-redirect-carrier.md).

Not covered by a test: the cross-origin branch (jsdom cannot navigate the document), and an end-to-end run with a real compiled "use server" function in a SolidStart app. The tests reproduce the wire shape of solid's maskRedirect rather than running it.

Disclosure. An AI system running an autonomous software project found the issue, wrote the fix and the tests, and wrote this description; I'm the account holder submitting it and I have reviewed the patch. It is free to take, change or drop.

If it's useful: I'd take the next scoped fix like this one for a fixed $400 on Solid's Development Support terms - a reviewed PR with tests, invoiced as one Open Collective expense only after you merge it.

@changeset-bot

changeset-bot Bot commented Sep 15, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 77721aa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@solidjs/router Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant