Fix internal route redirection with absolute urls outside basePath

packages/next/src/server/app-render/action-handler.ts

@@ -239,6 +239,36 @@ async function createForwardedActionResponse(
   return RenderResult.fromStatic('{}')
 }
 
+/**
+ * Returns the parsed redirect URL if we deem that it is hosted by us.
+ *
+ * We handle both relative and absolute redirect URLs.
+ *
+ * In case the redirect URL is not relative to the application we return `null`.
+ */
+function getAppRelativeRedirectUrl(
+  basePath: string,
+  host: Host,
+  redirectUrl: string
+): URL | null {
+  if (redirectUrl.startsWith('/')) {
+    // Make sure we are appending the basePath to relative URLS
+    return new URL(`${basePath}${redirectUrl}`, 'http://n')
+  }
+
+  const parsedRedirectUrl = new URL(redirectUrl)
+
+  if (host?.value !== parsedRedirectUrl.host) {
+    return null
+  }
+
+  // At this point the hosts are the same, just confirm we
+  // are routing to a path underneath the `basePath`
+  return parsedRedirectUrl.pathname.startsWith(basePath)
+    ? parsedRedirectUrl
+    : null
+}
+
 async function createRedirectRenderResult(
   req: BaseNextRequest,
   res: BaseNextResponse,
@@ -252,14 +282,15 @@ async function createRedirectRenderResult(
   // If we're redirecting to another route of this Next.js application, we'll
   // try to stream the response from the other worker path. When that works,
   // we can save an extra roundtrip and avoid a full page reload.
-  // When the redirect URL starts with a `/`, or to the same host as application,
-  // we treat it as an app-relative redirect.
-  const parsedRedirectUrl = new URL(redirectUrl, 'http://n')
-  const isAppRelativeRedirect =
-    redirectUrl.startsWith('/') ||
-    (originalHost && originalHost.value === parsedRedirectUrl.host)
-
-  if (isAppRelativeRedirect) {
+  // When the redirect URL starts with a `/` or is to the same host, under the
+  // `basePath` we treat it as an app-relative redirect;
+  const appRelativeRedirectUrl = getAppRelativeRedirectUrl(
+    basePath,
+    originalHost,
+    redirectUrl
+  )
+
+  if (appRelativeRedirectUrl) {
     if (!originalHost) {
       throw new Error(
         'Invariant: Missing `host` header from a forwarded Server Actions request.'
@@ -278,7 +309,7 @@ async function createRedirectRenderResult(
       process.env.__NEXT_PRIVATE_ORIGIN || `${proto}://${originalHost.value}`
 
     const fetchUrl = new URL(
-      `${origin}${basePath}${parsedRedirectUrl.pathname}${parsedRedirectUrl.search}`
+      `${origin}${appRelativeRedirectUrl.pathname}${appRelativeRedirectUrl.search}`
     )
 
     if (staticGenerationStore.revalidatedTags) {

test/e2e/app-dir/actions/app-action.test.ts

@@ -809,25 +809,61 @@ describe('app-dir action handling', () => {
       }, 'Prefix: HELLO, WORLD')
     })
 
-    it('should handle redirect to a relative URL in a single pass', async () => {
-      const browser = await next.browser('/client/edge')
+    it.each(['relative', 'absolute'])(
+      `should handle calls to redirect() with a %s URL in a single pass`,
+      async (redirectType) => {
+        const initialPagePath = '/client/redirects'
+        const destinationPagePath = '/redirect-target'
 
-      await waitFor(3000)
+        const browser = await next.browser(initialPagePath)
 
-      let requests = []
+        const requests: Request[] = []
+        const responses: Response[] = []
 
-      browser.on('request', (req: Request) => {
-        requests.push(new URL(req.url()).pathname)
-      })
+        browser.on('request', (req: Request) => {
+          const url = req.url()
 
-      await browser.elementByCss('#redirect').click()
+          if (
+            url.includes(initialPagePath) ||
+            url.includes(destinationPagePath)
+          ) {
+            requests.push(req)
+          }
+        })
 
-      // no other requests should be made
-      expect(requests).toEqual(['/client/edge'])
-    })
+        browser.on('response', (res: Response) => {
+          const url = res.url()
 
-    it('should handle regular redirects', async () => {
-      const browser = await next.browser('/client/edge')
+          if (
+            url.includes(initialPagePath) ||
+            url.includes(destinationPagePath)
+          ) {
+            responses.push(res)
+          }
+        })
+
+        await browser.elementById(`redirect-${redirectType}`).click()
+        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)
+        expect(responses).toHaveLength(1)
+
+        const request = requests[0]
+        const response = responses[0]
+
+        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/redirects')
 
       await browser.elementByCss('#redirect-external').click()
 
@@ -876,36 +912,57 @@ describe('app-dir action handling', () => {
       await check(() => browser.elementByCss('#count').text(), '2')
     })
 
-    it('should handle redirect to a relative URL in a single pass', async () => {
-      let responseCode: number
-      const browser = await next.browser('/client', {
-        beforePageLoad(page) {
-          page.on('response', async (res: Response) => {
-            const headers = await res.allHeaders()
-            if (headers['x-action-redirect']) {
-              responseCode = res.status()
-            }
-          })
-        },
-      })
+    it.each(['relative', 'absolute'])(
+      `should handle calls to redirect() with a %s URL in a single pass`,
+      async (redirectType) => {
+        const initialPagePath = '/client/redirects'
+        const destinationPagePath = '/redirect-target'
 
-      await waitFor(3000)
+        const browser = await next.browser(initialPagePath)
 
-      let requests = []
+        const requests: Request[] = []
+        const responses: Response[] = []
 
-      browser.on('request', (req: Request) => {
-        requests.push(new URL(req.url()).pathname)
-      })
+        browser.on('request', (req: Request) => {
+          const url = req.url()
 
-      await browser.elementByCss('#redirect').click()
+          if (
+            url.includes(initialPagePath) ||
+            url.includes(destinationPagePath)
+          ) {
+            requests.push(req)
+          }
+        })
 
-      // no other requests should be made
-      expect(requests).toEqual(['/client'])
-      await check(() => responseCode, 303)
-    })
+        browser.on('response', (res: Response) => {
+          const url = res.url()
+
+          if (
+            url.includes(initialPagePath) ||
+            url.includes(destinationPagePath)
+          ) {
+            responses.push(res)
+          }
+        })
+
+        await browser.elementById(`redirect-${redirectType}`).click()
+        await check(() => browser.url(), `${next.url}${destinationPagePath}`)
+
+        // no other requests should be made
+        expect(requests).toHaveLength(1)
+        expect(responses).toHaveLength(1)
+
+        const request = requests[0]
+        const response = responses[0]
+
+        expect(request.url()).toEqual(`${next.url}${initialPagePath}`)
+        expect(request.method()).toEqual('POST')
+        expect(response.status()).toEqual(303)
+      }
+    )
 
-    it('should handle regular redirects', async () => {
-      const browser = await next.browser('/client')
+    it('should handle calls to redirect() with external URLs', async () => {
+      const browser = await next.browser('/client/redirects')
 
       await browser.elementByCss('#redirect-external').click()
 

test/e2e/app-dir/actions/app/client/edge/page.js

@@ -38,10 +38,20 @@ export default function Counter() {
       </button>
       <form>
         <button
-          id="redirect"
+          id="redirect-relative"
           formAction={() => redirectAction('/redirect-target')}
         >
-          redirect
+          redirect to a relative URL
+        </button>
+      </form>
+      <form>
+        <button
+          id="redirect-absolute"
+          formAction={() =>
+            redirectAction(`${location.origin}/redirect-target`)
+          }
+        >
+          redirect to a absolute URL
         </button>
       </form>
       <form>

test/e2e/app-dir/actions/app/client/page.js

@@ -56,36 +56,6 @@ export default function Counter() {
       >
         *2
       </button>
-      <form>
-        <button
-          id="redirect"
-          formAction={() => redirectAction('/redirect-target')}
-        >
-          redirect
-        </button>
-      </form>
-      <form>
-        <button
-          id="redirect-external"
-          formAction={() =>
-            redirectAction(
-              'https://next-data-api-endpoint.vercel.app/api/random?page'
-            )
-          }
-        >
-          redirect external
-        </button>
-      </form>
-      <form>
-        <button
-          id="redirect-absolute"
-          formAction={() =>
-            redirectAction(location.origin + '/redirect-target')
-          }
-        >
-          redirect internal with domain
-        </button>
-      </form>
       <form>
         <button
           id="redirect-pages"

test/e2e/app-dir/actions/app/client/redirects/page.js

@@ -0,0 +1,40 @@
+'use client'
+
+import { redirectAction } from '../actions'
+
+export default function Page() {
+  return (
+    <div>
+      <form>
+        <button
+          id="redirect-relative"
+          formAction={() => redirectAction('/redirect-target')}
+        >
+          redirect relative
+        </button>
+      </form>
+      <form>
+        <button
+          id="redirect-external"
+          formAction={() =>
+            redirectAction(
+              'https://next-data-api-endpoint.vercel.app/api/random?page'
+            )
+          }
+        >
+          redirect external
+        </button>
+      </form>
+      <form>
+        <button
+          id="redirect-absolute"
+          formAction={() =>
+            redirectAction(location.origin + '/redirect-target')
+          }
+        >
+          redirect internal with domain
+        </button>
+      </form>
+    </div>
+  )
+}

test/e2e/app-dir/app-basepath/app/client/action.js

@@ -0,0 +1,9 @@
+'use server'
+
+import 'server-only'
+
+import { redirect } from 'next/navigation'
+
+export async function redirectAction(path) {
+  redirect(path)
+}

test/e2e/app-dir/app-basepath/app/client/page.js

@@ -0,0 +1,36 @@
+'use client'
+
+import { redirectAction } from './action'
+
+export default function Page() {
+  return (
+    <div>
+      <form>
+        <button
+          id="redirect-relative"
+          formAction={() => redirectAction('/another')}
+        >
+          redirect internal with relative path
+        </button>
+      </form>
+      <form>
+        <button
+          id="redirect-absolute-internal"
+          formAction={() => redirectAction(location.origin + '/base/another')}
+        >
+          redirect internal with domain (including basePath)
+        </button>
+      </form>
+      <form>
+        <button
+          id="redirect-absolute-external"
+          formAction={() =>
+            redirectAction(location.origin + '/outsideBasePath')
+          }
+        >
+          redirect external with domain (excluding basePath)
+        </button>
+      </form>
+    </div>
+  )
+}