Min html 404 (#49954)

github · Apr 1, 2024 · 268e692 · 268e692
1 parent a123b01
commit 268e692
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 4 deletions.
diff --git 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 = `
+<!doctype html>
+<html lang=en>
+<meta charset=utf-8>
+<title>404 - GitHub Docs</title>
+<style>body{font-family:system-ui,sans-serif;margin:3rem}a{color:#0969DA}</style>
+<p style=font-weight:500>GitHub Docs</p>
+<p>Page not found.</p>
+<p><a href=/>Return to home.</a></p>
+<small>
+&copy; ${new Date().getFullYear()} GitHub, Inc.
+ &bull; <a href=https://docs.github.com/site-policy/github-terms/github-terms-of-service>Terms</a>
+ &bull; <a href=https://docs.github.com/site-policy/privacy-policies/github-privacy-statement>Privacy</a>
+</small>
+`.replace(/\n/g, '')
diff --git 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
@@ -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')
     },
   )