From 217656ad1620da36271fcba55325abd65593b9b0 Mon Sep 17 00:00:00 2001 From: jm Date: Fri, 23 Oct 2026 21:53:00 +0200 Subject: [PATCH 1/4] JCU: opt-in loopback endpoint listing the SSR cache contents Backport of dataquest-dev/dspace-angular#1653 (77756f2164) translated to DSpace 9.3: LRUCache named export from lru-cache 11, node:http per the branch lint rules, npm scripts, and explicit ts-node compiler options because tsconfig.json uses moduleResolution bundler. --- .github/workflows/build.yml | 3 + config/config.example.yml | 10 +++ package.json | 1 + scripts/test-ssr-cache-stats.js | 107 +++++++++++++++++++++++ server.ts | 61 +++++++++++++- src/backend/ssr-cache-stats.ts | 122 +++++++++++++++++++++++++++ src/config/cache-config.interface.ts | 8 ++ src/config/default-app-config.ts | 5 ++ src/environments/environment.test.ts | 4 + 9 files changed, 319 insertions(+), 2 deletions(-) create mode 100644 scripts/test-ssr-cache-stats.js create mode 100644 src/backend/ssr-cache-stats.ts diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6797d792377..9188371089c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -90,6 +90,9 @@ jobs: - name: Run lint run: npm run lint:nobuild -- --quiet + - name: Run SSR cache stats tests + run: npm run test:ssr-cache-stats + - name: Check for circular dependencies run: npm run check-circ-deps diff --git a/config/config.example.yml b/config/config.example.yml index 76d0d4a597e..345721724b9 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -106,6 +106,16 @@ cache: serverSide: # Set to true to see all cache hits/misses/refreshes in your console logs. Useful for debugging SSR caching issues. debug: false + # Lets operators on this machine see what the caches below hold: GET /app/cache-stats (JSON). + # Each Node process reports its own cache. Off by default. + stats: + enabled: false + # 0 = answer on the UI port, but only to requests made directly from this host: connecting from 127.0.0.1 and + # carrying no proxy headers (Forwarded, Via, X-Forwarded-*, X-Real-IP). Everyone else gets 404. A same-host reverse + # proxy that adds none of those headers would pass this check, so use a port number there. + # A port number (e.g. 4001) = listen on 127.0.0.1: instead; under pm2 cluster mode worker N uses + N, + # which is the only way to pick a specific worker. + port: 0 # When enabled (i.e. max > 0), known bots will be sent pages from a server side cache specific for bots. # (Keep in mind, bot detection cannot be guaranteed. It is possible some bots will bypass this cache.) botCache: diff --git a/package.json b/package.json index 8c607d89dbf..434657a332f 100644 --- a/package.json +++ b/package.json @@ -4,6 +4,7 @@ "scripts": { "ng": "ng", "test:rest": "ts-node --project ./tsconfig.ts-node.json scripts/test-rest.ts", + "test:ssr-cache-stats": "node --test scripts/test-ssr-cache-stats.js", "start": "npm run start:prod", "start:dev": "cross-env NODE_ENV=development npm run serve --watch", "start:prod": "npm run build:prod && cross-env NODE_ENV=production npm run serve:ssr", diff --git a/scripts/test-ssr-cache-stats.js b/scripts/test-ssr-cache-stats.js new file mode 100644 index 00000000000..2ef73726c5e --- /dev/null +++ b/scripts/test-ssr-cache-stats.js @@ -0,0 +1,107 @@ +const assert = require('node:assert/strict'); +const { once } = require('node:events'); +const { test } = require('node:test'); +const { setTimeout: sleep } = require('node:timers/promises'); +const { LRUCache } = require('lru-cache'); + +process.env.TS_NODE_PROJECT = 'tsconfig.ts-node.json'; +// tsconfig.json uses moduleResolution 'bundler', which ts-node cannot combine with the commonjs output it needs. +process.env.TS_NODE_COMPILER_OPTIONS = JSON.stringify({ module: 'commonjs', moduleResolution: 'node10' }); +require('ts-node/register/transpile-only'); +const { + snapshotCache, startSsrCacheStatsServer, isDirectLoopbackRequest +} = require('../src/backend/ssr-cache-stats.ts'); + +const statsOf = (caches) => ({ + host: 'node-1', pid: 1, instance: 0, generatedAt: 'now', uptimeSeconds: 2, + memory: { rss: 1, heapUsed: 1 }, caches +}); + +test('snapshot reports entries, HTML byte sizes and TTL without changing LRU order', () => { + const cache = new LRUCache({ max: 3, ttl: 60000 }); + cache.set('/small', { page: 'é', headers: { 'content-type': 'text/html' } }); + cache.set('/large', { page: 'larger', headers: {} }); + const before = [...cache.keys()]; + + const stats = snapshotCache('bot', cache, true); + + assert.equal(stats.entryCount, 2); + assert.equal(stats.totalHtmlBytes, 8); + assert.deepEqual(stats.entries.map(e => e.key), ['/large', '/small']); + assert.equal(stats.maxEntries, 3); + assert.equal(stats.configuredTtlMs, 60000); + assert.equal(stats.entries[0].remainingTtlMs > 0, true); + assert.equal(stats.entries[0].stale, false); + assert.deepEqual([...cache.keys()], before); +}); + +test('snapshot counts expired entries even when the cache does not serve stale pages', async () => { + const cache = new LRUCache({ max: 2, ttl: 1, allowStale: false }); + cache.set('/expired', { page: 'x' }); + await sleep(5); + assert.equal(cache.size, 1); + + const stats = snapshotCache('anonymous', cache, true); + + assert.equal(stats.entryCount, 1); + assert.equal(stats.staleCount, 1); + assert.deepEqual(stats.entries[0], { key: '/expired', htmlBytes: 1, remainingTtlMs: 0, stale: true }); + assert.equal(cache.size, 1); +}); + +test('snapshot of a missing cache and of a cache without TTL', () => { + assert.deepEqual(snapshotCache('bot', undefined, false), { + name: 'bot', enabled: false, maxEntries: 0, configuredTtlMs: 0, entryCount: 0, staleCount: 0, totalHtmlBytes: 0, entries: [] + }); + const cache = new LRUCache({ max: 1 }); + cache.set('/forever', { page: 'x' }); + assert.equal(snapshotCache('bot', cache, true).entries[0].remainingTtlMs, null); +}); + +test('stats server listens on loopback and answers only GET /app/cache-stats', async () => { + const payload = statsOf([]); + const server = startSsrCacheStatsServer(0, () => payload); + try { + await once(server, 'listening'); + const address = server.address(); + assert.equal(address.address, '127.0.0.1'); + const base = `http://127.0.0.1:${address.port}`; + + const json = await fetch(`${base}/app/cache-stats`); + assert.equal(json.status, 200); + assert.equal(json.headers.get('cache-control'), 'no-store'); + assert.deepEqual(await json.json(), payload); + + assert.equal((await fetch(`${base}/other`)).status, 404); + assert.equal((await fetch(`${base}/app/cache-stats`, { method: 'POST' })).status, 404); + + } finally { + await new Promise((resolve, reject) => server.close(error => error ? reject(error) : resolve())); + } +}); + +test('direct loopback check accepts local sockets without proxy headers only', () => { + const request = (remoteAddress, headers = {}) => ({ socket: { remoteAddress }, headers }); + for (const address of ['127.0.0.1', '::1', '::ffff:127.0.0.1']) { + assert.equal(isDirectLoopbackRequest(request(address)), true, address); + } + for (const address of ['10.0.0.5', '::ffff:10.0.0.5', '0.0.0.0', '', undefined]) { + assert.equal(isDirectLoopbackRequest(request(address)), false, String(address)); + } + for (const header of ['forwarded', 'via', 'x-forwarded-for', 'x-forwarded-host', 'x-forwarded-proto', 'x-real-ip']) { + assert.equal(isDirectLoopbackRequest(request('127.0.0.1', { [header]: '' })), false, header); + } + assert.equal(isDirectLoopbackRequest(request('127.0.0.1', { 'user-agent': 'curl' })), true); +}); + +test('stats server reports a failed bind on its error event instead of throwing', async () => { + const blocker = startSsrCacheStatsServer(0, () => statsOf([])); + await once(blocker, 'listening'); + try { + const clash = startSsrCacheStatsServer(blocker.address().port, () => statsOf([])); + const [error] = await once(clash, 'error'); + assert.equal(error.code, 'EADDRINUSE'); + } finally { + await new Promise((resolve) => blocker.close(resolve)); + } +}); diff --git a/server.ts b/server.ts index b87780e9cbb..7b65d2605cc 100644 --- a/server.ts +++ b/server.ts @@ -33,6 +33,7 @@ import { createHttpTerminator } from 'http-terminator'; import { readFileSync } from 'fs'; import { join } from 'path'; +import { hostname } from 'os'; import { enableProdMode } from '@angular/core'; @@ -58,6 +59,15 @@ import { RESPONSE, } from './src/express.tokens'; import { SsrExcludePatterns } from "./src/config/ssr-config.interface"; +import { + createSsrCacheStatsHandler, + isDirectLoopbackRequest, + snapshotCache, + SSR_CACHE_STATS_PATH, + SsrCacheEntry, + SsrCacheStats, + startSsrCacheStatsServer +} from './src/backend/ssr-cache-stats'; /* * Set path for the browser application's dist folder @@ -77,10 +87,10 @@ appConfig.themes.forEach(themeConfig => hashedFileMapping.addThemeStyle(themeCon hashedFileMapping.save(); // cache of SSR pages for known bots, only enabled in production mode -let botCache: LRUCache; +let botCache: LRUCache; // cache of SSR pages for anonymous users. Disabled by default, and only available in production mode -let anonymousCache: LRUCache; +let anonymousCache: LRUCache; // extend environment with app config for server extendEnvironmentWithAppConfig(environment, appConfig); @@ -210,6 +220,17 @@ export function app() { */ server.get('/app/health', healthCheck); + /** + * SSR cache contents for operators on this host (see cache.serverSide.stats). With stats.port = 0 they are + * served here, but only to requests made directly from this host: loopback socket address (not req.ip, which + * honours X-Forwarded-For when `trust proxy` is on) and no proxy headers, so traffic relayed by a same-host + * reverse proxy is refused too. Everyone else gets the same 404 as an unknown route. + */ + if (environment.cache.serverSide.stats.enabled && environment.cache.serverSide.stats.port === 0) { + const statsHandler = createSsrCacheStatsHandler(collectCacheStats); + server.get(SSR_CACHE_STATS_PATH, (req, res) => isDirectLoopbackRequest(req) ? statsHandler(req, res) : res.sendStatus(404)); + } + /** * Default sending all incoming requests to ngApp() function, after first checking for a cached * copy of the page (see cacheCheck()) @@ -644,9 +665,45 @@ function run() { }); } +/** pm2 cluster mode numbers its workers 0..n-1 in NODE_APP_INSTANCE; a single process reports 0. */ +const CLUSTER_INSTANCE = Number(process.env.NODE_APP_INSTANCE) || 0; + +function collectCacheStats(): SsrCacheStats { + const memory = process.memoryUsage(); + return { + host: hostname(), + pid: process.pid, + instance: CLUSTER_INSTANCE, + generatedAt: new Date().toISOString(), + uptimeSeconds: Math.round(process.uptime()), + memory: { rss: memory.rss, heapUsed: memory.heapUsed }, + caches: [ + snapshotCache('bot', botCache, environment.production && botCacheEnabled()), + snapshotCache('anonymous', anonymousCache, environment.production && anonymousCacheEnabled()) + ] + }; +} + +/** + * Serve the cache stats on a loopback-only port of their own. Every pm2 cluster worker gets basePort + its + * instance number, because a port shared by the workers would be answered by an arbitrary one. + * A failed bind is logged and never stops the UI. + */ +function startCacheStatsServer(basePort: number) { + const port = basePort + CLUSTER_INSTANCE; + const statsServer = startSsrCacheStatsServer(port, collectCacheStats); + statsServer.on('listening', () => console.log(`SSR cache stats listening at http://127.0.0.1:${port}${SSR_CACHE_STATS_PATH}`)); + statsServer.on('error', (e: Error) => console.error(`SSR cache stats server not started on port ${port}: ${e.message}`)); + process.on('SIGINT', () => statsServer.close()); +} + function start() { logStartupMessage(environment); + if (environment.cache.serverSide.stats.enabled && environment.cache.serverSide.stats.port > 0) { + startCacheStatsServer(environment.cache.serverSide.stats.port); + } + /* * If SSL is enabled * - Read credentials from configuration files diff --git a/src/backend/ssr-cache-stats.ts b/src/backend/ssr-cache-stats.ts new file mode 100644 index 00000000000..9df35b81910 --- /dev/null +++ b/src/backend/ssr-cache-stats.ts @@ -0,0 +1,122 @@ +import { + createServer, + IncomingMessage, + ServerResponse, +} from 'node:http'; + +import { LRUCache } from 'lru-cache'; + +export interface SsrCacheEntry { + page: string; + headers?: Record; +} + +export interface SsrCacheEntryStats { + key: string; + htmlBytes: number; + remainingTtlMs: number | null; + stale: boolean; +} + +export interface SsrCacheSnapshot { + name: string; + enabled: boolean; + maxEntries: number; + configuredTtlMs: number; + entryCount: number; + staleCount: number; + totalHtmlBytes: number; + entries: SsrCacheEntryStats[]; +} + +export interface SsrCacheStats { + host: string; + pid: number; + instance: number; + generatedAt: string; + uptimeSeconds: number; + memory: { + rss: number; + heapUsed: number; + }; + caches: SsrCacheSnapshot[]; +} + +export const SSR_CACHE_STATS_PATH = '/app/cache-stats'; + +/** + * Describe every entry resident in an SSR page cache, largest page first. + * `dump()` visits stale entries too (unlike `forEach`, which skips them when `allowStale` is false); + * `dump()`, `peek()` and `has()` leave recency and TTL untouched. Byte counting is O(total cached HTML). + */ +export function snapshotCache(name: string, cache: LRUCache | undefined, enabled: boolean): SsrCacheSnapshot { + const entries: SsrCacheEntryStats[] = []; + if (cache) { + for (const [key, { value }] of cache.dump()) { + const remainingTtl = cache.getRemainingTTL(key); + entries.push({ + key, + htmlBytes: Buffer.byteLength(value.page, 'utf8'), + remainingTtlMs: Number.isFinite(remainingTtl) ? Math.max(0, Math.round(remainingTtl)) : null, + stale: !cache.has(key), + }); + } + } + entries.sort((a, b) => b.htmlBytes - a.htmlBytes || a.key.localeCompare(b.key)); + return { + name, + enabled, + maxEntries: cache ? cache.max : 0, + configuredTtlMs: cache ? cache.ttl : 0, + entryCount: entries.length, + staleCount: entries.filter(entry => entry.stale).length, + totalHtmlBytes: entries.reduce((sum, entry) => sum + entry.htmlBytes, 0), + entries, + }; +} + +const LOOPBACK_ADDRESSES = ['127.0.0.1', '::1', '::ffff:127.0.0.1']; +/** Headers a reverse proxy adds; a request carrying one came through the proxy even if its socket is loopback. */ +const PROXY_HEADERS = ['forwarded', 'via', 'x-forwarded-for', 'x-forwarded-host', 'x-forwarded-proto', 'x-real-ip']; + +/** + * True when the request was made directly on this host: loopback socket address and no proxy headers. + * A same-host reverse proxy also connects from loopback, so the socket address alone is not enough. + * A proxy that adds none of these headers cannot be told apart; use a separate port there instead. + */ +export function isDirectLoopbackRequest(request: IncomingMessage): boolean { + const address = request.socket.remoteAddress; + return address !== undefined && LOOPBACK_ADDRESSES.includes(address) && + !PROXY_HEADERS.some(header => request.headers[header] !== undefined); +} + +/** Request handler answering GET with the stats as JSON; any other method gets 404. */ +export function createSsrCacheStatsHandler(getStats: () => SsrCacheStats): (request: IncomingMessage, response: ServerResponse) => void { + return (request, response) => { + response.setHeader('Cache-Control', 'no-store'); + if (request.method !== 'GET') { + response.writeHead(404).end(); + return; + } + response.writeHead(200, { 'Content-Type': 'application/json; charset=utf-8' }).end(JSON.stringify(getStats())); + }; +} + +/** + * Serve the cache stats on 127.0.0.1:port (0 = ephemeral) under /app/cache-stats only. + * A failed bind surfaces on the returned server's 'error' event. + */ +export function startSsrCacheStatsServer(port: number, getStats: () => SsrCacheStats): ReturnType { + if (!Number.isInteger(port) || port < 0 || port > 65535) { + throw new Error('SSR cache stats port must be an integer from 0 to 65535'); + } + const handler = createSsrCacheStatsHandler(getStats); + const server = createServer((request, response) => { + if ((request.url || '/').split('?')[0] !== SSR_CACHE_STATS_PATH) { + response.writeHead(404).end(); + return; + } + handler(request, response); + }); + return server.listen(port, '127.0.0.1'); +} diff --git a/src/config/cache-config.interface.ts b/src/config/cache-config.interface.ts index c5609fef8f4..0b750263b5f 100644 --- a/src/config/cache-config.interface.ts +++ b/src/config/cache-config.interface.ts @@ -17,6 +17,14 @@ export interface CacheConfig extends Config { debug: boolean, // List of response headers to save into the cache headers: string[], + // Report the contents of the caches below to operators on this host: GET /app/cache-stats (JSON) + stats: { + enabled: boolean, + // 0 = serve it on the UI port to requests made directly from this host (loopback socket, no proxy headers). + // >0 = listen on 127.0.0.1: instead; under pm2 cluster mode worker N listens on + N, so each + // worker's own cache can be inspected. Prefer a port when a same-host reverse proxy adds no X-Forwarded-* headers. + port: number, + }, // Cache specific to known bots. Allows you to serve cached contents to bots only. botCache: { // Maximum number of pages (rendered via SSR) to cache. Setting max=0 disables the cache. diff --git a/src/config/default-app-config.ts b/src/config/default-app-config.ts index 431db5acd02..67a088cc29c 100644 --- a/src/config/default-app-config.ts +++ b/src/config/default-app-config.ts @@ -96,6 +96,11 @@ export class DefaultAppConfig implements AppConfig { debug: false, // Link header is used for signposting functionality headers: ['Link'], + // Cache stats endpoint for operators, off by default + stats: { + enabled: false, + port: 0, + }, // Cache specific to known bots. Allows you to serve cached contents to bots only. // Defaults to caching 1,000 pages. Each page expires after 1 day botCache: { diff --git a/src/environments/environment.test.ts b/src/environments/environment.test.ts index 537567b0218..6f46ed90174 100644 --- a/src/environments/environment.test.ts +++ b/src/environments/environment.test.ts @@ -88,6 +88,10 @@ export const environment: BuildConfig = { serverSide: { debug: false, headers: ['Link'], + stats: { + enabled: false, + port: 0, + }, botCache: { max: 0, timeToLive: 24 * 60 * 60 * 1000, // 1 day From de8cc3cb3b4ed2fcc4ebc71f99169fd44e4ce1f4 Mon Sep 17 00:00:00 2001 From: jm Date: Fri, 23 Oct 2026 22:06:40 +0200 Subject: [PATCH 2/4] JCU: fix(ssr): cache stats route falls through like an unknown path, invalid port never stops the UI Review findings: non-loopback callers got Express's plain 404 instead of the catch-all route, so the feature was detectable; an invalid stats port threw synchronously before the UI started; 'enabled' could serialize as 0 or be dropped. Test now covers the query string, trailing slash and HEAD. --- scripts/test-ssr-cache-stats.js | 3 +++ server.ts | 21 ++++++++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/scripts/test-ssr-cache-stats.js b/scripts/test-ssr-cache-stats.js index 2ef73726c5e..f8b18b0bac9 100644 --- a/scripts/test-ssr-cache-stats.js +++ b/scripts/test-ssr-cache-stats.js @@ -72,7 +72,10 @@ test('stats server listens on loopback and answers only GET /app/cache-stats', a assert.equal(json.headers.get('cache-control'), 'no-store'); assert.deepEqual(await json.json(), payload); + assert.equal((await fetch(`${base}/app/cache-stats?x=1`)).status, 200); assert.equal((await fetch(`${base}/other`)).status, 404); + assert.equal((await fetch(`${base}/app/cache-stats/`)).status, 404); + assert.equal((await fetch(`${base}/app/cache-stats`, { method: 'HEAD' })).status, 404); assert.equal((await fetch(`${base}/app/cache-stats`, { method: 'POST' })).status, 404); } finally { diff --git a/server.ts b/server.ts index 7b65d2605cc..420bcad6f32 100644 --- a/server.ts +++ b/server.ts @@ -224,11 +224,11 @@ export function app() { * SSR cache contents for operators on this host (see cache.serverSide.stats). With stats.port = 0 they are * served here, but only to requests made directly from this host: loopback socket address (not req.ip, which * honours X-Forwarded-For when `trust proxy` is on) and no proxy headers, so traffic relayed by a same-host - * reverse proxy is refused too. Everyone else gets the same 404 as an unknown route. + * reverse proxy is refused too. Everyone else falls through to the catch-all route, exactly as for an unknown path. */ if (environment.cache.serverSide.stats.enabled && environment.cache.serverSide.stats.port === 0) { const statsHandler = createSsrCacheStatsHandler(collectCacheStats); - server.get(SSR_CACHE_STATS_PATH, (req, res) => isDirectLoopbackRequest(req) ? statsHandler(req, res) : res.sendStatus(404)); + server.get(SSR_CACHE_STATS_PATH, (req, res, next) => isDirectLoopbackRequest(req) ? statsHandler(req, res) : next()); } /** @@ -678,8 +678,8 @@ function collectCacheStats(): SsrCacheStats { uptimeSeconds: Math.round(process.uptime()), memory: { rss: memory.rss, heapUsed: memory.heapUsed }, caches: [ - snapshotCache('bot', botCache, environment.production && botCacheEnabled()), - snapshotCache('anonymous', anonymousCache, environment.production && anonymousCacheEnabled()) + snapshotCache('bot', botCache, !!(environment.production && botCacheEnabled())), + snapshotCache('anonymous', anonymousCache, !!(environment.production && anonymousCacheEnabled())) ] }; } @@ -687,13 +687,20 @@ function collectCacheStats(): SsrCacheStats { /** * Serve the cache stats on a loopback-only port of their own. Every pm2 cluster worker gets basePort + its * instance number, because a port shared by the workers would be answered by an arbitrary one. - * A failed bind is logged and never stops the UI. + * An invalid port or a failed bind is logged and never stops the UI. */ function startCacheStatsServer(basePort: number) { const port = basePort + CLUSTER_INSTANCE; - const statsServer = startSsrCacheStatsServer(port, collectCacheStats); + const notStarted = (e: Error) => console.error(`SSR cache stats server not started on port ${port}: ${e.message}`); + let statsServer: ReturnType; + try { + statsServer = startSsrCacheStatsServer(port, collectCacheStats); + } catch (e) { + notStarted(e); + return; + } statsServer.on('listening', () => console.log(`SSR cache stats listening at http://127.0.0.1:${port}${SSR_CACHE_STATS_PATH}`)); - statsServer.on('error', (e: Error) => console.error(`SSR cache stats server not started on port ${port}: ${e.message}`)); + statsServer.on('error', notStarted); process.on('SIGINT', () => statsServer.close()); } From 3739be72cdd829c187568f884b4ad9a7519977a0 Mon Sep 17 00:00:00 2001 From: jm Date: Fri, 23 Oct 2026 22:19:55 +0200 Subject: [PATCH 3/4] JCU: fix(ssr): tighten the cache stats guards after review Refuse every X-Forwarded-* header, not just three of them; narrow the caught value (useUnknownInCatchVariables on DSpace 9 branches); never bind the stats listener to the UI port; log an invalid stats port such as a YAML string instead of ignoring it; let loopback HEAD fall through like other clients; answer 500 instead of crashing if the snapshot throws. --- config/config.example.yml | 4 ++-- scripts/test-ssr-cache-stats.js | 2 +- server.ts | 20 +++++++++++++++----- src/backend/ssr-cache-stats.ts | 14 +++++++++++--- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index 345721724b9..37ae63052c5 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -111,8 +111,8 @@ cache: stats: enabled: false # 0 = answer on the UI port, but only to requests made directly from this host: connecting from 127.0.0.1 and - # carrying no proxy headers (Forwarded, Via, X-Forwarded-*, X-Real-IP). Everyone else gets 404. A same-host reverse - # proxy that adds none of those headers would pass this check, so use a port number there. + # carrying no proxy headers (Forwarded, Via, X-Forwarded-*, X-Real-IP). Everyone else is handled by the app like any + # unknown path. A same-host reverse proxy that adds none of those headers would pass this check, so use a port number there. # A port number (e.g. 4001) = listen on 127.0.0.1: instead; under pm2 cluster mode worker N uses + N, # which is the only way to pick a specific worker. port: 0 diff --git a/scripts/test-ssr-cache-stats.js b/scripts/test-ssr-cache-stats.js index f8b18b0bac9..39f4ea25c16 100644 --- a/scripts/test-ssr-cache-stats.js +++ b/scripts/test-ssr-cache-stats.js @@ -91,7 +91,7 @@ test('direct loopback check accepts local sockets without proxy headers only', ( for (const address of ['10.0.0.5', '::ffff:10.0.0.5', '0.0.0.0', '', undefined]) { assert.equal(isDirectLoopbackRequest(request(address)), false, String(address)); } - for (const header of ['forwarded', 'via', 'x-forwarded-for', 'x-forwarded-host', 'x-forwarded-proto', 'x-real-ip']) { + for (const header of ['forwarded', 'via', 'x-forwarded-for', 'x-forwarded-host', 'x-forwarded-proto', 'x-forwarded-port', 'x-forwarded-ssl', 'x-real-ip']) { assert.equal(isDirectLoopbackRequest(request('127.0.0.1', { [header]: '' })), false, header); } assert.equal(isDirectLoopbackRequest(request('127.0.0.1', { 'user-agent': 'curl' })), true); diff --git a/server.ts b/server.ts index 420bcad6f32..acafded7826 100644 --- a/server.ts +++ b/server.ts @@ -228,7 +228,7 @@ export function app() { */ if (environment.cache.serverSide.stats.enabled && environment.cache.serverSide.stats.port === 0) { const statsHandler = createSsrCacheStatsHandler(collectCacheStats); - server.get(SSR_CACHE_STATS_PATH, (req, res, next) => isDirectLoopbackRequest(req) ? statsHandler(req, res) : next()); + server.get(SSR_CACHE_STATS_PATH, (req, res, next) => req.method === 'GET' && isDirectLoopbackRequest(req) ? statsHandler(req, res) : next()); } /** @@ -687,11 +687,15 @@ function collectCacheStats(): SsrCacheStats { /** * Serve the cache stats on a loopback-only port of their own. Every pm2 cluster worker gets basePort + its * instance number, because a port shared by the workers would be answered by an arbitrary one. - * An invalid port or a failed bind is logged and never stops the UI. + * An invalid port, a clash with the UI port or a failed bind is logged and never stops the UI. */ function startCacheStatsServer(basePort: number) { const port = basePort + CLUSTER_INSTANCE; - const notStarted = (e: Error) => console.error(`SSR cache stats server not started on port ${port}: ${e.message}`); + const notStarted = (e: unknown) => console.error(`SSR cache stats server not started on port ${port}: ${e instanceof Error ? e.message : String(e)}`); + if (port === environment.ui.port) { + notStarted('it is the UI port'); + return; + } let statsServer: ReturnType; try { statsServer = startSsrCacheStatsServer(port, collectCacheStats); @@ -707,8 +711,14 @@ function startCacheStatsServer(basePort: number) { function start() { logStartupMessage(environment); - if (environment.cache.serverSide.stats.enabled && environment.cache.serverSide.stats.port > 0) { - startCacheStatsServer(environment.cache.serverSide.stats.port); + const statsConfig = environment.cache.serverSide.stats; + if (statsConfig.enabled && statsConfig.port !== 0) { + // port 0 is served by the Express app, see app() + if (Number.isInteger(statsConfig.port) && statsConfig.port > 0) { + startCacheStatsServer(statsConfig.port); + } else { + console.error(`SSR cache stats server not started: invalid port ${JSON.stringify(statsConfig.port)}`); + } } /* diff --git a/src/backend/ssr-cache-stats.ts b/src/backend/ssr-cache-stats.ts index 9df35b81910..b6df5836aac 100644 --- a/src/backend/ssr-cache-stats.ts +++ b/src/backend/ssr-cache-stats.ts @@ -77,7 +77,8 @@ export function snapshotCache(name: string, cache: LRUCache request.headers[header] !== undefined); + !Object.keys(request.headers).some(header => PROXY_HEADERS.includes(header) || header.startsWith(PROXY_HEADER_PREFIX)); } /** Request handler answering GET with the stats as JSON; any other method gets 404. */ @@ -98,7 +99,14 @@ export function createSsrCacheStatsHandler(getStats: () => SsrCacheStats): (requ response.writeHead(404).end(); return; } - response.writeHead(200, { 'Content-Type': 'application/json; charset=utf-8' }).end(JSON.stringify(getStats())); + let body: string; + try { + body = JSON.stringify(getStats()); + } catch { + response.writeHead(500).end(); + return; + } + response.writeHead(200, { 'Content-Type': 'application/json; charset=utf-8' }).end(body); }; } From 2a2e64088361ddb705b74348bdc0103981125c7d Mon Sep 17 00:00:00 2001 From: jm Date: Fri, 23 Oct 2026 22:30:04 +0200 Subject: [PATCH 4/4] JCU: test(ssr): cover the 500 path of the cache stats handler, tidy the guards Review nits: exact-path check on the Express route (non-strict routing matched /app/cache-stats/ and case variants), warn when the stats are on the UI port while ui.useProxies is on, distinguish a bind failure from a later server error in the log, drop a stale peek() mention. --- scripts/test-ssr-cache-stats.js | 12 ++++++++++++ server.ts | 14 +++++++++++--- src/backend/ssr-cache-stats.ts | 2 +- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/scripts/test-ssr-cache-stats.js b/scripts/test-ssr-cache-stats.js index 39f4ea25c16..05644bf5c56 100644 --- a/scripts/test-ssr-cache-stats.js +++ b/scripts/test-ssr-cache-stats.js @@ -83,6 +83,18 @@ test('stats server listens on loopback and answers only GET /app/cache-stats', a } }); +test('stats server answers 500 when the snapshot throws', async () => { + const server = startSsrCacheStatsServer(0, () => { throw new Error('boom'); }); + try { + await once(server, 'listening'); + const response = await fetch(`http://127.0.0.1:${server.address().port}/app/cache-stats`); + assert.equal(response.status, 500); + assert.equal(response.headers.get('cache-control'), 'no-store'); + } finally { + await new Promise((resolve) => server.close(resolve)); + } +}); + test('direct loopback check accepts local sockets without proxy headers only', () => { const request = (remoteAddress, headers = {}) => ({ socket: { remoteAddress }, headers }); for (const address of ['127.0.0.1', '::1', '::ffff:127.0.0.1']) { diff --git a/server.ts b/server.ts index acafded7826..25432610ec1 100644 --- a/server.ts +++ b/server.ts @@ -227,8 +227,12 @@ export function app() { * reverse proxy is refused too. Everyone else falls through to the catch-all route, exactly as for an unknown path. */ if (environment.cache.serverSide.stats.enabled && environment.cache.serverSide.stats.port === 0) { + if (environment.ui.useProxies) { + console.warn('SSR cache stats are served on the UI port behind a proxy; a same-host proxy adding no forwarding headers would expose them. Prefer cache.serverSide.stats.port > 0.'); + } const statsHandler = createSsrCacheStatsHandler(collectCacheStats); - server.get(SSR_CACHE_STATS_PATH, (req, res, next) => req.method === 'GET' && isDirectLoopbackRequest(req) ? statsHandler(req, res) : next()); + server.get(SSR_CACHE_STATS_PATH, (req, res, next) => + req.method === 'GET' && req.path === SSR_CACHE_STATS_PATH && isDirectLoopbackRequest(req) ? statsHandler(req, res) : next()); } /** @@ -703,8 +707,12 @@ function startCacheStatsServer(basePort: number) { notStarted(e); return; } - statsServer.on('listening', () => console.log(`SSR cache stats listening at http://127.0.0.1:${port}${SSR_CACHE_STATS_PATH}`)); - statsServer.on('error', notStarted); + let listening = false; + statsServer.on('listening', () => { + listening = true; + console.log(`SSR cache stats listening at http://127.0.0.1:${port}${SSR_CACHE_STATS_PATH}`); + }); + statsServer.on('error', (e: Error) => listening ? console.error(`SSR cache stats server on port ${port}: ${e.message}`) : notStarted(e)); process.on('SIGINT', () => statsServer.close()); } diff --git a/src/backend/ssr-cache-stats.ts b/src/backend/ssr-cache-stats.ts index b6df5836aac..f828bc2022a 100644 --- a/src/backend/ssr-cache-stats.ts +++ b/src/backend/ssr-cache-stats.ts @@ -47,7 +47,7 @@ export const SSR_CACHE_STATS_PATH = '/app/cache-stats'; /** * Describe every entry resident in an SSR page cache, largest page first. * `dump()` visits stale entries too (unlike `forEach`, which skips them when `allowStale` is false); - * `dump()`, `peek()` and `has()` leave recency and TTL untouched. Byte counting is O(total cached HTML). + * `dump()` and `has()` leave recency and TTL untouched. Byte counting is O(total cached HTML). */ export function snapshotCache(name: string, cache: LRUCache | undefined, enabled: boolean): SsrCacheSnapshot { const entries: SsrCacheEntryStats[] = [];