Fix `public/` file name encoding (#10022)

* Update serving of files from public folder to handle special chars

* Update tests and match handling in dev mode

* Fix windows public file handling

* Remove colon test to make git on windows happy

* Remove un-used files

* Add missing await

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

@@ -647,30 +647,35 @@ export default class Server {
   }
 
   protected generatePublicRoutes(): Route[] {
-    const routes: Route[] = []
-    const publicFiles = recursiveReadDirSync(this.publicDir)
-
-    publicFiles.forEach(path => {
-      const unixPath = path.replace(/\\/g, '/')
-      // Only include public files that will not replace a page path
-      // this should not occur now that we check this during build
-      if (!this.pagesManifest![unixPath]) {
-        routes.push({
-          match: route(unixPath),
-          type: 'route',
-          name: 'public catchall',
-          fn: async (req, res, _params, parsedUrl) => {
-            const p = join(this.publicDir, unixPath)
-            await this.serveStatic(req, res, p, parsedUrl)
+    const publicFiles = new Set(
+      recursiveReadDirSync(this.publicDir).map(p => p.replace(/\\/g, '/'))
+    )
+
+    return [
+      {
+        match: route('/:path*'),
+        name: 'public folder catchall',
+        fn: async (req, res, params, parsedUrl) => {
+          const path = `/${(params.path || []).join('/')}`
+
+          if (publicFiles.has(path)) {
+            await this.serveStatic(
+              req,
+              res,
+              // we need to re-encode it since send decodes it
+              join(this.dir, 'public', encodeURIComponent(path)),
+              parsedUrl
+            )
             return {
               finished: true,
             }
-          },
-        })
-      }
-    })
-
-    return routes
+          }
+          return {
+            finished: false,
+          }
+        },
+      } as Route,
+    ]
   }
 
   protected getDynamicRoutes() {

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

@@ -258,14 +258,14 @@ export default class DevServer extends Server {
   protected async _beforeCatchAllRender(
     req: IncomingMessage,
     res: ServerResponse,
-    _params: Params,
+    params: Params,
     parsedUrl: UrlWithParsedQuery
   ) {
     const { pathname } = parsedUrl
-
+    const path = `/${(params.path || []).join('/')}`
     // check for a public file, throwing error if there's a
     // conflicting page
-    if (await this.hasPublicFile(pathname!)) {
+    if (await this.hasPublicFile(path)) {
       if (await this.hasPage(pathname!)) {
         const err = new Error(
           `A conflicting public file and page file was found for path ${pathname} https://err.sh/zeit/next.js/conflicting-public-file-page`
@@ -274,7 +274,7 @@ export default class DevServer extends Server {
         await this.renderError(err, req, res, pathname!, {})
         return true
       }
-      await this.servePublic(req, res, pathname!)
+      await this.servePublic(req, res, path)
       return true
     }
 
@@ -484,7 +484,8 @@ export default class DevServer extends Server {
   }
 
   servePublic(req: IncomingMessage, res: ServerResponse, path: string) {
-    const p = join(this.publicDir, path)
+    const p = join(this.publicDir, encodeURIComponent(path))
+    // we need to re-encode it since send decodes it
     return this.serveStatic(req, res, p)
   }
 

test/integration/dynamic-routing/public/hello copy.txt

+hello world copy
\ No newline at end of file

test/integration/dynamic-routing/public/hello%20copy.txt

+hello world %20
\ No newline at end of file

test/integration/dynamic-routing/public/hello+copy.txt

+hello world +
\ No newline at end of file

test/integration/dynamic-routing/test/index.test.js

@@ -350,6 +350,34 @@ function runTests(dev) {
     expect(data).toMatch(/hello world/)
   })
 
+  it('should serve file with space from public folder', async () => {
+    const res = await fetchViaHTTP(appPort, '/hello copy.txt')
+    const text = (await res.text()).trim()
+    expect(text).toBe('hello world copy')
+    expect(res.status).toBe(200)
+  })
+
+  it('should serve file with plus from public folder', async () => {
+    const res = await fetchViaHTTP(appPort, '/hello+copy.txt')
+    const text = (await res.text()).trim()
+    expect(text).toBe('hello world +')
+    expect(res.status).toBe(200)
+  })
+
+  it('should serve file from public folder encoded', async () => {
+    const res = await fetchViaHTTP(appPort, '/hello%20copy.txt')
+    const text = (await res.text()).trim()
+    expect(text).toBe('hello world copy')
+    expect(res.status).toBe(200)
+  })
+
+  it('should serve file with %20 from public folder', async () => {
+    const res = await fetchViaHTTP(appPort, '/hello%2520copy.txt')
+    const text = (await res.text()).trim()
+    expect(text).toBe('hello world %20')
+    expect(res.status).toBe(200)
+  })
+
   if (dev) {
     it('should work with HMR correctly', async () => {
       const browser = await webdriver(appPort, '/post-1/comments')
@@ -389,6 +417,10 @@ function runTests(dev) {
         join(appDir, '.next/routes-manifest.json')
       )
 
+      for (const route of manifest.dynamicRoutes) {
+        route.regex = normalizeRegEx(route.regex)
+      }
+
       expect(manifest).toEqual({
         version: 1,
         basePath: '',