Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initialize ALS with cookies in middleware #65008

Merged
merged 1 commit into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2818,6 +2818,10 @@ export default async function build(
// normalize header values as initialHeaders
// must be Record<string, string>
for (const key of headerKeys) {
// set-cookie is already handled - the middleware cookie setting case
// isn't needed for the prerender manifest since it can't read cookies
if (key === 'x-middleware-set-cookie') continue

let value = exportHeaders[key]

if (Array.isArray(value)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,26 @@ export const RequestAsyncStorageWrapper: AsyncStorageWrapper<
},
get cookies() {
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
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']}`
}

// Seal the cookies object that'll freeze out any methods that could
// mutate the underlying data.
cache.cookies = getCookies(req.headers)
cache.cookies = getCookies(
combinedCookies
? {
...req.headers,
cookie: combinedCookies,
}
: req.headers
)
}

return cache.cookies
Expand Down
31 changes: 29 additions & 2 deletions packages/next/src/server/web/spec-extension/response.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { I18NConfig } from '../../config-shared'
import { NextURL } from '../next-url'
import { toNodeOutgoingHttpHeaders, validateURL } from '../utils'
import { ReflectAdapter } from './adapters/reflect'

import { ResponseCookies } from './cookies'

Expand Down Expand Up @@ -41,11 +42,37 @@ export class NextResponse<Body = unknown> extends Response {
constructor(body?: BodyInit | null, init: ResponseInit = {}) {
super(body, init)

const headers = this.headers
const cookies = new ResponseCookies(headers)

const cookiesProxy = new Proxy(cookies, {
get(target, prop, receiver) {
switch (prop) {
case 'delete':
case 'set': {
return (...args: [string, string]) => {
const result = Reflect.apply(target[prop], target, args)
const newHeaders = new Headers(headers)

if (result instanceof ResponseCookies) {
headers.set('x-middleware-set-cookie', result.toString())
}

handleMiddlewareField(init, newHeaders)
return result
}
}
default:
return ReflectAdapter.get(target, prop, receiver)
}
},
})

this[INTERNALS] = {
cookies: new ResponseCookies(this.headers),
cookies: cookiesProxy,
url: init.url
? new NextURL(init.url, {
headers: toNodeOutgoingHttpHeaders(this.headers),
headers: toNodeOutgoingHttpHeaders(headers),
nextConfig: init.nextConfig,
})
: undefined,
Expand Down
37 changes: 36 additions & 1 deletion test/e2e/app-dir/app-middleware/app-middleware.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-env jest */
import path from 'path'
import cheerio from 'cheerio'
import { check, withQuery } from 'next-test-utils'
import { check, retry, withQuery } from 'next-test-utils'
import { createNextDescribe, FileRef } from 'e2e-utils'
import type { Response } from 'node-fetch'

Expand Down Expand Up @@ -134,6 +134,41 @@ createNextDescribe(
expect(bypassCookie).toBeDefined()
})
})

it('should be possible to modify cookies & read them in an RSC in a single request', async () => {
const browser = await next.browser('/rsc-cookies')

const initialRandom1 = await browser.elementById('rsc-cookie-1').text()
const initialRandom2 = await browser.elementById('rsc-cookie-2').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+/)

await browser.refresh()

const refreshedRandom1 = await browser.elementById('rsc-cookie-1').text()
const refreshedRandom2 = await browser.elementById('rsc-cookie-2').text()

// the cookies should be refreshed and have new values
expect(refreshedRandom1).toMatch(/Cookie 1: \d+\.\d+/)
expect(refreshedRandom2).toMatch(/Cookie 2: \d+\.\d+/)
expect(refreshedRandom1).not.toBe(initialRandom1)
expect(refreshedRandom2).not.toBe(initialRandom2)

// navigate to delete cookies route
await browser.elementByCss('[href="/rsc-cookies-delete"]').click()
await retry(async () => {
// only the first cookie should be deleted
expect(await browser.elementById('rsc-cookie-1').text()).toBe(
'Cookie 1:'
)

expect(await browser.elementById('rsc-cookie-2').text()).toMatch(
/Cookie 2: \d+\.\d+/
)
})
})
}
)

Expand Down
13 changes: 13 additions & 0 deletions test/e2e/app-dir/app-middleware/app/rsc-cookies-delete/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { cookies } from 'next/headers'

export default function Page() {
const rscCookie1 = cookies().get('rsc-cookie-value-1')?.value
const rscCookie2 = cookies().get('rsc-cookie-value-2')?.value

return (
<div>
<p id="rsc-cookie-1">Cookie 1: {rscCookie1}</p>
<p id="rsc-cookie-2">Cookie 2: {rscCookie2}</p>
</div>
)
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/app-middleware/app/rsc-cookies/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { cookies } from 'next/headers'
import Link from 'next/link'

export default function Page() {
const rscCookie1 = cookies().get('rsc-cookie-value-1')?.value
const rscCookie2 = cookies().get('rsc-cookie-value-2')?.value

return (
<div>
<p id="rsc-cookie-1">Cookie 1: {rscCookie1}</p>
<p id="rsc-cookie-2">Cookie 2: {rscCookie2}</p>
<Link href="/rsc-cookies-delete">To Delete Cookies Route</Link>
</div>
)
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/app-middleware/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ export async function middleware(request) {
return NextResponse.rewrite(request.nextUrl)
}

if (request.nextUrl.pathname === '/rsc-cookies') {
const res = NextResponse.next()
res.cookies.set('rsc-cookie-value-1', `${Math.random()}`)
res.cookies.set('rsc-cookie-value-2', `${Math.random()}`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also work with cookies() directly (not from res)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah perhaps -- although writing via cookies() has been documented as only working in server actions/route handlers. So I think making this change could be separate from this while making the canonical middleware use-case work correctly.


return res
}

if (request.nextUrl.pathname === '/rsc-cookies-delete') {
const res = NextResponse.next()
res.cookies.delete('rsc-cookie-value-1')

return res
}

return NextResponse.next({
request: {
headers: headersFromRequest,
Expand Down
Loading