From c45e63f9d4dec6ec6a07d944f4cbf09ddf281d47 Mon Sep 17 00:00:00 2001 From: Peter Bengtsson Date: Mon, 10 Jun 2024 12:11:44 -0400 Subject: [PATCH 1/3] Port `handle-redirects.js` to TypeScript (#51075) --- src/frame/middleware/index.ts | 2 +- ...andle-redirects.js => handle-redirects.ts} | 38 +++++++++++-------- src/types.ts | 7 ++++ 3 files changed, 30 insertions(+), 17 deletions(-) rename src/redirects/middleware/{handle-redirects.js => handle-redirects.ts} (83%) diff --git a/src/frame/middleware/index.ts b/src/frame/middleware/index.ts index 955f2aeeb017..4fae48222959 100644 --- a/src/frame/middleware/index.ts +++ b/src/frame/middleware/index.ts @@ -22,7 +22,7 @@ import reloadTree from './reload-tree' import context from './context/context' import shortVersions from '@/versions/middleware/short-versions.js' import languageCodeRedirects from '@/redirects/middleware/language-code-redirects.js' -import handleRedirects from '@/redirects/middleware/handle-redirects.js' +import handleRedirects from '@/redirects/middleware/handle-redirects' import findPage from './find-page.js' import blockRobots from './block-robots.js' import archivedEnterpriseVersionsAssets from '@/archives/middleware/archived-enterprise-versions-assets.js' diff --git a/src/redirects/middleware/handle-redirects.js b/src/redirects/middleware/handle-redirects.ts similarity index 83% rename from src/redirects/middleware/handle-redirects.js rename to src/redirects/middleware/handle-redirects.ts index edae5ba3b349..a009eb2bda6b 100644 --- a/src/redirects/middleware/handle-redirects.js +++ b/src/redirects/middleware/handle-redirects.ts @@ -1,11 +1,15 @@ -import patterns from '#src/frame/lib/patterns.js' -import { URL } from 'url' -import { pathLanguagePrefixed } from '#src/languages/lib/languages.js' -import { deprecatedWithFunctionalRedirects } from '#src/versions/lib/enterprise-server-releases.js' +import type { NextFunction, Response } from 'express' + +import patterns from '@/frame/lib/patterns.js' +import { pathLanguagePrefixed } from '@/languages/lib/languages.js' +import { deprecatedWithFunctionalRedirects } from '@/versions/lib/enterprise-server-releases.js' import getRedirect from '../lib/get-redirect.js' -import { defaultCacheControl, languageCacheControl } from '#src/frame/middleware/cache-control.js' +import { defaultCacheControl, languageCacheControl } from '@/frame/middleware/cache-control.js' +import { ExtendedRequest, URLSearchParamsTypes } from '@/types' + +export default function handleRedirects(req: ExtendedRequest, res: Response, next: NextFunction) { + if (!req.context) throw new Error('Request not contextualized') -export default function handleRedirects(req, res, next) { // never redirect assets if (patterns.assetPaths.test(req.path)) return next() @@ -27,7 +31,7 @@ export default function handleRedirects(req, res, next) { // begin redirect handling let redirect = req.path - let queryParams = req._parsedUrl.query + let queryParams = req.originalUrl.includes('?') ? req.originalUrl.split('?')[1] : null // Redirect `/some/uri?q=stuff` to `/en/search?query=stuff` // Redirect `/some/uri?query=stuff` to `/en/search?query=stuff` @@ -43,9 +47,9 @@ export default function handleRedirects(req, res, next) { const hasQuery = 'query' in req.query if ((hasQ && !hasQuery) || (hasQuery && !onSearch && !onGraphqlExplorer)) { const language = getLanguage(req) - const sp = new URLSearchParams(req.query) + const sp = new URLSearchParams(req.query as URLSearchParamsTypes) if (sp.has('q') && !sp.has('query')) { - sp.set('query', sp.get('q')) + sp.set('query', sp.get('q')!) sp.delete('q') } @@ -87,6 +91,7 @@ export default function handleRedirects(req, res, next) { // But for example, a `/authentication/connecting-to-github-with-ssh` // needs to become `/en/authentication/connecting-to-github-with-ssh` const possibleRedirectTo = `/en${req.path}` + if (!req.context.pages) throw new Error('req.context.pages not yet set') if (possibleRedirectTo in req.context.pages || isDeprecatedVersion(req.path)) { const language = getLanguage(req) @@ -98,11 +103,12 @@ export default function handleRedirects(req, res, next) { } // do not redirect a path to itself - // req._parsedUrl.path includes query params whereas req.path does not - if (redirect === req._parsedUrl.path) { + if (redirect === req.originalUrl) { return next() } + if (!req.context.pages) throw new Error('req.context.pages not yet set') + // do not redirect if the redirected page can't be found if ( !(req.context.pages[removeQueryParams(redirect)] || isDeprecatedVersion(req.path)) && @@ -127,13 +133,13 @@ export default function handleRedirects(req, res, next) { return res.redirect(permanent ? 301 : 302, redirect) } -function getLanguage(req, default_ = 'en') { +function getLanguage(req: ExtendedRequest, default_ = 'en') { // req.context.userLanguage, if it truthy, is always a valid supported // language. It's whatever was in the user's request in lib/languages.js - return req.context.userLanguage || default_ + return req.context!.userLanguage || default_ } -function usePermanentRedirect(req) { +function usePermanentRedirect(req: ExtendedRequest) { // If the redirect was to essentially swap `enterprise-server@latest` // for `enterprise-server@3.x` then, we definitely don't want to // do a permanent redirect. @@ -152,11 +158,11 @@ function usePermanentRedirect(req) { return false } -function removeQueryParams(redirect) { +function removeQueryParams(redirect: string) { return new URL(redirect, 'https://docs.github.com').pathname } -function isDeprecatedVersion(path) { +function isDeprecatedVersion(path: string) { // When we rewrote how redirects work, from a lookup model to a // functional model, the enterprise-server releases that got // deprecated since then fall between the cracks. Especially diff --git a/src/types.ts b/src/types.ts index 4ff46d4d592c..3089692dec0e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -72,6 +72,7 @@ export type Context = { enterpriseServerVersions?: string[] enterpriseServerReleases?: typeof enterpriseServerReleases languages?: Languages + redirectNotFound?: string } type Language = { @@ -172,3 +173,9 @@ export type Version = { export type AllVersions = { [name: string]: Version } + +// Use this when constructing a URLSearchParams object from a `req.query`. +// E.g. `const sp = new URLSearchParams(req.query as URLSearchParamsTypes)` +// It's useful because otherwise you might get a TypeScript error that +// is not possible to happen at runtime. +export type URLSearchParamsTypes = string | string[][] | Record | URLSearchParams From 841a765e2405770ab3541f9968aa05d094b155c7 Mon Sep 17 00:00:00 2001 From: Peter Bengtsson Date: Mon, 10 Jun 2024 12:24:43 -0400 Subject: [PATCH 2/3] Port `manifest-json.js` to TypeScript (#51085) --- src/frame/middleware/index.ts | 2 +- .../{manifest-json.js => manifest-json.ts} | 15 ++++++++++++--- src/frame/tests/{manifest.js => manifest.ts} | 18 +++++++++++++++--- src/tests/helpers/e2etest.js | 8 ++++---- 4 files changed, 32 insertions(+), 11 deletions(-) rename src/frame/middleware/{manifest-json.js => manifest-json.ts} (85%) rename src/frame/tests/{manifest.js => manifest.ts} (86%) diff --git a/src/frame/middleware/index.ts b/src/frame/middleware/index.ts index 4fae48222959..46d5c3263c0a 100644 --- a/src/frame/middleware/index.ts +++ b/src/frame/middleware/index.ts @@ -28,7 +28,7 @@ import blockRobots from './block-robots.js' import archivedEnterpriseVersionsAssets from '@/archives/middleware/archived-enterprise-versions-assets.js' import api from './api' import healthz from './healthz' -import manifestJson from './manifest-json.js' +import manifestJson from './manifest-json' import remoteIP from './remote-ip.js' import buildInfo from './build-info.js' import archivedEnterpriseVersions from '@/archives/middleware/archived-enterprise-versions.js' diff --git a/src/frame/middleware/manifest-json.js b/src/frame/middleware/manifest-json.ts similarity index 85% rename from src/frame/middleware/manifest-json.js rename to src/frame/middleware/manifest-json.ts index 933eda49ab20..83b3395ad1bb 100644 --- a/src/frame/middleware/manifest-json.js +++ b/src/frame/middleware/manifest-json.ts @@ -1,3 +1,4 @@ +import type { NextFunction, Request, Response } from 'express' import fs from 'fs' import path from 'path' @@ -17,7 +18,13 @@ const ICONS = [ './assets/images/site/apple-touch-icon-512x512.png', ] -export default async function manifestJson(req, res, next) { +type Icon = { + sizes: string + src: string + type?: string +} + +export default async function manifestJson(req: Request, res: Response, next: NextFunction) { if (!req.url.startsWith('/manifest.json')) { return next() } @@ -28,6 +35,8 @@ export default async function manifestJson(req, res, next) { return res.redirect(302, '/manifest.json') } + const icons: Icon[] = [] + // This is modelled after https://github.com/manifest.json const manifest = { // In the future we might want to have a different manifest for each @@ -40,10 +49,10 @@ export default async function manifestJson(req, res, next) { short_name: 'GitHub Docs', start_url: '/', display: 'standalone', - icons: [], + icons, } for (const icon of ICONS) { - for (const sizes of path.basename(icon).match(/\d+x\d+/g)) { + for (const sizes of path.basename(icon).match(/\d+x\d+/g) || []) { const stats = fs.statSync(icon) const split = icon.slice(1).split(path.sep) const hash = `${stats.size}` diff --git a/src/frame/tests/manifest.js b/src/frame/tests/manifest.ts similarity index 86% rename from src/frame/tests/manifest.js rename to src/frame/tests/manifest.ts index 294d4d04fdd8..dcdc74eb5c2c 100644 --- a/src/frame/tests/manifest.js +++ b/src/frame/tests/manifest.ts @@ -1,8 +1,20 @@ import { describe, expect, test } from 'vitest' import sharp from 'sharp' -import { SURROGATE_ENUMS } from '#src/frame/middleware/set-fastly-surrogate-key.js' -import { get, getDOM } from '#src/tests/helpers/e2etest.js' +import { SURROGATE_ENUMS } from '@/frame/middleware/set-fastly-surrogate-key.js' +import { get, getDOM } from '@/tests/helpers/e2etest.js' + +type Manifest = { + name: string + short_name: string + start_url: string + display: string + icons: { + sizes: string + src: string + type?: string + }[] +} describe('manifest', () => { test('download manifest from HTML and check content', async () => { @@ -19,7 +31,7 @@ describe('manifest', () => { expect(res.headers['surrogate-control']).toMatch(/max-age=[1-9]/) expect(res.headers['surrogate-key']).toBe(`${SURROGATE_ENUMS.DEFAULT} no-language`) - const manifest = JSON.parse(res.body) + const manifest: Manifest = JSON.parse(res.body) expect(manifest.name).toBe('GitHub Docs') expect(manifest.short_name).toBe('GitHub Docs') expect(manifest.start_url).toBe('/') diff --git a/src/tests/helpers/e2etest.js b/src/tests/helpers/e2etest.js index ac1443d6b508..6a7b646fb3ac 100644 --- a/src/tests/helpers/e2etest.js +++ b/src/tests/helpers/e2etest.js @@ -6,11 +6,11 @@ export async function get( route, { method = 'get', - body, + body = undefined, followRedirects = false, followAllRedirects = false, - headers, - responseType, + headers = undefined, + responseType = '', retries = 0, } = {}, ) { @@ -23,7 +23,7 @@ export async function get( retry: { limit: retries }, throwHttpErrors: false, followRedirect: followAllRedirects || followRedirects, - responseType, + responseType: responseType || undefined, }, isUndefined, ) From 081e9f157e89ad6b121fc517d12608208fc30ebe Mon Sep 17 00:00:00 2001 From: docs-bot <77750099+docs-bot@users.noreply.github.com> Date: Mon, 10 Jun 2024 09:34:15 -0700 Subject: [PATCH 3/3] Update audit log event data (#51089) --- src/audit-logs/lib/config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/audit-logs/lib/config.json b/src/audit-logs/lib/config.json index 59d95b183eec..f53e0d043aff 100644 --- a/src/audit-logs/lib/config.json +++ b/src/audit-logs/lib/config.json @@ -3,5 +3,5 @@ "apiOnlyEvents": "This event is not available in the web interface, only via the REST API, audit log streaming, or JSON/CSV exports.", "apiRequestEvent": "This event is only available via audit log streaming." }, - "sha": "388fbe0209d465ccdc56d5b34fefa9e47d2a4d94" + "sha": "02af3186061478ec55d60b2b9e535b446e55cc19" } \ No newline at end of file