fix: cookie decoding for Node and Cloudflare (opennextjs#889)

Author: vicb

.changeset/hot-chicken-ring.md

@@ -0,0 +1,5 @@
+---
+"@opennextjs/aws": patch
+---
+
+fix cookie decoding for Node and Cloudflare

packages/open-next/package.json

@@ -48,6 +48,7 @@
     "@tsconfig/node18": "^1.0.1",
     "aws4fetch": "^1.0.18",
     "chalk": "^5.3.0",
+    "cookie": "^1.0.2",
     "esbuild": "catalog:",
     "express": "5.0.1",
     "path-to-regexp": "^6.3.0",

packages/open-next/src/http/openNextResponse.ts

@@ -10,7 +10,7 @@ import { Transform } from "node:stream";
 
 import type { StreamCreator } from "types/open-next";
 import { debug } from "../adapters/logger";
-import { parseCookies, parseHeaders } from "./util";
+import { parseHeaders, parseSetCookieHeader } from "./util";
 
 const SET_COOKIE_HEADER = "set-cookie";
 const CANNOT_BE_USED = "This cannot be used in OpenNext";
@@ -152,7 +152,7 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse {
               ...this.initialHeaders,
               ...this.headers,
             };
-      const initialCookies = parseCookies(
+      const initialCookies = parseSetCookieHeader(
         this.initialHeaders[SET_COOKIE_HEADER]?.toString(),
       );
       this._cookies =

packages/open-next/src/http/util.ts

@@ -28,16 +28,27 @@ export const convertHeader = (header: http.OutgoingHttpHeader) => {
   return String(header);
 };
 
-export function parseCookies(
+/**
+ * Parses a (comma-separated) list of Set-Cookie headers
+ *
+ * @param cookies A comma-separated list of Set-Cookie headers or a list of Set-Cookie headers
+ * @returns A list of Set-Cookie header
+ */
+export function parseSetCookieHeader(
   cookies: string | string[] | null | undefined,
 ): string[] {
   if (!cookies) {
     return [];
   }
 
-  return typeof cookies === "string"
-    ? cookies.split(/(?<!Expires=\w+),/i).map((c) => c.trim())
-    : cookies;
+  if (typeof cookies === "string") {
+    // Split the cookie string on ",".
+    // Note that "," can also appear in the Expires value (i.e. `Expires=Thu, 01 June`)
+    // so we have to skip it with a negative lookbehind.
+    return cookies.split(/(?<!Expires=\w+),/i).map((c) => c.trim());
+  }
+
+  return cookies;
 }
 
 /**

packages/open-next/src/overrides/converters/aws-apigw-v2.ts

@@ -2,7 +2,7 @@ import type {
   APIGatewayProxyEventV2,
   APIGatewayProxyResultV2,
 } from "aws-lambda";
-import { parseCookies } from "http/util";
+import { parseSetCookieHeader } from "http/util";
 import type { InternalEvent, InternalResult } from "types/open-next";
 import type { Converter } from "types/overrides";
 import { fromReadableStream } from "utils/stream";
@@ -122,7 +122,7 @@ async function convertToApiGatewayProxyResultV2(
   const response: APIGatewayProxyResultV2 = {
     statusCode: result.statusCode,
     headers,
-    cookies: parseCookies(result.headers["set-cookie"]),
+    cookies: parseSetCookieHeader(result.headers["set-cookie"]),
     body,
     isBase64Encoded: result.isBase64Encoded,
   };

packages/open-next/src/overrides/converters/aws-cloudfront.ts

@@ -7,7 +7,7 @@ import type {
   CloudFrontRequestEvent,
   CloudFrontRequestResult,
 } from "aws-lambda";
-import { parseCookies } from "http/util";
+import { parseSetCookieHeader } from "http/util";
 import type {
   InternalEvent,
   InternalResult,
@@ -133,10 +133,12 @@ function convertToCloudfrontHeaders(
     )
     .forEach(([key, value]) => {
       if (key === "set-cookie") {
-        cloudfrontHeaders[key] = parseCookies(`${value}`).map((cookie) => ({
-          key,
-          value: cookie,
-        }));
+        cloudfrontHeaders[key] = parseSetCookieHeader(`${value}`).map(
+          (cookie) => ({
+            key,
+            value: cookie,
+          }),
+        );
         return;
       }
 

packages/open-next/src/overrides/converters/edge.ts

@@ -1,6 +1,7 @@
 import { Buffer } from "node:buffer";
 
-import { parseCookies } from "http/util";
+import cookieParser from "cookie";
+import { parseSetCookieHeader } from "http/util";
 import type {
   InternalEvent,
   InternalResult,
@@ -29,11 +30,11 @@ const converter: Converter<InternalEvent, InternalResult | MiddlewareResult> = {
     const rawPath = url.pathname;
     const method = event.method;
     const shouldHaveBody = method !== "GET" && method !== "HEAD";
-    const cookies: Record<string, string> = Object.fromEntries(
-      parseCookies(event.headers.get("cookie")).map((cookie) =>
-        cookie.split("="),
-      ),
-    );
+
+    const cookieHeader = event.headers.get("cookie");
+    const cookies = cookieHeader
+      ? (cookieParser.parse(cookieHeader) as Record<string, string>)
+      : {};
 
     return {
       type: "core",
@@ -81,7 +82,7 @@ const converter: Converter<InternalEvent, InternalResult | MiddlewareResult> = {
       if (key === "set-cookie" && typeof value === "string") {
         // If the value is a string, we need to parse it into an array
         // This is the case for middleware direct result
-        const cookies = parseCookies(value);
+        const cookies = parseSetCookieHeader(value);
         for (const cookie of cookies) {
           headers.append(key, cookie);
         }

packages/open-next/src/overrides/converters/node.ts

@@ -1,6 +1,6 @@
 import type { IncomingMessage } from "node:http";
 
-import { parseCookies } from "http/util";
+import cookieParser from "cookie";
 import type { InternalResult } from "types/open-next";
 import type { Converter } from "types/overrides";
 import { extractHostFromHeaders, getQueryFromSearchParams } from "./utils.js";
@@ -30,6 +30,12 @@ const converter: Converter = {
       `${req.protocol ? req.protocol : "http"}://${extractHostFromHeaders(headers)}${req.url}`,
     );
     const query = getQueryFromSearchParams(url.searchParams);
+
+    const cookieHeader = req.headers.cookie;
+    const cookies = cookieHeader
+      ? (cookieParser.parse(cookieHeader) as Record<string, string>)
+      : {};
+
     return {
       type: "core",
       method: req.method ?? "GET",
@@ -42,12 +48,7 @@ const converter: Converter = {
         req.socket.remoteAddress ??
         "::1",
       query,
-      cookies: Object.fromEntries(
-        parseCookies(req.headers.cookie)?.map((cookie) => {
-          const [key, value] = cookie.split("=");
-          return [key, value];
-        }) ?? [],
-      ),
+      cookies,
     };
   },
   // Nothing to do here, it's streaming

packages/tests-unit/tests/converters/node.test.ts

@@ -186,7 +186,7 @@ describe("convertFrom", () => {
         headers: {
           "content-length": "2",
           "content-type": "application/json",
-          cookie: ["foo=bar", "hello=world"],
+          cookie: "foo=bar; hello=world",
         },
         remoteAddress: "::1",
         body: Buffer.from("{}"),
@@ -201,7 +201,7 @@ describe("convertFrom", () => {
       headers: {
         "content-length": "2",
         "content-type": "application/json",
-        cookie: "foo=bar,hello=world",
+        cookie: "foo=bar; hello=world",
       },
       remoteAddress: "::1",
       body: Buffer.from("{}"),

packages/tests-unit/tests/http/utils.test.ts

@@ -1,14 +1,14 @@
-import { parseCookies } from "@opennextjs/aws/http/util.js";
+import { parseSetCookieHeader } from "@opennextjs/aws/http/util.js";
 
-describe("parseCookies", () => {
+describe("parseSetCookieHeader", () => {
   it("returns an empty list if cookies is emptyish", () => {
-    expect(parseCookies("")).toEqual([]);
-    expect(parseCookies(null)).toEqual([]);
-    expect(parseCookies(undefined)).toEqual([]);
-    expect(parseCookies([])).toEqual([]);
+    expect(parseSetCookieHeader("")).toEqual([]);
+    expect(parseSetCookieHeader(null)).toEqual([]);
+    expect(parseSetCookieHeader(undefined)).toEqual([]);
+    expect(parseSetCookieHeader([])).toEqual([]);
   });
   it("parse single cookie", () => {
-    const cookies = parseCookies(
+    const cookies = parseSetCookieHeader(
       "cookie1=value1; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; Path=/",
     );
     expect(cookies).toEqual([
@@ -17,7 +17,7 @@ describe("parseCookies", () => {
   });
   it("parse multiple cookies", () => {
     // NOTE: expires is lower case but still works
-    const cookies = parseCookies(
+    const cookies = parseSetCookieHeader(
       "cookie1=value1; expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; Path=/, cookie2=value2; HttpOnly; Secure",
     );
     expect(cookies).toEqual([
@@ -26,15 +26,15 @@ describe("parseCookies", () => {
     ]);
   });
   it("return if cookies is already an array", () => {
-    const cookies = parseCookies([
+    const cookies = parseSetCookieHeader([
       "cookie1=value1; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; Path=/",
     ]);
     expect(cookies).toEqual([
       "cookie1=value1; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; Path=/",
     ]);
   });
   it("parses w/o Expire", () => {
-    const cookies = parseCookies(
+    const cookies = parseSetCookieHeader(
       "cookie1=value1; HttpOnly; Secure; Path=/, cookie2=value2; HttpOnly=false; Secure=True; Domain=example.com; Path=/api",
     );
     expect(cookies).toEqual([