Skip to content

Commit 9c9aaed

Browse files
ztannerijjk
andauthored
fix router handling when setting a location response header (#82588)
When middleware sets a `Location` header on the response, we should not assume that the target URL is the one we want to handle the request. We only want to route to the underlying location header when a rewrite is explicitly triggered rather than relying on the presence of the response header. --------- Co-authored-by: JJ Kasper <[email protected]>
1 parent b1c0885 commit 9c9aaed

File tree

3 files changed

+68
-10
lines changed

3 files changed

+68
-10
lines changed

packages/next/src/server/lib/router-utils/resolve-routes.ts

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ import { formatHostname } from '../format-hostname'
1818
import { toNodeOutgoingHttpHeaders } from '../../web/utils'
1919
import { isAbortError } from '../../pipe-readable'
2020
import { getHostname } from '../../../shared/lib/get-hostname'
21-
import { getRedirectStatus } from '../../../lib/redirect-status'
21+
import {
22+
getRedirectStatus,
23+
allowedStatusCodes,
24+
} from '../../../lib/redirect-status'
2225
import { normalizeRepeatedSlashes } from '../../../shared/lib/utils'
2326
import { getRelativeURL } from '../../../shared/lib/router/utils/relativize-url'
2427
import { addPathPrefix } from '../../../shared/lib/router/utils/add-path-prefix'
@@ -659,15 +662,36 @@ export function getResolveRoutes(
659662

660663
if (middlewareHeaders['location']) {
661664
const value = middlewareHeaders['location'] as string
662-
const rel = getRelativeURL(value, initUrl)
663-
resHeaders['location'] = rel
664-
parsedUrl = url.parse(rel, true)
665665

666-
return {
667-
parsedUrl,
668-
resHeaders,
669-
finished: true,
670-
statusCode: middlewareRes.status,
666+
// Only process Location header as a redirect if it has a proper redirect status
667+
// This prevents a Location header with non-redirect status from being treated as a redirect
668+
const isRedirectStatus = allowedStatusCodes.has(
669+
middlewareRes.status
670+
)
671+
672+
if (isRedirectStatus) {
673+
// Process as redirect: update parsedUrl and convert to relative URL
674+
const rel = getRelativeURL(value, initUrl)
675+
resHeaders['location'] = rel
676+
parsedUrl = url.parse(rel, true)
677+
678+
return {
679+
parsedUrl,
680+
resHeaders,
681+
finished: true,
682+
statusCode: middlewareRes.status,
683+
}
684+
} else {
685+
// Not a redirect: just pass through the Location header
686+
resHeaders['location'] = value
687+
688+
return {
689+
parsedUrl,
690+
resHeaders,
691+
finished: true,
692+
bodyStream,
693+
statusCode: middlewareRes.status,
694+
}
671695
}
672696
}
673697

test/e2e/app-dir/app-middleware/app-middleware.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { nextTestSetup, FileRef } from 'e2e-utils'
66
import type { Response } from 'node-fetch'
77

88
describe('app-dir with middleware', () => {
9-
const { next } = nextTestSetup({
9+
const { next, isNextDeploy } = nextTestSetup({
1010
files: __dirname,
1111
})
1212

@@ -248,6 +248,29 @@ describe('app-dir with middleware', () => {
248248

249249
await browser.deleteCookies()
250250
})
251+
252+
// TODO: This consistently 404s on Vercel deployments. It technically
253+
// doesn't repro the bug we're trying to fix but we need to figure out
254+
// why the handling is different.
255+
if (!isNextDeploy) {
256+
it('should not incorrectly treat a Location header as a rewrite', async () => {
257+
const res = await next.fetch('/test-location-header')
258+
259+
// Should get status 200 (not a redirect status)
260+
expect(res.status).toBe(200)
261+
262+
// Should get the JSON response associated with the route,
263+
// and not follow the redirect
264+
const json = await res.json()
265+
expect(json).toEqual({ foo: 'bar' })
266+
267+
// Ensure the provided location is still on the response
268+
const locationHeader = res.headers.get('location')
269+
expect(locationHeader).toBe(
270+
'https://next-data-api-endpoint.vercel.app/api/random'
271+
)
272+
})
273+
}
251274
})
252275

253276
describe('app dir - middleware without pages dir', () => {

test/e2e/app-dir/app-middleware/middleware.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,17 @@ export async function middleware(request) {
7878
return res
7979
}
8080

81+
if (request.nextUrl.pathname === '/test-location-header') {
82+
return NextResponse.json(
83+
{ foo: 'bar' },
84+
{
85+
headers: {
86+
location: 'https://next-data-api-endpoint.vercel.app/api/random',
87+
},
88+
}
89+
)
90+
}
91+
8192
return NextResponse.next({
8293
request: {
8394
headers: headersFromRequest,

0 commit comments

Comments
 (0)