From 1ca23ce3260f8b9362f44c37b7dc8cfa8dc03aa0 Mon Sep 17 00:00:00 2001
From: Philippe Serhal <philippe.serhal@netlify.com>
Date: Thu, 22 Aug 2024 16:39:33 -0400
Subject: [PATCH] fix: ignore middleware rewrite when redirecting

The Next.js behaviour is for a middleware redirect to take precedence over a middleware rewrite when
both are present at once.
---
 edge-runtime/lib/response.ts            |  3 ++-
 tests/fixtures/middleware/middleware.ts | 10 +++++++++
 tests/integration/edge-handler.test.ts  | 30 +++++++++++++++++++++----
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/edge-runtime/lib/response.ts b/edge-runtime/lib/response.ts
index 6e2366354b..498da4e298 100644
--- a/edge-runtime/lib/response.ts
+++ b/edge-runtime/lib/response.ts
@@ -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')
diff --git a/tests/fixtures/middleware/middleware.ts b/tests/fixtures/middleware/middleware.ts
index 5140197933..6490d7c538 100644
--- a/tests/fixtures/middleware/middleware.ts
+++ b/tests/fixtures/middleware/middleware.ts
@@ -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 })
 }
 
diff --git a/tests/integration/edge-handler.test.ts b/tests/integration/edge-handler.test.ts
index 78949d2d6c..10300b9ed4 100644
--- a/tests/integration/edge-handler.test.ts
+++ b/tests/integration/edge-handler.test.ts
@@ -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)
@@ -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', () => {
@@ -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')
@@ -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')