Fix duplicate headers in response to static assets if overrides speci…

…fied by user (#70127) Fixes #70086 and #64864 (partially) ### What? When user wants to override for certain headers such as `content-type` or `cache-control` via `next.config` file or through middleware, nextjs sends multiple header values by combining the user's headers with its own default values for such headers. An example is demonstrated here: https://github.com/user-attachments/assets/7b38331b-9137-485d-9285-d0b0d0e1e5ac ### Why? Duplicate header values are a problem. ### How? Maintaining a list of headers which cannot have duplicate values, and then checking if the user has overridden any of those headers. If so, user overrides are respected and sent over. Demonstration of the correct behaviour after this fix: Using `next.config` file: https://github.com/user-attachments/assets/65e2aafb-dffc-47f4-bfcf-cf26a66865db Using `middleware`: https://github.com/user-attachments/assets/67636145-10eb-4504-ad78-800c1307c550 --------- Co-authored-by: JJ Kasper <jj@jjsweb.site>
vercel · Sep 23, 2024 · b3f0744 · b3f0744
1 parent 86c63b9
commit b3f0744
Show file tree

Hide file tree

Showing 12 changed files with 109 additions and 1 deletion.
diff --git a/packages/next/src/server/send-response.ts b/packages/next/src/server/send-response.ts
@@ -27,6 +27,13 @@ export async function sendResponse(
     res.statusCode = response.status
     res.statusMessage = response.statusText
 
+    // can add more headers to this list if needed
+    const headersWithMultipleValuesAllowed = [
+      'set-cookie',
+      'www-authenticate',
+      'proxy-authenticate',
+    ]
+
     // Copy over the response headers.
     response.headers?.forEach((value, name) => {
       // The append handling is special cased for `set-cookie`.
@@ -36,7 +43,15 @@ export async function sendResponse(
           res.appendHeader(name, cookie)
         }
       } else {
-        res.appendHeader(name, value)
+        // only append the header if it is either not present in the outbound response
+        // or if the header supports multiple values
+        const isHeaderPresent = typeof res.getHeader(name) !== 'undefined'
+        if (
+          headersWithMultipleValuesAllowed.includes(name.toLowerCase()) ||
+          !isHeaderPresent
+        ) {
+          res.appendHeader(name, value)
+        }
       }
     })
 

diff --git a/test/e2e/app-dir/no-duplicate-headers-middleware/app/favicon.ico b/test/e2e/app-dir/no-duplicate-headers-middleware/app/favicon.ico
diff --git a/test/e2e/app-dir/no-duplicate-headers-middleware/app/layout.tsx b/test/e2e/app-dir/no-duplicate-headers-middleware/app/layout.tsx
@@ -0,0 +1,8 @@
+import { ReactNode } from 'react'
+export default function Root({ children }: { children: ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/no-duplicate-headers-middleware/app/page.tsx b/test/e2e/app-dir/no-duplicate-headers-middleware/app/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p>hello world</p>
+}
diff --git a/test/e2e/app-dir/no-duplicate-headers-middleware/middleware.ts b/test/e2e/app-dir/no-duplicate-headers-middleware/middleware.ts
@@ -0,0 +1,13 @@
+import { NextResponse } from 'next/server'
+import type { NextRequest } from 'next/server'
+
+export function middleware(request: NextRequest) {
+  if (request.nextUrl.pathname === '/favicon.ico') {
+    return NextResponse.next({
+      headers: {
+        'Cache-Control': 'max-age=1234',
+        'Content-Type': 'image/vnd.microsoft.icon',
+      },
+    })
+  }
+}
diff --git a/test/e2e/app-dir/no-duplicate-headers-middleware/next.config.js b/test/e2e/app-dir/no-duplicate-headers-middleware/next.config.js
@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig
diff --git a/test/e2e/app-dir/no-duplicate-headers-middleware/no-duplicate-headers-middleware.test.ts b/test/e2e/app-dir/no-duplicate-headers-middleware/no-duplicate-headers-middleware.test.ts
@@ -0,0 +1,14 @@
+import { nextTestSetup } from 'e2e-utils'
+
+describe('no-duplicate-headers-next-config', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  it('should prioritise headers in middleware for static assets', async () => {
+    const res = await next.fetch('favicon.ico')
+    expect(res.status).toBe(200)
+    expect(res.headers.get('cache-control')).toBe('max-age=1234')
+    expect(res.headers.get('content-type')).toBe('image/vnd.microsoft.icon')
+  })
+})
diff --git a/test/e2e/app-dir/no-duplicate-headers-next-config/app/favicon.ico b/test/e2e/app-dir/no-duplicate-headers-next-config/app/favicon.ico
diff --git a/test/e2e/app-dir/no-duplicate-headers-next-config/app/layout.tsx b/test/e2e/app-dir/no-duplicate-headers-next-config/app/layout.tsx
@@ -0,0 +1,8 @@
+import { ReactNode } from 'react'
+export default function Root({ children }: { children: ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/no-duplicate-headers-next-config/app/page.tsx b/test/e2e/app-dir/no-duplicate-headers-next-config/app/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p>hello world</p>
+}
diff --git a/test/e2e/app-dir/no-duplicate-headers-next-config/next.config.js b/test/e2e/app-dir/no-duplicate-headers-next-config/next.config.js
@@ -0,0 +1,24 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {
+  async headers() {
+    return [
+      {
+        source: '/favicon.ico',
+        headers: [
+          {
+            key: 'cache-control',
+            value: 'max-age=1234',
+          },
+          {
+            key: 'content-type',
+            value: 'image/vnd.microsoft.icon',
+          },
+        ],
+      },
+    ]
+  },
+}
+
+module.exports = nextConfig
diff --git a/test/e2e/app-dir/no-duplicate-headers-next-config/no-duplicate-headers-next-config.test.ts b/test/e2e/app-dir/no-duplicate-headers-next-config/no-duplicate-headers-next-config.test.ts
@@ -0,0 +1,14 @@
+import { nextTestSetup } from 'e2e-utils'
+
+describe('no-duplicate-headers-next-config', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  it('should prioritise headers in next config for static assets', async () => {
+    const res = await next.fetch('favicon.ico')
+    expect(res.status).toBe(200)
+    expect(res.headers.get('cache-control')).toBe('max-age=1234')
+    expect(res.headers.get('content-type')).toBe('image/vnd.microsoft.icon')
+  })
+})