Ensure external beforeFiles rewrites are handled with next/link (#33888)

This ensures we properly handle external `beforeFiles` rewrites client-side so that a different result doesn't occur client-side versus on a direct visit. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Fixes: #32348
vercel · Feb 2, 2022 · 7b1bace · 7b1bace
1 parent ee2725f
commit 7b1bace
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 1 deletion.
diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts
@@ -1104,6 +1104,11 @@ export default class Router implements BaseRouter {
           (p: string) => resolveDynamicRoute(p, pages),
           this.locales
         )
+
+        if (rewritesResult.externalDest) {
+          location.href = as
+          return true
+        }
         resolvedAs = rewritesResult.asPath
 
         if (rewritesResult.matchedPage && rewritesResult.resolvedHref) {
@@ -1689,6 +1694,10 @@ export default class Router implements BaseRouter {
         (p: string) => resolveDynamicRoute(p, pages),
         this.locales
       )
+
+      if (rewritesResult.externalDest) {
+        return
+      }
       resolvedAs = delLocale(delBasePath(rewritesResult.asPath), this.locale)
 
       if (rewritesResult.matchedPage && rewritesResult.resolvedHref) {

diff --git a/packages/next/shared/lib/router/utils/resolve-rewrites.ts b/packages/next/shared/lib/router/utils/resolve-rewrites.ts
@@ -25,8 +25,10 @@ export default function resolveRewrites(
   parsedAs: ReturnType<typeof parseRelativeUrl>
   asPath: string
   resolvedHref?: string
+  externalDest?: boolean
 } {
   let matchedPage = false
+  let externalDest = false
   let parsedAs = parseRelativeUrl(asPath)
   let fsPathname = removePathTrailingSlash(
     normalizeLocalePath(delBasePath(parsedAs.pathname), locales).pathname
@@ -65,6 +67,7 @@ export default function resolveRewrites(
     if (params) {
       if (!rewrite.destination) {
         // this is a proxied rewrite which isn't handled on the client
+        externalDest = true
         return true
       }
       const destRes = prepareDestination({
@@ -103,7 +106,7 @@ export default function resolveRewrites(
   for (let i = 0; i < rewrites.beforeFiles.length; i++) {
     // we don't end after match in beforeFiles to allow
     // continuing through all beforeFiles rewrites
-    handleRewrite(rewrites.beforeFiles[i])
+    finished = handleRewrite(rewrites.beforeFiles[i]) || false
   }
   matchedPage = pages.includes(fsPathname)
 
@@ -139,5 +142,6 @@ export default function resolveRewrites(
     parsedAs,
     matchedPage,
     resolvedHref,
+    externalDest,
   }
 }
diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js
@@ -220,6 +220,10 @@ module.exports = {
           source: '/old-blog/:path*',
           destination: '/blog/:path*',
         },
+        {
+          source: '/overridden',
+          destination: 'https://example.com',
+        },
       ],
     }
   },

diff --git a/test/integration/custom-routes/pages/nav.js b/test/integration/custom-routes/pages/nav.js
@@ -40,6 +40,10 @@ const Page = () => (
       <a id="to-old-blog">to /old-blog/post-1</a>
     </Link>
     <br />
+    <Link href="/overridden">
+      <a id="to-before-files-overridden">to /overridden</a>
+    </Link>
+    <br />
   </>
 )
 export default Page
diff --git a/test/integration/custom-routes/pages/overridden.js b/test/integration/custom-routes/pages/overridden.js
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p>this page is overridden by a beforeFiles rewrite</p>
+}
diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js
@@ -39,6 +39,19 @@ let appPort
 let app
 
 const runTests = (isDev = false) => {
+  it('should handle external beforeFiles rewrite correctly', async () => {
+    const res = await fetchViaHTTP(appPort, '/overridden')
+    expect(res.status).toBe(200)
+    expect(await res.text()).toContain('Example Domain')
+
+    const browser = await webdriver(appPort, '/nav')
+    await browser.elementByCss('#to-before-files-overridden').click()
+    await check(
+      () => browser.eval('document.documentElement.innerHTML'),
+      /Example Domain/
+    )
+  })
+
   it('should handle has query encoding correctly', async () => {
     for (const expected of [
       {
@@ -1697,6 +1710,11 @@ const runTests = (isDev = false) => {
               ),
               source: '/old-blog/:path*',
             },
+            {
+              destination: 'https://example.com',
+              regex: normalizeRegEx('^\\/overridden(?:\\/)?$'),
+              source: '/overridden',
+            },
           ],
           afterFiles: [
             {
@@ -2028,6 +2046,12 @@ const runTests = (isDev = false) => {
             regex: '^/nav(?:/)?$',
             routeKeys: {},
           },
+          {
+            namedRegex: '^/overridden(?:/)?$',
+            page: '/overridden',
+            regex: '^/overridden(?:/)?$',
+            routeKeys: {},
+          },
           {
             namedRegex: '^/redirect\\-override(?:/)?$',
             page: '/redirect-override',