From 6000527a224f707a156b991578cdfd3edb0311aa Mon Sep 17 00:00:00 2001 From: rururux Date: Wed, 27 Aug 2025 02:01:00 +0900 Subject: [PATCH 1/3] fix(react-router): Ensure custom cookie transformers are used exclusively --- .changeset/famous-donkeys-poke.md | 5 ++ .../__tests__/server-runtime/cookies-test.ts | 70 +++++++++++++++++++ .../lib/server-runtime/cookies.ts | 26 ++++--- 3 files changed, 90 insertions(+), 11 deletions(-) create mode 100644 .changeset/famous-donkeys-poke.md diff --git a/.changeset/famous-donkeys-poke.md b/.changeset/famous-donkeys-poke.md new file mode 100644 index 0000000000..ce2996e902 --- /dev/null +++ b/.changeset/famous-donkeys-poke.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +fix(react-router): Ensure custom cookie transformers are used exclusively diff --git a/packages/react-router/__tests__/server-runtime/cookies-test.ts b/packages/react-router/__tests__/server-runtime/cookies-test.ts index 18edc90bc4..25f9f98612 100644 --- a/packages/react-router/__tests__/server-runtime/cookies-test.ts +++ b/packages/react-router/__tests__/server-runtime/cookies-test.ts @@ -196,6 +196,76 @@ describe("cookies", () => { ); }); }); + + describe("encode", () => { + it("encodes the cookie using default encoding when no encode function is provided", async () => { + let rawCookieValue = "cookie"; + let cookie = createCookie("my-cookie"); + let setCookie = await cookie.serialize(rawCookieValue); + expect(setCookie).not.toContain("my-cookie=cookie"); + }); + + it("encodes the cookie using the provided encode function at initialization", async () => { + let rawCookieValue = "cookie"; + let encodeFn = (str: string) => { + // Check that the value is not encoded before calling encodeFn + expect(str).toBe(rawCookieValue); + return str; + }; + let cookie = createCookie("my-cookie", { encode: encodeFn }); + let setCookie = await cookie.serialize(rawCookieValue); + expect(setCookie).toContain("my-cookie=cookie"); + }); + + it("encodes the cookie using the provided encode function when specified during serialization", async () => { + let rawCookieValue = "cookie"; + let encodeFn = (str: string) => { + // Check that the value is not encoded before calling encodeFn + expect(str).toBe(rawCookieValue); + return str; + }; + let cookie = createCookie("my-cookie"); + let setCookie = await cookie.serialize(rawCookieValue, { + encode: encodeFn, + }); + expect(setCookie).toContain("my-cookie=cookie"); + }); + }); + + describe("decode", () => { + it("decodes cookie using default decode function", async () => { + let rawCookieValue = "cookie"; + let cookie = createCookie("my-cookie"); + let setCookie = await cookie.serialize(rawCookieValue); + let value = await cookie.parse(getCookieFromSetCookie(setCookie)); + expect(value).toBe(rawCookieValue); + }); + + it("decodes cookie using provided encode and decode functions during initialization", async () => { + let rawCookieValue = "cookie"; + let encodeFn = (str: string) => str; + let decodeFn = (str: string) => str; + let cookie = createCookie("my-cookie", { + encode: encodeFn, + decode: decodeFn, + }); + let setCookie = await cookie.serialize(rawCookieValue); + let value = await cookie.parse(getCookieFromSetCookie(setCookie)); + expect(value).toBe(rawCookieValue); + }); + + it("decodes cookie using provided decode function during parsing", async () => { + let rawCookieValue = "cookie"; + let encodeFn = (str: string) => str; + let decodeFn = (str: string) => str; + let cookie = createCookie("my-cookie", { encode: encodeFn }); + let setCookie = await cookie.serialize(rawCookieValue); + let value = await cookie.parse(getCookieFromSetCookie(setCookie), { + decode: decodeFn, + }); + expect(value).toBe(rawCookieValue); + }); + }); }); function spyConsole() { diff --git a/packages/react-router/lib/server-runtime/cookies.ts b/packages/react-router/lib/server-runtime/cookies.ts index 42ffeab6d6..9119569b1c 100644 --- a/packages/react-router/lib/server-runtime/cookies.ts +++ b/packages/react-router/lib/server-runtime/cookies.ts @@ -97,11 +97,12 @@ export const createCookie = ( }, async parse(cookieHeader, parseOptions) { if (!cookieHeader) return null; - let cookies = parse(cookieHeader, { ...options, ...parseOptions }); + let { decode: decodeFn = decodeData } = { ...options, ...parseOptions }; + let cookies = parse(cookieHeader); if (name in cookies) { let value = cookies[name]; if (typeof value === "string" && value !== "") { - let decoded = await decodeCookieValue(value, secrets); + let decoded = await decodeCookieValue(decodeFn, value, secrets); return decoded; } else { return ""; @@ -110,14 +111,15 @@ export const createCookie = ( return null; } }, - async serialize(value, serializeOptions) { + async serialize(value, _serializeOptions) { + const { encode: encodeFn = encodeData, ...serializeOptions } = { + ...options, + ..._serializeOptions, + }; return serialize( name, - value === "" ? "" : await encodeCookieValue(value, secrets), - { - ...options, - ...serializeOptions, - }, + value === "" ? "" : await encodeCookieValue(encodeFn, value, secrets), + serializeOptions, ); }, }; @@ -141,10 +143,11 @@ export const isCookie: IsCookieFunction = (object): object is Cookie => { }; async function encodeCookieValue( + encodeFn: (value: string) => string, value: any, secrets: string[], ): Promise { - let encoded = encodeData(value); + let encoded = encodeFn(value); if (secrets.length > 0) { encoded = await sign(encoded, secrets[0]); @@ -154,6 +157,7 @@ async function encodeCookieValue( } async function decodeCookieValue( + decodeFn: (value: string) => any, value: string, secrets: string[], ): Promise { @@ -161,14 +165,14 @@ async function decodeCookieValue( for (let secret of secrets) { let unsignedValue = await unsign(value, secret); if (unsignedValue !== false) { - return decodeData(unsignedValue); + return decodeFn(unsignedValue); } } return null; } - return decodeData(value); + return decodeFn(value); } function encodeData(value: any): string { From e85393cc03d7d840fa40e74af8d348f6ddb538d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=E3=83=A4=E3=82=B9=E3=83=92=E3=83=87=20=E9=A0=88?= =?UTF-8?q?=E8=97=A4?= Date: Wed, 27 Aug 2025 04:58:50 +0900 Subject: [PATCH 2/3] retry From 0f58862409d91cb566a626b4dfed14f300d13dab Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 8 Sep 2025 15:40:21 -0400 Subject: [PATCH 3/3] Avoid internal encode/decode logic via identity function --- .../__tests__/server-runtime/cookies-test.ts | 107 +++++++++--------- .../lib/server-runtime/cookies.ts | 39 ++++--- 2 files changed, 80 insertions(+), 66 deletions(-) diff --git a/packages/react-router/__tests__/server-runtime/cookies-test.ts b/packages/react-router/__tests__/server-runtime/cookies-test.ts index 25f9f98612..6127282765 100644 --- a/packages/react-router/__tests__/server-runtime/cookies-test.ts +++ b/packages/react-router/__tests__/server-runtime/cookies-test.ts @@ -197,73 +197,78 @@ describe("cookies", () => { }); }); - describe("encode", () => { - it("encodes the cookie using default encoding when no encode function is provided", async () => { - let rawCookieValue = "cookie"; + describe("custom encoding/decoding", () => { + it("uses default base64 encoding when no functions are provided", async () => { + let rawCookieValue = "hello world"; let cookie = createCookie("my-cookie"); let setCookie = await cookie.serialize(rawCookieValue); - expect(setCookie).not.toContain("my-cookie=cookie"); + expect(setCookie).toContain("my-cookie=ImhlbGxvIHdvcmxkIg%3D%3D;"); + let parsed = await cookie.parse(getCookieFromSetCookie(setCookie)); + expect(parsed).toBe(rawCookieValue); }); - it("encodes the cookie using the provided encode function at initialization", async () => { - let rawCookieValue = "cookie"; - let encodeFn = (str: string) => { - // Check that the value is not encoded before calling encodeFn - expect(str).toBe(rawCookieValue); - return str; - }; - let cookie = createCookie("my-cookie", { encode: encodeFn }); + it("uses custom implementations when provided at initialization", async () => { + let rawCookieValue = "hello world"; + let cookie = createCookie("my-cookie", { + encode(str: string) { + expect(str).toBe(rawCookieValue); // not encoded yet + return encodeURIComponent(str.toUpperCase()); + }, + decode(str: string) { + expect(str).toBe("HELLO%20WORLD"); + return decodeURIComponent(str.toLowerCase()); + }, + }); let setCookie = await cookie.serialize(rawCookieValue); - expect(setCookie).toContain("my-cookie=cookie"); + expect(setCookie).toContain("my-cookie=HELLO%20WORLD;"); + let parsed = await cookie.parse(getCookieFromSetCookie(setCookie)); + expect(parsed).toBe(rawCookieValue); }); - it("encodes the cookie using the provided encode function when specified during serialization", async () => { - let rawCookieValue = "cookie"; - let encodeFn = (str: string) => { - // Check that the value is not encoded before calling encodeFn - expect(str).toBe(rawCookieValue); - return str; - }; + it("uses custom implementations when provided at usage time", async () => { + let rawCookieValue = "hello world"; let cookie = createCookie("my-cookie"); let setCookie = await cookie.serialize(rawCookieValue, { - encode: encodeFn, + encode(str: string) { + expect(str).toBe(rawCookieValue); // not encoded yet + return encodeURIComponent(str.toUpperCase()); + }, }); - expect(setCookie).toContain("my-cookie=cookie"); - }); - }); - - describe("decode", () => { - it("decodes cookie using default decode function", async () => { - let rawCookieValue = "cookie"; - let cookie = createCookie("my-cookie"); - let setCookie = await cookie.serialize(rawCookieValue); - let value = await cookie.parse(getCookieFromSetCookie(setCookie)); - expect(value).toBe(rawCookieValue); + expect(setCookie).toContain("my-cookie=HELLO%20WORLD;"); + let parsed = await cookie.parse(getCookieFromSetCookie(setCookie), { + decode(str: string) { + expect(str).toBe("HELLO%20WORLD"); + return decodeURIComponent(str.toLowerCase()); + }, + }); + expect(parsed).toBe(rawCookieValue); }); - it("decodes cookie using provided encode and decode functions during initialization", async () => { - let rawCookieValue = "cookie"; - let encodeFn = (str: string) => str; - let decodeFn = (str: string) => str; + it("uses custom implementations when using signed cookies", async () => { + let rawCookieValue = "hello world"; let cookie = createCookie("my-cookie", { - encode: encodeFn, - decode: decodeFn, + secrets: ["s3cr3t"], + encode(str: string) { + expect(str).toBe(rawCookieValue); // not encoded yet + return encodeURIComponent(str.toUpperCase()); + }, + decode(str: string) { + expect(str).toBe("HELLO%20WORLD"); + return decodeURIComponent(str.toLowerCase()); + }, }); let setCookie = await cookie.serialize(rawCookieValue); - let value = await cookie.parse(getCookieFromSetCookie(setCookie)); - expect(value).toBe(rawCookieValue); - }); + expect(setCookie).toContain( + "my-cookie=HELLO%20WORLD.4bKWgOIqYxcP3KMCHWBmoKEQth3NPQ9yrTRurGMgS40;", + ); + let parsed = await cookie.parse(getCookieFromSetCookie(setCookie)); + expect(parsed).toBe(rawCookieValue); - it("decodes cookie using provided decode function during parsing", async () => { - let rawCookieValue = "cookie"; - let encodeFn = (str: string) => str; - let decodeFn = (str: string) => str; - let cookie = createCookie("my-cookie", { encode: encodeFn }); - let setCookie = await cookie.serialize(rawCookieValue); - let value = await cookie.parse(getCookieFromSetCookie(setCookie), { - decode: decodeFn, - }); - expect(value).toBe(rawCookieValue); + // Fails if the cookie value is tampered with + parsed = await cookie.parse( + "my-cookie=HELLO%20MARS.4bKWgOIqYxcP3KMCHWBmoKEQth3NPQ9yrTRurGMgS40", + ); + expect(parsed).toBe(null); }); }); }); diff --git a/packages/react-router/lib/server-runtime/cookies.ts b/packages/react-router/lib/server-runtime/cookies.ts index 9119569b1c..b76fbe0738 100644 --- a/packages/react-router/lib/server-runtime/cookies.ts +++ b/packages/react-router/lib/server-runtime/cookies.ts @@ -97,12 +97,17 @@ export const createCookie = ( }, async parse(cookieHeader, parseOptions) { if (!cookieHeader) return null; - let { decode: decodeFn = decodeData } = { ...options, ...parseOptions }; - let cookies = parse(cookieHeader); + let opts = { ...options, ...parseOptions }; + let cookies = parse(cookieHeader, { + ...opts, + // If they provided a custom decode function, skip internal decoding by + // passing an identity function here + decode: opts.decode ? (v) => v : undefined, + }); if (name in cookies) { let value = cookies[name]; if (typeof value === "string" && value !== "") { - let decoded = await decodeCookieValue(decodeFn, value, secrets); + let decoded = await decodeCookieValue(value, secrets, opts.decode); return decoded; } else { return ""; @@ -111,16 +116,18 @@ export const createCookie = ( return null; } }, - async serialize(value, _serializeOptions) { - const { encode: encodeFn = encodeData, ...serializeOptions } = { - ...options, - ..._serializeOptions, - }; - return serialize( - name, - value === "" ? "" : await encodeCookieValue(encodeFn, value, secrets), - serializeOptions, - ); + async serialize(value, serializeOptions) { + let opts = { ...options, ...serializeOptions }; + let encoded = + value === "" + ? "" + : await encodeCookieValue(value, secrets, opts.encode); + return serialize(name, encoded, { + ...opts, + // If they provided a custom encode function, skip internal encoding by + // passing an identity function here + encode: opts.encode ? (v) => v : undefined, + }); }, }; }; @@ -143,10 +150,11 @@ export const isCookie: IsCookieFunction = (object): object is Cookie => { }; async function encodeCookieValue( - encodeFn: (value: string) => string, value: any, secrets: string[], + encode: ((value: string) => string) | undefined, ): Promise { + let encodeFn = encode || encodeData; let encoded = encodeFn(value); if (secrets.length > 0) { @@ -157,10 +165,11 @@ async function encodeCookieValue( } async function decodeCookieValue( - decodeFn: (value: string) => any, value: string, secrets: string[], + decode: ((value: string) => any) | undefined, ): Promise { + let decodeFn = decode ?? decodeData; if (secrets.length > 0) { for (let secret of secrets) { let unsignedValue = await unsign(value, secret);