Fix internal route redirection with absolute urls outside basePath

When performing a redirect() with an absolute path, action-handler
attempts to detect whether the resource is hosted by NextJS. If we
believe it is, we then attempt to stream it.

Previously we were not accounting for basePath which caused absolute
redirects to resources on the same host, but not underneath the
basePath, to be resolved by NextJS. Since the resource is outside the
basePath we resolve a 404 page which returns back as `text/x-component`
and is thus streamed back to the client within the original POST
request.

This PR adds a check for the presence of the basePath within absolute
redirect URLs. This fixes the above problem.

Signed-off-by: Chris Frank <chris@cfrank.org>

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

@@ -239,6 +239,39 @@ 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 (!host) {
+    return null
+  }
+
+  const parsedRedirectUrl = new URL(redirectUrl, 'http://n')
+
+  if (redirectUrl.startsWith('/')) {
+    return parsedRedirectUrl
+  }
+
+  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 +285,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)
+  // 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 (isAppRelativeRedirect) {
+  if (appRelativeRedirectUrl) {
     if (!originalHost) {
       throw new Error(
         'Invariant: Missing `host` header from a forwarded Server Actions request.'
@@ -278,7 +312,7 @@ async function createRedirectRenderResult(
       process.env.__NEXT_PRIVATE_ORIGIN || `${proto}://${originalHost.value}`
 
     const fetchUrl = new URL(
-      `${origin}${basePath}${parsedRedirectUrl.pathname}${parsedRedirectUrl.search}`
+      `${origin}${basePath}${appRelativeRedirectUrl.pathname}${appRelativeRedirectUrl.search}`
     )
 
     if (staticGenerationStore.revalidatedTags) {

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

@@ -772,24 +772,39 @@ 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 browser = await next.browser('/client/edge')
 
-      await waitFor(3000)
+        const requests: Request[] = []
+        const responses: Response[] = []
 
-      let requests = []
+        browser.on('request', (req: Request) => {
+          requests.push(req)
+        })
 
-      browser.on('request', (req: Request) => {
-        requests.push(new URL(req.url()).pathname)
-      })
+        browser.on('response', (res: Response) => {
+          responses.push(res)
+        })
 
-      await browser.elementByCss('#redirect').click()
+        await browser.elementById(`redirect-${redirectType}`).click()
+        await check(() => browser.url(), /\/redirect-target/)
 
-      // no other requests should be made
-      expect(requests).toEqual(['/client/edge'])
-    })
+        // 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}/client/edge`)
+        expect(request.method()).toEqual('POST')
+        expect(response.status()).toEqual(303)
+      }
+    )
 
-    it('should handle regular redirects', async () => {
+    it('should handle calls to redirect() with external URLs', async () => {
       const browser = await next.browser('/client/edge')
 
       await browser.elementByCss('#redirect-external').click()
@@ -839,36 +854,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()
+
+          if (
+            url.includes(initialPagePath) ||
+            url.includes(destinationPagePath)
+          ) {
+            requests.push(req)
+          }
+        })
 
-      await browser.elementByCss('#redirect').click()
+        browser.on('response', (res: Response) => {
+          const url = res.url()
 
-      // no other requests should be made
-      expect(requests).toEqual(['/client'])
-      await check(() => responseCode, 303)
-    })
+          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>
+  )
+}