Fix basePath and public folder check ending routes early (#16356)

This corrects the basePath being required check for filesystem routes to not consider the public folder catch-all route since it always matches even if the public file isn't present and instead moves the basePath check inside of the public-folder catch-all. Tests already exist that catch this by adding a public folder to the existing `basepath` test suite

Fixes: #16332
Closes: #16350

packages/next/next-server/server/next-server.ts

@@ -783,6 +783,14 @@ export default class Server {
         name: 'public folder catchall',
         fn: async (req, res, params, parsedUrl) => {
           const pathParts: string[] = params.path || []
+          const { basePath } = this.nextConfig
+
+          // if basePath is defined require it be present
+          if (basePath) {
+            if (pathParts[0] !== basePath.substr(1)) return { finished: false }
+            pathParts.shift()
+          }
+
           const path = `/${pathParts.join('/')}`
 
           if (publicFiles.has(path)) {

packages/next/next-server/server/router.ts

@@ -37,6 +37,7 @@ export type PageChecker = (pathname: string) => Promise<boolean>
 const customRouteTypes = new Set(['rewrite', 'redirect', 'header'])
 
 function replaceBasePath(basePath: string, pathname: string) {
+  // If replace ends up replacing the full url it'll be `undefined`, meaning we have to default it to `/`
   return pathname!.replace(basePath, '') || '/'
 }
 
@@ -166,9 +167,10 @@ export default class Router {
       const originalPathname = currentPathname
       const requireBasePath = testRoute.requireBasePath !== false
       const isCustomRoute = customRouteTypes.has(testRoute.type)
+      const isPublicFolderCatchall = testRoute.name === 'public folder catchall'
+      const keepBasePath = isCustomRoute || isPublicFolderCatchall
 
-      if (!isCustomRoute) {
-        // If replace ends up replacing the full url it'll be `undefined`, meaning we have to default it to `/`
+      if (!keepBasePath) {
         currentPathname = replaceBasePath(this.basePath, currentPathname!)
       }
 
@@ -178,7 +180,7 @@ export default class Router {
       if (newParams) {
         // since we require basePath be present for non-custom-routes we
         // 404 here when we matched an fs route
-        if (!isCustomRoute) {
+        if (!keepBasePath) {
           if (!originallyHadBasePath && !(req as any)._nextDidRewrite) {
             if (requireBasePath) {
               // consider this a non-match so the 404 renders
@@ -201,7 +203,7 @@ export default class Router {
 
         // since the fs route didn't match we need to re-add the basePath
         // to continue checking rewrites with the basePath present
-        if (!isCustomRoute) {
+        if (!keepBasePath) {
           parsedUrlUpdated.pathname = originalPathname
         }
 
@@ -272,7 +274,6 @@ export default class Router {
         }
       }
     }
-
     return false
   }
 }

test/integration/basepath/public/data.txt

@@ -0,0 +1 @@
+hello world

test/integration/basepath/test/index.test.js

@@ -135,6 +135,17 @@ const runTests = (context, dev = false) => {
     })
   }
 
+  it('should 404 for public file without basePath', async () => {
+    const res = await fetchViaHTTP(context.appPort, '/data.txt')
+    expect(res.status).toBe(404)
+  })
+
+  it('should serve public file with basePath correctly', async () => {
+    const res = await fetchViaHTTP(context.appPort, '/docs/data.txt')
+    expect(res.status).toBe(200)
+    expect(await res.text()).toBe('hello world')
+  })
+
   it('should rewrite with basePath by default', async () => {
     const html = await renderViaHTTP(context.appPort, '/docs/rewrite-1')
     expect(html).toContain('getServerSideProps')