vercel · abhi12299 · Sep 9, 2024 · Sep 10, 2024 · Sep 10, 2024 · Sep 10, 2024
diff --git a/packages/next/src/server/lib/patch-set-header.ts b/packages/next/src/server/lib/patch-set-header.ts
@@ -6,7 +6,9 @@ type PatchableResponse = {
 
 /**
  * Ensure cookies set in middleware are merged and not overridden by API
- * routes/getServerSideProps.
+ * routes/getServerSideProps. Since middleware always runs before API routes
+ * and actions, any duplicate cookie set downstream are given priority
+ * and overlapping cookie names set by middleware are be removed.
  *
  * @param req Incoming request
  * @param res Outgoing response
@@ -28,22 +30,41 @@ export function patchSetHeaderWithCookieSupport(
 
     if (name.toLowerCase() === 'set-cookie') {
       const middlewareValue = getRequestMeta(req, 'middlewareCookie')
-
       if (
         !middlewareValue ||
         !Array.isArray(value) ||
         !value.every((item, idx) => item === middlewareValue[idx])
       ) {
+        let valueAsArray: string[] =
+          typeof value === 'string'
+            ? [value]
+            : Array.isArray(value)
+              ? value
+              : []
+
+        // this step is necessary because when merging cookies in send-response.ts
+        // the "value" argument will contain the cookies set by the middleware
+        // so we remove those in this step by exact match
+        valueAsArray = valueAsArray.filter(
+          (cookie) => !middlewareValue?.includes(cookie)
+        )
+        const cookieNamesToSet = new Set<string>()
+        for (const cookie of valueAsArray) {
+          const [cookieName] = cookie.split('=')
+          cookieNamesToSet.add(cookieName.trim())
+        }
+
+        const middlewareCookiesToSet = []
+        for (const mwCookie of middlewareValue || []) {
+          const [cookieName] = mwCookie.split('=')
+          if (!cookieNamesToSet.has(cookieName.trim())) {
+            middlewareCookiesToSet.push(mwCookie)
+          }
+        }
+
         value = [
           // TODO: (wyattjoh) find out why this is called multiple times resulting in duplicate cookies being added
-          ...new Set([
-            ...(middlewareValue || []),
-            ...(typeof value === 'string'
-              ? [value]
-              : Array.isArray(value)
-                ? value
-                : []),
-          ]),
+          ...new Set([...middlewareCookiesToSet, ...valueAsArray]),
         ]
       }
     }

diff --git a/packages/next/src/server/web/adapter.ts b/packages/next/src/server/web/adapter.ts
@@ -198,7 +198,7 @@ export async function adapter(
 
   const event = new NextFetchEvent({ request, page: params.page })
   let response
-  let cookiesFromResponse
+  let cookiesFromResponse: string[] | undefined
 
   response = await propagator(request, () => {
     // we only care to make async storage available for middleware
@@ -242,7 +242,10 @@ export async function adapter(
                 url: request.nextUrl,
                 renderOpts: {
                   onUpdateCookies: (cookies) => {
-                    cookiesFromResponse = cookies
+                    if (!cookiesFromResponse) {
+                      cookiesFromResponse = []
+                    }
+                    cookiesFromResponse.push(...cookies)
                   },
                   previewProps,
                   waitUntil,
@@ -279,7 +282,9 @@ export async function adapter(
   }
 
   if (response && cookiesFromResponse) {
-    response.headers.set('set-cookie', cookiesFromResponse)
+    for (const cookie of cookiesFromResponse) {
+      response.headers.append('set-cookie', cookie)
+    }
   }
 
   /**

diff --git a/test/e2e/app-dir/cookies-dedup/app/actions.ts b/test/e2e/app-dir/cookies-dedup/app/actions.ts
@@ -0,0 +1,8 @@
+'use server'
+
+import { cookies } from 'next/headers'
+
+export async function cookieAction() {
+  const cookieStore = await cookies()
+  cookieStore.set('common-cookie', 'from-action')
+}
diff --git a/test/e2e/app-dir/cookies-dedup/app/api/route.ts b/test/e2e/app-dir/cookies-dedup/app/api/route.ts
@@ -0,0 +1,6 @@
+export async function GET() {
+  return new Response('', {
+    status: 200,
+    headers: { 'Set-Cookie': `common-cookie=from-api; Path=/` },
+  })
+}
diff --git a/test/e2e/app-dir/cookies-dedup/app/layout.tsx b/test/e2e/app-dir/cookies-dedup/app/layout.tsx
@@ -0,0 +1,9 @@
+import React from 'react'
+
+export default function Root({ children }: { children: React.ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/cookies-dedup/app/page.tsx b/test/e2e/app-dir/cookies-dedup/app/page.tsx
@@ -0,0 +1,21 @@
+'use client'
+
+import React from 'react'
+import { cookieAction } from './actions'
+
+export default function Page() {
+  const api = async () => {
+    await fetch('/api')
+  }
+
+  return (
+    <>
+      <button id="action" onClick={() => cookieAction()}>
+        click
+      </button>
+      <button id="api" onClick={() => api()}>
+        click
+      </button>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/cookies-dedup/cookies-dedup.test.ts b/test/e2e/app-dir/cookies-dedup/cookies-dedup.test.ts
@@ -0,0 +1,35 @@
+import { nextTestSetup } from 'e2e-utils'
+
+describe('cookies-dedup', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  it('cookies set by middleware should be removed if action sets the same cookie', async () => {
+    const browser = await next.browser('/')
+    const url = await browser.url()
+    await browser.waitForElementByCss('button#action')
+
+    const actionResponsePromise = browser.waitForResponse(url)
+    await browser.elementByCss('button#action').click()
+    const actionResponse = await actionResponsePromise
+
+    const headers = await actionResponse.allHeaders()
+    const setCookieHeaders = headers['set-cookie']
+    expect(setCookieHeaders).toEqual('common-cookie=from-action; Path=/')
+  })
+
+  it('cookies set by middleware should be removed if api route sets the same cookie', async () => {
+    const browser = await next.browser('/')
+    const url = await browser.url()
+    await browser.waitForElementByCss('button#api')
+
+    const apiResponsePromise = browser.waitForResponse(`${url}api`)
+    await browser.elementByCss('button#api').click()
+    const apiResponse = await apiResponsePromise
+
+    const headers = await apiResponse.allHeaders()
+    const setCookieHeaders = headers['set-cookie']
+    expect(setCookieHeaders).toEqual('common-cookie=from-api; Path=/')
+  })
+})
diff --git a/test/e2e/app-dir/cookies-dedup/middleware.ts b/test/e2e/app-dir/cookies-dedup/middleware.ts
@@ -0,0 +1,7 @@
+import { NextResponse } from 'next/server'
+
+export default function () {
+  const response = NextResponse.next()
+  response.cookies.set('common-cookie', 'from-middleware')
+  return response
+}
diff --git a/test/e2e/app-dir/cookies-dedup/next.config.js b/test/e2e/app-dir/cookies-dedup/next.config.js
@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig
diff --git a/test/lib/browsers/base.ts b/test/lib/browsers/base.ts
@@ -1,3 +1,5 @@
+import { Response } from 'playwright'
+
 export type Event = 'request' | 'response'
 
 /**
@@ -110,4 +112,13 @@ export abstract class BrowserInterface<TCurrent = any> {
   abstract websocketFrames(): Promise<any[]>
   abstract url(): Promise<string>
   abstract waitForIdleNetwork(): Promise<void>
+  abstract waitForResponse(
+    urlOrPredicate:
+      | string
+      | RegExp
+      | ((response: Response) => boolean | Promise<boolean>),
+    options?: {
+      timeout?: number
+    }
+  ): Promise<Response>
 }
diff --git a/test/lib/browsers/playwright.ts b/test/lib/browsers/playwright.ts
@@ -9,6 +9,7 @@ import {
   Page,
   ElementHandle,
   devices,
+  Response,
 } from 'playwright'
 import path from 'path'
 
@@ -491,4 +492,18 @@ export class Playwright extends BrowserInterface {
       return page.waitForLoadState('networkidle')
     })
   }
+
+  waitForResponse(
+    urlOrPredicate:
+      | string
+      | RegExp
+      | ((response: Response) => boolean | Promise<boolean>),
+    options?: {
+      timeout?: number
+    }
+  ): Promise<Response> {
+    return this.chain(() => {
+      return page.waitForResponse(urlOrPredicate, options)
+    })
+  }
 }