Fix bugs with thrown data headers and duplciated set-cookie headers (remix-run#12846)

brophdawg11 · web-flow · commit 8f4229047d7b · 2025-01-23T10:57:32.000-05:00
diff --git a/.changeset/rich-pans-fail.md b/.changeset/rich-pans-fail.md
@@ -0,0 +1,6 @@
+---
+"react-router": patch
+---
+
+- Properly bubble headers as `errorHeaders` when throwing a `data()` result
+- Avoid duplication of `Set-Cookie` headers could be duplicated if also returned from `headers`
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -0,0 +1,4 @@
+{
+  "typescript.tsdk": "node_modules/typescript/lib",
+  "typescript.enablePromptUseWorkspaceTsdk": true
+}
diff --git a/integration/headers-test.ts b/integration/headers-test.ts
@@ -4,7 +4,7 @@ import { UNSAFE_ServerMode as ServerMode } from "react-router";
 import { createFixture, js } from "./helpers/create-fixture.js";
 import type { Fixture } from "./helpers/create-fixture.js";
 
-test.describe.skip("headers export", () => {
+test.describe("headers export", () => {
   let ROOT_HEADER_KEY = "X-Test";
   let ROOT_HEADER_VALUE = "SUCCESS";
   let ACTION_HKEY = "X-Test-Action";
@@ -416,4 +416,57 @@ test.describe.skip("headers export", () => {
       ])
     );
   });
+
+  test("does not duplicate set-cookie headers also returned via headers() function", async () => {
+    let fixture = await createFixture(
+      {
+        files: {
+          "app/root.tsx": js`
+            import { Links, Meta, Outlet, Scripts } from "react-router";
+
+            export default function Root() {
+              return (
+                <html lang="en">
+                  <head>
+                    <Meta />
+                    <Links />
+                  </head>
+                  <body>
+                    <Outlet />
+                    <Scripts />
+                  </body>
+                </html>
+              );
+            }
+          `,
+
+          "app/routes/_index.tsx": js`
+            export function headers({ loaderHeaders }) {
+              return loaderHeaders;
+            }
+
+            export function loader() {
+              return new Response(null, {
+                headers: {
+                  "X-Test": "value",
+                  "Set-Cookie": "cookie=yum"
+                }
+              })
+            }
+
+            export default function Index() {
+              return <div>Heyo!</div>
+            }
+          `,
+        },
+      },
+      ServerMode.Test
+    );
+    let response = await fixture.requestDocument("/");
+    expect([...response.headers.entries()]).toEqual([
+      ["content-type", "text/html"],
+      ["set-cookie", "cookie=yum"],
+      ["x-test", "value"],
+    ]);
+  });
 });
diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts
@@ -4867,15 +4867,25 @@ async function convertDataStrategyResultToDataResult(
           type: ResultType.error,
           error: result.data,
           statusCode: result.init?.status,
+          headers: result.init?.headers
+            ? new Headers(result.init.headers)
+            : undefined,
         };
       }
 
       // Convert thrown data() to ErrorResponse instances
-      result = new ErrorResponseImpl(
-        result.init?.status || 500,
-        undefined,
-        result.data
-      );
+      return {
+        type: ResultType.error,
+        error: new ErrorResponseImpl(
+          result.init?.status || 500,
+          undefined,
+          result.data
+        ),
+        statusCode: isRouteErrorResponse(result) ? result.status : undefined,
+        headers: result.init?.headers
+          ? new Headers(result.init.headers)
+          : undefined,
+      };
     }
 
     return {
diff --git a/packages/react-router/lib/server-runtime/headers.ts b/packages/react-router/lib/server-runtime/headers.ts
@@ -98,8 +98,11 @@ function prependCookies(parentHeaders: Headers, childHeaders: Headers): void {
 
   if (parentSetCookieString) {
     let cookies = splitCookiesString(parentSetCookieString);
+    let childCookies = new Set(childHeaders.getSetCookie());
     cookies.forEach((cookie) => {
-      childHeaders.append("Set-Cookie", cookie);
+      if (!childCookies.has(cookie)) {
+        childHeaders.append("Set-Cookie", cookie);
+      }
     });
   }
 }

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +  "typescript.tsdk": "node_modules/typescript/lib",
 +  "typescript.enablePromptUseWorkspaceTsdk": true
 +}
Original file line number	Diff line number	Diff line change
`@@ -98,8 +98,11 @@ function prependCookies(parentHeaders: Headers, childHeaders: Headers): void {`
`98`	`98`
`99`	`99`	`if (parentSetCookieString) {`
`100`	`100`	`let cookies = splitCookiesString(parentSetCookieString);`
	`101`	`+ let childCookies = new Set(childHeaders.getSetCookie());`
`101`	`102`	`cookies.forEach((cookie) => {`
`102`		`- childHeaders.append("Set-Cookie", cookie);`
	`103`	`+ if (!childCookies.has(cookie)) {`
	`104`	`+ childHeaders.append("Set-Cookie", cookie);`
	`105`	`+ }`
`103`	`106`	`});`
`104`	`107`	`}`
`105`	`108`	`}`