diff --git a/public/analytics.js b/public/analytics.js index 5cbf4d7f4..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) { @@ -3151,6 +3158,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); })(); 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() {}, });