From d58bd492f9aa4661460d784c21cbc796092fc407 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 9 Jan 2020 16:23:24 -0600 Subject: [PATCH 1/6] Update serving of files from public folder to handle special chars --- .../next/next-server/server/next-server.ts | 44 ++++++++++--------- .../dynamic-routing/public/hello copy.txt | 1 + .../dynamic-routing/public/hello%20copy.txt | 1 + .../dynamic-routing/public/hello+copy.txt | 1 + .../dynamic-routing/test/index.test.js | 21 +++++++++ .../serverless/public/hello copy.txt | 1 + .../serverless/public/hello%20copy.txt | 1 + .../serverless/public/hello+copy.txt | 1 + 8 files changed, 50 insertions(+), 21 deletions(-) create mode 100644 test/integration/dynamic-routing/public/hello copy.txt create mode 100644 test/integration/dynamic-routing/public/hello%20copy.txt create mode 100644 test/integration/dynamic-routing/public/hello+copy.txt create mode 100644 test/integration/serverless/public/hello copy.txt create mode 100644 test/integration/serverless/public/hello%20copy.txt create mode 100644 test/integration/serverless/public/hello+copy.txt diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 2fbe9e6dd8ade..d031bd9525214 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -647,30 +647,32 @@ 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)) + + return [ + { + match: route('/:path*'), + name: 'public folder catchall', + fn: async (req, res, params, parsedUrl) => { + const path = `/${(params.path || []).join('/')}` + + if (publicFiles.has(path)) { + this.serveStatic( + req, + res, + join(this.dir, 'public', path), + parsedUrl + ) return { finished: true, } - }, - }) - } - }) - - return routes + } + return { + finished: false, + } + }, + } as Route, + ] } protected getDynamicRoutes() { diff --git a/test/integration/dynamic-routing/public/hello copy.txt b/test/integration/dynamic-routing/public/hello copy.txt new file mode 100644 index 0000000000000..95d09f2b10159 --- /dev/null +++ b/test/integration/dynamic-routing/public/hello copy.txt @@ -0,0 +1 @@ +hello world \ No newline at end of file diff --git a/test/integration/dynamic-routing/public/hello%20copy.txt b/test/integration/dynamic-routing/public/hello%20copy.txt new file mode 100644 index 0000000000000..95d09f2b10159 --- /dev/null +++ b/test/integration/dynamic-routing/public/hello%20copy.txt @@ -0,0 +1 @@ +hello world \ No newline at end of file diff --git a/test/integration/dynamic-routing/public/hello+copy.txt b/test/integration/dynamic-routing/public/hello+copy.txt new file mode 100644 index 0000000000000..95d09f2b10159 --- /dev/null +++ b/test/integration/dynamic-routing/public/hello+copy.txt @@ -0,0 +1 @@ +hello world \ No newline at end of file diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index 492639cbd8994..e97faaea6d1a4 100644 --- a/test/integration/dynamic-routing/test/index.test.js +++ b/test/integration/dynamic-routing/test/index.test.js @@ -350,6 +350,27 @@ 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') + 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 with %20 from public folder', async () => { + const res = await fetchViaHTTP(appPort, '/hello%20copy.txt') + const text = (await res.text()).trim() + expect(text).toBe('hello world') + expect(res.status).toBe(200) + }) + if (dev) { it('should work with HMR correctly', async () => { const browser = await webdriver(appPort, '/post-1/comments') diff --git a/test/integration/serverless/public/hello copy.txt b/test/integration/serverless/public/hello copy.txt new file mode 100644 index 0000000000000..3b18e512dba79 --- /dev/null +++ b/test/integration/serverless/public/hello copy.txt @@ -0,0 +1 @@ +hello world diff --git a/test/integration/serverless/public/hello%20copy.txt b/test/integration/serverless/public/hello%20copy.txt new file mode 100644 index 0000000000000..3b18e512dba79 --- /dev/null +++ b/test/integration/serverless/public/hello%20copy.txt @@ -0,0 +1 @@ +hello world diff --git a/test/integration/serverless/public/hello+copy.txt b/test/integration/serverless/public/hello+copy.txt new file mode 100644 index 0000000000000..3b18e512dba79 --- /dev/null +++ b/test/integration/serverless/public/hello+copy.txt @@ -0,0 +1 @@ +hello world From c829d94e30f2c957160416de5fcdb63bbfedcd75 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 9 Jan 2020 17:01:05 -0600 Subject: [PATCH 2/6] Update tests and match handling in dev mode --- .../next/next-server/server/next-server.ts | 3 ++- packages/next/server/next-dev-server.ts | 11 ++++---- .../dynamic-routing/public/hello copy.txt | 2 +- .../dynamic-routing/public/hello%20copy.txt | 2 +- .../dynamic-routing/public/hello+copy.txt | 2 +- .../dynamic-routing/public/hello:copy.txt | 1 + .../dynamic-routing/test/index.test.js | 26 ++++++++++++++++--- 7 files changed, 34 insertions(+), 13 deletions(-) create mode 100644 test/integration/dynamic-routing/public/hello:copy.txt diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index d031bd9525214..1e0a9889106d3 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -660,7 +660,8 @@ export default class Server { this.serveStatic( req, res, - join(this.dir, 'public', path), + // we need to re-encode it since send decodes it + join(this.dir, 'public', encodeURIComponent(path)), parsedUrl ) return { diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 83e49e21f6381..b37f5e3a0cd21 100644 --- a/packages/next/server/next-dev-server.ts +++ b/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) } diff --git a/test/integration/dynamic-routing/public/hello copy.txt b/test/integration/dynamic-routing/public/hello copy.txt index 95d09f2b10159..48941bc083e7e 100644 --- a/test/integration/dynamic-routing/public/hello copy.txt +++ b/test/integration/dynamic-routing/public/hello copy.txt @@ -1 +1 @@ -hello world \ No newline at end of file +hello world copy \ No newline at end of file diff --git a/test/integration/dynamic-routing/public/hello%20copy.txt b/test/integration/dynamic-routing/public/hello%20copy.txt index 95d09f2b10159..c619e3b1f68fb 100644 --- a/test/integration/dynamic-routing/public/hello%20copy.txt +++ b/test/integration/dynamic-routing/public/hello%20copy.txt @@ -1 +1 @@ -hello world \ No newline at end of file +hello world %20 \ No newline at end of file diff --git a/test/integration/dynamic-routing/public/hello+copy.txt b/test/integration/dynamic-routing/public/hello+copy.txt index 95d09f2b10159..2c6733445659c 100644 --- a/test/integration/dynamic-routing/public/hello+copy.txt +++ b/test/integration/dynamic-routing/public/hello+copy.txt @@ -1 +1 @@ -hello world \ No newline at end of file +hello world + \ No newline at end of file diff --git a/test/integration/dynamic-routing/public/hello:copy.txt b/test/integration/dynamic-routing/public/hello:copy.txt new file mode 100644 index 0000000000000..442ea9b9cdcea --- /dev/null +++ b/test/integration/dynamic-routing/public/hello:copy.txt @@ -0,0 +1 @@ +hello world : \ No newline at end of file diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index e97faaea6d1a4..f25d1e6344d86 100644 --- a/test/integration/dynamic-routing/test/index.test.js +++ b/test/integration/dynamic-routing/test/index.test.js @@ -353,21 +353,35 @@ function runTests(dev) { 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') + 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(text).toBe('hello world +') expect(res.status).toBe(200) }) - it('should serve file with %20 from public folder', async () => { + 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') + 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) + }) + + it('should serve file with colon 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) }) @@ -410,6 +424,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: '', From adbdb017653ba5653fae7e3eca3c2ebd5f96847d Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 9 Jan 2020 17:13:38 -0600 Subject: [PATCH 3/6] Fix windows public file handling --- packages/next/next-server/server/next-server.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 1e0a9889106d3..eb44613bd8b32 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -647,7 +647,9 @@ export default class Server { } protected generatePublicRoutes(): Route[] { - const publicFiles = new Set(recursiveReadDirSync(this.publicDir)) + const publicFiles = new Set( + recursiveReadDirSync(this.publicDir).map(p => p.replace(/\\/g, '/')) + ) return [ { From 1ba0c8672460fa17ccb27ea2ab106c438b52a79e Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 9 Jan 2020 18:07:37 -0600 Subject: [PATCH 4/6] Remove colon test to make git on windows happy --- test/integration/dynamic-routing/public/hello:copy.txt | 1 - test/integration/dynamic-routing/test/index.test.js | 7 ------- 2 files changed, 8 deletions(-) delete mode 100644 test/integration/dynamic-routing/public/hello:copy.txt diff --git a/test/integration/dynamic-routing/public/hello:copy.txt b/test/integration/dynamic-routing/public/hello:copy.txt deleted file mode 100644 index 442ea9b9cdcea..0000000000000 --- a/test/integration/dynamic-routing/public/hello:copy.txt +++ /dev/null @@ -1 +0,0 @@ -hello world : \ No newline at end of file diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index f25d1e6344d86..643c6407b82a3 100644 --- a/test/integration/dynamic-routing/test/index.test.js +++ b/test/integration/dynamic-routing/test/index.test.js @@ -378,13 +378,6 @@ function runTests(dev) { expect(res.status).toBe(200) }) - it('should serve file with colon 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) - }) - if (dev) { it('should work with HMR correctly', async () => { const browser = await webdriver(appPort, '/post-1/comments') From 4846665f8f4734151f22f28c7a09ca83d3ed0463 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 9 Jan 2020 19:03:12 -0600 Subject: [PATCH 5/6] Remove un-used files --- test/integration/serverless/public/hello copy.txt | 1 - test/integration/serverless/public/hello%20copy.txt | 1 - test/integration/serverless/public/hello+copy.txt | 1 - 3 files changed, 3 deletions(-) delete mode 100644 test/integration/serverless/public/hello copy.txt delete mode 100644 test/integration/serverless/public/hello%20copy.txt delete mode 100644 test/integration/serverless/public/hello+copy.txt diff --git a/test/integration/serverless/public/hello copy.txt b/test/integration/serverless/public/hello copy.txt deleted file mode 100644 index 3b18e512dba79..0000000000000 --- a/test/integration/serverless/public/hello copy.txt +++ /dev/null @@ -1 +0,0 @@ -hello world diff --git a/test/integration/serverless/public/hello%20copy.txt b/test/integration/serverless/public/hello%20copy.txt deleted file mode 100644 index 3b18e512dba79..0000000000000 --- a/test/integration/serverless/public/hello%20copy.txt +++ /dev/null @@ -1 +0,0 @@ -hello world diff --git a/test/integration/serverless/public/hello+copy.txt b/test/integration/serverless/public/hello+copy.txt deleted file mode 100644 index 3b18e512dba79..0000000000000 --- a/test/integration/serverless/public/hello+copy.txt +++ /dev/null @@ -1 +0,0 @@ -hello world From 1c8920dbc92cf9ef0ffc4c6a562b9a2ad9fd54d5 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 9 Jan 2020 19:44:21 -0600 Subject: [PATCH 6/6] Add missing await --- packages/next/next-server/server/next-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index eb44613bd8b32..63c7e363d7fee 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -659,7 +659,7 @@ export default class Server { const path = `/${(params.path || []).join('/')}` if (publicFiles.has(path)) { - this.serveStatic( + await this.serveStatic( req, res, // we need to re-encode it since send decodes it