diff --git a/apps/netscli-gui/src/tools/presentation.test.ts b/apps/netscli-gui/src/tools/presentation.test.ts index ce03c144..d1a49907 100644 --- a/apps/netscli-gui/src/tools/presentation.test.ts +++ b/apps/netscli-gui/src/tools/presentation.test.ts @@ -76,9 +76,27 @@ describe('buildCommand', () => { pcap.form.filter = 'tcp port 443'; pcap.form.max_packets = '50'; expect(buildCommand(pcap)).toBe( - 'netscli pcap --interface Ethernet --duration 10 --filter "tcp port 443" --max-packets 50', + 'netscli pcap --interface Ethernet --duration 10 --filter "tcp port 443" --max-packets 50 --json', ); }); + + it('omits -p when the ports field is empty, because that is what runs', () => { + // The preview used to substitute the GUI's five-port placeholder while + // execution sent no port list and the core used its own default of three. + // The copied command, the History entry and the saved bundle all carried + // the wrong string. + const scan = createTab('scan'); + scan.form.host = 'router.local'; + scan.form.ports = ''; + expect(buildCommand(scan)).toBe('netscli scan router.local --json'); + }); + + it('escapes quotes in a capture filter so the preview stays paste-able', () => { + const pcap = createTab('pcap'); + pcap.form.interface = 'eth0'; + pcap.form.filter = 'host "example"'; + expect(buildCommand(pcap)).toContain('--filter "host \\"example\\""'); + }); }); describe('tabIdentity', () => { diff --git a/apps/netscli-gui/src/tools/presentation/columns.ts b/apps/netscli-gui/src/tools/presentation/columns.ts index 91e69ee7..26072100 100644 --- a/apps/netscli-gui/src/tools/presentation/columns.ts +++ b/apps/netscli-gui/src/tools/presentation/columns.ts @@ -1,6 +1,7 @@ import type { ToolResult } from '../../types/app'; import type { ResultColumn, ResultRow, ToolKind } from '../types'; import { buildRows } from './rows'; +import { inspectPorts } from './ports'; export function columnsFor( kind: ToolKind, @@ -13,8 +14,8 @@ export function columnsFor( const hasPorts = hasColumnData(rows, 'ports'); const hasMac = hasColumnData(rows, 'mac'); - const inspectPorts = result?.kind === 'inspect' ? (result.data.ports ?? result.data.open_ports) : []; - if (kind === 'inspect' && result?.kind === 'inspect' && inspectPorts.length === 0) { + const inspectPortList = result?.kind === 'inspect' ? inspectPorts(result.data) : []; + if (kind === 'inspect' && result?.kind === 'inspect' && inspectPortList.length === 0) { return [ { key: 'host', label: 'Host', mono: true, width: 220 }, { key: 'ip', label: 'IP', mono: true, width: 220 }, diff --git a/apps/netscli-gui/src/tools/presentation/commands.ts b/apps/netscli-gui/src/tools/presentation/commands.ts index ce968186..d61a6df9 100644 --- a/apps/netscli-gui/src/tools/presentation/commands.ts +++ b/apps/netscli-gui/src/tools/presentation/commands.ts @@ -1,11 +1,17 @@ -import { DEFAULT_PORTS } from '../registry'; import type { WorkspaceTab } from '../types'; export function buildCommand(tab: WorkspaceTab): string { const form = tab.form; switch (tab.kind) { case 'scan': - return `netscli scan ${form.host || ''} -p ${form.ports || DEFAULT_PORTS} --json`; + // `-p` is omitted when the field is empty, exactly as `inspect` and + // `sweep` below already do. It used to substitute the GUI's + // DEFAULT_PORTS placeholder (22,80,443,8080,8443) while execution sent + // no port list at all and the core fell back to its own default of + // 22,80,443 — so the preview claimed five ports, three were scanned, + // and that wrong string was what Ctrl+Shift+C copied, what the History + // menu recorded, and what got stored with the saved result. + return `netscli scan ${form.host || ''}${form.ports ? ` -p ${form.ports}` : ''} --json`; case 'ping': return `netscli ping ${form.host || ''}${form.count ? ` --count ${form.count}` : ''} --json`; case 'trace': { @@ -45,8 +51,13 @@ export function buildCommand(tab: WorkspaceTab): string { const parts = ['netscli pcap']; if (form.interface) parts.push(`--interface ${form.interface}`); if (form.duration) parts.push(`--duration ${form.duration}`); - if (form.filter) parts.push(`--filter "${form.filter}"`); + // Escape quotes rather than interpolating raw: a BPF filter containing + // a double quote produced a preview that would not parse if pasted. + if (form.filter) parts.push(`--filter "${form.filter.replace(/"/g, '\\"')}"`); if (form.max_packets) parts.push(`--max-packets ${form.max_packets}`); + // The capture branch omitted --json while every other command here + // includes it, so this one preview did not match what the app runs. + parts.push('--json'); return parts.join(' '); } } diff --git a/apps/netscli-gui/src/tools/presentation/ports.ts b/apps/netscli-gui/src/tools/presentation/ports.ts index 77d31a1d..c9e0c296 100644 --- a/apps/netscli-gui/src/tools/presentation/ports.ts +++ b/apps/netscli-gui/src/tools/presentation/ports.ts @@ -1,4 +1,22 @@ -import type { PortResult } from '../../types/netscli'; +import type { InspectResult, PortResult } from '../../types/netscli'; + +/** + * Which ports an inspect actually checked, everywhere. + * + * Three call sites had drifted into two different answers: the row builder + * used `ports?.length ? ports : open_ports`, while the column set and the + * status-bar summary used `ports ?? open_ports`. A backend returning + * `ports: []` alongside a non-empty `open_ports` therefore rendered rows + * from `open_ports` while the columns were built from an empty list, so + * every cell came out blank. + * + * The length check is the right one to keep: `ports` is optional for older + * backends, and an empty list carries no more information than a missing + * one. + */ +export function inspectPorts(result: InspectResult): PortResult[] { + return result.ports?.length ? result.ports : result.open_ports; +} export function statusOf(port: PortResult): string { return port.status ?? (port.open ? 'open' : 'closed'); diff --git a/apps/netscli-gui/src/tools/presentation/rows.ts b/apps/netscli-gui/src/tools/presentation/rows.ts index d57a6d22..2531e173 100644 --- a/apps/netscli-gui/src/tools/presentation/rows.ts +++ b/apps/netscli-gui/src/tools/presentation/rows.ts @@ -1,7 +1,7 @@ import type { ToolResult } from '../../types/app'; import type { PortResult } from '../../types/netscli'; import type { ResultRow, ToolKind } from '../types'; -import { latencyOf, statusOf } from './ports'; +import { inspectPorts, latencyOf, statusOf } from './ports'; import { parseTraceLine, type TraceHopRow } from './traceLine'; function portRow(port: PortResult, index: number, kind: ToolKind): ResultRow { @@ -128,7 +128,7 @@ export function buildRows(result: ToolResult | null): ResultRow[] { ]; } case 'inspect': { - const ports = result.data.ports?.length ? result.data.ports : result.data.open_ports; + const ports = inspectPorts(result.data); const rows = ports.map((port, index) => portRow(port, index, 'inspect')); if (rows.length > 0) return rows; const ping = result.data.ping; diff --git a/apps/netscli-gui/src/tools/presentation/summaries.ts b/apps/netscli-gui/src/tools/presentation/summaries.ts index 34ffc273..10439bca 100644 --- a/apps/netscli-gui/src/tools/presentation/summaries.ts +++ b/apps/netscli-gui/src/tools/presentation/summaries.ts @@ -1,4 +1,5 @@ import type { ToolResult } from '../../types/app'; +import { inspectPorts } from './ports'; export function resultSummary(result: ToolResult | null): string { if (!result) return '0 results'; @@ -18,7 +19,7 @@ export function resultSummary(result: ToolResult | null): string { case 'reverse': return result.data.hostname ? 'reverse name found' : 'no reverse name'; case 'inspect': { - const ports = result.data.ports ?? result.data.open_ports; + const ports = inspectPorts(result.data); if (ports.length > 0) { const open = ports.filter((port) => port.open).length; return `${ports.length} ${ports.length === 1 ? 'port' : 'ports'} checked - ${open} open`; @@ -37,8 +38,25 @@ export function resultSummary(result: ToolResult | null): string { return `${result.data.length} interfaces`; case 'arp': return `${result.data.length} ARP entries`; - case 'pcap': - return `${'packets_captured' in result.data ? result.data.packets_captured : result.data.total_packets} packets`; + case 'pcap': { + // Say when the view is cut off. The core sets `packets_truncated` on a + // capture and `truncated` on a parse, and neither was read anywhere in + // the GUI: opening a 20,000-packet file with the default 1,000 limit + // showed 1,000 rows under a status bar reading "20000 packets", with + // nothing indicating the rest was missing — and Export CSV then wrote + // only the loaded rows. + // `in` has to narrow inline; hoisting the discriminant into a boolean + // loses the narrowing and the union members have no fields in common. + const data = result.data; + if ('packets_captured' in data) { + return data.packets_truncated + ? `${formatNumber(data.packets.length)} of ${formatNumber(data.packets_captured)} packets shown` + : `${formatNumber(data.packets_captured)} packets`; + } + return data.truncated + ? `${formatNumber(data.packets.length)} of ${formatNumber(data.total_packets)} packets shown` + : `${formatNumber(data.total_packets)} packets`; + } } } diff --git a/apps/netscli-gui/src/tools/summaries.test.ts b/apps/netscli-gui/src/tools/summaries.test.ts new file mode 100644 index 00000000..d1e9f8c5 --- /dev/null +++ b/apps/netscli-gui/src/tools/summaries.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest'; + +import { resultSummary } from './presentation'; + +describe('resultSummary — packet capture truncation', () => { + /** + * The core sets `packets_truncated` on a capture and `truncated` on a + * parse, and neither was read anywhere in the GUI. Opening a + * 20,000-packet file with the default 1,000 limit showed 1,000 rows under + * a status bar reading "20000 packets" — no indication the view was cut + * off, while Export CSV wrote only the loaded rows. + */ + const packets = (n: number) => Array.from({ length: n }, (_, i) => ({ index: i })) as never[]; + + it('says how many of how many when a parsed capture is truncated', () => { + const summary = resultSummary({ + kind: 'pcap', + data: { file_path: 'x.pcap', link_type: 1, total_packets: 20000, packets: packets(1000), truncated: true }, + } as never); + expect(summary).toBe('1000 of 20000 packets shown'); + }); + + it('says how many of how many when a live capture is truncated', () => { + const summary = resultSummary({ + kind: 'pcap', + data: { file_path: 'x.pcap', duration: { secs: 5, nanos: 0 }, packets_captured: 8000, packets: packets(1000), packets_truncated: true }, + } as never); + expect(summary).toBe('1000 of 8000 packets shown'); + }); + + it('reports a plain total when nothing was dropped', () => { + const summary = resultSummary({ + kind: 'pcap', + data: { file_path: 'x.pcap', link_type: 1, total_packets: 42, packets: packets(42), truncated: false }, + } as never); + expect(summary).toBe('42 packets'); + }); +});