Fix static file check with i18n (vercel#33503)

hensansi · Jan 21, 2022 · 69524c2 · 69524c2
1 parent 557a972
commit 69524c2
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 6 deletions.
diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts
@@ -740,7 +740,8 @@ export default abstract class Server {
             res,
             pathname,
             { ..._parsedUrl.query, _nextDataReq: '1' },
-            parsedUrl
+            parsedUrl,
+            true
           )
           return {
             finished: true,
@@ -962,7 +963,7 @@ export default abstract class Server {
         }
 
         try {
-          await this.render(req, res, pathname, query, parsedUrl)
+          await this.render(req, res, pathname, query, parsedUrl, true)
 
           return {
             finished: true,
@@ -1183,7 +1184,8 @@ export default abstract class Server {
     res: BaseNextResponse,
     pathname: string,
     query: NextParsedUrlQuery = {},
-    parsedUrl?: NextUrlWithParsedQuery
+    parsedUrl?: NextUrlWithParsedQuery,
+    internalRender = false
   ): Promise<void> {
     if (!pathname.startsWith('/')) {
       console.warn(
@@ -1206,6 +1208,7 @@ export default abstract class Server {
     // we don't modify the URL for _next/data request but still
     // call render so we special case this to prevent an infinite loop
     if (
+      !internalRender &&
       !this.minimalMode &&
       !query._nextDataReq &&
       (req.url?.match(/^\/_next\//) ||

diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts
@@ -216,7 +216,7 @@ export default class DevServer extends Server {
 
             const mergedQuery = { ...urlQuery, ...query }
 
-            await this.render(req, res, page, mergedQuery, parsedUrl)
+            await this.render(req, res, page, mergedQuery, parsedUrl, true)
             return {
               finished: true,
             }

diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts
@@ -601,14 +601,16 @@ export default class NextNodeServer extends BaseServer {
     res: BaseNextResponse | ServerResponse,
     pathname: string,
     query?: NextParsedUrlQuery,
-    parsedUrl?: NextUrlWithParsedQuery
+    parsedUrl?: NextUrlWithParsedQuery,
+    internal = false
   ): Promise<void> {
     return super.render(
       this.normalizeReq(req),
       this.normalizeRes(res),
       pathname,
       query,
-      parsedUrl
+      parsedUrl,
+      internal
     )
   }
 

diff --git a/packages/next/server/router.ts b/packages/next/server/router.ts
@@ -78,6 +78,7 @@ export default class Router {
   dynamicRoutes: DynamicRoutes
   useFileSystemPublicRoutes: boolean
   locales: string[]
+  seenRequests: Set<any>
 
   constructor({
     basePath = '',
@@ -123,6 +124,7 @@ export default class Router {
     this.dynamicRoutes = dynamicRoutes
     this.useFileSystemPublicRoutes = useFileSystemPublicRoutes
     this.locales = locales
+    this.seenRequests = new Set()
   }
 
   setDynamicRoutes(routes: DynamicRoutes = []) {
@@ -138,6 +140,13 @@ export default class Router {
     res: BaseNextResponse,
     parsedUrl: NextUrlWithParsedQuery
   ): Promise<boolean> {
+    if (this.seenRequests.has(req)) {
+      throw new Error(
+        `Invariant: request has already been processed: ${req.url}, this is an internal error please open an issue.`
+      )
+    }
+    this.seenRequests.add(req)
+
     // memoize page check calls so we don't duplicate checks for pages
     const pageChecks: { [name: string]: Promise<boolean> } = {}
     const memoizedPageChecker = async (p: string): Promise<boolean> => {
@@ -360,6 +369,7 @@ export default class Router {
           ) {
             if (requireBasePath) {
               // consider this a non-match so the 404 renders
+              this.seenRequests.delete(req)
               return false
             }
             // page checker occurs before rewrites so we need to continue
@@ -374,6 +384,7 @@ export default class Router {
 
         // The response was handled
         if (result.finished) {
+          this.seenRequests.delete(req)
           return true
         }
 
@@ -397,11 +408,13 @@ export default class Router {
         // check filesystem
         if (testRoute.check === true) {
           if (await applyCheckTrue(parsedUrlUpdated)) {
+            this.seenRequests.delete(req)
             return true
           }
         }
       }
     }
+    this.seenRequests.delete(req)
     return false
   }
 }
diff --git a/test/integration/i18n-support/test/shared.js b/test/integration/i18n-support/test/shared.js
@@ -1,6 +1,7 @@
 /* eslint-env jest */
 
 import url from 'url'
+import glob from 'glob'
 import fs from 'fs-extra'
 import cheerio from 'cheerio'
 import { join } from 'path'
@@ -56,6 +57,26 @@ export function runTests(ctx) {
     })
   }
 
+  it('should 404 for locale prefixed static assets correctly', async () => {
+    const assets = glob.sync('**/*.js', {
+      cwd: join(ctx.appDir, '.next/static'),
+    })
+
+    for (const locale of locales) {
+      for (const asset of assets) {
+        // _next/static asset
+        const res = await fetchViaHTTP(
+          ctx.appPort,
+          `${ctx.basePath || ''}/${locale}/_next/static/${asset}`,
+          undefined,
+          { redirect: 'manual' }
+        )
+        expect(res.status).toBe(404)
+        expect(await res.text()).toContain('could not be found')
+      }
+    }
+  })
+
   it('should redirect external domain correctly', async () => {
     const res = await fetchViaHTTP(
       ctx.appPort,