From 556ad698fba6efefa17e272a0cddef5ebb66fbc8 Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Wed, 15 May 2024 16:54:49 -0700 Subject: [PATCH] fix middleware cookie initialization (#65820) When we provide the `set-cookie` string in `x-middleware-set-cookie`, we need to ensure that multiple values are properly delimited. We also make sure the cookies that get passed into `RequestCookies` aren't in `ResponseCookie` form, to prevent something like `Path=/` from being part of `cookies()`. --- .../request-async-storage-wrapper.ts | 39 ++++++++++--------- .../src/server/web/spec-extension/cookies.ts | 1 + .../src/server/web/spec-extension/response.ts | 9 ++++- .../app-middleware/app-middleware.test.ts | 2 + .../app/rsc-cookies-delete/page.js | 1 + .../app-middleware/app/rsc-cookies/page.js | 1 + 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/packages/next/src/server/async-storage/request-async-storage-wrapper.ts b/packages/next/src/server/async-storage/request-async-storage-wrapper.ts index 833b3ad1e0392..634e4c13be2ee 100644 --- a/packages/next/src/server/async-storage/request-async-storage-wrapper.ts +++ b/packages/next/src/server/async-storage/request-async-storage-wrapper.ts @@ -17,9 +17,9 @@ import { RequestCookiesAdapter, type ReadonlyRequestCookies, } from '../web/spec-extension/adapters/request-cookies' -import type { ResponseCookies } from '../web/spec-extension/cookies' -import { RequestCookies } from '../web/spec-extension/cookies' +import { ResponseCookies, RequestCookies } from '../web/spec-extension/cookies' import { DraftModeProvider } from './draft-mode-provider' +import { splitCookiesString } from '../web/utils' function getHeaders(headers: Headers | IncomingHttpHeaders): ReadonlyHeaders { const cleaned = HeadersAdapter.from(headers) @@ -30,13 +30,6 @@ function getHeaders(headers: Headers | IncomingHttpHeaders): ReadonlyHeaders { return HeadersAdapter.seal(cleaned) } -function getCookies( - headers: Headers | IncomingHttpHeaders -): ReadonlyRequestCookies { - const cookies = new RequestCookies(HeadersAdapter.from(headers)) - return RequestCookiesAdapter.seal(cookies) -} - function getMutableCookies( headers: Headers | IncomingHttpHeaders, onUpdateCookies?: (cookies: string[]) => void @@ -103,24 +96,32 @@ export const RequestAsyncStorageWrapper: AsyncStorageWrapper< if (!cache.cookies) { // if middleware is setting cookie(s), then include those in // the initial cached cookies so they can be read in render - let combinedCookies + const requestCookies = new RequestCookies( + HeadersAdapter.from(req.headers) + ) + if ( 'x-middleware-set-cookie' in req.headers && typeof req.headers['x-middleware-set-cookie'] === 'string' ) { - combinedCookies = `${req.headers.cookie}; ${req.headers['x-middleware-set-cookie']}` + const setCookieValue = req.headers['x-middleware-set-cookie'] + const responseHeaders = new Headers() + + for (const cookie of splitCookiesString(setCookieValue)) { + responseHeaders.append('set-cookie', cookie) + } + + const responseCookies = new ResponseCookies(responseHeaders) + + // Transfer cookies from ResponseCookies to RequestCookies + for (const cookie of responseCookies.getAll()) { + requestCookies.set(cookie.name, cookie.value ?? '') + } } // Seal the cookies object that'll freeze out any methods that could // mutate the underlying data. - cache.cookies = getCookies( - combinedCookies - ? { - ...req.headers, - cookie: combinedCookies, - } - : req.headers - ) + cache.cookies = RequestCookiesAdapter.seal(requestCookies) } return cache.cookies diff --git a/packages/next/src/server/web/spec-extension/cookies.ts b/packages/next/src/server/web/spec-extension/cookies.ts index bfa953c5c9e8c..1fd37f5075b2b 100644 --- a/packages/next/src/server/web/spec-extension/cookies.ts +++ b/packages/next/src/server/web/spec-extension/cookies.ts @@ -1,4 +1,5 @@ export { RequestCookies, ResponseCookies, + stringifyCookie, } from 'next/dist/compiled/@edge-runtime/cookies' diff --git a/packages/next/src/server/web/spec-extension/response.ts b/packages/next/src/server/web/spec-extension/response.ts index c680c2117a191..0e855ef701de6 100644 --- a/packages/next/src/server/web/spec-extension/response.ts +++ b/packages/next/src/server/web/spec-extension/response.ts @@ -1,3 +1,4 @@ +import { stringifyCookie } from '../../web/spec-extension/cookies' import type { I18NConfig } from '../../config-shared' import { NextURL } from '../next-url' import { toNodeOutgoingHttpHeaders, validateURL } from '../utils' @@ -55,7 +56,13 @@ export class NextResponse
extends Response { const newHeaders = new Headers(headers) if (result instanceof ResponseCookies) { - headers.set('x-middleware-set-cookie', result.toString()) + headers.set( + 'x-middleware-set-cookie', + result + .getAll() + .map((cookie) => stringifyCookie(cookie)) + .join(',') + ) } handleMiddlewareField(init, newHeaders) diff --git a/test/e2e/app-dir/app-middleware/app-middleware.test.ts b/test/e2e/app-dir/app-middleware/app-middleware.test.ts index 40a0d36d9baad..778bce5918ad9 100644 --- a/test/e2e/app-dir/app-middleware/app-middleware.test.ts +++ b/test/e2e/app-dir/app-middleware/app-middleware.test.ts @@ -140,10 +140,12 @@ createNextDescribe( const initialRandom1 = await browser.elementById('rsc-cookie-1').text() const initialRandom2 = await browser.elementById('rsc-cookie-2').text() + const totalCookies = await browser.elementById('total-cookies').text() // cookies were set in middleware, assert they are present and match the Math.random() pattern expect(initialRandom1).toMatch(/Cookie 1: \d+\.\d+/) expect(initialRandom2).toMatch(/Cookie 2: \d+\.\d+/) + expect(totalCookies).toBe('Total Cookie Length: 2') await browser.refresh() diff --git a/test/e2e/app-dir/app-middleware/app/rsc-cookies-delete/page.js b/test/e2e/app-dir/app-middleware/app/rsc-cookies-delete/page.js index 9bdfed8530e18..38245781cbd8d 100644 --- a/test/e2e/app-dir/app-middleware/app/rsc-cookies-delete/page.js +++ b/test/e2e/app-dir/app-middleware/app/rsc-cookies-delete/page.js @@ -8,6 +8,7 @@ export default function Page() {