From a0f9ab42fd828a647c0a1d281c17cbc77ef826c3 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Tue, 11 May 2021 13:53:34 +0200 Subject: [PATCH 1/9] Make sure 404 pages do not get cached by a CDN when using next start Fixes #22579 This still needs more work / tests. --- .../webpack/loaders/next-serverless-loader/page-handler.ts | 4 ++-- packages/next/next-server/server/next-server.ts | 5 +++-- 2 files changed, 5 insertions(+), 4 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 35682cfc37225..118b7d56ac4e2 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 @@ -347,7 +347,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) { poweredByHeader, }, { - private: isPreviewMode, + private: isPreviewMode || renderOpts.isNotFound, stateful: !!getServerSideProps, revalidate: renderOpts.revalidate, } @@ -388,7 +388,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) { poweredByHeader, }, { - private: isPreviewMode, + private: isPreviewMode || renderOpts.isNotFound, stateful: !!getServerSideProps, revalidate: renderOpts.revalidate, } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 0b367f43a1a4e..3b3217b2187ee 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1592,7 +1592,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 || Boolean(cachedData.isNotFound), stateful: false, // GSP response revalidate: cachedData.curRevalidate !== undefined @@ -1820,7 +1821,7 @@ export default class Server { const revalidateOptions = !this.renderOpts.dev || (hasServerProps && !isDataReq) ? { - private: isPreviewMode, + private: isPreviewMode || isNotFound, stateful: !isSSG, revalidate: sprRevalidate, } From e9b115d26a91b4bcb248801f8cfda5864d53260e Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 23 Jun 2021 15:27:03 +0800 Subject: [PATCH 2/9] add integration test --- test/integration/404-page/test/index.test.js | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index 7f4aa59b04cba..2df06355095d3 100644 --- a/test/integration/404-page/test/index.test.js +++ b/test/integration/404-page/test/index.test.js @@ -61,6 +61,38 @@ const runTests = (mode = 'server') => { expect('/404' in manifest).toBe(true) }) } + + if (mode === 'server') { + it('should always revalidate custom 404 page without ssg', async () => { + const res404 = await fetchViaHTTP(appPort, '/404') + const resNext = await fetchViaHTTP(appPort, '/_next/abc') + + expect(res404.headers.get('Cache-Control')).toBe(null) + expect(resNext.headers.get('Cache-Control')).toBe( + 'no-cache, no-store, max-age=0, must-revalidate' + ) + }) + + it('should use swr cache on custom 404 page with ssg', 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 + ` + ) + const swrCacheControl = 's-maxage=31536000, stale-while-revalidate' + const res404 = await fetchViaHTTP(appPort, '/404') + const resNext = await fetchViaHTTP(appPort, '/_next/abc') + await fs.remove(pages404) + await fs.move(`${pages404}.bak`, pages404) + + expect(res404.headers.get('Cache-Control')).toBe(swrCacheControl) + expect(resNext.headers.get('Cache-Control')).toBe(swrCacheControl) + }) + } } describe('404 Page Support', () => { From 67cb3a6f0450b29982d53120ac850c47c9654610 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 23 Jun 2021 17:15:38 +0800 Subject: [PATCH 3/9] fix test --- test/integration/404-page/test/index.test.js | 70 +++++++++++--------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index 2df06355095d3..9019aa194cf72 100644 --- a/test/integration/404-page/test/index.test.js +++ b/test/integration/404-page/test/index.test.js @@ -61,38 +61,6 @@ const runTests = (mode = 'server') => { expect('/404' in manifest).toBe(true) }) } - - if (mode === 'server') { - it('should always revalidate custom 404 page without ssg', async () => { - const res404 = await fetchViaHTTP(appPort, '/404') - const resNext = await fetchViaHTTP(appPort, '/_next/abc') - - expect(res404.headers.get('Cache-Control')).toBe(null) - expect(resNext.headers.get('Cache-Control')).toBe( - 'no-cache, no-store, max-age=0, must-revalidate' - ) - }) - - it('should use swr cache on custom 404 page with ssg', 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 - ` - ) - const swrCacheControl = 's-maxage=31536000, stale-while-revalidate' - const res404 = await fetchViaHTTP(appPort, '/404') - const resNext = await fetchViaHTTP(appPort, '/_next/abc') - await fs.remove(pages404) - await fs.move(`${pages404}.bak`, pages404) - - expect(res404.headers.get('Cache-Control')).toBe(swrCacheControl) - expect(resNext.headers.get('Cache-Control')).toBe(swrCacheControl) - }) - } } describe('404 Page Support', () => { @@ -140,6 +108,44 @@ describe('404 Page Support', () => { runTests('serverless') }) + it('should revalidate for custom 404 page while stale with ssg', 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 + ` + ) + appPort = await findPort() + await nextBuild(appDir) + app = await nextStart(appDir, appPort) + const swrCacheControl = `s-maxage=31536000, stale-while-revalidate` + const res404 = await fetchViaHTTP(appPort, '/404') + const resNext = await fetchViaHTTP(appPort, '/_next/abc') + await fs.remove(pages404) + await fs.move(`${pages404}.bak`, pages404) + await killApp(app) + + expect(res404.headers.get('Cache-Control')).toBe(swrCacheControl) + expect(resNext.headers.get('Cache-Control')).toBe(swrCacheControl) + }) + + it('should revalidate for custom 404 page without ssg', async () => { + appPort = await findPort() + await nextBuild(appDir) + app = await nextStart(appDir, appPort) + const res404 = await fetchViaHTTP(appPort, '/404') + const resNext = await fetchViaHTTP(appPort, '/_next/abc') + + expect(res404.headers.get('Cache-Control')).toBe(null) + expect(resNext.headers.get('Cache-Control')).toBe( + 'no-cache, no-store, max-age=0, must-revalidate' + ) + await killApp(app) + }) + it('falls back to _error correctly without pages/404', async () => { await fs.move(pages404, `${pages404}.bak`) appPort = await findPort() From 63a5498af61fa3d1e7f076ee67a656e1700bd4ea Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 24 Jun 2021 14:50:19 +0800 Subject: [PATCH 4/9] use is404Page --- .../loaders/next-serverless-loader/page-handler.ts | 4 ++-- test/integration/404-page/test/index.test.js | 8 ++++---- 2 files changed, 6 insertions(+), 6 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 118b7d56ac4e2..dfdc7fd494a3a 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 @@ -347,7 +347,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) { poweredByHeader, }, { - private: isPreviewMode || renderOpts.isNotFound, + private: isPreviewMode || renderOpts.is404Page, stateful: !!getServerSideProps, revalidate: renderOpts.revalidate, } @@ -388,7 +388,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) { poweredByHeader, }, { - private: isPreviewMode || renderOpts.isNotFound, + private: isPreviewMode || renderOpts.is404Page, stateful: !!getServerSideProps, revalidate: renderOpts.revalidate, } diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index 9019aa194cf72..bb0fc94773a5e 100644 --- a/test/integration/404-page/test/index.test.js +++ b/test/integration/404-page/test/index.test.js @@ -108,7 +108,7 @@ describe('404 Page Support', () => { runTests('serverless') }) - it('should revalidate for custom 404 page while stale with ssg', async () => { + it('should revalidate for custom 404 page with ssg', async () => { await fs.move(pages404, `${pages404}.bak`) await fs.writeFile( pages404, @@ -118,8 +118,8 @@ describe('404 Page Support', () => { export default page ` ) - appPort = await findPort() await nextBuild(appDir) + appPort = await findPort() app = await nextStart(appDir, appPort) const swrCacheControl = `s-maxage=31536000, stale-while-revalidate` const res404 = await fetchViaHTTP(appPort, '/404') @@ -133,17 +133,17 @@ describe('404 Page Support', () => { }) it('should revalidate for custom 404 page without ssg', async () => { - appPort = await findPort() await nextBuild(appDir) + appPort = await findPort() app = await nextStart(appDir, appPort) const res404 = await fetchViaHTTP(appPort, '/404') const resNext = await fetchViaHTTP(appPort, '/_next/abc') + await killApp(app) expect(res404.headers.get('Cache-Control')).toBe(null) expect(resNext.headers.get('Cache-Control')).toBe( 'no-cache, no-store, max-age=0, must-revalidate' ) - await killApp(app) }) it('falls back to _error correctly without pages/404', async () => { From f0b3b7c120704209a2790a36d8a0e0608f54e813 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 24 Jun 2021 14:36:55 -0500 Subject: [PATCH 5/9] update check --- .../webpack/loaders/next-serverless-loader/page-handler.ts | 2 +- packages/next/next-server/server/next-server.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 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 dfdc7fd494a3a..9628cec9c1e4f 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 @@ -347,7 +347,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) { poweredByHeader, }, { - private: isPreviewMode || renderOpts.is404Page, + private: isPreviewMode || page === '/404', stateful: !!getServerSideProps, revalidate: renderOpts.revalidate, } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 9559b49d2716f..aac844377470d 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1611,7 +1611,7 @@ export default class Server { const revalidateOptions = !this.renderOpts.dev ? { // When the page is 404 cache-control should not be added - private: isPreviewMode || Boolean(cachedData.isNotFound), + private: isPreviewMode || is404Page, stateful: false, // GSP response revalidate: cachedData.curRevalidate !== undefined @@ -1839,7 +1839,7 @@ export default class Server { const revalidateOptions = !this.renderOpts.dev || (hasServerProps && !isDataReq) ? { - private: isPreviewMode || isNotFound, + private: isPreviewMode || is404Page, stateful: !isSSG, revalidate: sprRevalidate, } From 5dd12aff46f71bbcca66279385f3a82cda93277a Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 25 Jun 2021 16:01:59 +0800 Subject: [PATCH 6/9] fix integration test --- test/integration/404-page/test/index.test.js | 13 ++++++++----- .../not-found-revalidate/test/index.test.js | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index bb0fc94773a5e..f1c4ba055265b 100644 --- a/test/integration/404-page/test/index.test.js +++ b/test/integration/404-page/test/index.test.js @@ -108,7 +108,7 @@ describe('404 Page Support', () => { runTests('serverless') }) - it('should revalidate for custom 404 page with ssg', async () => { + it('should not cache for custom 404 page with ssg', async () => { await fs.move(pages404, `${pages404}.bak`) await fs.writeFile( pages404, @@ -121,18 +121,21 @@ describe('404 Page Support', () => { await nextBuild(appDir) appPort = await findPort() app = await nextStart(appDir, appPort) - const swrCacheControl = `s-maxage=31536000, stale-while-revalidate` const res404 = await fetchViaHTTP(appPort, '/404') const resNext = await fetchViaHTTP(appPort, '/_next/abc') await fs.remove(pages404) await fs.move(`${pages404}.bak`, pages404) await killApp(app) - expect(res404.headers.get('Cache-Control')).toBe(swrCacheControl) - expect(resNext.headers.get('Cache-Control')).toBe(swrCacheControl) + expect(res404.headers.get('Cache-Control')).toBe( + 'private, no-cache, no-store, max-age=0, must-revalidate' + ) + expect(resNext.headers.get('Cache-Control')).toBe( + 's-maxage=31536000, stale-while-revalidate' + ) }) - it('should revalidate for custom 404 page without ssg', async () => { + it('should not cache for custom 404 page without ssg', async () => { await nextBuild(appDir) appPort = await findPort() app = await nextStart(appDir, appPort) diff --git a/test/integration/not-found-revalidate/test/index.test.js b/test/integration/not-found-revalidate/test/index.test.js index ec43e616e4ca4..5e052189760b3 100644 --- a/test/integration/not-found-revalidate/test/index.test.js +++ b/test/integration/not-found-revalidate/test/index.test.js @@ -89,7 +89,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) From 378a54321b3e240f030134e969ae15103376dfd5 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 25 Jun 2021 16:50:25 +0800 Subject: [PATCH 7/9] fix integration test --- test/integration/404-page/test/index.test.js | 9 +++------ .../not-found-revalidate/test/index.test.js | 10 ++++------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index f1c4ba055265b..2a9318a4a945a 100644 --- a/test/integration/404-page/test/index.test.js +++ b/test/integration/404-page/test/index.test.js @@ -126,13 +126,10 @@ describe('404 Page Support', () => { await fs.remove(pages404) await fs.move(`${pages404}.bak`, pages404) await killApp(app) - - expect(res404.headers.get('Cache-Control')).toBe( + const privateCache = 'private, no-cache, no-store, max-age=0, must-revalidate' - ) - expect(resNext.headers.get('Cache-Control')).toBe( - 's-maxage=31536000, stale-while-revalidate' - ) + expect(res404.headers.get('Cache-Control')).toBe(privateCache) + expect(resNext.headers.get('Cache-Control')).toBe(privateCache) }) it('should not cache for custom 404 page without ssg', async () => { diff --git a/test/integration/not-found-revalidate/test/index.test.js b/test/integration/not-found-revalidate/test/index.test.js index 5e052189760b3..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) From 5e7e8ce3e15d8af45d349fecf38c094a71ee96ac Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 2 Jul 2021 02:13:16 +0800 Subject: [PATCH 8/9] add ssg revalidate case --- test/integration/404-page/test/index.test.js | 51 ++++++++++++++------ 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index 2a9318a4a945a..9207f184d5703 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,7 +111,7 @@ describe('404 Page Support', () => { runTests('serverless') }) - it('should not cache for custom 404 page with ssg', async () => { + it('should cache for custom 404 page on ssg without revalidate', async () => { await fs.move(pages404, `${pages404}.bak`) await fs.writeFile( pages404, @@ -121,29 +124,49 @@ describe('404 Page Support', () => { await nextBuild(appDir) appPort = await findPort() app = await nextStart(appDir, appPort) - const res404 = await fetchViaHTTP(appPort, '/404') - const resNext = await fetchViaHTTP(appPort, '/_next/abc') + 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('s-maxage=31536000, stale-while-revalidate') + expect(cacheNext).toBe('s-maxage=31536000, stale-while-revalidate') + }) + + it('should not cache for custom 404 page on ssg with revalidate', async () => { + await fs.move(pages404, `${pages404}.bak`) + await fs.writeFile( + pages404, + ` + const page = () => 'custom 404 page' + export async function getStaticProps() { return { props: {}, revalidate: 10 } } + 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) - const privateCache = - 'private, no-cache, no-store, max-age=0, must-revalidate' - expect(res404.headers.get('Cache-Control')).toBe(privateCache) - expect(resNext.headers.get('Cache-Control')).toBe(privateCache) + + expect(cache404).toBe('s-maxage=10, stale-while-revalidate') + expect(cacheNext).toBe('s-maxage=10, stale-while-revalidate') }) - it('should not cache for custom 404 page without ssg', async () => { + it('should not cache for custom 404 page on production mode', async () => { await nextBuild(appDir) appPort = await findPort() app = await nextStart(appDir, appPort) - const res404 = await fetchViaHTTP(appPort, '/404') - const resNext = await fetchViaHTTP(appPort, '/_next/abc') + const cache404 = await getCacheHeader(appPort, '/404') + const cacheNext = await getCacheHeader(appPort, '/_next/abc') await killApp(app) - expect(res404.headers.get('Cache-Control')).toBe(null) - expect(resNext.headers.get('Cache-Control')).toBe( - 'no-cache, no-store, max-age=0, must-revalidate' - ) + 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 () => { From f7bf27765145e1ac56717e93b12190787cae9c19 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 2 Jul 2021 12:05:49 +0800 Subject: [PATCH 9/9] update test --- test/integration/404-page/test/index.test.js | 24 +++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index 9207f184d5703..13724e1b089d1 100644 --- a/test/integration/404-page/test/index.test.js +++ b/test/integration/404-page/test/index.test.js @@ -111,7 +111,7 @@ describe('404 Page Support', () => { runTests('serverless') }) - it('should cache for custom 404 page on ssg without revalidate', async () => { + it('should not cache for custom 404 page with gssp and revalidate disabled', async () => { await fs.move(pages404, `${pages404}.bak`) await fs.writeFile( pages404, @@ -130,17 +130,21 @@ describe('404 Page Support', () => { await fs.move(`${pages404}.bak`, pages404) await killApp(app) - expect(cache404).toBe('s-maxage=31536000, stale-while-revalidate') - expect(cacheNext).toBe('s-maxage=31536000, stale-while-revalidate') + 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 on ssg with revalidate', async () => { + 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: 10 } } + export async function getStaticProps() { return { props: {}, revalidate: 1 } } export default page ` ) @@ -153,11 +157,15 @@ describe('404 Page Support', () => { await fs.move(`${pages404}.bak`, pages404) await killApp(app) - expect(cache404).toBe('s-maxage=10, stale-while-revalidate') - expect(cacheNext).toBe('s-maxage=10, stale-while-revalidate') + 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 on production mode', async () => { + it('should not cache for custom 404 page without gssp', async () => { await nextBuild(appDir) appPort = await findPort() app = await nextStart(appDir, appPort)