From 268e692bfbe28337c0b3de5dfed4352ee57cda78 Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Mon, 1 Apr 2024 10:19:15 -0700 Subject: [PATCH] Min html 404 (#49954) --- src/frame/lib/constants.js | 17 +++++++++++++++++ src/frame/middleware/render-page.js | 3 ++- src/shielding/tests/shielding.js | 16 +++++++++++++--- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/frame/lib/constants.js b/src/frame/lib/constants.js index 2b5787182f78..250450778ca6 100644 --- a/src/frame/lib/constants.js +++ b/src/frame/lib/constants.js @@ -17,3 +17,20 @@ export const MAX_REQUEST_TIMEOUT = process.env.REQUEST_TIMEOUT ? parseInt(process.env.REQUEST_TIMEOUT, 10) : DEFAULT_MAX_REQUEST_TIMEOUT export const TRANSLATIONS_FIXTURE_ROOT = process.env.TRANSLATIONS_FIXTURE_ROOT + +// Minimum required HTML for 404: W3C valid, no external, legal. +export const minimumNotFoundHtml = ` + + + +404 - GitHub Docs + +

GitHub Docs

+

Page not found.

+

Return to home.

+ +© ${new Date().getFullYear()} GitHub, Inc. + • Terms + • Privacy + +`.replace(/\n/g, '') diff --git a/src/frame/middleware/render-page.js b/src/frame/middleware/render-page.js index 7af230aebf2e..bab7c37e8cc8 100644 --- a/src/frame/middleware/render-page.js +++ b/src/frame/middleware/render-page.js @@ -11,6 +11,7 @@ import { allVersions } from '#src/versions/lib/all-versions.js' import { isConnectionDropped } from './halt-on-dropped-connection.js' import { nextHandleRequest } from './next.js' import { defaultCacheControl } from './cache-control.js' +import { minimumNotFoundHtml } from '../lib/constants.js' const STATSD_KEY_RENDER = 'middleware.render_page' const STATSD_KEY_404 = 'middleware.render_404' @@ -59,7 +60,7 @@ export default async function renderPage(req, res) { if (!pathLanguagePrefixed(req.path)) { defaultCacheControl(res) - return res.status(404).type('text').send('Not found') + return res.status(404).type('html').send(minimumNotFoundHtml) } // The rest is "unhandled" requests where we don't have the page diff --git a/src/shielding/tests/shielding.js b/src/shielding/tests/shielding.js index f045566811eb..719804120678 100644 --- a/src/shielding/tests/shielding.js +++ b/src/shielding/tests/shielding.js @@ -126,18 +126,28 @@ describe('rate limiting', () => { }) describe('404 pages and their content-type', () => { + const exampleNonLanguage404plain = ['/_next/image/foo'] + test.each(exampleNonLanguage404plain)( + 'non-language 404 response is plain text and cacheable: %s', + async (pathname) => { + const res = await get(pathname) + expect(res.statusCode).toBe(404) + expect(res.headers['content-type']).toMatch('text/plain') + expect(res.headers['cache-control']).toMatch('public') + }, + ) + const exampleNonLanguage404s = [ - '/_next/image/foo', '/wp-content/themes/seotheme/db.php?u', '/enterprise/3.1/_next/static/chunks/616-910d0397bafa52e0.js', '/~root', ] test.each(exampleNonLanguage404s)( - 'non-language 404 response is plain text and cacheable: %s', + 'non-language 404 response is html text and cacheable: %s', async (pathname) => { const res = await get(pathname) expect(res.statusCode).toBe(404) - expect(res.headers['content-type']).toMatch('text/plain') + expect(res.headers['content-type']).toMatch('text/html; charset=utf-8') expect(res.headers['cache-control']).toMatch('public') }, )