1.0 ergonomics: replace 13 bespoke *_from_pcap helpers with one generic pcap::messages::<P>() #86

New issue

Closed

opened 2026-06-27 11:53:36 +00:00 by p13marc · 0 comments

p13marc commented

2026-06-27 11:53:36 +00:00

(Migrated from github.com)

Pre-1.0 ergonomics. Additive-then-deprecate (mild break OK).

Problem — 13 near-identical `*_from_pcap` helpers, inconsistently named, covering only 8 of 28 parsers

The per-parser pcap iterators are mechanically identical (open PcapFlowSource, .sessions/.datagrams, filter SessionEvent::Application → (key, message)):

http::{requests,responses,exchanges}_from_pcap (src/http/pcap_iter.rs)
dns::{messages,exchanges}_from_pcap, tls::{client_hellos,handshakes}_from_pcap,
quic::initials_from_pcap, smb/kerberos/ldap/ssh::messages_from_pcap,
pcap::flow_summaries_from_pcap (the one outlier — drives a raw tracker, eager Vec).

Problems:

Naming is ad-hoc: messages_from_pcap vs client_hellos_from_pcap vs initials_from_pcap vs requests_from_pcap. No rule.
Coverage is arbitrary: only 8 of 28 parsers have one. The other 20 (icmp/arp/ndp/dhcp/lldp/cdp/ntp/ssdp/tftp/mdns/netbios_ns/ftp/smtp/wireguard/modbus/dnp3/snmp/radius/stun/rdp) don't — so the convenience is a coin-flip per protocol.
Maintenance tax: every new parser needs a hand-written, copy-pasted helper to feel first-class.
CLAUDE.md under-documents which exist (omits tls/quic/dns::exchanges).

Proposed direction — one generic typed entry, drop the bespoke 13

PcapFlowSource::sessions(ext, parser) / .datagrams(ext, parser) are already generic — the 13 helpers add nothing but a name. Replace them with a single ergonomic, discoverable API so every parser is equally first-class:

// One call, works for any SessionParser / DatagramParser:
for (key, msg) in flowscope::pcap::messages::<TlsParser>("trace.pcap")? { ... }
for (key, msg) in flowscope::pcap::messages::<DnsUdpParser>("trace.pcap")? { ... }

(or a small PcapMessages::<P>::open(path) builder). One generic function → uniform name, uniform behaviour, zero per-parser code, automatic coverage for all 28 + future parsers. The multi-parser case stays Driver::run_pcap (driver/typed.rs:398).

Keep flow_summaries_from_pcap (it's genuinely different — no parser, raw flow records) but rename for consistency (pcap::flow_summaries).

Migration

Deprecate the 13 named helpers (keep one release as #[deprecated] aliases delegating to the generic), document the one-call replacement in the migration recipe, delete next major.

Effort: S–M. High DX payoff; shrinks the public surface.

**Pre-1.0 ergonomics. Additive-then-deprecate (mild break OK).** ## Problem — 13 near-identical `*_from_pcap` helpers, inconsistently named, covering only 8 of 28 parsers The per-parser pcap iterators are mechanically identical (open `PcapFlowSource`, `.sessions`/`.datagrams`, filter `SessionEvent::Application` → `(key, message)`): - `http::{requests,responses,exchanges}_from_pcap` (`src/http/pcap_iter.rs`) - `dns::{messages,exchanges}_from_pcap`, `tls::{client_hellos,handshakes}_from_pcap`, `quic::initials_from_pcap`, `smb/kerberos/ldap/ssh::messages_from_pcap`, `pcap::flow_summaries_from_pcap` (the one outlier — drives a raw tracker, eager `Vec`). Problems: - **Naming is ad-hoc**: `messages_from_pcap` vs `client_hellos_from_pcap` vs `initials_from_pcap` vs `requests_from_pcap`. No rule. - **Coverage is arbitrary**: only 8 of 28 parsers have one. The other 20 (icmp/arp/ndp/dhcp/lldp/cdp/ntp/ssdp/tftp/mdns/netbios_ns/ftp/smtp/wireguard/modbus/dnp3/snmp/radius/stun/rdp) don't — so the convenience is a coin-flip per protocol. - **Maintenance tax**: every new parser needs a hand-written, copy-pasted helper to feel first-class. - CLAUDE.md under-documents which exist (omits tls/quic/dns::exchanges). ## Proposed direction — one generic typed entry, drop the bespoke 13 `PcapFlowSource::sessions(ext, parser)` / `.datagrams(ext, parser)` are **already generic** — the 13 helpers add nothing but a name. Replace them with a single ergonomic, discoverable API so *every* parser is equally first-class: ```rust // One call, works for any SessionParser / DatagramParser: for (key, msg) in flowscope::pcap::messages::<TlsParser>("trace.pcap")? { ... } for (key, msg) in flowscope::pcap::messages::<DnsUdpParser>("trace.pcap")? { ... } ``` (or a small `PcapMessages::<P>::open(path)` builder). One generic function → uniform name, uniform behaviour, zero per-parser code, automatic coverage for all 28 + future parsers. The multi-parser case stays `Driver::run_pcap` (`driver/typed.rs:398`). Keep `flow_summaries_from_pcap` (it's genuinely different — no parser, raw flow records) but rename for consistency (`pcap::flow_summaries`). ## Migration Deprecate the 13 named helpers (keep one release as `#[deprecated]` aliases delegating to the generic), document the one-call replacement in the migration recipe, delete next major. Effort: S–M. High DX payoff; shrinks the public surface.