From a5c8ba227223935707552969d3898f9254ca4bfa Mon Sep 17 00:00:00 2001 From: SandObserver <260779319+SandObserver@users.noreply.github.com> Date: Mon, 19 Oct 2026 00:44:23 -0400 Subject: [PATCH] Fix: limit widget data cache keys to declared query values Build the widget-data cache key and ctx.params from the endpoint and the names a widget manifest lists in a new params field. Cap cached replies at 16 per widget. System Summary declares seed. --- api/src/latest.js | 28 +++++++++--- api/src/widget-data.js | 27 +++++++++--- api/src/widgets.js | 13 ++++++ api/test/http-integration.test.js | 61 +++++++++++++++++++++++++++ api/test/latest.test.js | 27 +++++++++++- api/test/widget-data.test.js | 27 +++++++++++- api/test/widget-manifests.test.js | 1 + api/test/widgets.test.js | 22 ++++++++++ ui/widgets/system-summary/widget.json | 3 ++ 9 files changed, 197 insertions(+), 12 deletions(-) diff --git a/api/src/latest.js b/api/src/latest.js index db69f911..6de6ce2c 100644 --- a/api/src/latest.js +++ b/api/src/latest.js @@ -4,13 +4,28 @@ /** @typedef {{ ok: true, value: any } | { ok: false, error: unknown }} Outcome */ -/** @type {Map | null }>} */ +/** @type {Map | null, group: string | null }>} */ const _entries = new Map(); function prune(now, keepMs) { for (const [key, e] of _entries) if (!e.pending && now - e.at >= keepMs) _entries.delete(key); } +/* Drops the oldest settled entries of a group until at most max remain. A + refresh in flight is never dropped. */ +function cap(group, max) { + /** @type {{ key: string, at: number }[]} */ + const settled = []; + let count = 0; + for (const [key, e] of _entries) { + if (e.group !== group) continue; + count++; + if (!e.pending) settled.push({ key, at: e.at }); + } + settled.sort((a, b) => a.at - b.at); + for (let i = 0; count > max && i < settled.length; i++, count--) _entries.delete(settled[i].key); +} + /** @param {number} ms @param {Promise} pending @returns {Promise} */ function within(ms, pending) { let timer; @@ -21,13 +36,15 @@ function within(ms, pending) { } /** @param {string} key @param {() => any} refresh - @param {{ holdMs: number, keepMs: number }} opts @returns {Promise} */ -async function latest(key, refresh, { holdMs, keepMs }) { + @param {{ holdMs: number, keepMs: number, group?: string, maxPerGroup?: number }} opts + @returns {Promise} */ +async function latest(key, refresh, { holdMs, keepMs, group, maxPerGroup }) { const now = Date.now(); prune(now, keepMs); let e = _entries.get(key); + const created = !e; if (!e) { - e = { started: 0, at: 0, outcome: null, pending: null }; + e = { started: 0, at: 0, outcome: null, pending: null, group: group ?? null }; _entries.set(key, e); } const entry = e; @@ -45,6 +62,7 @@ async function latest(key, refresh, { holdMs, keepMs }) { return outcome; }); } + if (created && group != null && maxPerGroup) cap(group, maxPerGroup); const fresh = await within(entry.started + holdMs - now, entry.pending); if (fresh) return fresh; return entry.outcome && Date.now() - entry.at < keepMs ? entry.outcome : null; @@ -54,4 +72,4 @@ function reset() { _entries.clear(); } -module.exports = { latest, reset }; +module.exports = { latest, reset, _size: () => _entries.size }; diff --git a/api/src/widget-data.js b/api/src/widget-data.js index dc866f09..d183447e 100644 --- a/api/src/widget-data.js +++ b/api/src/widget-data.js @@ -105,6 +105,21 @@ async function runWidgetModule(name, file, ctx) { return await fn(ctx); } +/* Only these reach data.js and the cache key. Passing the raw query lets each + distinct value store its own cached reply. */ +function widgetParams(searchParams, manifest) { + const out = new URLSearchParams(); + for (const k of ['endpoint', ...(manifest?.params || [])]) { + const v = searchParams.get(k); + if (v !== null) out.set(k, v); + } + return out; +} + +/* Declared values still come from the request. Without this cap one client can + fill the heap. */ +const KEPT_PER_WIDGET = 16; + async function getWidgetData(item, entry, endpointName, searchParams, fetch, row = null, isOptions = false) { const wc = item.widgetConfig || {}; /* An options fetch must run the real path, or the editor shows fabricated @@ -130,17 +145,19 @@ on('GET', '/api/widget-data/:id', async (req, res) => { const entry = getRegistry()[item.widgetType]; if (!entry) return json(res, 404, { error: 'unknown widget type', kind: KIND.INVALID }); - const u = new URL(req.url, 'http://x'); - const endpointName = u.searchParams.get('endpoint') || ''; + const params = widgetParams(new URL(req.url, 'http://x').searchParams, entry.manifest); + const endpointName = params.get('endpoint') || ''; const refresh = () => - getWidgetData(item, entry, endpointName, u.searchParams, fetchUnchecked).catch(e => { + getWidgetData(item, entry, endpointName, params, fetchUnchecked).catch(e => { log.error('widget-data failed', { widget: item.widgetType, id: item.id, error: log.reason(e) }); throw e; }); - const outcome = await latest(`${cfg._rev}:${item.id}?${u.searchParams}`, refresh, { + const outcome = await latest(`${cfg._rev}:${item.id}?${params}`, refresh, { holdMs: WIDGET_HOLD_MS, keepMs: WIDGET_KEEP_MS, + group: `${cfg._rev}:${item.id}`, + maxPerGroup: KEPT_PER_WIDGET, }); if (!outcome) return json(res, 202, { pending: true }); if (outcome.ok) return json(res, outcome.value.status, outcome.value.body); @@ -210,4 +227,4 @@ on('POST', '/api/widget-options/:id', async (req, res) => { } }); -module.exports = { getWidgetData, normalizeBase, dataFnContext, resolveRow, labelOptions }; +module.exports = { getWidgetData, widgetParams, normalizeBase, dataFnContext, resolveRow, labelOptions }; diff --git a/api/src/widgets.js b/api/src/widgets.js index acfa92a1..903cacb5 100644 --- a/api/src/widgets.js +++ b/api/src/widgets.js @@ -7,6 +7,8 @@ const { loadConfig } = require('./config'); const WIDGETS_PATH = process.env.WIDGETS_PATH || '/usr/share/nginx/html/widgets'; +const MAX_PARAMS = 8; +const PARAM_NAME = /^[A-Za-z][A-Za-z0-9_-]{0,31}$/; const VALID_SIZES = new Set(['small', 'medium', 'large', 'xlarge']); const VALID_CARDS = new Set(['dark', 'graphite', 'light', 'translucent']); /* Mirrors GLYPH_NAMES in ui/js/widget-glyphs.js. The two cannot share a module @@ -180,6 +182,17 @@ function validateManifest(name, m) { } } + if (m.params !== undefined) { + if (!Array.isArray(m.params) || m.params.length > MAX_PARAMS) + errs.push(`"params" must be an array of at most ${MAX_PARAMS} names`); + else + m.params.forEach((p, i) => { + if (typeof p !== 'string' || !PARAM_NAME.test(p)) errs.push(`params[${i}] must be a query parameter name`); + else if (p === 'endpoint') errs.push('params: "endpoint" is always passed and must not be listed'); + else if (m.params.indexOf(p) !== i) errs.push(`params: "${p}" is listed twice`); + }); + } + if (m.views !== undefined) { if (typeof m.views !== 'object' || Array.isArray(m.views) || !Object.keys(m.views).length) errs.push('"views" must be a non-empty object'); diff --git a/api/test/http-integration.test.js b/api/test/http-integration.test.js index 019add3e..879c354f 100644 --- a/api/test/http-integration.test.js +++ b/api/test/http-integration.test.js @@ -223,6 +223,67 @@ test('a widget whose service stalls answers pending, then with the data once it } }); +test('query values a widget does not read share one refresh and one cached reply', async () => { + let release; + const released = new Promise(r => { + release = r; + }); + let calls = 0; + const upstream = http.createServer(async (_, res) => { + calls++; + await released; + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"n":7}'); + }); + await new Promise(r => upstream.listen(0, '127.0.0.1', r)); + const cfg = loadConfig(); + const url = `http://127.0.0.1:${upstream.address().port}/`; + saveConfig({ + ...cfg, + items: [...(cfg.items || []), { id: 'junk-w', type: 'widget', widgetType: 'fixture', widgetConfig: { url } }], + }); + try { + const replies = await Promise.all( + [1, 2, 3, 4, 5].map(n => req('GET', `/api/widget-data/junk-w?_=${n}`, { cookie: validCookie })), + ); + assert.deepEqual( + replies.map(r => r.status), + [202, 202, 202, 202, 202], + ); + assert.equal(calls, 1); + } finally { + release(); + saveConfig(cfg); + upstream.closeAllConnections?.(); + await new Promise(r => upstream.close(r)); + } +}); + +test('a query value the widget manifest declares reaches its data function', async () => { + const manifest = path.join(_fx, 'widget.json'); + const dataFn = path.join(_fx, 'data.js'); + const savedManifest = fs.readFileSync(manifest, 'utf8'); + const savedData = fs.readFileSync(dataFn, 'utf8'); + fs.writeFileSync(manifest, JSON.stringify({ ...JSON.parse(savedManifest), params: ['range'] })); + fs.writeFileSync(dataFn, 'module.exports = async ctx => ({ q: ctx.params.toString() });\n'); + delete require.cache[require.resolve(dataFn)]; + const { loadRegistry } = require('../src/widgets'); + loadRegistry(); + const cfg = loadConfig(); + saveConfig({ ...cfg, items: [...(cfg.items || []), { id: 'param-w', type: 'widget', widgetType: 'fixture' }] }); + try { + const r = await req('GET', '/api/widget-data/param-w?endpoint=e&range=week&_=1', { cookie: validCookie }); + assert.equal(r.status, 200); + assert.deepEqual(r.body, { q: 'endpoint=e&range=week' }); + } finally { + saveConfig(cfg); + fs.writeFileSync(manifest, savedManifest); + fs.writeFileSync(dataFn, savedData); + delete require.cache[require.resolve(dataFn)]; + loadRegistry(); + } +}); + test('OPTIONS is no longer specially routed', async () => { const r = await req('OPTIONS', '/api/config', { cookie: validCookie }); assert.equal(r.status, 404, 'it must fall through like any other unmatched method'); diff --git a/api/test/latest.test.js b/api/test/latest.test.js index e0f0952f..9cb5798d 100644 --- a/api/test/latest.test.js +++ b/api/test/latest.test.js @@ -1,6 +1,6 @@ const { test, beforeEach } = require('node:test'); const assert = require('node:assert/strict'); -const { latest, reset } = require('../src/latest'); +const { latest, reset, _size } = require('../src/latest'); const OPTS = { holdMs: 30, keepMs: 60_000 }; const wait = ms => new Promise(r => setTimeout(r, ms)); @@ -65,3 +65,28 @@ test('different keys do not share a result', async () => { await latest('a', async () => 'a', OPTS); assert.deepEqual(await latest('b', async () => 'b', OPTS), { ok: true, value: 'b' }); }); + +test('a group keeps at most maxPerGroup entries, dropping the oldest settled ones', async () => { + const opts = { ...OPTS, group: 'w1', maxPerGroup: 3 }; + for (let i = 0; i < 10; i++) await latest(`w1?${i}`, async () => i, opts); + assert.equal(_size(), 3); + assert.deepEqual(await latest('w1?9', () => wait(50).then(() => 'again'), { ...opts, holdMs: 0 }), { + ok: true, + value: 9, + }); +}); + +test('the cap never drops a refresh in flight, and leaves other groups alone', async () => { + await latest('other', async () => 0, OPTS); + const opts = { ...OPTS, holdMs: 0, group: 'w1', maxPerGroup: 2 }; + const slow = () => wait(100).then(() => 1); + for (let i = 0; i < 5; i++) await latest(`w1?${i}`, slow, opts); + assert.equal(_size(), 6, 'five refreshes in flight plus the other group'); + await wait(150); + await latest('w1?new', async () => 2, opts); + assert.equal(_size(), 3); + assert.deepEqual(await latest('other', () => wait(50).then(() => 'x'), { ...OPTS, holdMs: 0 }), { + ok: true, + value: 0, + }); +}); diff --git a/api/test/widget-data.test.js b/api/test/widget-data.test.js index 95812327..eceefaa6 100644 --- a/api/test/widget-data.test.js +++ b/api/test/widget-data.test.js @@ -4,7 +4,7 @@ process.env.CONFIG_PATH = path.join(tmpDir('wd'), 'apps.json'); const { test } = require('node:test'); const assert = require('node:assert/strict'); -const { normalizeBase, dataFnContext, resolveRow } = require('../src/widget-data'); +const { normalizeBase, dataFnContext, resolveRow, widgetParams } = require('../src/widget-data'); const { saveConfig } = require('../src/config'); test('normalizeBase adds a scheme, trims, and strips trailing slashes', () => { @@ -23,6 +23,31 @@ test('a widget with no data.js reports no data source', async () => { assert.match(out.body.error, /no data source/); }); +test('widgetParams keeps the endpoint and declared params and drops everything else', () => { + const q = new URLSearchParams('endpoint=system&seed=1&_=123&x=y'); + assert.equal(widgetParams(q, { params: ['seed'] }).toString(), 'endpoint=system&seed=1'); + assert.equal(widgetParams(q, {}).toString(), 'endpoint=system'); + assert.equal(widgetParams(new URLSearchParams('_=1'), {}).toString(), ''); +}); + +test('every query value a widget reads is declared in its manifest', () => { + const fs = require('node:fs'); + const widgets = path.join(__dirname, '..', '..', 'ui', 'widgets'); + for (const name of fs.readdirSync(widgets)) { + const file = path.join(widgets, name, 'data.js'); + if (!fs.existsSync(file)) continue; + const manifest = JSON.parse(fs.readFileSync(path.join(widgets, name, 'widget.json'), 'utf8')); + const src = fs.readFileSync(file, 'utf8'); + for (const [, key] of src.matchAll(/params\??\.get\(\s*['"]([^'"]+)['"]/g)) { + assert.equal( + widgetParams(new URLSearchParams({ [key]: 'v' }), manifest).get(key), + 'v', + `${name}/data.js reads "${key}" but widget.json does not list it in "params"`, + ); + } + } +}); + /* ── Group-row resolution for an options fetch ──────────────────────────── */ test('resolveRow returns the addressed row', () => { diff --git a/api/test/widget-manifests.test.js b/api/test/widget-manifests.test.js index d00cc9b4..2e41c9f8 100644 --- a/api/test/widget-manifests.test.js +++ b/api/test/widget-manifests.test.js @@ -29,6 +29,7 @@ const KNOWN_KEYS = new Set([ 'views', 'viewField', 'defaultView', + 'params', 'entryVersions', ]); diff --git a/api/test/widgets.test.js b/api/test/widgets.test.js index 39f89fed..25d47b25 100644 --- a/api/test/widgets.test.js +++ b/api/test/widgets.test.js @@ -98,6 +98,28 @@ test('loadRegistry returns an empty registry when the directory is unreadable', assert.deepEqual(plain(widgets.loadRegistry()), {}); }); +/* ── Query parameters ───────────────────────────────────────────────────── */ + +const paramErrs = params => widgets.validateManifest('w', { name: 'w', label: 'W', sizes: ['small'], params }).errors; + +test('validateManifest accepts a list of query parameter names', () => { + assert.deepEqual(paramErrs(['seed', 'range_2']), []); +}); + +test('validateManifest rejects params that are not a short list of names', () => { + assert.equal(paramErrs('seed').length, 1); + assert.equal(paramErrs(Array.from({ length: 9 }, (_, i) => 'p' + i)).length, 1); + assert.equal(paramErrs(['']).length, 1); + assert.equal(paramErrs(['a b']).length, 1); + assert.equal(paramErrs([7]).length, 1); + assert.equal(paramErrs(['x'.repeat(33)]).length, 1); +}); + +test('validateManifest rejects a listed endpoint and a repeated name', () => { + assert.match(paramErrs(['endpoint'])[0], /always passed/); + assert.match(paramErrs(['seed', 'seed'])[0], /listed twice/); +}); + /* ── Field type validation ──────────────────────────────────────────────── */ const base = { name: 'w', label: 'W', sizes: ['small'] }; diff --git a/ui/widgets/system-summary/widget.json b/ui/widgets/system-summary/widget.json index b3af2761..e7b8f59f 100644 --- a/ui/widgets/system-summary/widget.json +++ b/ui/widgets/system-summary/widget.json @@ -8,6 +8,9 @@ "medium" ], "card": "dark", + "params": [ + "seed" + ], "fields": [ { "key": "statProvider",