Skip to content

[Backport v14] add additional x-middleware-set-cookie filtering (#75561) #75870

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

Merged
merged 3 commits into from
Feb 10, 2025
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
6 changes: 6 additions & 0 deletions packages/next/src/server/lib/router-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { getNextPathnameInfo } from '../../shared/lib/router/utils/get-next-path
import { getHostname } from '../../shared/lib/get-hostname'
import { detectDomainLocale } from '../../shared/lib/i18n/detect-domain-locale'
import { normalizedAssetPrefix } from '../../shared/lib/normalized-asset-prefix'
import { filterInternalHeaders } from './server-ipc/utils'

const debug = setupDebug('next:router-server:main')
const isNextFont = (pathname: string | null) =>
Expand Down Expand Up @@ -149,6 +150,11 @@ export async function initialize(opts: {
require('./render-server') as typeof import('./render-server')

const requestHandlerImpl: WorkerRequestHandler = async (req, res) => {
// internal headers should not be honored by the request handler
if (!process.env.NEXT_PRIVATE_TEST_HEADERS) {
filterInternalHeaders(req.headers)
}

if (
!opts.minimalMode &&
config.i18n &&
Expand Down
8 changes: 8 additions & 0 deletions packages/next/src/server/lib/router-utils/resolve-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,14 @@ export function getResolveRoutes(
) {
continue
}

// for set-cookie, the header shouldn't be added to the response
// as it's only needed for the request to the middleware function.
if (key === 'x-middleware-set-cookie') {
req.headers[key] = value
continue
}

if (value) {
resHeaders[key] = value
req.headers[key] = value
Expand Down
23 changes: 23 additions & 0 deletions packages/next/src/server/lib/server-ipc/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,26 @@ export const filterReqHeaders = (
}
return headers as Record<string, undefined | string | string[]>
}

// These are headers that are only used internally and should
// not be honored from the external request
const INTERNAL_HEADERS = [
'x-middleware-rewrite',
'x-middleware-redirect',
'x-middleware-set-cookie',
'x-middleware-skip',
'x-middleware-override-headers',
'x-middleware-next',
'x-now-route-matches',
'x-matched-path',
]

export const filterInternalHeaders = (
headers: Record<string, undefined | string | string[]>
) => {
for (const header in headers) {
if (INTERNAL_HEADERS.includes(header)) {
delete headers[header]
}
}
}
5 changes: 5 additions & 0 deletions packages/next/src/server/send-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ export async function sendResponse(

// Copy over the response headers.
response.headers?.forEach((value, name) => {
// `x-middleware-set-cookie` is an internal header not needed for the response
if (name.toLowerCase() === 'x-middleware-set-cookie') {
return
}

// The append handling is special cased for `set-cookie`.
if (name.toLowerCase() === 'set-cookie') {
// TODO: (wyattjoh) replace with native response iteration when we can upgrade undici
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/app-middleware/app-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,18 @@ createNextDescribe(
await browser.deleteCookies()
})

it('should omit internal headers for middleware cookies', async () => {
const response = await next.fetch('/rsc-cookies/cookie-options')
expect(response.status).toBe(200)
expect(response.headers.get('x-middleware-set-cookie')).toBeNull()

const response2 = await next.fetch('/cookies/api')
expect(response2.status).toBe(200)
expect(response2.headers.get('x-middleware-set-cookie')).toBeNull()
expect(response2.headers.get('set-cookie')).toBeDefined()
expect(response2.headers.get('set-cookie')).toContain('example')
})

it('should respect cookie options of merged middleware cookies', async () => {
const browser = await next.browser('/rsc-cookies/cookie-options')

Expand Down
11 changes: 11 additions & 0 deletions test/e2e/app-dir/app-middleware/app/cookies/api/route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { NextResponse } from 'next/server'

export function GET() {
const response = new NextResponse()
response.cookies.set({
name: 'example',
value: 'example',
})

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

export default async function Page() {
const cookieLength = (await cookies()).size
return <div id="cookies">cookies: {cookieLength}</div>
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('Required Server Files', () => {
}
await fs.rename(join(appDir, 'pages'), join(appDir, 'pages-bak'))

process.env.NEXT_PRIVATE_TEST_HEADERS = '1'
nextApp = nextServer({
conf: {},
dir: appDir,
Expand All @@ -57,6 +58,7 @@ describe('Required Server Files', () => {
console.log(`Listening at ::${appPort}`)
})
afterAll(async () => {
delete process.env.NEXT_PRIVATE_TEST_HEADERS
if (server) server.close()
await fs.rename(join(appDir, 'pages-bak'), join(appDir, 'pages'))
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('required server files app router', () => {
}) => {
// test build against environment with next support
process.env.NOW_BUILDER = nextEnv ? '1' : ''
process.env.NEXT_PRIVATE_TEST_HEADERS = '1'

next = await createNext({
files: {
Expand Down Expand Up @@ -96,6 +97,7 @@ describe('required server files app router', () => {
await setupNext({ nextEnv: true, minimalMode: true })
})
afterAll(async () => {
delete process.env.NEXT_PRIVATE_TEST_HEADERS
await next.destroy()
if (server) await killApp(server)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('required server files i18n', () => {

beforeAll(async () => {
let wasmPkgIsAvailable = false
process.env.NEXT_PRIVATE_TEST_HEADERS = '1'

const res = await nodeFetch(
`https://registry.npmjs.com/@next/swc-wasm-nodejs/-/swc-wasm-nodejs-${
Expand Down Expand Up @@ -128,6 +129,7 @@ describe('required server files i18n', () => {
)
})
afterAll(async () => {
delete process.env.NEXT_PRIVATE_TEST_HEADERS
await next.destroy()
if (server) await killApp(server)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('required server files app router', () => {
}) => {
// test build against environment with next support
process.env.NOW_BUILDER = nextEnv ? '1' : ''
process.env.NEXT_PRIVATE_TEST_HEADERS = '1'

next = await createNext({
files: {
Expand Down Expand Up @@ -106,6 +107,7 @@ describe('required server files app router', () => {
await setupNext({ nextEnv: true, minimalMode: true })
})
afterAll(async () => {
delete process.env.NEXT_PRIVATE_TEST_HEADERS
await next.destroy()
if (server) await killApp(server)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('required server files', () => {
}) => {
// test build against environment with next support
process.env.NOW_BUILDER = nextEnv ? '1' : ''
process.env.NEXT_PRIVATE_TEST_HEADERS = '1'

next = await createNext({
files: {
Expand Down Expand Up @@ -139,6 +140,7 @@ describe('required server files', () => {
await setupNext({ nextEnv: true, minimalMode: true })
})
afterAll(async () => {
delete process.env.NEXT_PRIVATE_TEST_HEADERS
await next.destroy()
if (server) await killApp(server)
})
Expand Down
2 changes: 2 additions & 0 deletions test/production/standalone-mode/response-cache/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe('minimal-mode-response-cache', () => {
beforeAll(async () => {
// test build against environment with next support
process.env.NOW_BUILDER = '1'
process.env.NEXT_PRIVATE_TEST_HEADERS = '1'

next = await createNext({
files: new FileRef(join(__dirname, 'app')),
Expand Down Expand Up @@ -84,6 +85,7 @@ describe('minimal-mode-response-cache', () => {
appPort = `http://127.0.0.1:${port}`
})
afterAll(async () => {
delete process.env.NEXT_PRIVATE_TEST_HEADERS
await next.destroy()
if (server) await killApp(server)
})
Expand Down
Loading