From 270487d7979d86105c73489be81cc0077493ebd3 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Fri, 2 Jul 2021 10:40:13 +0200 Subject: [PATCH] Make sure 404 pages do not get cached by a CDN when using next start (#24983) Co-authored-by: Jiachi Liu Co-authored-by: JJ Kasper --- .../next-serverless-loader/page-handler.ts | 4 +- packages/next/server/next-server.ts | 5 +- test/integration/404-page/test/index.test.js | 69 +++++++++++++++++++ .../not-found-revalidate/test/index.test.js | 12 ++-- 4 files changed, 79 insertions(+), 11 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts index 00e2f4c5ab244..a09f276f17471 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts @@ -344,7 +344,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) { poweredByHeader, }, { - private: isPreviewMode, + private: isPreviewMode || page === '/404', stateful: !!getServerSideProps, revalidate: renderOpts.revalidate, } @@ -385,7 +385,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) { poweredByHeader, }, { - private: isPreviewMode, + private: isPreviewMode || renderOpts.is404Page, stateful: !!getServerSideProps, revalidate: renderOpts.revalidate, } diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts index cc5e9963003c6..329709e17b9e1 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -1614,7 +1614,8 @@ export default class Server { const revalidateOptions = !this.renderOpts.dev ? { - private: isPreviewMode, + // When the page is 404 cache-control should not be added + private: isPreviewMode || is404Page, stateful: false, // GSP response revalidate: cachedData.curRevalidate !== undefined @@ -1842,7 +1843,7 @@ export default class Server { const revalidateOptions = !this.renderOpts.dev || (hasServerProps && !isDataReq) ? { - private: isPreviewMode, + private: isPreviewMode || is404Page, stateful: !isSSG, revalidate: sprRevalidate, } diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index 7f4aa59b04cba..13724e1b089d1 100644 --- a/test/integration/404-page/test/index.test.js +++ b/test/integration/404-page/test/index.test.js @@ -25,6 +25,9 @@ let nextConfigContent let appPort let app +const getCacheHeader = async (port, pathname) => + (await fetchViaHTTP(port, pathname)).headers.get('Cache-Control') + const runTests = (mode = 'server') => { it('should use pages/404', async () => { const html = await renderViaHTTP(appPort, '/abc') @@ -108,6 +111,72 @@ describe('404 Page Support', () => { runTests('serverless') }) + it('should not cache for custom 404 page with gssp and revalidate disabled', async () => { + await fs.move(pages404, `${pages404}.bak`) + await fs.writeFile( + pages404, + ` + const page = () => 'custom 404 page' + export async function getStaticProps() { return { props: {} } } + export default page + ` + ) + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + const cache404 = await getCacheHeader(appPort, '/404') + const cacheNext = await getCacheHeader(appPort, '/_next/abc') + await fs.remove(pages404) + await fs.move(`${pages404}.bak`, pages404) + await killApp(app) + + expect(cache404).toBe( + 'private, no-cache, no-store, max-age=0, must-revalidate' + ) + expect(cacheNext).toBe( + 'private, no-cache, no-store, max-age=0, must-revalidate' + ) + }) + + it('should not cache for custom 404 page with gssp and revalidate enabled', async () => { + await fs.move(pages404, `${pages404}.bak`) + await fs.writeFile( + pages404, + ` + const page = () => 'custom 404 page' + export async function getStaticProps() { return { props: {}, revalidate: 1 } } + export default page + ` + ) + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + const cache404 = await getCacheHeader(appPort, '/404') + const cacheNext = await getCacheHeader(appPort, '/_next/abc') + await fs.remove(pages404) + await fs.move(`${pages404}.bak`, pages404) + await killApp(app) + + expect(cache404).toBe( + 'private, no-cache, no-store, max-age=0, must-revalidate' + ) + expect(cacheNext).toBe( + 'private, no-cache, no-store, max-age=0, must-revalidate' + ) + }) + + it('should not cache for custom 404 page without gssp', async () => { + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + const cache404 = await getCacheHeader(appPort, '/404') + const cacheNext = await getCacheHeader(appPort, '/_next/abc') + await killApp(app) + + expect(cache404).toBe(null) + expect(cacheNext).toBe('no-cache, no-store, max-age=0, must-revalidate') + }) + it('falls back to _error correctly without pages/404', async () => { await fs.move(pages404, `${pages404}.bak`) appPort = await findPort() diff --git a/test/integration/not-found-revalidate/test/index.test.js b/test/integration/not-found-revalidate/test/index.test.js index ec43e616e4ca4..002c0ffa2b4b9 100644 --- a/test/integration/not-found-revalidate/test/index.test.js +++ b/test/integration/not-found-revalidate/test/index.test.js @@ -22,9 +22,9 @@ const runTests = () => { let res = await fetchViaHTTP(appPort, '/fallback-blocking/hello') let $ = cheerio.load(await res.text()) - expect(res.headers.get('cache-control')).toBe( - 's-maxage=1, stale-while-revalidate' - ) + const privateCache = + 'private, no-cache, no-store, max-age=0, must-revalidate' + expect(res.headers.get('cache-control')).toBe(privateCache) expect(res.status).toBe(404) expect(JSON.parse($('#props').text()).notFound).toBe(true) @@ -32,9 +32,7 @@ const runTests = () => { res = await fetchViaHTTP(appPort, '/fallback-blocking/hello') $ = cheerio.load(await res.text()) - expect(res.headers.get('cache-control')).toBe( - 's-maxage=1, stale-while-revalidate' - ) + expect(res.headers.get('cache-control')).toBe(privateCache) expect(res.status).toBe(404) expect(JSON.parse($('#props').text()).notFound).toBe(true) @@ -89,7 +87,7 @@ const runTests = () => { let $ = cheerio.load(await res.text()) expect(res.headers.get('cache-control')).toBe( - 's-maxage=1, stale-while-revalidate' + 'private, no-cache, no-store, max-age=0, must-revalidate' ) expect(res.status).toBe(404) expect(JSON.parse($('#props').text()).notFound).toBe(true)