From d9523f23a052df7fb7515055c1ca68938cd6dce2 Mon Sep 17 00:00:00 2001 From: Kpa-clawbot <259247574+Kpa-clawbot@users.noreply.github.com> Date: Fri, 27 Mar 2026 21:28:50 -0700 Subject: [PATCH] fix: harden node detail rendering with Number() casts and Array.isArray guards, fixes #190 Add defensive type safety to node detail page rendering: - Wrap all .toFixed() calls with Number() to handle string values from Go backend - Use Array.isArray() for hash_sizes_seen instead of || [] fallback - Apply same fixes to both full-screen and side-panel views - Add 9 new tests for renderHashInconsistencyWarning and renderNodeBadges with hash_size_inconsistent data (including non-array edge cases) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- public/analytics.js | 2 +- public/index.html | 54 ++++++++-------- public/nodes.js | 56 +++++++++++++---- test-frontend-helpers.js | 131 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 203 insertions(+), 40 deletions(-) diff --git a/public/analytics.js b/public/analytics.js index e7879002..fb4310e3 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -956,7 +956,7 @@ ${inconsistent.map((n, i) => { const roleColor = window.ROLE_COLORS?.[n.role] || '#6b7280'; const prefix = n.hash_size ? n.public_key.slice(0, n.hash_size * 2).toUpperCase() : '?'; - const sizeBadges = (n.hash_sizes_seen || []).map(s => { + const sizeBadges = (Array.isArray(n.hash_sizes_seen) ? n.hash_sizes_seen : []).map(s => { const c = s >= 3 ? '#16a34a' : s === 2 ? '#86efac' : '#f97316'; const fg = s === 2 ? '#064e3b' : '#fff'; return '' + s + 'B'; diff --git a/public/index.html b/public/index.html index 89172197..26799465 100644 --- a/public/index.html +++ b/public/index.html @@ -22,9 +22,9 @@ - - - + + + @@ -81,29 +81,29 @@
- - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/public/nodes.js b/public/nodes.js index 41ccf67b..355c1a48 100644 --- a/public/nodes.js +++ b/public/nodes.js @@ -152,7 +152,8 @@ function renderHashInconsistencyWarning(n) { if (!n.hash_size_inconsistent) return ''; - return `
Adverts show varying hash sizes (${(n.hash_sizes_seen||[]).join('-byte, ')}-byte). This is a known bug where automatic adverts ignore the configured multibyte path setting. Fixed in repeater v1.14.1.
`; + const sizes = Array.isArray(n.hash_sizes_seen) ? n.hash_sizes_seen : []; + return `
Adverts show varying hash sizes (${sizes.join('-byte, ')}-byte). This is a known bug where automatic adverts ignore the configured multibyte path setting. Fixed in repeater v1.14.1.
`; } let directNode = null; // set when navigating directly to #/nodes/:pubkey @@ -244,9 +245,15 @@ const statusLabel = si.statusLabel; const statusExplanation = si.explanation; + const dupMap = buildDupNameMap(_allNodes); + const dupBadge = dupNameBadge(n.name, n.public_key, dupMap); + const dupKeys = n.name && dupMap[n.name.toLowerCase()] ? dupMap[n.name.toLowerCase()].filter(function(k) { return k !== n.public_key; }) : []; + const dupSection = dupKeys.length ? '
Also known as: ' + dupKeys.map(function(k) { return '' + escapeHtml(k.slice(0, 12)) + '…'; }).join(', ') + '
' : ''; + body.innerHTML = `
-
${escapeHtml(n.name || '(unnamed)')}
+
${escapeHtml(n.name || '(unnamed)')}${dupBadge}
+ ${dupSection}
${renderNodeBadges(n, roleColor)}
${renderHashInconsistencyWarning(n)}
${n.public_key}
@@ -270,10 +277,10 @@ First Seen${n.first_seen ? new Date(n.first_seen).toLocaleString() : '—'} Total Packets${stats.totalTransmissions || stats.totalPackets || n.advert_count || 0}${stats.totalObservations && stats.totalObservations !== (stats.totalTransmissions || stats.totalPackets) ? ' (seen ' + stats.totalObservations + '×)' : ''} Packets Today${stats.packetsToday || 0} - ${stats.avgSnr != null ? `Avg SNR${stats.avgSnr.toFixed(1)} dB` : ''} + ${stats.avgSnr != null ? `Avg SNR${Number(stats.avgSnr).toFixed(1)} dB` : ''} ${stats.avgHops ? `Avg Hops${stats.avgHops}` : ''} - ${hasLoc ? `Location${n.lat.toFixed(5)}, ${n.lon.toFixed(5)}` : ''} - Hash Prefix${n.hash_size ? '' + n.public_key.slice(0, n.hash_size * 2).toUpperCase() + ' (' + n.hash_size + '-byte)' : 'Unknown'}${n.hash_size_inconsistent ? ' ⚠️ varies' : ''} + ${hasLoc ? `Location${Number(n.lat).toFixed(5)}, ${Number(n.lon).toFixed(5)}` : ''} + Hash Prefix${n.hash_size ? '' + n.public_key.slice(0, n.hash_size * 2).toUpperCase() + ' (' + n.hash_size + '-byte)' : 'Unknown'}${n.hash_size_inconsistent ? ' ⚠️ varies' : ''} ${observers.length ? `
@@ -286,8 +293,8 @@ ${escapeHtml(o.observer_name || o.observer_id)} ${o.iata ? escapeHtml(o.iata) : '—'} ${o.packetCount} - ${o.avgSnr != null ? o.avgSnr.toFixed(1) + ' dB' : '—'} - ${o.avgRssi != null ? o.avgRssi.toFixed(0) + ' dBm' : '—'} + ${o.avgSnr != null ? Number(o.avgSnr).toFixed(1) + ' dB' : '—'} + ${o.avgRssi != null ? Number(o.avgRssi).toFixed(0) + ' dBm' : '—'} `).join('')} @@ -433,6 +440,27 @@ let _allNodes = null; // cached full node list + // Build a map of lowercased name → count of distinct pubkeys sharing that name + function buildDupNameMap(allNodes) { + var map = {}; + (allNodes || []).forEach(function(n) { + if (!n.name) return; + var key = n.name.toLowerCase(); + if (!map[key]) map[key] = []; + if (map[key].indexOf(n.public_key) === -1) map[key].push(n.public_key); + }); + return map; + } + + function dupNameBadge(name, pubkey, dupMap) { + if (!name || !dupMap) return ''; + var keys = dupMap[name.toLowerCase()]; + if (!keys || keys.length <= 1) return ''; + var others = keys.filter(function(k) { return k !== pubkey; }); + var title = keys.length + ' nodes share this name (' + others.map(function(k) { return k.slice(0, 8) + '…'; }).join(', ') + ')'; + return ' (' + keys.length + ')'; + } + async function loadNodes(refreshOnly) { try { // Fetch all nodes once, filter client-side @@ -637,6 +665,7 @@ return aFav - bFav; }); + const dupMap = buildDupNameMap(_allNodes); tbody.innerHTML = sorted.map(n => { const roleColor = ROLE_COLORS[n.role] || '#6b7280'; const isClaimed = myKeys.has(n.public_key); @@ -644,7 +673,7 @@ const status = getNodeStatus(n.role || 'companion', lastSeenTime ? new Date(lastSeenTime).getTime() : 0); const lastSeenClass = status === 'active' ? 'last-seen-active' : 'last-seen-stale'; return ` - ${favStar(n.public_key, 'node-fav')}${isClaimed ? '★ ' : ''}${n.name || '(unnamed)'} + ${favStar(n.public_key, 'node-fav')}${isClaimed ? '★ ' : ''}${n.name || '(unnamed)'}${dupNameBadge(n.name, n.public_key, dupMap)} ${truncate(n.public_key, 16)} ${n.role} ${timeAgo(n.last_heard || n.last_seen)} @@ -696,9 +725,12 @@ const roleColor = si.roleColor; const totalPackets = stats.totalTransmissions || stats.totalPackets || n.advert_count || 0; + const dupMap = buildDupNameMap(_allNodes); + const dupBadge = dupNameBadge(n.name, n.public_key, dupMap); + panel.innerHTML = `
-
${escapeHtml(n.name || '(unnamed)')}
+
${escapeHtml(n.name || '(unnamed)')}${dupBadge}
${renderNodeBadges(n, roleColor)} 🔍 Details 📊 Analytics @@ -721,9 +753,9 @@
First Seen
${n.first_seen ? new Date(n.first_seen).toLocaleString() : '—'}
Total Packets
${totalPackets}
Packets Today
${stats.packetsToday || 0}
- ${stats.avgSnr != null ? `
Avg SNR
${stats.avgSnr.toFixed(1)} dB
` : ''} + ${stats.avgSnr != null ? `
Avg SNR
${Number(stats.avgSnr).toFixed(1)} dB
` : ''} ${stats.avgHops ? `
Avg Hops
${stats.avgHops}
` : ''} - ${hasLoc ? `
Location
${n.lat.toFixed(5)}, ${n.lon.toFixed(5)}
` : ''} + ${hasLoc ? `
Location
${Number(n.lat).toFixed(5)}, ${Number(n.lon).toFixed(5)}
` : ''}
@@ -733,7 +765,7 @@
${observers.map(o => `
${escapeHtml(o.observer_name || o.observer_id)}${o.iata ? ' ' + escapeHtml(o.iata) + '' : ''} - ${o.packetCount} pkts · ${o.avgSnr != null ? 'SNR ' + o.avgSnr.toFixed(1) + 'dB' : ''}${o.avgRssi != null ? ' · RSSI ' + o.avgRssi.toFixed(0) : ''} + ${o.packetCount} pkts · ${o.avgSnr != null ? 'SNR ' + Number(o.avgSnr).toFixed(1) + 'dB' : ''}${o.avgRssi != null ? ' · RSSI ' + Number(o.avgRssi).toFixed(0) : ''}
`).join('')}
` : ''} diff --git a/test-frontend-helpers.js b/test-frontend-helpers.js index 71e6e358..507d41e1 100644 --- a/test-frontend-helpers.js +++ b/test-frontend-helpers.js @@ -206,6 +206,15 @@ console.log('\n=== nodes.js: getStatusTooltip / getStatusInfo (extracted) ==='); ).replace( /function sortNodes/, 'window.__nodesExport.sortNodes = sortNodes; function sortNodes' + ).replace( + /function buildDupNameMap/, + 'window.__nodesExport.buildDupNameMap = buildDupNameMap; function buildDupNameMap' + ).replace( + /function renderHashInconsistencyWarning/, + 'window.__nodesExport.renderHashInconsistencyWarning = renderHashInconsistencyWarning; function renderHashInconsistencyWarning' + ).replace( + /function dupNameBadge/, + 'window.__nodesExport.dupNameBadge = dupNameBadge; function dupNameBadge' ); // Provide required globals @@ -291,6 +300,128 @@ console.log('\n=== nodes.js: getStatusTooltip / getStatusInfo (extracted) ==='); assert.ok(Array.isArray(result)); }); } + + if (ex.buildDupNameMap) { + const buildDupNameMap = ex.buildDupNameMap; + test('buildDupNameMap returns empty for no nodes', () => { + const m = buildDupNameMap([]); + assert.strictEqual(Object.keys(m).length, 0); + }); + test('buildDupNameMap groups nodes by lowercase name', () => { + const m = buildDupNameMap([ + { name: 'Alpha', public_key: 'key1' }, + { name: 'alpha', public_key: 'key2' }, + { name: 'Beta', public_key: 'key3' }, + ]); + assert.strictEqual(m['alpha'].length, 2); + assert.ok(m['alpha'].includes('key1')); + assert.ok(m['alpha'].includes('key2')); + assert.strictEqual(m['beta'].length, 1); + }); + test('buildDupNameMap ignores unnamed nodes', () => { + const m = buildDupNameMap([ + { name: '', public_key: 'key1' }, + { name: null, public_key: 'key2' }, + { name: 'Alpha', public_key: 'key3' }, + ]); + assert.strictEqual(Object.keys(m).length, 1); + }); + test('buildDupNameMap deduplicates same pubkey', () => { + const m = buildDupNameMap([ + { name: 'Alpha', public_key: 'key1' }, + { name: 'Alpha', public_key: 'key1' }, + ]); + assert.strictEqual(m['alpha'].length, 1); + }); + } + + if (ex.dupNameBadge) { + const dupNameBadge = ex.dupNameBadge; + test('dupNameBadge returns empty for unique name', () => { + const m = { 'alpha': ['key1'] }; + assert.strictEqual(dupNameBadge('Alpha', 'key1', m), ''); + }); + test('dupNameBadge returns badge for duplicate names', () => { + const m = { 'alpha': ['key1', 'key2'] }; + const html = dupNameBadge('Alpha', 'key1', m); + assert.ok(html.includes('(2)')); + assert.ok(html.includes('dup-name-badge')); + }); + test('dupNameBadge returns empty for null name', () => { + assert.strictEqual(dupNameBadge(null, 'key1', {}), ''); + }); + test('dupNameBadge returns empty for null map', () => { + assert.strictEqual(dupNameBadge('Alpha', 'key1', null), ''); + }); + test('dupNameBadge shows count of 3 for three duplicates', () => { + const m = { 'alpha': ['key1', 'key2', 'key3'] }; + const html = dupNameBadge('Alpha', 'key1', m); + assert.ok(html.includes('(3)')); + }); + } + + // --- renderHashInconsistencyWarning tests (fixes #190) --- + if (ex.renderHashInconsistencyWarning) { + const warn = ex.renderHashInconsistencyWarning; + test('renderHashInconsistencyWarning returns empty for consistent node', () => { + assert.strictEqual(warn({ hash_size_inconsistent: false }), ''); + }); + test('renderHashInconsistencyWarning returns empty for undefined flag', () => { + assert.strictEqual(warn({}), ''); + }); + test('renderHashInconsistencyWarning renders with valid array', () => { + const html = warn({ hash_size_inconsistent: true, hash_sizes_seen: [1, 2] }); + assert.ok(html.includes('1-byte, 2-byte')); + assert.ok(html.includes('varying hash sizes')); + }); + test('renderHashInconsistencyWarning handles missing hash_sizes_seen', () => { + const html = warn({ hash_size_inconsistent: true }); + assert.ok(html.includes('varying hash sizes')); + // Should not crash — renders with empty sizes + assert.ok(html.includes('-byte')); + }); + test('renderHashInconsistencyWarning handles non-array hash_sizes_seen', () => { + const html = warn({ hash_size_inconsistent: true, hash_sizes_seen: '[1, 2]' }); + assert.ok(html.includes('varying hash sizes')); + // String should be treated as empty array (Array.isArray guard) + assert.ok(html.includes('-byte')); + }); + test('renderHashInconsistencyWarning handles null hash_sizes_seen', () => { + const html = warn({ hash_size_inconsistent: true, hash_sizes_seen: null }); + assert.ok(html.includes('varying hash sizes')); + }); + } + + // --- renderNodeBadges with hash_size_inconsistent (fixes #190) --- + if (ex.renderNodeBadges) { + test('renderNodeBadges handles hash_size_inconsistent node', () => { + const html = ex.renderNodeBadges({ + role: 'room', public_key: '9dc3e069d1b336c4af33167d3838147ca6449e12c1e1bdaa92fdfc0ecfdd98bc', + hash_size: 2, hash_size_inconsistent: true, hash_sizes_seen: [1, 2], + last_heard: new Date().toISOString() + }, '#16a34a'); + assert.ok(html.includes('room')); + assert.ok(html.includes('9DC3')); + assert.ok(html.includes('variable hash size')); + }); + test('renderNodeBadges handles null hash_size', () => { + const html = ex.renderNodeBadges({ + role: 'room', public_key: 'abcdef1234567890', + hash_size: null, hash_size_inconsistent: false, + last_heard: new Date().toISOString() + }, '#16a34a'); + assert.ok(html.includes('room')); + assert.ok(!html.includes('variable hash size')); + }); + test('renderNodeBadges handles string hash_sizes_seen gracefully', () => { + const html = ex.renderNodeBadges({ + role: 'repeater', public_key: 'abcdef1234567890', + hash_size: 2, hash_size_inconsistent: true, hash_sizes_seen: '[1, 2]', + last_heard: new Date().toISOString() + }, '#dc2626'); + assert.ok(html.includes('variable hash size')); + }); + } } // ===== HOP-RESOLVER TESTS =====