diff --git a/.changeset/tidy-donkeys-cheat.md b/.changeset/tidy-donkeys-cheat.md new file mode 100644 index 000000000..1572fd0c1 --- /dev/null +++ b/.changeset/tidy-donkeys-cheat.md @@ -0,0 +1,5 @@ +--- +"@solidjs/start": patch +--- + +Apply cookies set on a returned or thrown response during single flight mutations. `redirect(to, { headers: { "Set-Cookie": ... } })` previously only reached the browser: the single flight re-render of the redirect target still ran with the old request cookies, so queries reading that cookie saw stale values. Those cookies are now merged into the request the re-render sees, matching what a browser round trip would have sent. diff --git a/apps/tests/src/e2e/single-flight-cookie.test.ts b/apps/tests/src/e2e/single-flight-cookie.test.ts new file mode 100644 index 000000000..f93cef4f5 --- /dev/null +++ b/apps/tests/src/e2e/single-flight-cookie.test.ts @@ -0,0 +1,12 @@ +import { expect, test } from "@playwright/test"; + +test.describe("single flight mutation", () => { + test("should apply a cookie set by a thrown redirect to the same flight", async ({ page }) => { + await page.goto("/single-flight-cookie"); + await expect(page.locator("#cookie-value")).toHaveText("none"); + + await page.getByRole("button", { name: "set cookie" }).click(); + + await expect(page.locator("#cookie-value")).toHaveText("1234"); + }); +}); diff --git a/apps/tests/src/routes/single-flight-cookie.tsx b/apps/tests/src/routes/single-flight-cookie.tsx new file mode 100644 index 000000000..3750e4b44 --- /dev/null +++ b/apps/tests/src/routes/single-flight-cookie.tsx @@ -0,0 +1,38 @@ +import { action, createAsync, query, redirect } from "@solidjs/router"; +import { getRequestEvent } from "solid-js/web"; + +const readCookie = query(async () => { + "use server"; + const cookies = getRequestEvent()!.request.headers.get("cookie") ?? ""; + const match = /(?:^|;\s*)single_flight_cookie=([^;]*)/.exec(cookies); + return match ? match[1] : "none"; +}, "single-flight-cookie"); + +const setCookie = action(async () => { + "use server"; + throw redirect("/single-flight-cookie", { + headers: { + "Set-Cookie": "single_flight_cookie=1234; Path=/; HttpOnly; SameSite=Lax; Max-Age=3600", + }, + }); +}, "single-flight-cookie-set"); + +// the preload is what the single flight re-render runs to collect data for the +// redirect target, so the mutation response carries the fresh cookie value +export const route = { + preload: () => readCookie(), +}; + +export default function SingleFlightCookie() { + const value = createAsync(() => readCookie(), { deferStream: true }); + + return ( +
+

Single Flight Cookie

+ +
+ +
+
+ ); +} diff --git a/packages/start/src/fns/handler.spec.ts b/packages/start/src/fns/handler.spec.ts index 29dad2f25..16d2726cc 100644 --- a/packages/start/src/fns/handler.spec.ts +++ b/packages/start/src/fns/handler.spec.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; +import { parseCookies } from "h3"; import type { FetchEvent } from "../server/types.ts"; vi.mock("h3", () => ({ @@ -24,12 +25,15 @@ vi.mock("../server/fetchEvent.ts", () => ({ mergeResponseHeaders: vi.fn(), })); -function createMockFetchEvent(headers: Record = {}): FetchEvent { +function createMockFetchEvent( + headers: Record = {}, + setCookies: string[] = [], +): FetchEvent { return { request: new Request("http://localhost/test", { headers }), response: { headers: { - getSetCookie: () => [], + getSetCookie: () => [...setCookies], }, }, nativeEvent: {}, @@ -38,10 +42,11 @@ function createMockFetchEvent(headers: Record = {}): FetchEvent } describe("createSingleFlightHeaders", () => { - let createSingleFlightHeaders: (sourceEvent: FetchEvent) => Headers; + let createSingleFlightHeaders: (sourceEvent: FetchEvent, result?: unknown) => Headers; beforeEach(async () => { vi.clearAllMocks(); + vi.mocked(parseCookies).mockReturnValue({}); const module = await import("./handler.ts"); createSingleFlightHeaders = module.createSingleFlightHeaders; }); @@ -82,4 +87,74 @@ describe("createSingleFlightHeaders", () => { expect(sourceEvent.request.headers.get("cookie")).toBe(originalCookieHeader); expect(sourceEvent.request.headers.get("cf-ray")).toBe(originalCfRay); }); + + it("should apply cookies set on the event response", () => { + const sourceEvent = createMockFetchEvent({}, [ + "val=1234; Path=/; HttpOnly; SameSite=Lax; Max-Age=3600", + ]); + + const result = createSingleFlightHeaders(sourceEvent); + + expect(result.get("cookie")).toBe("val=1234"); + }); + + it("should apply cookies set on a thrown redirect response", () => { + const sourceEvent = createMockFetchEvent(); + const redirect = new Response(null, { + status: 302, + headers: { + Location: "/", + "Set-Cookie": "val=1234; Path=/; HttpOnly; SameSite=Lax; Max-Age=3600", + }, + }); + + const result = createSingleFlightHeaders(sourceEvent, redirect); + + expect(result.get("cookie")).toBe("val=1234"); + }); + + it("should let response cookies win over ones already on the request", () => { + vi.mocked(parseCookies).mockReturnValue({ session: "old" }); + const sourceEvent = createMockFetchEvent({ cookie: "session=old" }); + const redirect = new Response(null, { + status: 302, + headers: { "Set-Cookie": "session=new; Path=/" }, + }); + + const result = createSingleFlightHeaders(sourceEvent, redirect); + + expect(result.get("cookie")).toBe("session=new"); + }); + + it("should remove cookies cleared by the response", () => { + vi.mocked(parseCookies).mockReturnValue({ session: "abc123" }); + const sourceEvent = createMockFetchEvent({ cookie: "session=abc123" }); + const redirect = new Response(null, { + status: 302, + headers: { "Set-Cookie": "session=; Path=/; Max-Age=0" }, + }); + + const result = createSingleFlightHeaders(sourceEvent, redirect); + + expect(result.get("cookie")).toBe(null); + }); + + it("should not copy non-cookie response headers onto the request", () => { + const sourceEvent = createMockFetchEvent(); + const redirect = new Response(null, { + status: 302, + headers: { Location: "/", "X-Revalidate": "user" }, + }); + + const result = createSingleFlightHeaders(sourceEvent, redirect); + + expect(result.get("location")).toBe(null); + expect(result.get("x-revalidate")).toBe(null); + }); + + it("should ignore non-Response results", () => { + const sourceEvent = createMockFetchEvent(); + + expect(() => createSingleFlightHeaders(sourceEvent, { some: "value" })).not.toThrow(); + }); }); diff --git a/packages/start/src/fns/handler.ts b/packages/start/src/fns/handler.ts index 470992e76..2683087a9 100644 --- a/packages/start/src/fns/handler.ts +++ b/packages/start/src/fns/handler.ts @@ -241,7 +241,7 @@ async function handleNoJS(result: any, request: Request, parsed: any[], thrown?: } let App: any; -export function createSingleFlightHeaders(sourceEvent: FetchEvent) { +export function createSingleFlightHeaders(sourceEvent: FetchEvent, result?: unknown) { // cookie handling logic is pretty simplistic so this might be imperfect // unclear if h3 internals are available on all platforms but we need a way to // update request headers on the underlying H3 event. @@ -249,6 +249,11 @@ export function createSingleFlightHeaders(sourceEvent: FetchEvent) { const headers = new Headers(sourceEvent.request.headers); const cookies = parseCookies(sourceEvent.nativeEvent); const SetCookies = sourceEvent.response.headers.getSetCookie(); + // cookies attached to the returned/thrown response (eg. `redirect(to, { headers })`) + // haven't been merged onto the event response yet, but a browser round trip would + // have sent them back with the next request, so apply them here too. They come after + // the ones on the event response, so they win on conflict. + if (result instanceof Response) SetCookies.push(...result.headers.getSetCookie()); headers.delete("cookie"); // let useH3Internals = false; // if (sourceEvent.nativeEvent.node?.req) { @@ -294,7 +299,7 @@ async function handleSingleFlight(sourceEvent: FetchEvent, result: any): Promise } const event = { ...sourceEvent } as PageEvent; event.request = new Request(url, { - headers: createSingleFlightHeaders(sourceEvent), + headers: createSingleFlightHeaders(sourceEvent, result), }); return await provideRequestEvent(event, async () => { await createPageEvent(event);