From 831bb630f98f5f21c60cd98901ba4fe411e90e66 Mon Sep 17 00:00:00 2001 From: Mikael Pettersson Date: Thu, 3 Oct 2024 18:52:21 +0200 Subject: [PATCH 1/3] fix: don't Base64 encode cookie if encode is set Right now, if a developer adds an encode/decode function when creating a cookie, it's applied after the cookie is Base64 encoded. We should not Base64 encode a cookie if the developer have added a `encode`/`decode` override to the cookie options. Unfortunately this makes the override and sign/secret options mutually exclusive. BREAKING CHANGE: If you override the cookie encode/decode functions, you can't sign the cookie with a secret --- packages/remix-server-runtime/cookies.ts | 27 ++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/remix-server-runtime/cookies.ts b/packages/remix-server-runtime/cookies.ts index 80884fa8fc2..a48a6b03e45 100644 --- a/packages/remix-server-runtime/cookies.ts +++ b/packages/remix-server-runtime/cookies.ts @@ -109,17 +109,30 @@ export const createCookieFactory = }, async parse(cookieHeader, parseOptions) { if (!cookieHeader) return null; + if (secrets.length > 0 && !!parseOptions?.decode) + throw new Error("unsign is not supported with decode option"); + let shouldEncode = !parseOptions?.decode; let cookies = parse(cookieHeader, { ...options, ...parseOptions }); return name in cookies ? cookies[name] === "" ? "" - : await decodeCookieValue(unsign, cookies[name], secrets) + : await decodeCookieValue( + unsign, + cookies[name], + secrets, + shouldEncode + ) : null; }, async serialize(value, serializeOptions) { + if (secrets.length > 0 && !!serializeOptions?.encode) + throw new Error("sign is not supported with encode option"); + let shouldEncode = !serializeOptions?.encode; return serialize( name, - value === "" ? "" : await encodeCookieValue(sign, value, secrets), + value === "" + ? "" + : await encodeCookieValue(sign, value, secrets, shouldEncode), { ...options, ...serializeOptions, @@ -149,9 +162,10 @@ export const isCookie: IsCookieFunction = (object): object is Cookie => { async function encodeCookieValue( sign: SignFunction, value: any, - secrets: string[] + secrets: string[], + shouldEncode: boolean ): Promise { - let encoded = encodeData(value); + let encoded = shouldEncode ? encodeData(value) : value; if (secrets.length > 0) { encoded = await sign(encoded, secrets[0]); @@ -163,7 +177,8 @@ async function encodeCookieValue( async function decodeCookieValue( unsign: UnsignFunction, value: string, - secrets: string[] + secrets: string[], + shouldDecode: boolean ): Promise { if (secrets.length > 0) { for (let secret of secrets) { @@ -176,7 +191,7 @@ async function decodeCookieValue( return null; } - return decodeData(value); + return shouldDecode ? decodeData(value) : value; } function encodeData(value: any): string { From 455a148093bda71c15429eb5f950fc9c820b5285 Mon Sep 17 00:00:00 2001 From: Mikael Pettersson Date: Thu, 3 Oct 2024 19:16:04 +0200 Subject: [PATCH 2/3] test: add tests for encode/decode overrrides adds tests for the encode/decode options. --- .../__tests__/cookies-test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/remix-server-runtime/__tests__/cookies-test.ts b/packages/remix-server-runtime/__tests__/cookies-test.ts index 9cce2903978..e1a4e3a0c4e 100644 --- a/packages/remix-server-runtime/__tests__/cookies-test.ts +++ b/packages/remix-server-runtime/__tests__/cookies-test.ts @@ -193,6 +193,38 @@ describe("cookies", () => { ); }); }); + + describe("encode/decode options", () => { + it("Will not Base64 encode/decode the cookie if `encode` and `decode` are set to custom functions", async () => { + let testValue = { hello: "world" }; + let cookie = createCookie("my-cookie", { + encode: (value) => JSON.stringify(value), + decode: (value) => JSON.parse(value), + }); + let setCookie = await cookie.serialize(testValue); + + expect(setCookie).toBe("my-cookie=" + JSON.stringify(testValue)); + }); + + it("Will not allow `encode` and `decode` to be set at the same time as `secrets`", async () => { + let secrets = ["foo"]; + let encode = (value) => JSON.stringify(value); + + expect(() => { + createCookie("my-cookie", { secrets, encode }); + }).toThrowErrorMatchingInlineSnapshot( + "sign is not supported with encode option" + ); + + let decode = (value) => JSON.parse(value); + + expect(() => { + createCookie("my-cookie", { secrets, decode }); + }).toThrowErrorMatchingInlineSnapshot( + "unsign is not supported with decode option" + ); + }); + }); }); function spyConsole() { From d307d8b2d99417d69494e47b001c4709ec42618b Mon Sep 17 00:00:00 2001 From: Mikael Pettersson Date: Thu, 3 Oct 2024 19:35:43 +0200 Subject: [PATCH 3/3] chore: sign CLA --- contributors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contributors.yml b/contributors.yml index 57082791773..1bd9de0a4b2 100644 --- a/contributors.yml +++ b/contributors.yml @@ -192,6 +192,7 @@ - eric-burel - ericchernuka - esamattis +- Evanion - evanwinter - everdimension - exegeteio