From 8e055bcc94e6af38f640906054f4bdfce79acf0c Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 18 Jan 2022 16:15:09 -0600 Subject: [PATCH] Fix static file check with i18n --- packages/next/server/base-server.ts | 9 ++++++--- packages/next/server/dev/next-dev-server.ts | 2 +- packages/next/server/next-server.ts | 6 ++++-- packages/next/server/router.ts | 13 ++++++++++++ test/integration/i18n-support/test/shared.js | 21 ++++++++++++++++++++ 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index 95beef8bdf6ff81..ad72d1de1b40183 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -958,7 +958,8 @@ export default abstract class Server { res, pathname, { ..._parsedUrl.query, _nextDataReq: '1' }, - parsedUrl + parsedUrl, + true ) return { finished: true, @@ -1344,7 +1345,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, @@ -1565,7 +1566,8 @@ export default abstract class Server { res: BaseNextResponse, pathname: string, query: NextParsedUrlQuery = {}, - parsedUrl?: NextUrlWithParsedQuery + parsedUrl?: NextUrlWithParsedQuery, + internalRender = false ): Promise { if (!pathname.startsWith('/')) { console.warn( @@ -1588,6 +1590,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 index 57d39949e1ca500..8577f70101b88e3 100644 --- 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 index d9f4669e085fb70..b365e003b9fb153 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -474,14 +474,16 @@ export default class NextNodeServer extends BaseServer { res: BaseNextResponse | ServerResponse, pathname: string, query?: NextParsedUrlQuery, - parsedUrl?: NextUrlWithParsedQuery + parsedUrl?: NextUrlWithParsedQuery, + internal = false ): Promise { 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 index 01d2ff94f461b26..e3d519d91b809cd 100644 --- 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 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 { + 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 } = {} const memoizedPageChecker = async (p: string): Promise => { @@ -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 index f24c92461b060d0..2235946b180fca8 100644 --- 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,