Skip to content

remove unnecessary internal middleware header from response #73482

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
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 @@ -47,6 +47,7 @@ import {
import { normalizedAssetPrefix } from '../../shared/lib/normalized-asset-prefix'
import { NEXT_PATCH_SYMBOL } from './patch-fetch'
import type { ServerInitResult } from './render-server'
import { filterInternalHeaders } from './server-ipc/utils'

const debug = setupDebug('next:router-server:main')
const isNextFont = (pathname: string | null) =>
Expand Down Expand Up @@ -164,6 +165,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 @@ -585,6 +585,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]
}
}
}
25 changes: 24 additions & 1 deletion test/e2e/app-dir/app-middleware/app-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { nextTestSetup, FileRef } from 'e2e-utils'
import type { Response } from 'node-fetch'

describe('app-dir with middleware', () => {
const { next } = nextTestSetup({
const { next, isNextDeploy } = nextTestSetup({
files: __dirname,
})

Expand Down Expand Up @@ -187,6 +187,29 @@ describe('app-dir with middleware', () => {
await browser.deleteCookies()
})

// TODO: Re-enable this test in deploy mode once Vercel has proper handling
if (!isNextDeploy) {
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()
})

it('should ignore x-middleware-set-cookie as a request header', async () => {
const $ = await next.render$(
'/cookies',
{},
{
headers: {
'x-middleware-set-cookie': 'test',
},
}
)

expect($('#cookies').text()).toBe('cookies: 0')
})
}

it('should be possible to read cookies that are set during the middleware handling of a server action', async () => {
const browser = await next.browser('/rsc-cookies')
const initialRandom1 = await browser.elementById('rsc-cookie-1').text()
Expand Down
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 @@ -26,6 +26,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 @@ -97,6 +98,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 @@ -25,6 +25,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 @@ -131,6 +132,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 @@ -28,6 +28,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 @@ -107,6 +108,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 @@ -29,6 +29,7 @@ describe('required server files', () => {
const setupNext = async ({ nextEnv }: { nextEnv?: boolean }) => {
// 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 @@ -152,6 +153,7 @@ describe('required server files', () => {
})

afterAll(async () => {
delete process.env.NEXT_PRIVATE_TEST_HEADERS
await next.destroy()
})

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