Make sure 404 pages do not get cached by a CDN when using next start (#…

…24983)

Co-authored-by: Jiachi Liu <inbox@huozhi.im>
Co-authored-by: JJ Kasper <jj@jjsweb.site>

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,
               }

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,
           }

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()

test/integration/not-found-revalidate/test/index.test.js

@@ -22,19 +22,17 @@ 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)
 
     await waitFor(1000)
     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)