From 55ad68e4c861cd9b14840b272ba7e3ffaa186903 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 15:41:48 +0200 Subject: [PATCH 1/3] test(analytics): pin ?tab= escaping, the remaining #191 gaps and the withQuery contract (#193) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Red on master: a ?tab= value with a quote throws in init(), a crafted value (x"],[data-tab="overview) matches a real button and leaves the page on "Loading analytics…", and withQuery mishandles a bare fragment, a lone separator and a path that already has a query. Green on master by design (they kill the mutants that survived the #191 review): the neighbor-graph URL, the Prefix Tool's own hash-sizes request (driven alone by dropping the api cache after the shared load), and the area filter visibility on mount. The fake tab bar now parses [data-tab="..."] selectors like a browser (a stray quote is a SyntaxError, a list returns the first match), and the test reads withQuery through window._analyticsWithQuery, a one-line seam beside the existing _analytics* test exports. Co-Authored-By: Claude Sonnet 5.5 --- public/analytics.js | 1 + test-analytics-tab-state-and-query.js | 184 +++++++++++++++++++++++++- 2 files changed, 182 insertions(+), 3 deletions(-) diff --git a/public/analytics.js b/public/analytics.js index 5cbf4d7f4..18963fd7a 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -3151,6 +3151,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _stopForeignTraf // Expose for testing if (typeof window !== 'undefined') { window._analyticsAssignTableIds = assignAnalyticsTableIds; + window._analyticsWithQuery = withQuery; window._analyticsDecorateChannels = decorateAnalyticsChannels; window._analyticsSortChannels = sortChannels; window._analyticsLoadChannelSort = loadChannelSort; diff --git a/test-analytics-tab-state-and-query.js b/test-analytics-tab-state-and-query.js index 8bfa628e2..0530ff232 100644 --- a/test-analytics-tab-state-and-query.js +++ b/test-analytics-tab-state-and-query.js @@ -4,6 +4,10 @@ * whenever a region or area filter was active, and got the SPA HTML. * #183: leaving #/analytics and coming back without ?tab= marked Overview * active but rendered the tab selected before leaving. + * #193: a ?tab= value must never reach a selector (a quote threw in init(); + * a crafted value matched a real button and left the page on + * "Loading analytics…"), the remaining #191 test gaps, and the + * withQuery(path, frag) contract. * * Runs the REAL app.js api() and the REAL analytics page in one vm with a * fake fetch and a small fake DOM whose tab buttons come from the markup @@ -50,6 +54,28 @@ function fakeEl(id) { }; } +// The one selector shape init() could build from ?tab=: a list of +// [data-tab="…"] attribute selectors. Parsed like a browser would: a stray +// quote is a SyntaxError, a comma starts another selector of the list, and +// querySelector returns the first match in document order. +function parseTabSelector(sel) { + const open = '[data-tab="'; + const values = []; + let i = 0; + for (;;) { + if (sel.slice(i, i + open.length) !== open) throw new SyntaxError("'" + sel + "' is not a valid selector."); + i += open.length; + let v = ''; + while (i < sel.length && sel[i] !== '"') { if (sel[i] === '\\') i++; v += sel[i++]; } + if (sel[i] !== '"' || sel[i + 1] !== ']') throw new SyntaxError("'" + sel + "' is not a valid selector."); + i += 2; + values.push(v); + if (i === sel.length) return values; + if (sel[i] !== ',') throw new SyntaxError("'" + sel + "' is not a valid selector."); + i++; + } +} + function pageEnv() { const fetchLog = []; let region = '', area = ''; @@ -62,6 +88,9 @@ function pageEnv() { const m = /\/api\/analytics\/(rf|topology|channels|hash-sizes|hash-collisions)(\?|$)/.exec(url); const key = m && { rf: 'rfData', topology: 'topoData', channels: 'chanData', 'hash-sizes': 'hashData', 'hash-collisions': 'collisionData' }[m[1]]; if (key) return respond(REAL[key]); + // The neighbor-graph tab only needs its request observed; failing it keeps + // the canvas renderer, which the fake DOM cannot host, out of the run. + if (/\/api\/analytics\/neighbor-graph/.test(url)) return { ok: false, status: 404, headers: { get: () => null }, json: async () => ({ error: 'not found' }) }; if (/\/api\/nodes\?/.test(url)) return respond({ nodes: [{ public_key: 'aabbccddeeff', name: 'R1', role: 'repeater' }] }); return respond({}); }; @@ -94,15 +123,18 @@ function pageEnv() { if (id === 'analyticsTabs') { els[id].querySelectorAll = (sel) => (sel === '.tab-btn' ? tabButtons() : []); els[id].querySelector = (sel) => { - const m = /^\[data-tab="([^"]+)"\]$/.exec(sel); - return m ? tabButtons().find((b) => b.dataset.tab === m[1]) || null : null; + const wanted = parseTabSelector(sel); + return tabButtons().find((b) => wanted.includes(b.dataset.tab)) || null; }; } + // The markup ships the area filter hidden; the page shows it per tab. + if (id === 'analyticsAreaFilter') els[id].style.display = 'none'; } return els[id]; }; const pages = {}; + let initError = null; const ctx = { window: { addEventListener() {}, removeEventListener() {}, dispatchEvent() {} }, document: { @@ -137,7 +169,16 @@ function pageEnv() { return { fetchLog, setFilters: (r, a) => { region = r ? '®ion=' + r : ''; area = a ? '&area=' + a : ''; }, - async mount(hash) { ctx.location.hash = hash; page.init(app); await flush(); }, + async mount(hash) { + ctx.location.hash = hash; + initError = null; + page.init(app).catch((e) => { initError = e; }); + await flush(); + }, + initError: () => initError, + invalidateApiCache: (prefix) => ctx.invalidateApiCache(prefix), + withQuery: ctx._analyticsWithQuery, + areaFilterDisplay: () => el('analyticsAreaFilter').style.display, destroy: () => page.destroy(), clickTab: async (tab) => { const btn = tabButtons().find((b) => b.dataset.tab === tab); @@ -247,6 +288,143 @@ function pageEnv() { assert.ok(MARKERS.overview.test(env.content()), 'content is not Overview: ' + env.content().slice(0, 200)); }); + console.log('\n=== #193: ?tab= never reaches a selector ==='); + + // Each value either throws in a selector (a quote) or builds a selector + // list that matches a real button ('overview', 'rf'). All must end on + // Overview, with Overview rendered: _currentTab is a real tab again. + const hostileTabs = ['x"', 'x"],[data-tab="overview', 'x"],[data-tab="rf', 'overview"]', '\\', '', 'overview', '__proto__']; + for (const value of hostileTabs) { + await test('?tab=' + JSON.stringify(value) + ' shows Overview with Overview active, no exception', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=' + encodeURIComponent(value)); + assert.strictEqual(env.initError(), null, 'init() threw: ' + (env.initError() && env.initError().message)); + assert.deepStrictEqual(env.activeTabs(), ['overview'], 'active button'); + assert.ok(MARKERS.overview.test(env.content()), 'content is not Overview: ' + env.content().slice(0, 200)); + }); + } + + await test('a hostile ?tab= after another tab was selected still ends on Overview', async () => { + const env = pageEnv(); + await env.mount('#/analytics'); + await env.clickTab('topology'); + env.destroy(); + await env.mount('#/analytics?tab=' + encodeURIComponent('x"],[data-tab="topology')); + assert.strictEqual(env.initError(), null, 'init() threw'); + assert.deepStrictEqual(env.activeTabs(), ['overview'], 'active button'); + assert.ok(MARKERS.overview.test(env.content()) && !MARKERS.topology.test(env.content()), 'content is not Overview'); + await env.clickTab('topology'); + assert.ok(MARKERS.topology.test(env.content()), 'tab switching broken after the hostile mount'); + }); + + console.log('\n=== #193: remaining #191 test gaps ==='); + + await test('the neighbor-graph tab asks for neighbor-graph?region=…&min_count=1&min_score=0', async () => { + const env = pageEnv(); + env.setFilters('SJC'); + await env.mount('#/analytics?tab=neighbor-graph'); + assert.ok(env.fetchLog.includes('/api/analytics/neighbor-graph?region=SJC&min_count=1&min_score=0'), env.fetchLog.join(', ')); + const env2 = pageEnv(); + await env2.mount('#/analytics?tab=neighbor-graph'); + assert.ok(env2.fetchLog.includes('/api/analytics/neighbor-graph?min_count=1&min_score=0'), env2.fetchLog.join(', ')); + }); + + // loadAnalytics asks for the same hash-sizes URL on every mount and api() + // dedupes by URL, so the Prefix Tool's own request is only visible when it + // goes out on its own: mount Overview, drop the cache, open the tab. + await test('the Prefix Tool\'s own hash-sizes request carries region and area', async () => { + const env = pageEnv(); + env.setFilters('SJC', 'north'); + await env.mount('#/analytics'); + env.invalidateApiCache('/analytics'); + env.fetchLog.length = 0; + await env.clickTab('prefix-tool'); + const urls = env.fetchLog.filter((u) => /\/api\/analytics\/hash-sizes/.test(u)); + assert.deepStrictEqual(urls, ['/api/analytics/hash-sizes?region=SJC&area=north']); + }); + + await test('the Prefix Tool\'s own hash-sizes request is plain without filters', async () => { + const env = pageEnv(); + await env.mount('#/analytics'); + env.invalidateApiCache('/analytics'); + env.fetchLog.length = 0; + await env.clickTab('prefix-tool'); + assert.deepStrictEqual(env.fetchLog.filter((u) => /\/api\/analytics\/hash-sizes/.test(u)), ['/api/analytics/hash-sizes']); + }); + + await test('the area filter is shown on the mounted tab only where it applies', async () => { + const env = pageEnv(); + await env.mount('#/analytics'); + assert.strictEqual(env.areaFilterDisplay(), '', 'Overview: area filter not shown'); + await env.mount('#/analytics?tab=prefix-tool'); + assert.strictEqual(env.areaFilterDisplay(), 'none', 'Prefix Tool: area filter not hidden'); + await env.clickTab('rf'); + assert.strictEqual(env.areaFilterDisplay(), '', 'RF: area filter not shown after the switch'); + await env.clickTab('prefix-tool'); + assert.strictEqual(env.areaFilterDisplay(), 'none', 'Prefix Tool: area filter not hidden after the switch'); + }); + + await test('returning to a plain #/analytics after a tab without the area filter shows it again', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=prefix-tool'); + env.destroy(); + await env.mount('#/analytics'); + assert.strictEqual(env.areaFilterDisplay(), ''); + }); + + console.log('\n=== #193: withQuery(path, frag) contract ==='); + + await test('withQuery is exposed for the test', async () => { + assert.strictEqual(typeof pageEnv().withQuery, 'function', 'window._analyticsWithQuery missing'); + }); + + const WQ = [ + // path, fragment, expected + ['/a', '', '/a'], + ['/a', undefined, '/a'], + ['/a', null, '/a'], + ['/a', '®ion=X', '/a?region=X'], + ['/a', '®ion=X&area=Y', '/a?region=X&area=Y'], + ['/a', '?region=X', '/a?region=X'], + ['/a', 'region=X', '/a?region=X'], + ['/a', '&', '/a'], + ['/a', '?', '/a'], + ['/a?x=1', '', '/a?x=1'], + ['/a?x=1', '®ion=X', '/a?x=1®ion=X'], + ['/a?x=1', '?region=X', '/a?x=1®ion=X'], + ['/a?x=1', 'region=X', '/a?x=1®ion=X'], + ['/a?x=1', '&', '/a?x=1'], + ]; + for (const [path, frag, want] of WQ) { + await test('withQuery(' + JSON.stringify(path) + ', ' + JSON.stringify(frag) + ') = ' + JSON.stringify(want), async () => { + assert.strictEqual(pageEnv().withQuery(path, frag), want); + }); + } + + // The pre-#179 call sites built their query strings by hand. The helper + // must produce exactly those strings for every filter combination. + await test('withQuery equals the hand-built query strings it replaced (every filter combination)', async () => { + const { withQuery } = pageEnv(); + const rqsV = ['', '®ion=SJC', '®ion=' + encodeURIComponent('A,B')]; + const aqsV = ['', '&area=north', '&area=' + encodeURIComponent('a&b')]; + const twV = ['', '&window=7d']; + const oldSep = (q) => { const t = q.slice(1); return t ? '?' + t : ''; }; + let compared = 0; + for (const r of rqsV) for (const a of aqsV) for (const t of twV) { + const cases = [ + [r + a, oldSep(r + a)], // hash-sizes, hash-collisions + [r + a + t, oldSep(r + a + t)], // rf, topology, relay-airtime-share + [r + t, oldSep(r + t)], // channels + [r, r ? '?' + r.slice(1) : ''], // distance + ]; + for (const [frag, want] of cases) { assert.strictEqual(withQuery('/x', frag), '/x' + want); compared++; } + const sep = r ? '?' + r.slice(1) : ''; // neighbor-graph + assert.strictEqual(withQuery('/x', r + '&min_count=1&min_score=0'), '/x' + sep + (sep ? '&' : '?') + 'min_count=1&min_score=0'); + compared++; + } + assert.strictEqual(compared, 90, 'URL pairs compared'); + }); + console.log('\n' + passed + ' passed, ' + failed + ' failed'); if (failed > 0) process.exit(1); })(); From 8e94f7372cf6cbb9b8f3c9f5ed1b8226be8fa30e Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 15:42:11 +0200 Subject: [PATCH 2/3] fix(analytics): escape-safe ?tab= lookup and a tolerant withQuery (#193) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit init() found the ?tab= button with an attribute selector built from the URL. A value with a quote threw and left the page on "Loading analytics…"; a crafted value (x"],[data-tab="overview) matched a real button while _currentTab took the arbitrary string. The button is now found by comparing dataset.tab over the tab buttons, and an unknown value falls back to Overview with the Overview button active. withQuery(path, frag) now accepts '', '&…', '?…' and a bare 'a=1', and joins with '&' when the path already has a '?'. The existing URLs stay byte-identical (90 URL pairs). Co-Authored-By: Claude Sonnet 5.5 --- public/analytics.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/public/analytics.js b/public/analytics.js index 18963fd7a..b362bf106 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -255,11 +255,15 @@ const _ap = new URLSearchParams(hashParams); // Every mount starts from the URL, falling back to Overview: the tab // selected before leaving the page must not survive into this mount (#183). + // The button is found by comparing data-tab, never by building a selector + // from the URL: a quote in ?tab= threw here, and a crafted value matched a + // real button while _currentTab took the arbitrary string (#193). const urlTab = _ap.get('tab'); - const urlTabBtn = urlTab && analyticsTabs.querySelector(`[data-tab="${urlTab}"]`); + const tabBtns = Array.from(analyticsTabs.querySelectorAll('.tab-btn')); + const urlTabBtn = urlTab ? tabBtns.find(b => b.dataset.tab === urlTab) : null; _currentTab = urlTabBtn ? urlTab : 'overview'; - const activeBtn = urlTabBtn || analyticsTabs.querySelector('[data-tab="overview"]'); - analyticsTabs.querySelectorAll('.tab-btn').forEach(b => b.classList.remove('active')); + const activeBtn = urlTabBtn || tabBtns.find(b => b.dataset.tab === 'overview'); + tabBtns.forEach(b => b.classList.remove('active')); if (activeBtn) activeBtn.classList.add('active'); // #749 — restore time window from URL. const urlWindow = _ap.get('window'); @@ -327,10 +331,13 @@ var _themeRefreshHandler = null; let _currentTab = 'overview'; - // Filter fragments ("®ion=…", "&area=…", "&window=…") all start with - // '&'; turn their concatenation into the path's query string (#179). + // Append a query fragment to a path (#179, #193). The filter fragments + // ("®ion=…", "&area=…", "&window=…") start with '&'; '?…' and a bare + // 'a=1' are taken too, and an empty fragment (or a lone '&' / '?') leaves + // the path as it is. The separator is '&' when the path already has a '?'. function withQuery(path, frag) { - return frag ? path + '?' + frag.slice(1) : path; + const q = frag ? String(frag).replace(/^[?&]/, '') : ''; + return q ? path + (path.indexOf('?') < 0 ? '?' : '&') + q : path; } async function loadAnalytics(startedAt) { From 8d5e30f2afd04b8df613f518ec44f81609b85c74 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 15:44:49 +0200 Subject: [PATCH 3/3] test(analytics): let the #1375 fake tab bar list its buttons (#193) init() now finds the ?tab= button by comparing dataset.tab over querySelectorAll('.tab-btn') instead of building an attribute selector, so the fake tab bar in test-issue-1375-scope-stats-fetch.js has to return its buttons. The assertions are unchanged. Follows the escape-safe lookup commit, which left this file failing. Co-Authored-By: Claude Sonnet 5.5 --- test-issue-1375-scope-stats-fetch.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test-issue-1375-scope-stats-fetch.js b/test-issue-1375-scope-stats-fetch.js index 51d576f51..2c5fe2d34 100644 --- a/test-issue-1375-scope-stats-fetch.js +++ b/test-issue-1375-scope-stats-fetch.js @@ -146,18 +146,18 @@ function makeSandbox() { id, innerHTML: '', textContent: '', value: '', style: {}, dataset: {}, classList: { add() {}, remove() {}, toggle() {}, contains() { return false; } }, addEventListener() {}, removeEventListener() {}, - // init()'s deep-link tab selector (analytics.js ~205) does - // `analyticsTabs.querySelector('[data-tab="scopes"]')` to find the - // tab button matching a `#/analytics?tab=scopes` hash, then reads - // it back truthy to set `_currentTab`. A generic `[data-tab="x"]` - // match (rather than special-casing the analyticsTabs id) covers - // that real production lookup without hand-modeling the tab bar's - // actual child markup. - querySelector(sel) { - const m = /\[data-tab="([^"]+)"\]/.exec(sel || ''); - return m ? elFor('tab-btn:' + m[1]) : null; + // init()'s deep-link lookup (#193) lists `analyticsTabs.querySelectorAll + // ('.tab-btn')` and matches `btn.dataset.tab` against ?tab= (it no + // longer builds a selector from the URL). A generic `.tab-btn` list of + // the two tabs this test drives (rather than special-casing the + // analyticsTabs id) covers that real production lookup without + // hand-modeling the tab bar's actual child markup. `#/analytics + // ?tab=scopes` therefore finds the Scopes button and sets _currentTab. + querySelector() { return null; }, + querySelectorAll(sel) { + if (sel !== '.tab-btn') return []; + return ['overview', 'scopes'].map((t) => { const b = elFor('tab-btn:' + t); b.dataset.tab = t; return b; }); }, - querySelectorAll() { return []; }, appendChild() {}, closest() { return null; }, getAttribute() { return null; }, setAttribute() {}, removeAttribute() {}, });