Skip to content

Commit

Permalink
fix: ignore middleware rewrite when redirecting
Browse files Browse the repository at this point in the history
The Next.js behaviour is for a middleware redirect to take precedence over a middleware rewrite when
both are present at once.
  • Loading branch information
serhalp committed Aug 27, 2024
1 parent 733a021 commit 1ca23ce
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 5 deletions.
3 changes: 2 additions & 1 deletion edge-runtime/lib/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,10 @@ export const buildResponse = async ({
const res = new Response(result.response.body, result.response)
request.headers.set('x-nf-next-middleware', 'skip')

let rewrite = res.headers.get('x-middleware-rewrite')
let redirect = res.headers.get('location')
let nextRedirect = res.headers.get('x-nextjs-redirect')
// If we have both a redirect and a rewrite, the redirect must take precedence
let rewrite = !redirect && !nextRedirect ? res.headers.get('x-middleware-rewrite') : false

// Data requests (i.e. requests for /_next/data ) need special handling
const isDataReq = request.headers.has('x-nextjs-data')
Expand Down
10 changes: 10 additions & 0 deletions tests/fixtures/middleware/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,16 @@ const getResponse = (request: NextRequest) => {
})
}

if (request.nextUrl.pathname === '/test/rewrite-and-redirect') {
return NextResponse.redirect(new URL('/other', request.url), {
status: 302,
statusText: 'Found',
headers: {
'x-middleware-rewrite': new URL('/test/should-not-be-rewritten', request.url).toString(),
},
})
}

return NextResponse.json({ error: 'Error' }, { status: 500 })
}

Expand Down
30 changes: 26 additions & 4 deletions tests/integration/edge-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('redirect', () => {
expect(response.status).toBe(307)
expect(response.headers.get('location'), 'added a location header').toBeTypeOf('string')
expect(
new URL(response.headers.get('location') as string).pathname,
new URL(response.headers.get('location')!).pathname,
'redirected to the correct path',
).toEqual('/other')
expect(origin.calls).toBe(0)
Expand All @@ -111,12 +111,34 @@ describe('redirect', () => {
expect(response.status).toBe(307)
expect(response.headers.get('location'), 'added a location header').toBeTypeOf('string')
expect(
new URL(response.headers.get('location') as string).pathname,
new URL(response.headers.get('location')!).pathname,
'redirected to the correct path',
).toEqual('/other')
expect(response.headers.get('x-header-from-redirect'), 'hello').toBe('hello')
expect(origin.calls).toBe(0)
})

test<FixtureTestContext>('should ignore x-middleware-rewrite when redirecting', async (ctx) => {
await createFixture('middleware', ctx)
await runPlugin(ctx)

const origin = new LocalServer()
const response = await invokeEdgeFunction(ctx, {
functions: ['___netlify-edge-handler-middleware'],
origin,
redirect: 'manual',
url: '/test/rewrite-and-redirect',
})

ctx.cleanup?.push(() => origin.stop())

expect(response.status).toBe(302)
expect(response.headers.get('location'), 'added a location header').toBeTypeOf('string')
expect(
new URL(response.headers.get('location')!).pathname,
'redirected to the correct path',
).toEqual('/other')
})
})

describe('rewrite', () => {
Expand Down Expand Up @@ -309,7 +331,7 @@ describe('should run middleware on data requests', () => {
expect(response.status).toBe(307)
expect(response.headers.get('location'), 'added a location header').toBeTypeOf('string')
expect(
new URL(response.headers.get('location') as string).pathname,
new URL(response.headers.get('location')!).pathname,
'redirected to the correct path',
).toEqual('/other')
expect(response.headers.get('x-header-from-redirect'), 'hello').toBe('hello')
Expand All @@ -333,7 +355,7 @@ describe('should run middleware on data requests', () => {
expect(response.status).toBe(307)
expect(response.headers.get('location'), 'added a location header').toBeTypeOf('string')
expect(
new URL(response.headers.get('location') as string).pathname,
new URL(response.headers.get('location')!).pathname,
'redirected to the correct path',
).toEqual('/other')
expect(response.headers.get('x-header-from-redirect'), 'hello').toBe('hello')
Expand Down

0 comments on commit 1ca23ce

Please sign in to comment.