From fef4757389947aae0e79e3423377fb39f85787b7 Mon Sep 17 00:00:00 2001
From: Chris Frank <chris@cfrank.org>
Date: Thu, 18 Apr 2024 09:30:32 -0700
Subject: [PATCH] Fix issue with redirect() duplicating basePath in absolute
 URLs

When encountering a absolute URL within a redirect() call from a server
action we were duplicating the basePath when it was present.

This commit moves responsibility for including the basePath to
`getAppRelativeRedirectUrl` which solves the issue.

Signed-off-by: Chris Frank <chris@cfrank.org>
---
 .../src/server/app-render/action-handler.ts   | 15 +++++------
 test/e2e/app-dir/actions/app-action.test.ts   | 27 ++++++++++++++-----
 test/e2e/app-dir/app-basepath/index.test.ts   |  6 +++--
 3 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/packages/next/src/server/app-render/action-handler.ts b/packages/next/src/server/app-render/action-handler.ts
index 9da2937673581e..29d4a97f2d4be3 100644
--- a/packages/next/src/server/app-render/action-handler.ts
+++ b/packages/next/src/server/app-render/action-handler.ts
@@ -251,17 +251,14 @@ function getAppRelativeRedirectUrl(
   host: Host,
   redirectUrl: string
 ): URL | null {
-  if (!host) {
-    return null
-  }
-
-  const parsedRedirectUrl = new URL(redirectUrl, 'http://n')
-
   if (redirectUrl.startsWith('/')) {
-    return parsedRedirectUrl
+    // Make sure we are appending the basePath to relative URLS
+    return new URL(`${basePath}${redirectUrl}`, 'http://n')
   }
 
-  if (host.value !== parsedRedirectUrl.host) {
+  const parsedRedirectUrl = new URL(redirectUrl)
+
+  if (host?.value !== parsedRedirectUrl.host) {
     return null
   }
 
@@ -312,7 +309,7 @@ async function createRedirectRenderResult(
       process.env.__NEXT_PRIVATE_ORIGIN || `${proto}://${originalHost.value}`
 
     const fetchUrl = new URL(
-      `${origin}${basePath}${appRelativeRedirectUrl.pathname}${appRelativeRedirectUrl.search}`
+      `${origin}${appRelativeRedirectUrl.pathname}${appRelativeRedirectUrl.search}`
     )
 
     if (staticGenerationStore.revalidatedTags) {
diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts
index 3aebf05b5ed3fd..688e24737ef5ea 100644
--- a/test/e2e/app-dir/actions/app-action.test.ts
+++ b/test/e2e/app-dir/actions/app-action.test.ts
@@ -775,21 +775,36 @@ describe('app-dir action handling', () => {
     it.each(['relative', 'absolute'])(
       `should handle calls to redirect() with a %s URL in a single pass`,
       async (redirectType) => {
-        const browser = await next.browser('/client/edge')
+        const initialPagePath = '/client/redirects'
+        const destinationPagePath = '/redirect-target'
+
+        const browser = await next.browser(initialPagePath)
 
         const requests: Request[] = []
         const responses: Response[] = []
 
         browser.on('request', (req: Request) => {
-          requests.push(req)
+          const url = req.url()
+
+          if (!url.includes('_next')) {
+            requests.push(req)
+          }
         })
 
         browser.on('response', (res: Response) => {
-          responses.push(res)
+          const url = res.url()
+
+          if (!url.includes('_next')) {
+            responses.push(res)
+          }
         })
 
         await browser.elementById(`redirect-${redirectType}`).click()
-        await check(() => browser.url(), /\/redirect-target/)
+        await check(() => browser.url(), `${next.url}${destinationPagePath}`)
+
+        expect(await browser.waitForElementByCss('#redirected').text()).toBe(
+          'redirected'
+        )
 
         // no other requests should be made
         expect(requests).toHaveLength(1)
@@ -798,14 +813,14 @@ describe('app-dir action handling', () => {
         const request = requests[0]
         const response = responses[0]
 
-        expect(request.url()).toEqual(`${next.url}/client/edge`)
+        expect(request.url()).toEqual(`${next.url}${initialPagePath}`)
         expect(request.method()).toEqual('POST')
         expect(response.status()).toEqual(303)
       }
     )
 
     it('should handle calls to redirect() with external URLs', async () => {
-      const browser = await next.browser('/client/edge')
+      const browser = await next.browser('/client/redirects')
 
       await browser.elementByCss('#redirect-external').click()
 
diff --git a/test/e2e/app-dir/app-basepath/index.test.ts b/test/e2e/app-dir/app-basepath/index.test.ts
index 42cea5acb6a558..ff4992c009f3df 100644
--- a/test/e2e/app-dir/app-basepath/index.test.ts
+++ b/test/e2e/app-dir/app-basepath/index.test.ts
@@ -135,6 +135,8 @@ describe('app dir - basepath', () => {
       await browser.elementById(buttonId).click()
       await check(() => browser.url(), /\/base\/another/)
 
+      expect(await browser.waitForElementByCss('#page-2').text()).toBe(`Page 2`)
+
       // verify that the POST request was only made to the action handler
       expect(requests).toHaveLength(1)
       expect(responses).toHaveLength(1)
@@ -160,7 +162,7 @@ describe('app dir - basepath', () => {
     browser.on('request', (req: Request) => {
       const url = req.url()
 
-      if (url.includes(initialPagePath) || url.includes(destinationPagePath)) {
+      if (!url.includes('_next')) {
         requests.push(req)
       }
     })
@@ -168,7 +170,7 @@ describe('app dir - basepath', () => {
     browser.on('response', (res: Response) => {
       const url = res.url()
 
-      if (url.includes(initialPagePath) || url.includes(destinationPagePath)) {
+      if (!url.includes('_next')) {
         responses.push(res)
       }
     })