Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Jul 2, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
poweredByHeader,
},
{
private: isPreviewMode,
private: isPreviewMode || page === '/404',
stateful: !!getServerSideProps,
revalidate: renderOpts.revalidate,
}
Expand Down Expand Up @@ -385,7 +385,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
poweredByHeader,
},
{
private: isPreviewMode,
private: isPreviewMode || renderOpts.is404Page,
stateful: !!getServerSideProps,
revalidate: renderOpts.revalidate,
}
Expand Down
5 changes: 3 additions & 2 deletions packages/next/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1613,7 +1613,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
Expand Down Expand Up @@ -1841,7 +1842,7 @@ export default class Server {
const revalidateOptions =
!this.renderOpts.dev || (hasServerProps && !isDataReq)
? {
private: isPreviewMode,
private: isPreviewMode || is404Page,
stateful: !isSSG,
revalidate: sprRevalidate,
}
Expand Down
69 changes: 69 additions & 0 deletions test/integration/404-page/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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: {} } }
huozhi marked this conversation as resolved.
Show resolved Hide resolved
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()
Expand Down
12 changes: 5 additions & 7 deletions test/integration/not-found-revalidate/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down