From eac4f804ddd2e42bf06c3befef8158ed3d5a27e5 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Fri, 21 Aug 2026 15:44:22 -0700 Subject: [PATCH 1/6] feat: --hex flag augments record-level output with raw record bytes Hex is an attribute of existing output formats, not a format of its own (the unit is the record, the meaning is "attach the original bytes"): - --format text --hex: HEX: line at the end of each block (render::text::format_record_with_hex) - --format json/json-pretty --hex: hex field injected into each record object - render::hex::format_record: shared hex source; re-encodes through MrtRecord::encode (round-trips byte-identically for standard records; round-trip proven in tests against crafted wire bytes) - unencodable records keep well-formed output + stderr warning - implies --level records; explicit error for unsupported formats (default/psv) Workflow target: filter the CLI output, paste the HEX line / hex field straight into wirescope's hex input. --- CHANGELOG.md | 1 + src/bin/main.rs | 59 ++++++++++++++++++++++--- src/render/hex.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++ src/render/mod.rs | 1 + src/render/text.rs | 18 ++++++++ 5 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 src/render/hex.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 355aa2b..1f479ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### Added * **Layered text output format** (`render::text::format_record`, `--format text`): one human-readable, indented block per MRT record — session context (`TIME`/`TYPE`/`FROM`/`TO`), `UPDATE:` sections with withdrawn/announced prefixes (including those carried in MP_REACH/MP_UNREACH) and every path attribute, `OPEN:` capabilities, session states, RIB entries, the peer table, and full legacy type-5 records. RFC 7606 validation findings render under `WARNINGS:` when present. The format is designed around this crate's own models and `Display` vocabulary — inspired by bgpdump's human-readable output, not byte-compatible with it. Rendering is a pure function of the record. In the CLI, `--format text` always uses record-level output (implies `--level records`); the other formats follow `--level` and default to elems. +* **`--hex` flag**: include each record's raw bytes as hex in record-level output — a `HEX:` line at the end of `--format text` blocks, a `hex` field in `--format json`/`json-pretty` records. Designed as the pipe from filtered CLI output into byte-level tools (e.g. wirescope's hex paste input): `bgpkit-parser -p 8.8.8.0/24 --format text --hex updates.mrt.gz` yields blocks whose `HEX:` line can be pasted straight into a dissector. Implies `--level records`; requires text/json/json-pretty formats. Records that cannot be re-encoded keep well-formed output and note the failure on stderr. * Record-level filter semantics are now explicit: records that produce no elems (KEEPALIVE, OPEN, NOTIFICATION, state changes) never match elem-oriented filters and are dropped from record iteration while filters are active — a `debug!` line notes each drop. ## v0.21.0 - 2026-08-21 diff --git a/src/bin/main.rs b/src/bin/main.rs index 2d15fe6..c24c505 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -69,6 +69,12 @@ struct Opts { #[clap(long)] psv: bool, + /// Include each record's raw bytes as hex: a `HEX:` line in text + /// blocks, a `hex` field in JSON records. Record-level only — implies + /// `--level records`; requires `--format text`, `json`, or `json-pretty`. + #[clap(long)] + hex: bool, + /// Count BGP elems #[clap(short, long)] elems_count: bool, @@ -257,13 +263,24 @@ fn main() { opts.format }; + if opts.hex + && !matches!( + output_format, + OutputFormat::Json | OutputFormat::JsonPretty | OutputFormat::Text + ) + { + eprintln!("Error: --hex requires --format text, json, or json-pretty"); + std::process::exit(1); + } + let recovery_config = RecoveryConfig::default(); // Element-level runs (element output or counting only elements) use the elem // iterators, which apply filters per element; everything else stays at the record // level. Counting both (-e -r) iterates records and converts once per record. let use_elem_stream = ((opts.elems_count && !opts.records_count) || (!opts.elems_count && !opts.records_count && matches!(opts.level, OutputLevel::Elems))) - && output_format != OutputFormat::Text; + && output_format != OutputFormat::Text + && !opts.hex; let result = match (opts.recover, use_elem_stream) { (true, true) => run_elems( @@ -287,6 +304,7 @@ fn main() { .into_recovering_record_iter(recovery_config) .map(|event| event.map_err(|error| error.to_string())), output_format, + opts.hex, opts.elems_count, opts.records_count, true, @@ -296,6 +314,7 @@ fn main() { .into_record_iter() .map(|record| Ok(RecoveryEvent::Item(record))), output_format, + opts.hex, opts.elems_count, opts.records_count, false, @@ -393,6 +412,7 @@ where fn run_records( events: I, output_format: OutputFormat, + include_hex: bool, elems_count_requested: bool, records_count_requested: bool, report_recovery: bool, @@ -425,7 +445,7 @@ where if records_count_requested { continue; } - let output = format_record(&record, output_format); + let output = format_record(&record, output_format, include_hex); if !write_output(&mut stdout, &output)? { return Ok(()); } @@ -480,17 +500,44 @@ fn format_elem(elem: &BgpElem, format: OutputFormat, index: usize) -> String { } } -fn format_record(record: &bgpkit_parser::MrtRecord, format: OutputFormat) -> String { +fn format_record( + record: &bgpkit_parser::MrtRecord, + format: OutputFormat, + include_hex: bool, +) -> String { + // Hex rendering re-encodes the record; records that cannot round-trip + // keep their output well-formed and note the failure on stderr. + let record_hex = include_hex.then(|| match bgpkit_parser::render::hex::format_record(record) { + Ok(hex) => Some(hex), + Err(error) => { + eprintln!( + "warning: record at {} cannot be re-encoded for --hex: {error}", + record.common_header.timestamp + ); + None + } + }); + let record_hex = record_hex.flatten(); + match format { OutputFormat::Json => { - let val = json!(record); + let mut val = json!(record); + if let (Some(hex), Some(obj)) = (&record_hex, val.as_object_mut()) { + obj.insert("hex".to_string(), json!(hex)); + } val.to_string() } OutputFormat::JsonPretty => { - let val = json!(record); + let mut val = json!(record); + if let (Some(hex), Some(obj)) = (&record_hex, val.as_object_mut()) { + obj.insert("hex".to_string(), json!(hex)); + } serde_json::to_string_pretty(&val).unwrap() } - OutputFormat::Text => bgpkit_parser::render::text::format_record(record), + OutputFormat::Text => match &record_hex { + Some(hex) => bgpkit_parser::render::text::format_record_with_hex(record, hex), + None => bgpkit_parser::render::text::format_record(record), + }, OutputFormat::Psv | OutputFormat::Default => { // Use the Display implementation for MrtRecord format!("{}", record) diff --git a/src/render/hex.rs b/src/render/hex.rs new file mode 100644 index 0000000..57d6a57 --- /dev/null +++ b/src/render/hex.rs @@ -0,0 +1,108 @@ +//! Hex rendering of MRT records. +//! +//! One record, one lowercase hex string — the paste format for +//! byte-level tools (e.g. wirescope's hex input). The record is +//! re-encoded through [`MrtRecord::encode`], which round-trips +//! byte-identically for standard records; records that cannot be +//! re-encoded surface the [`EncodingError`] to the caller. + +use crate::error::EncodingError; +use crate::models::MrtRecord; + +/// Render one MRT record as a single lowercase hex string (no separators). +/// +/// The string covers the whole record — MRT common header, BGP4MP +/// subheader, and embedded BGP message — so pasting it into a layered +/// dissector preserves the session context. +pub fn format_record(record: &MrtRecord) -> Result { + let bytes = record.encode()?; + let mut out = String::with_capacity(bytes.len() * 2); + for byte in bytes.iter() { + out.push_str(&format!("{byte:02x}")); + } + Ok(out) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::*; + use std::net::IpAddr; + use std::str::FromStr; + + /// Wire bytes for one BGP4MP_MESSAGE_AS4 record wrapping a minimal + /// UPDATE. Hex rendering must reproduce these bytes exactly. + fn wire() -> Vec { + let mut attrs = Vec::new(); + attrs.extend_from_slice(&[0x40, 0x01, 0x01, 0x00]); // ORIGIN igp + attrs.extend_from_slice(&[0x40, 0x02, 0x06, 0x02, 0x01]); + attrs.extend_from_slice(&65001u32.to_be_bytes()); // AS_PATH 65001 + attrs.extend_from_slice(&[0x40, 0x03, 0x04, 192, 0, 2, 254]); + + let mut update = Vec::new(); + update.extend_from_slice(&0u16.to_be_bytes()); + update.extend_from_slice(&(attrs.len() as u16).to_be_bytes()); + update.extend_from_slice(&attrs); + update.extend_from_slice(&[24, 203, 0, 113]); // 203.0.113.0/24 + + let mut bgp = vec![0xFF; 16]; + bgp.extend_from_slice(&((19 + update.len()) as u16).to_be_bytes()); + bgp.push(2); + bgp.extend_from_slice(&update); + + let mut body = Vec::new(); + body.extend_from_slice(&64496u32.to_be_bytes()); + body.extend_from_slice(&64497u32.to_be_bytes()); + body.extend_from_slice(&0u16.to_be_bytes()); + body.extend_from_slice(&1u16.to_be_bytes()); + body.extend_from_slice(&[192, 0, 2, 1]); + body.extend_from_slice(&[192, 0, 2, 2]); + body.extend_from_slice(&bgp); + + let mut wire = Vec::new(); + wire.extend_from_slice(&1_700_000_000u32.to_be_bytes()); + wire.extend_from_slice(&(EntryType::BGP4MP as u16).to_be_bytes()); + wire.extend_from_slice(&(Bgp4MpType::MessageAs4 as u16).to_be_bytes()); + wire.extend_from_slice(&(body.len() as u32).to_be_bytes()); + wire.extend_from_slice(&body); + wire + } + + #[test] + fn hex_render_round_trips_wire_bytes() { + let wire = wire(); + let mut cursor = std::io::Cursor::new(wire.clone()); + let record = crate::parser::mrt::parse_mrt_record(&mut cursor).unwrap(); + + let hex = format_record(&record).unwrap(); + let expected: String = wire.iter().map(|b| format!("{b:02x}")).collect(); + assert_eq!(hex, expected); + assert!(!hex.contains(|c: char| c.is_uppercase())); + } + + #[test] + fn keepalive_record_renders() { + let record = MrtRecord { + common_header: CommonHeader { + timestamp: 5, + microsecond_timestamp: None, + entry_type: EntryType::BGP4MP, + entry_subtype: Bgp4MpType::MessageAs4 as u16, + length: 0, + }, + message: MrtMessage::Bgp4Mp(Bgp4MpEnum::Message(Bgp4MpMessage { + msg_type: Bgp4MpType::MessageAs4, + peer_asn: Asn::new_32bit(64496), + local_asn: Asn::new_32bit(64497), + interface_index: 0, + peer_ip: IpAddr::from_str("192.0.2.1").unwrap(), + local_ip: IpAddr::from_str("192.0.2.2").unwrap(), + bgp_message: BgpMessage::KeepAlive, + })), + }; + let hex = format_record(&record).unwrap(); + // header(12) + subheader(20) + BGP header(19) = 51 bytes = 102 chars + assert_eq!(hex.len(), 102); + assert!(hex.starts_with("0000000500100004")); + } +} diff --git a/src/render/mod.rs b/src/render/mod.rs index 5c599c6..9777c45 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -25,4 +25,5 @@ for record in parser.into_record_iter() { ``` */ +pub mod hex; pub mod text; diff --git a/src/render/text.rs b/src/render/text.rs index de1636a..bc1db63 100644 --- a/src/render/text.rs +++ b/src/render/text.rs @@ -14,6 +14,15 @@ use crate::models::*; const INDENT: &str = " "; +/// Render a record block with a trailing `HEX:` line carrying the raw +/// bytes (e.g. from [`crate::render::hex::format_record`]), so the block +/// can be pasted straight into a byte-level dissector. +pub fn format_record_with_hex(record: &MrtRecord, hex: &str) -> String { + let mut out = format_record(record); + out.push_str(&format!("{INDENT}HEX: {hex}\n")); + out +} + /// Render one MRT record as a layered text block. /// /// Pure function of the record; see the [module docs](self) for scope and @@ -468,6 +477,15 @@ mod tests { attributes } + #[test] + fn renders_hex_line_variant() { + let text = format_record_with_hex(&update_record(full_attributes()), "deadbeef"); + assert!(text.trim_end().ends_with(" HEX: deadbeef")); + // everything before the HEX line is the plain block + let plain = format_record(&update_record(full_attributes())); + assert!(text.starts_with(&plain)); + } + #[test] fn renders_layered_update_block() { let text = format_record(&update_record(full_attributes())); From 3d1aca3b78a37c1fe8d3369e0906f74239f7c052 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Fri, 21 Aug 2026 15:58:02 -0700 Subject: [PATCH 2/6] =?UTF-8?q?fix:=20address=20review=20on=20--hex=20PR?= =?UTF-8?q?=20=E2=80=94=20original=20bytes,=20no=20re-encode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - hex now comes from the record's ORIGINAL wire bytes via a new library iterator, BgpkitParser::into_filtered_raw_record_iter(): raw MRT records with the same record-level filter semantics as into_record_iter (no-elem records drop under filters, PeerIndexTable passes, empty-filter fast path skips parsing entirely). Re-encoding the parsed model was observable-wrong: BGP-LS attributes re-encode in HashMap iteration order, varying between runs. - render::hex reduced to a pure encode(&[u8]) — no model dependency (fixes the --no-default-features build break) and no per-byte format! allocations - CLI: --hex routes through the raw pipeline; --hex + --recover is an explicit error (recovering iterators wrap parsed records) - byte-exactness verified against a real fixture: the hex field equals the record's bytes in the decompressed file exactly --- CHANGELOG.md | 2 +- src/bin/main.rs | 153 ++++++++++++++++++++++++++-------------- src/parser/iters/mod.rs | 27 ++++++- src/parser/iters/raw.rs | 149 +++++++++++++++++++++++++++++++++++++- src/render/hex.rs | 120 ++++++++----------------------- src/render/mod.rs | 5 ++ src/render/text.rs | 2 +- 7 files changed, 311 insertions(+), 147 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f479ef..2aca515 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. ### Added * **Layered text output format** (`render::text::format_record`, `--format text`): one human-readable, indented block per MRT record — session context (`TIME`/`TYPE`/`FROM`/`TO`), `UPDATE:` sections with withdrawn/announced prefixes (including those carried in MP_REACH/MP_UNREACH) and every path attribute, `OPEN:` capabilities, session states, RIB entries, the peer table, and full legacy type-5 records. RFC 7606 validation findings render under `WARNINGS:` when present. The format is designed around this crate's own models and `Display` vocabulary — inspired by bgpdump's human-readable output, not byte-compatible with it. Rendering is a pure function of the record. In the CLI, `--format text` always uses record-level output (implies `--level records`); the other formats follow `--level` and default to elems. -* **`--hex` flag**: include each record's raw bytes as hex in record-level output — a `HEX:` line at the end of `--format text` blocks, a `hex` field in `--format json`/`json-pretty` records. Designed as the pipe from filtered CLI output into byte-level tools (e.g. wirescope's hex paste input): `bgpkit-parser -p 8.8.8.0/24 --format text --hex updates.mrt.gz` yields blocks whose `HEX:` line can be pasted straight into a dissector. Implies `--level records`; requires text/json/json-pretty formats. Records that cannot be re-encoded keep well-formed output and note the failure on stderr. +* **`--hex` flag**: include each record's original wire bytes as hex in record-level output — a `HEX:` line at the end of `--format text` blocks, a `hex` field in `--format json`/`json-pretty` records. Designed as the pipe from filtered CLI output into byte-level tools (e.g. wirescope's hex paste input): `bgpkit-parser -p 8.8.8.0/24 --format text --hex updates.mrt.gz` yields blocks whose `HEX:` line can be pasted straight into a dissector. Bytes come from a new library iterator, `BgpkitParser::into_filtered_raw_record_iter()` — raw MRT records with the same record-level filter semantics as `into_record_iter`, yielding the untouched original bytes (no re-encoding, so attribute-ordering quirks such as BGP-LS hash-map iteration can never alter the output). The flag implies `--level records`, requires text/json/json-pretty formats, and is mutually exclusive with `--recover`. * Record-level filter semantics are now explicit: records that produce no elems (KEEPALIVE, OPEN, NOTIFICATION, state changes) never match elem-oriented filters and are dropped from record iteration while filters are active — a `debug!` line notes each drop. ## v0.21.0 - 2026-08-21 diff --git a/src/bin/main.rs b/src/bin/main.rs index c24c505..536c6f5 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -272,6 +272,10 @@ fn main() { eprintln!("Error: --hex requires --format text, json, or json-pretty"); std::process::exit(1); } + if opts.hex && opts.recover { + eprintln!("Error: --hex does not support --recover"); + std::process::exit(1); + } let recovery_config = RecoveryConfig::default(); // Element-level runs (element output or counting only elements) use the elem @@ -282,43 +286,53 @@ fn main() { && output_format != OutputFormat::Text && !opts.hex; - let result = match (opts.recover, use_elem_stream) { - (true, true) => run_elems( - parser - .into_recovering_elem_iter(recovery_config) - .map(|event| event.map_err(|error| error.to_string())), - output_format, - opts.elems_count, - true, - ), - (false, true) => run_elems( - parser - .into_elem_iter() - .map(|elem| Ok(RecoveryEvent::Item(elem))), - output_format, - opts.elems_count, - false, - ), - (true, false) => run_records( - parser - .into_recovering_record_iter(recovery_config) - .map(|event| event.map_err(|error| error.to_string())), + // The hex path always works on original wire bytes from the + // raw-record pipeline; it cannot share run_records, which holds + // parsed records. + let result = if opts.hex { + run_hex_records( + parser.into_filtered_raw_record_iter(), output_format, - opts.hex, opts.elems_count, opts.records_count, - true, - ), - (false, false) => run_records( - parser - .into_record_iter() - .map(|record| Ok(RecoveryEvent::Item(record))), - output_format, - opts.hex, - opts.elems_count, - opts.records_count, - false, - ), + ) + } else { + match (opts.recover, use_elem_stream) { + (true, true) => run_elems( + parser + .into_recovering_elem_iter(recovery_config) + .map(|event| event.map_err(|error| error.to_string())), + output_format, + opts.elems_count, + true, + ), + (false, true) => run_elems( + parser + .into_elem_iter() + .map(|elem| Ok(RecoveryEvent::Item(elem))), + output_format, + opts.elems_count, + false, + ), + (true, false) => run_records( + parser + .into_recovering_record_iter(recovery_config) + .map(|event| event.map_err(|error| error.to_string())), + output_format, + opts.elems_count, + opts.records_count, + true, + ), + (false, false) => run_records( + parser + .into_record_iter() + .map(|record| Ok(RecoveryEvent::Item(record))), + output_format, + opts.elems_count, + opts.records_count, + false, + ), + } }; if let Err(error) = result { eprintln!("{error}"); @@ -412,7 +426,6 @@ where fn run_records( events: I, output_format: OutputFormat, - include_hex: bool, elems_count_requested: bool, records_count_requested: bool, report_recovery: bool, @@ -445,7 +458,7 @@ where if records_count_requested { continue; } - let output = format_record(&record, output_format, include_hex); + let output = format_record(&record, output_format, None); if !write_output(&mut stdout, &output)? { return Ok(()); } @@ -466,6 +479,56 @@ where terminal_error.map_or(Ok(()), Err) } +/// Hex-augmented output over the raw-record pipeline: hex comes from the +/// original wire bytes (never a re-encoding), the block/JSON body from the +/// parsed record. +fn run_hex_records( + records: impl Iterator, + output_format: OutputFormat, + elems_count_requested: bool, + records_count_requested: bool, +) -> Result<(), String> { + let mut stdout = std::io::stdout(); + let mut elementor = Elementor::new(); + let mut records_count = 0usize; + let mut elems_count = 0usize; + + for raw in records { + records_count += 1; + if elems_count_requested || records_count_requested { + if elems_count_requested { + if let Ok(record) = raw.parse() { + elems_count += elementor.record_to_elems(record).len(); + } + } + continue; + } + let hex = bgpkit_parser::render::hex::encode(raw.raw_bytes().as_ref()); + let record = match raw.parse() { + Ok(record) => record, + Err(error) => { + eprintln!("warning: skipping unparseable record: {error}"); + continue; + } + }; + let output = format_record(&record, output_format, Some(&hex)); + if !write_output(&mut stdout, &output)? { + return Ok(()); + } + } + + match (elems_count_requested, records_count_requested) { + (true, true) => { + println!("total records: {records_count}"); + println!("total elems: {elems_count}"); + } + (false, true) => println!("total records: {records_count}"), + (true, false) => println!("total elems: {elems_count}"), + (false, false) => {} + } + Ok(()) +} + fn write_output(stdout: &mut std::io::Stdout, output: &str) -> Result { if let Err(error) = writeln!(stdout, "{output}") { if error.kind() == std::io::ErrorKind::BrokenPipe { @@ -503,22 +566,8 @@ fn format_elem(elem: &BgpElem, format: OutputFormat, index: usize) -> String { fn format_record( record: &bgpkit_parser::MrtRecord, format: OutputFormat, - include_hex: bool, + record_hex: Option<&str>, ) -> String { - // Hex rendering re-encodes the record; records that cannot round-trip - // keep their output well-formed and note the failure on stderr. - let record_hex = include_hex.then(|| match bgpkit_parser::render::hex::format_record(record) { - Ok(hex) => Some(hex), - Err(error) => { - eprintln!( - "warning: record at {} cannot be re-encoded for --hex: {error}", - record.common_header.timestamp - ); - None - } - }); - let record_hex = record_hex.flatten(); - match format { OutputFormat::Json => { let mut val = json!(record); @@ -534,7 +583,7 @@ fn format_record( } serde_json::to_string_pretty(&val).unwrap() } - OutputFormat::Text => match &record_hex { + OutputFormat::Text => match record_hex { Some(hex) => bgpkit_parser::render::text::format_record_with_hex(record, hex), None => bgpkit_parser::render::text::format_record(record), }, diff --git a/src/parser/iters/mod.rs b/src/parser/iters/mod.rs index 927e8d9..273c14e 100644 --- a/src/parser/iters/mod.rs +++ b/src/parser/iters/mod.rs @@ -25,7 +25,7 @@ pub use diagnostic::{ DissectedDiagnosticEvent, DissectingDiagnosticIterator, }; pub use fallible::{FallibleElemIterator, FallibleRecordIterator}; -pub use raw::RawRecordIterator; +pub use raw::{FilteredRawRecordIterator, RawRecordIterator}; pub use recovery::{ RecoveringElemIterator, RecoveringRecordIterator, RecoveryConfig, RecoveryError, RecoveryEvent, RecoveryEvidence, RecoveryGap, @@ -114,6 +114,31 @@ impl BgpkitParser { RawRecordIterator::new(self) } + /// Creates an iterator over raw MRT records with record-level filter + /// semantics applied. + /// + /// Like [`into_raw_record_iter`](Self::into_raw_record_iter), but only + /// records passing the parser's filters are yielded (same semantics as + /// [`into_record_iter`](Self::into_record_iter): filters match on the + /// elem projection, so no-elem records such as KEEPALIVEs are dropped + /// while filters are active, and the `PeerIndexTable` always passes). + /// The yielded records carry their original wire bytes — no + /// re-encoding — which is what byte-exact consumers (hex output, + /// re-dissection) need. + /// + /// # Example + /// ```no_run + /// use bgpkit_parser::BgpkitParser; + /// + /// let parser = BgpkitParser::new("updates.mrt").unwrap(); + /// for raw in parser.into_filtered_raw_record_iter() { + /// println!("{}", raw.raw_bytes().len()); + /// } + /// ``` + pub fn into_filtered_raw_record_iter(self) -> FilteredRawRecordIterator { + FilteredRawRecordIterator::new(self) + } + /// Creates an opt-in iterator that reports skipped byte ranges while recovering MRT framing. /// /// Recovery never reconstructs a damaged record. It scans for a structurally valid boundary, diff --git a/src/parser/iters/raw.rs b/src/parser/iters/raw.rs index 80ed3a8..19de51b 100644 --- a/src/parser/iters/raw.rs +++ b/src/parser/iters/raw.rs @@ -11,8 +11,8 @@ when possible and continue processing the remaining data. It also supports confi warning messages and core dump generation for debugging purposes. */ -use crate::parser::iters::write_mrt_core_dump; -use crate::{chunk_mrt_record, BgpkitParser, ParserError, RawMrtRecord}; +use crate::parser::iters::{record_matches_filters, write_mrt_core_dump}; +use crate::{chunk_mrt_record, BgpkitParser, Elementor, Filter, ParserError, RawMrtRecord}; use log::{error, warn}; use std::io::Read; @@ -89,3 +89,148 @@ impl Iterator for RawRecordIterator { } } } + +/// Iterator over raw MRT records with record-level filter semantics +/// applied. +/// +/// Behaves like [`RawRecordIterator`] for chunking, but only yields the +/// raw bytes of records that pass the parser's filters under the same +/// semantics as [`RecordIterator`](crate::RecordIterator): filters match +/// on the elem projection, so records that produce no elems (KEEPALIVE, +/// OPEN, NOTIFICATION, state changes) are dropped while filters are +/// active, and the `PeerIndexTable` always passes through so RIB peer +/// resolution keeps working. +/// +/// The yielded [`RawMrtRecord`]s carry the *original* wire bytes — no +/// re-encoding is involved — which is what byte-exact outputs (hex dumps, +/// re-dissection) require. +pub struct FilteredRawRecordIterator { + inner: RawRecordIterator, + elementor: Elementor, + filters: Vec, +} + +impl FilteredRawRecordIterator { + pub(crate) fn new(parser: BgpkitParser) -> Self { + let filters = parser.filters.clone(); + FilteredRawRecordIterator { + inner: RawRecordIterator::new(parser), + elementor: Elementor::new(), + filters, + } + } +} + +impl Iterator for FilteredRawRecordIterator { + type Item = RawMrtRecord; + + fn next(&mut self) -> Option { + if self.filters.is_empty() { + // No filtering needed: yield the raw chunks untouched without + // paying for a parse. + return self.inner.next(); + } + loop { + let raw = self.inner.next()?; + let record = match raw.clone().parse() { + Ok(record) => record, + Err(_) => continue, // skip unparseable, like the record iterator + }; + if record_matches_filters(&record, &self.filters, &mut self.elementor) { + return Some(raw); + } + } + } +} + +#[cfg(test)] +mod filtered_tests { + use super::*; + use crate::models::*; + use std::io::Cursor; + + fn bgp4mp_record(timestamp: u32, bgp_message: BgpMessage) -> Vec { + let body_of = |update: &BgpUpdateMessage| update.encode(AsnLength::Bits32).unwrap(); + let body = match &bgp_message { + BgpMessage::Update(update) => body_of(update), + BgpMessage::KeepAlive => Vec::new().into(), + _ => unreachable!("test builds updates and keepalives only"), + }; + let mut bgp = vec![0xFF; 16]; + bgp.extend_from_slice(&((19 + body.len()) as u16).to_be_bytes()); + bgp.push(bgp_message.msg_type() as u8); + bgp.extend_from_slice(&body); + + let mut mrt_body = Vec::new(); + mrt_body.extend_from_slice(&64496u32.to_be_bytes()); + mrt_body.extend_from_slice(&64497u32.to_be_bytes()); + mrt_body.extend_from_slice(&0u16.to_be_bytes()); + mrt_body.extend_from_slice(&1u16.to_be_bytes()); + mrt_body.extend_from_slice(&[192, 0, 2, 1]); + mrt_body.extend_from_slice(&[192, 0, 2, 2]); + mrt_body.extend_from_slice(&bgp); + + let mut wire = Vec::new(); + wire.extend_from_slice(×tamp.to_be_bytes()); + wire.extend_from_slice(&(EntryType::BGP4MP as u16).to_be_bytes()); + wire.extend_from_slice(&(Bgp4MpType::MessageAs4 as u16).to_be_bytes()); + wire.extend_from_slice(&(mrt_body.len() as u32).to_be_bytes()); + wire.extend_from_slice(&mrt_body); + wire + } + + fn keepalive(timestamp: u32) -> Vec { + bgp4mp_record(timestamp, BgpMessage::KeepAlive) + } + + fn update(timestamp: u32) -> Vec { + let mut attributes = Attributes::default(); + attributes.add_attr(AttributeValue::Origin(Origin::IGP).into()); + attributes.add_attr(AttributeValue::AsPath(AsPath::from_sequence([65000])).into()); + attributes.add_attr(AttributeValue::NextHop("192.0.2.254".parse().unwrap()).into()); + bgp4mp_record( + timestamp, + BgpMessage::Update(BgpUpdateMessage { + withdrawn_prefixes: vec![], + attributes, + announced_prefixes: vec!["198.51.100.0/24".parse().unwrap()], + }), + ) + } + + #[test] + fn no_filters_yield_original_bytes_exactly() { + let mut input = keepalive(1); + let upd = update(2); + input.extend_from_slice(&upd); + + let parser = BgpkitParser::from_reader(Cursor::new(input.clone())); + let yielded: Vec> = parser + .into_filtered_raw_record_iter() + .map(|raw| raw.raw_bytes().to_vec()) + .collect(); + assert_eq!(yielded.len(), 2); + assert_eq!(yielded[0], input[..input.len() - upd.len()].to_vec()); + assert_eq!(yielded[1], upd); + } + + #[test] + fn filters_drop_no_elem_records_and_preserve_bytes() { + let keep = keepalive(1); + let upd = update(2); + let mut input = keep.clone(); + input.extend_from_slice(&upd); + + let parser = BgpkitParser::from_reader(Cursor::new(input)) + .add_filter("prefix", "198.51.100.0/24") + .unwrap(); + let yielded: Vec> = parser + .into_filtered_raw_record_iter() + .map(|raw| raw.raw_bytes().to_vec()) + .collect(); + + // the keepalive is dropped; the update passes with byte-exact bytes + assert_eq!(yielded.len(), 1); + assert_eq!(yielded[0], upd); + } +} diff --git a/src/render/hex.rs b/src/render/hex.rs index 57d6a57..e9cd837 100644 --- a/src/render/hex.rs +++ b/src/render/hex.rs @@ -1,108 +1,48 @@ -//! Hex rendering of MRT records. +//! Hex encoding of record bytes. //! //! One record, one lowercase hex string — the paste format for -//! byte-level tools (e.g. wirescope's hex input). The record is -//! re-encoded through [`MrtRecord::encode`], which round-trips -//! byte-identically for standard records; records that cannot be -//! re-encoded surface the [`EncodingError`] to the caller. +//! byte-level tools (e.g. wirescope's hex input). Encoding operates on +//! the record's *original* bytes (see +//! [`BgpkitParser::into_filtered_raw_record_iter`](crate::BgpkitParser::into_filtered_raw_record_iter)), never on a +//! re-encoding of the parsed model, so attribute ordering quirks (e.g. +//! BGP-LS hash-map iteration order) cannot alter the output. -use crate::error::EncodingError; -use crate::models::MrtRecord; - -/// Render one MRT record as a single lowercase hex string (no separators). +/// Encode bytes as a single lowercase hex string (no separators). /// -/// The string covers the whole record — MRT common header, BGP4MP -/// subheader, and embedded BGP message — so pasting it into a layered -/// dissector preserves the session context. -pub fn format_record(record: &MrtRecord) -> Result { - let bytes = record.encode()?; +/// Writes the two digits directly into the output buffer; no +/// per-byte allocations. +pub fn encode(bytes: &[u8]) -> String { + const HEX: &[u8; 16] = b"0123456789abcdef"; let mut out = String::with_capacity(bytes.len() * 2); - for byte in bytes.iter() { - out.push_str(&format!("{byte:02x}")); + for &byte in bytes { + out.push(HEX[(byte >> 4) as usize] as char); + out.push(HEX[(byte & 0x0f) as usize] as char); } - Ok(out) + out } #[cfg(test)] mod tests { use super::*; - use crate::models::*; - use std::net::IpAddr; - use std::str::FromStr; - - /// Wire bytes for one BGP4MP_MESSAGE_AS4 record wrapping a minimal - /// UPDATE. Hex rendering must reproduce these bytes exactly. - fn wire() -> Vec { - let mut attrs = Vec::new(); - attrs.extend_from_slice(&[0x40, 0x01, 0x01, 0x00]); // ORIGIN igp - attrs.extend_from_slice(&[0x40, 0x02, 0x06, 0x02, 0x01]); - attrs.extend_from_slice(&65001u32.to_be_bytes()); // AS_PATH 65001 - attrs.extend_from_slice(&[0x40, 0x03, 0x04, 192, 0, 2, 254]); - - let mut update = Vec::new(); - update.extend_from_slice(&0u16.to_be_bytes()); - update.extend_from_slice(&(attrs.len() as u16).to_be_bytes()); - update.extend_from_slice(&attrs); - update.extend_from_slice(&[24, 203, 0, 113]); // 203.0.113.0/24 - - let mut bgp = vec![0xFF; 16]; - bgp.extend_from_slice(&((19 + update.len()) as u16).to_be_bytes()); - bgp.push(2); - bgp.extend_from_slice(&update); - - let mut body = Vec::new(); - body.extend_from_slice(&64496u32.to_be_bytes()); - body.extend_from_slice(&64497u32.to_be_bytes()); - body.extend_from_slice(&0u16.to_be_bytes()); - body.extend_from_slice(&1u16.to_be_bytes()); - body.extend_from_slice(&[192, 0, 2, 1]); - body.extend_from_slice(&[192, 0, 2, 2]); - body.extend_from_slice(&bgp); - - let mut wire = Vec::new(); - wire.extend_from_slice(&1_700_000_000u32.to_be_bytes()); - wire.extend_from_slice(&(EntryType::BGP4MP as u16).to_be_bytes()); - wire.extend_from_slice(&(Bgp4MpType::MessageAs4 as u16).to_be_bytes()); - wire.extend_from_slice(&(body.len() as u32).to_be_bytes()); - wire.extend_from_slice(&body); - wire - } #[test] - fn hex_render_round_trips_wire_bytes() { - let wire = wire(); - let mut cursor = std::io::Cursor::new(wire.clone()); - let record = crate::parser::mrt::parse_mrt_record(&mut cursor).unwrap(); - - let hex = format_record(&record).unwrap(); - let expected: String = wire.iter().map(|b| format!("{b:02x}")).collect(); - assert_eq!(hex, expected); - assert!(!hex.contains(|c: char| c.is_uppercase())); + fn encodes_golden_values() { + assert_eq!(encode(&[]), ""); + assert_eq!(encode(&[0x00]), "00"); + assert_eq!(encode(&[0xde, 0xad, 0xbe, 0xef]), "deadbeef"); + assert_eq!(encode(&[0xff, 0x10, 0x0a]), "ff100a"); } #[test] - fn keepalive_record_renders() { - let record = MrtRecord { - common_header: CommonHeader { - timestamp: 5, - microsecond_timestamp: None, - entry_type: EntryType::BGP4MP, - entry_subtype: Bgp4MpType::MessageAs4 as u16, - length: 0, - }, - message: MrtMessage::Bgp4Mp(Bgp4MpEnum::Message(Bgp4MpMessage { - msg_type: Bgp4MpType::MessageAs4, - peer_asn: Asn::new_32bit(64496), - local_asn: Asn::new_32bit(64497), - interface_index: 0, - peer_ip: IpAddr::from_str("192.0.2.1").unwrap(), - local_ip: IpAddr::from_str("192.0.2.2").unwrap(), - bgp_message: BgpMessage::KeepAlive, - })), - }; - let hex = format_record(&record).unwrap(); - // header(12) + subheader(20) + BGP header(19) = 51 bytes = 102 chars - assert_eq!(hex.len(), 102); - assert!(hex.starts_with("0000000500100004")); + fn encodes_all_byte_values_lowercase() { + let all: Vec = (0..=255u8).collect(); + let hex = encode(&all); + assert_eq!(hex.len(), 512); + assert!(!hex.contains(|c: char| c.is_uppercase())); + // spot checks across the range + assert_eq!(&hex[0..2], "00"); + assert_eq!(&hex[9 * 2..10 * 2], "09"); + assert_eq!(&hex[10 * 2..12 * 2], "0a0b"); + assert_eq!(&hex[510..512], "ff"); } } diff --git a/src/render/mod.rs b/src/render/mod.rs index 9777c45..61e798b 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -8,6 +8,11 @@ validation warnings. The format is designed around this crate's own models (reusing the leaf `Display` implementations), not around any external tool's output; it is *inspired by bgpdump's human-readable output*. +The [`hex`] module encodes raw record bytes (as yielded by +[`BgpkitParser::into_filtered_raw_record_iter`](crate::BgpkitParser::into_filtered_raw_record_iter)) +as single hex strings — the paste format for byte-level dissectors such as +[wirescope](https://wirescope.labs.bgpkit.com). + Rendering is a pure function of the record: no iterators, no I/O, no session state. RIB entries reference peers by their table index because the peer table lives in a separate, earlier record. diff --git a/src/render/text.rs b/src/render/text.rs index bc1db63..b6ce197 100644 --- a/src/render/text.rs +++ b/src/render/text.rs @@ -15,7 +15,7 @@ use crate::models::*; const INDENT: &str = " "; /// Render a record block with a trailing `HEX:` line carrying the raw -/// bytes (e.g. from [`crate::render::hex::format_record`]), so the block +/// bytes (e.g. from [`crate::render::hex::encode`]), so the block /// can be pasted straight into a byte-level dissector. pub fn format_record_with_hex(record: &MrtRecord, hex: &str) -> String { let mut out = format_record(record); From d19b0de70894415a883e96e4ce598fc88389286b Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Fri, 21 Aug 2026 16:07:34 -0700 Subject: [PATCH 3/6] fix: hex count-run semantics + silent parse failures in filtered raw iter - count-only runs (-e/-r) ignore --hex entirely: they emit no hex, so they keep the normal elem/record counting pipelines and their per-elem filter semantics (verified: -e and -e --hex report identical totals); the format/recover validations also skip count-only runs - FilteredRawRecordIterator mirrors RecordIterator's body-parse failure handling under filters: logged via error! and core-dumped when enabled, never silently dropped --- CHANGELOG.md | 2 +- src/bin/main.rs | 63 +++++++++++++---------------------------- src/parser/iters/raw.rs | 11 ++++++- 3 files changed, 31 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aca515..0ce1201 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. ### Added * **Layered text output format** (`render::text::format_record`, `--format text`): one human-readable, indented block per MRT record — session context (`TIME`/`TYPE`/`FROM`/`TO`), `UPDATE:` sections with withdrawn/announced prefixes (including those carried in MP_REACH/MP_UNREACH) and every path attribute, `OPEN:` capabilities, session states, RIB entries, the peer table, and full legacy type-5 records. RFC 7606 validation findings render under `WARNINGS:` when present. The format is designed around this crate's own models and `Display` vocabulary — inspired by bgpdump's human-readable output, not byte-compatible with it. Rendering is a pure function of the record. In the CLI, `--format text` always uses record-level output (implies `--level records`); the other formats follow `--level` and default to elems. -* **`--hex` flag**: include each record's original wire bytes as hex in record-level output — a `HEX:` line at the end of `--format text` blocks, a `hex` field in `--format json`/`json-pretty` records. Designed as the pipe from filtered CLI output into byte-level tools (e.g. wirescope's hex paste input): `bgpkit-parser -p 8.8.8.0/24 --format text --hex updates.mrt.gz` yields blocks whose `HEX:` line can be pasted straight into a dissector. Bytes come from a new library iterator, `BgpkitParser::into_filtered_raw_record_iter()` — raw MRT records with the same record-level filter semantics as `into_record_iter`, yielding the untouched original bytes (no re-encoding, so attribute-ordering quirks such as BGP-LS hash-map iteration can never alter the output). The flag implies `--level records`, requires text/json/json-pretty formats, and is mutually exclusive with `--recover`. +* **`--hex` flag**: include each record's original wire bytes as hex in record-level output — a `HEX:` line at the end of `--format text` blocks, a `hex` field in `--format json`/`json-pretty` records. Designed as the pipe from filtered CLI output into byte-level tools (e.g. wirescope's hex paste input): `bgpkit-parser -p 8.8.8.0/24 --format text --hex updates.mrt.gz` yields blocks whose `HEX:` line can be pasted straight into a dissector. Bytes come from a new library iterator, `BgpkitParser::into_filtered_raw_record_iter()` — raw MRT records with the same record-level filter semantics as `into_record_iter`, yielding the untouched original bytes (no re-encoding, so attribute-ordering quirks such as BGP-LS hash-map iteration can never alter the output). The flag implies `--level records`, requires text/json/json-pretty formats, is mutually exclusive with `--recover`, and is ignored by count-only runs (`-e`/`-r`), which keep the normal counting pipelines and their per-elem filter semantics. * Record-level filter semantics are now explicit: records that produce no elems (KEEPALIVE, OPEN, NOTIFICATION, state changes) never match elem-oriented filters and are dropped from record iteration while filters are active — a `debug!` line notes each drop. ## v0.21.0 - 2026-08-21 diff --git a/src/bin/main.rs b/src/bin/main.rs index 536c6f5..772644a 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -71,7 +71,8 @@ struct Opts { /// Include each record's raw bytes as hex: a `HEX:` line in text /// blocks, a `hex` field in JSON records. Record-level only — implies - /// `--level records`; requires `--format text`, `json`, or `json-pretty`. + /// `--level records`; requires `--format text`, `json`, or + /// `json-pretty`. Count-only runs (-e/-r) ignore this flag. #[clap(long)] hex: bool, @@ -263,18 +264,21 @@ fn main() { opts.format }; - if opts.hex - && !matches!( + // Count-only runs emit no hex, so the flag is ignored there (and the + // format/recover constraints do not apply). + let counting = opts.elems_count || opts.records_count; + if opts.hex && !counting { + if !matches!( output_format, OutputFormat::Json | OutputFormat::JsonPretty | OutputFormat::Text - ) - { - eprintln!("Error: --hex requires --format text, json, or json-pretty"); - std::process::exit(1); - } - if opts.hex && opts.recover { - eprintln!("Error: --hex does not support --recover"); - std::process::exit(1); + ) { + eprintln!("Error: --hex requires --format text, json, or json-pretty"); + std::process::exit(1); + } + if opts.recover { + eprintln!("Error: --hex does not support --recover"); + std::process::exit(1); + } } let recovery_config = RecoveryConfig::default(); @@ -288,14 +292,11 @@ fn main() { // The hex path always works on original wire bytes from the // raw-record pipeline; it cannot share run_records, which holds - // parsed records. - let result = if opts.hex { - run_hex_records( - parser.into_filtered_raw_record_iter(), - output_format, - opts.elems_count, - opts.records_count, - ) + // parsed records. Count-only runs emit no hex, so they keep the + // normal elem/record counting pipelines and their semantics + // (per-elem filtering for -e). + let result = if opts.hex && !counting { + run_hex_records(parser.into_filtered_raw_record_iter(), output_format) } else { match (opts.recover, use_elem_stream) { (true, true) => run_elems( @@ -485,24 +486,10 @@ where fn run_hex_records( records: impl Iterator, output_format: OutputFormat, - elems_count_requested: bool, - records_count_requested: bool, ) -> Result<(), String> { let mut stdout = std::io::stdout(); - let mut elementor = Elementor::new(); - let mut records_count = 0usize; - let mut elems_count = 0usize; for raw in records { - records_count += 1; - if elems_count_requested || records_count_requested { - if elems_count_requested { - if let Ok(record) = raw.parse() { - elems_count += elementor.record_to_elems(record).len(); - } - } - continue; - } let hex = bgpkit_parser::render::hex::encode(raw.raw_bytes().as_ref()); let record = match raw.parse() { Ok(record) => record, @@ -516,16 +503,6 @@ fn run_hex_records( return Ok(()); } } - - match (elems_count_requested, records_count_requested) { - (true, true) => { - println!("total records: {records_count}"); - println!("total elems: {elems_count}"); - } - (false, true) => println!("total records: {records_count}"), - (true, false) => println!("total elems: {elems_count}"), - (false, false) => {} - } Ok(()) } diff --git a/src/parser/iters/raw.rs b/src/parser/iters/raw.rs index 19de51b..9c5ad55 100644 --- a/src/parser/iters/raw.rs +++ b/src/parser/iters/raw.rs @@ -108,15 +108,18 @@ pub struct FilteredRawRecordIterator { inner: RawRecordIterator, elementor: Elementor, filters: Vec, + core_dump: bool, } impl FilteredRawRecordIterator { pub(crate) fn new(parser: BgpkitParser) -> Self { let filters = parser.filters.clone(); + let core_dump = parser.core_dump; FilteredRawRecordIterator { inner: RawRecordIterator::new(parser), elementor: Elementor::new(), filters, + core_dump, } } } @@ -132,9 +135,15 @@ impl Iterator for FilteredRawRecordIterator { } loop { let raw = self.inner.next()?; + // Body-parse failures keep the record iterator's diagnostics: + // logged and core-dumped when enabled, never silent. let record = match raw.clone().parse() { Ok(record) => record, - Err(_) => continue, // skip unparseable, like the record iterator + Err(error) => { + error!("parser error: {error}"); + write_mrt_core_dump(self.core_dump, Some(raw.raw_bytes().to_vec())); + continue; + } }; if record_matches_filters(&record, &self.filters, &mut self.elementor) { return Some(raw); From 30a0ade22fb0114b41832cacfee8db3f99165c57 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Fri, 21 Aug 2026 16:31:45 -0700 Subject: [PATCH 4/6] fix: third review round on --hex PR - use_elem_stream no longer special-cases --hex: count-only runs keep the elem pipeline and per-elem filter semantics (verified parity with filters: -e -p and -e --hex -p report identical totals) - FilteredRawRecordIterator yields (raw, Option): the parse done for filtering is reused by consumers instead of reparsed; the no-filter fast path still skips parsing - shortened Zebra BGP4MP records are detected and warned once in the raw pipeline, matching the record iterator's data-quality diagnostic --- src/bin/main.rs | 29 +++++++++++++++++--------- src/parser/iters/mod.rs | 6 ++++-- src/parser/iters/raw.rs | 45 ++++++++++++++++++++++++++++++++--------- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 772644a..78f44ca 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -287,8 +287,7 @@ fn main() { // level. Counting both (-e -r) iterates records and converts once per record. let use_elem_stream = ((opts.elems_count && !opts.records_count) || (!opts.elems_count && !opts.records_count && matches!(opts.level, OutputLevel::Elems))) - && output_format != OutputFormat::Text - && !opts.hex; + && output_format != OutputFormat::Text; // The hex path always works on original wire bytes from the // raw-record pipeline; it cannot share run_records, which holds @@ -484,19 +483,29 @@ where /// original wire bytes (never a re-encoding), the block/JSON body from the /// parsed record. fn run_hex_records( - records: impl Iterator, + records: impl Iterator< + Item = ( + bgpkit_parser::RawMrtRecord, + Option, + ), + >, output_format: OutputFormat, ) -> Result<(), String> { let mut stdout = std::io::stdout(); - for raw in records { + for (raw, parsed) in records { let hex = bgpkit_parser::render::hex::encode(raw.raw_bytes().as_ref()); - let record = match raw.parse() { - Ok(record) => record, - Err(error) => { - eprintln!("warning: skipping unparseable record: {error}"); - continue; - } + // The iterator parsed the record for filtering when filters are + // active; reuse that parse instead of a second pass. + let record = match parsed { + Some(record) => record, + None => match raw.parse() { + Ok(record) => record, + Err(error) => { + eprintln!("warning: skipping unparseable record: {error}"); + continue; + } + }, }; let output = format_record(&record, output_format, Some(&hex)); if !write_output(&mut stdout, &output)? { diff --git a/src/parser/iters/mod.rs b/src/parser/iters/mod.rs index 273c14e..7718841 100644 --- a/src/parser/iters/mod.rs +++ b/src/parser/iters/mod.rs @@ -124,14 +124,16 @@ impl BgpkitParser { /// while filters are active, and the `PeerIndexTable` always passes). /// The yielded records carry their original wire bytes — no /// re-encoding — which is what byte-exact consumers (hex output, - /// re-dissection) need. + /// re-dissection) need. When filters required a parse for matching, + /// the parsed record is yielded alongside so consumers do not parse + /// the bytes twice; the no-filter fast path yields `None` for it. /// /// # Example /// ```no_run /// use bgpkit_parser::BgpkitParser; /// /// let parser = BgpkitParser::new("updates.mrt").unwrap(); - /// for raw in parser.into_filtered_raw_record_iter() { + /// for (raw, _) in parser.into_filtered_raw_record_iter() { /// println!("{}", raw.raw_bytes().len()); /// } /// ``` diff --git a/src/parser/iters/raw.rs b/src/parser/iters/raw.rs index 9c5ad55..386769a 100644 --- a/src/parser/iters/raw.rs +++ b/src/parser/iters/raw.rs @@ -12,7 +12,10 @@ warning messages and core dump generation for debugging purposes. */ use crate::parser::iters::{record_matches_filters, write_mrt_core_dump}; -use crate::{chunk_mrt_record, BgpkitParser, Elementor, Filter, ParserError, RawMrtRecord}; +use crate::parser::mrt::mrt_record::raw_record_uses_zebra_compat; +use crate::{ + chunk_mrt_record, BgpkitParser, Elementor, Filter, MrtRecord, ParserError, RawMrtRecord, +}; use log::{error, warn}; use std::io::Read; @@ -103,12 +106,19 @@ impl Iterator for RawRecordIterator { /// /// The yielded [`RawMrtRecord`]s carry the *original* wire bytes — no /// re-encoding is involved — which is what byte-exact outputs (hex dumps, -/// re-dissection) require. +/// re-dissection) require. When filters required a parse for matching, +/// the parsed record is yielded alongside (`Some`), so consumers that +/// render the record do not need to parse the bytes a second time; the +/// no-filter fast path skips parsing and yields `None`. +/// +/// Shortened Zebra BGP4MP records are detected and warned about once, +/// matching the record iterator's data-quality diagnostic. pub struct FilteredRawRecordIterator { inner: RawRecordIterator, elementor: Elementor, filters: Vec, core_dump: bool, + warned_zebra_compat: bool, } impl FilteredRawRecordIterator { @@ -120,21 +130,34 @@ impl FilteredRawRecordIterator { elementor: Elementor::new(), filters, core_dump, + warned_zebra_compat: false, + } + } + + fn warn_zebra_once(&mut self, raw: &RawMrtRecord) { + if !self.warned_zebra_compat && raw_record_uses_zebra_compat(raw) { + warn!( + "recovered shortened Zebra BGP4MP records with missing envelope fields; substituting IPv4 zero addresses and interface index 0 (further occurrences for this parser will not be logged)" + ); + self.warned_zebra_compat = true; } } } impl Iterator for FilteredRawRecordIterator { - type Item = RawMrtRecord; + type Item = (RawMrtRecord, Option); - fn next(&mut self) -> Option { + fn next(&mut self) -> Option { if self.filters.is_empty() { // No filtering needed: yield the raw chunks untouched without // paying for a parse. - return self.inner.next(); + let raw = self.inner.next()?; + self.warn_zebra_once(&raw); + return Some((raw, None)); } loop { let raw = self.inner.next()?; + self.warn_zebra_once(&raw); // Body-parse failures keep the record iterator's diagnostics: // logged and core-dumped when enabled, never silent. let record = match raw.clone().parse() { @@ -146,7 +169,7 @@ impl Iterator for FilteredRawRecordIterator { } }; if record_matches_filters(&record, &self.filters, &mut self.elementor) { - return Some(raw); + return Some((raw, Some(record))); } } } @@ -216,7 +239,7 @@ mod filtered_tests { let parser = BgpkitParser::from_reader(Cursor::new(input.clone())); let yielded: Vec> = parser .into_filtered_raw_record_iter() - .map(|raw| raw.raw_bytes().to_vec()) + .map(|(raw, _)| raw.raw_bytes().to_vec()) .collect(); assert_eq!(yielded.len(), 2); assert_eq!(yielded[0], input[..input.len() - upd.len()].to_vec()); @@ -233,13 +256,15 @@ mod filtered_tests { let parser = BgpkitParser::from_reader(Cursor::new(input)) .add_filter("prefix", "198.51.100.0/24") .unwrap(); - let yielded: Vec> = parser + let yielded: Vec<(Vec, bool)> = parser .into_filtered_raw_record_iter() - .map(|raw| raw.raw_bytes().to_vec()) + .map(|(raw, record)| (raw.raw_bytes().to_vec(), record.is_some())) .collect(); // the keepalive is dropped; the update passes with byte-exact bytes + // and its parse carried along (no re-parse needed downstream) assert_eq!(yielded.len(), 1); - assert_eq!(yielded[0], upd); + assert_eq!(yielded[0].0, upd); + assert!(yielded[0].1); } } From 2261364af77313590e925ff6bd95d505aa97d2d4 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Fri, 21 Aug 2026 16:40:34 -0700 Subject: [PATCH 5/6] =?UTF-8?q?fix:=20fourth=20review=20round=20=E2=80=94?= =?UTF-8?q?=20delegate=20parser=20state,=20shared=20error=20policy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - FilteredRawRecordIterator's Zebra warning delegates to the parser's own warn_zebra_compat_once() state instead of a second iterator-local flag: disable_warnings() is honored, and a parser that already warned before conversion cannot warn again - body-parse error handling extracted into a shared variant-aware policy (handle_record_parse_error) used by both RecordIterator and the filtered raw iterator: TruncatedMsg/Unsupported and labeled-NLRI failures warn under show_warnings, ParseError with core dumps enabled stops after writing the dump so later failures cannot overwrite it, IO errors stop — identical to the historical RecordIterator behavior --- src/parser/iters/default.rs | 52 +++----------------------------- src/parser/iters/mod.rs | 60 ++++++++++++++++++++++++++++++++++++- src/parser/iters/raw.rs | 33 ++++++++++---------- 3 files changed, 81 insertions(+), 64 deletions(-) diff --git a/src/parser/iters/default.rs b/src/parser/iters/default.rs index 0edb466..9408f65 100644 --- a/src/parser/iters/default.rs +++ b/src/parser/iters/default.rs @@ -1,12 +1,10 @@ /*! Default iterator implementations that skip errors and return successfully parsed items. */ -use crate::error::ParserError; use crate::models::*; -use crate::parser::iters::{record_matches_filters, write_mrt_core_dump}; +use crate::parser::iters::{handle_record_parse_error, record_matches_filters}; use crate::parser::BgpkitParser; use crate::{Elementor, Filterable}; -use log::{error, warn}; use std::io::Read; /********* @@ -49,52 +47,10 @@ impl Iterator for RecordIterator { } } Err(e) => { - match e.error { - ParserError::TruncatedMsg(err_str) | ParserError::Unsupported(err_str) => { - if self.parser.options.show_warnings { - warn!("parser warn: {}", err_str); - } - write_mrt_core_dump(self.parser.core_dump, e.bytes); - continue; - } - ParserError::ParseError(err_str) => { - error!("parser error: {}", err_str); - write_mrt_core_dump(self.parser.core_dump, e.bytes); - if self.parser.core_dump { - None - } else { - continue; - } - } - ParserError::EofExpected => { - // normal end of file - None - } - ParserError::IoError(err) | ParserError::EofError(err) => { - // when reaching IO error, stop iterating - error!("{:?}", err); - write_mrt_core_dump(self.parser.core_dump, e.bytes); - None - } - #[cfg(feature = "oneio")] - ParserError::OneIoError(_) => None, - ParserError::FilterError(_) => { - // this should not happen at this stage - None - } - // Labeled NLRI parsing errors - treat as malformed and skip - ParserError::InvalidLabeledNlriLength - | ParserError::TruncatedLabeledNlri - | ParserError::TruncatedPrefix - | ParserError::MaxLabelStackDepthExceeded - | ParserError::PeerMaxLabelsExceeded - | ParserError::InvalidPrefix => { - if self.parser.options.show_warnings { - warn!("parser warn: labeled NLRI parsing error: {:?}", e.error); - } - continue; - } + if handle_record_parse_error(&mut self.parser, e.error, e.bytes) { + continue; } + None } }; } diff --git a/src/parser/iters/mod.rs b/src/parser/iters/mod.rs index 7718841..2449e58 100644 --- a/src/parser/iters/mod.rs +++ b/src/parser/iters/mod.rs @@ -36,12 +36,13 @@ pub use update::{ UpdateIterator, }; +use crate::error::ParserError; use crate::models::BgpElem; use crate::models::{MrtMessage, MrtRecord, TableDumpV2Message}; use crate::parser::BgpkitParser; use crate::RawMrtRecord; use crate::{Elementor, Filter, Filterable}; -use log::debug; +use log::{debug, error, warn}; use std::io::Read; use std::path::Path; @@ -75,6 +76,63 @@ pub(crate) fn record_matches_filters( elems.iter().any(|element| element.match_filters(filters)) } +/// Shared body-parse error policy for record-producing iterators. +/// +/// Mirrors the historical `RecordIterator` behavior: warnings honor +/// `disable_warnings()`, core dumps are written for recoverable classes, +/// and a fatal `ParseError` with core dumps enabled stops the iterator so +/// a later failure cannot overwrite the dump. Returns `true` to continue +/// iterating, `false` to stop. +pub(crate) fn handle_record_parse_error( + parser: &mut crate::parser::BgpkitParser, + error: ParserError, + bytes: Option>, +) -> bool { + match error { + ParserError::TruncatedMsg(err_str) | ParserError::Unsupported(err_str) => { + if parser.options.show_warnings { + warn!("parser warn: {}", err_str); + } + write_mrt_core_dump(parser.core_dump, bytes); + true + } + ParserError::ParseError(err_str) => { + error!("parser error: {}", err_str); + write_mrt_core_dump(parser.core_dump, bytes); + // stop after writing the dump so later failures don't overwrite it + !parser.core_dump + } + ParserError::EofExpected => { + // normal end of file + false + } + ParserError::IoError(err) | ParserError::EofError(err) => { + // when reaching IO error, stop iterating + error!("{:?}", err); + write_mrt_core_dump(parser.core_dump, bytes); + false + } + #[cfg(feature = "oneio")] + ParserError::OneIoError(_) => false, + ParserError::FilterError(_) => { + // this should not happen at this stage + false + } + // Labeled NLRI parsing errors - treat as malformed and skip + ParserError::InvalidLabeledNlriLength + | ParserError::TruncatedLabeledNlri + | ParserError::TruncatedPrefix + | ParserError::MaxLabelStackDepthExceeded + | ParserError::PeerMaxLabelsExceeded + | ParserError::InvalidPrefix => { + if parser.options.show_warnings { + warn!("parser warn: labeled NLRI parsing error: {:?}", error); + } + true + } + } +} + pub(crate) fn write_mrt_core_dump(enabled: bool, bytes: Option>) { write_mrt_core_dump_to_path(enabled, bytes, "mrt_core_dump"); } diff --git a/src/parser/iters/raw.rs b/src/parser/iters/raw.rs index 386769a..31add07 100644 --- a/src/parser/iters/raw.rs +++ b/src/parser/iters/raw.rs @@ -11,7 +11,9 @@ when possible and continue processing the remaining data. It also supports confi warning messages and core dump generation for debugging purposes. */ -use crate::parser::iters::{record_matches_filters, write_mrt_core_dump}; +use crate::parser::iters::{ + handle_record_parse_error, record_matches_filters, write_mrt_core_dump, +}; use crate::parser::mrt::mrt_record::raw_record_uses_zebra_compat; use crate::{ chunk_mrt_record, BgpkitParser, Elementor, Filter, MrtRecord, ParserError, RawMrtRecord, @@ -117,29 +119,24 @@ pub struct FilteredRawRecordIterator { inner: RawRecordIterator, elementor: Elementor, filters: Vec, - core_dump: bool, - warned_zebra_compat: bool, } impl FilteredRawRecordIterator { pub(crate) fn new(parser: BgpkitParser) -> Self { let filters = parser.filters.clone(); - let core_dump = parser.core_dump; FilteredRawRecordIterator { inner: RawRecordIterator::new(parser), elementor: Elementor::new(), filters, - core_dump, - warned_zebra_compat: false, } } + /// Zebra detection delegates to the parser's own warn-once state, so + /// `disable_warnings()` is honored and a parser that already warned + /// before conversion does not warn again. fn warn_zebra_once(&mut self, raw: &RawMrtRecord) { - if !self.warned_zebra_compat && raw_record_uses_zebra_compat(raw) { - warn!( - "recovered shortened Zebra BGP4MP records with missing envelope fields; substituting IPv4 zero addresses and interface index 0 (further occurrences for this parser will not be logged)" - ); - self.warned_zebra_compat = true; + if raw_record_uses_zebra_compat(raw) { + self.inner.parser.warn_zebra_compat_once(); } } } @@ -158,13 +155,19 @@ impl Iterator for FilteredRawRecordIterator { loop { let raw = self.inner.next()?; self.warn_zebra_once(&raw); - // Body-parse failures keep the record iterator's diagnostics: - // logged and core-dumped when enabled, never silent. + // Body-parse failures share the record iterator's + // variant-aware error policy (warnings vs. errors, core + // dumps, stop-after-dump). let record = match raw.clone().parse() { Ok(record) => record, Err(error) => { - error!("parser error: {error}"); - write_mrt_core_dump(self.core_dump, Some(raw.raw_bytes().to_vec())); + if !handle_record_parse_error( + &mut self.inner.parser, + error, + Some(raw.raw_bytes().to_vec()), + ) { + return None; + } continue; } }; From 2470b8eac8e242e4382f13dad8d73fc93a2a234a Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Fri, 21 Aug 2026 16:51:36 -0700 Subject: [PATCH 6/6] =?UTF-8?q?fix:=20fifth=20review=20round=20=E2=80=94?= =?UTF-8?q?=20uniform=20error=20policy=20on=20every=20hex=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FilteredRawRecordIterator now parses every record body and routes all parse failures through the shared variant-aware policy, on filtered and unfiltered paths alike. The CLI hex runner's no-filter fallback (a bare warn-and-continue eprintln) is deleted along with the Option in the yielded item: consumers always receive (raw, record), never re-parse, and the pipeline cannot continue past errors the record pipeline would stop for (fatal ParseError with core dumps, IO errors). Note: the first two comments in this round restate the fourth-round findings (zebra warning state, per-class error policy) that 2261364 already addressed in this file; this change extends the same treatment to the previously-uncovered no-filter CLI path. --- src/bin/main.rs | 21 ++------------------- src/parser/iters/mod.rs | 7 ++++--- src/parser/iters/raw.rs | 37 +++++++++++++++++-------------------- 3 files changed, 23 insertions(+), 42 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 78f44ca..be5cd4c 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -483,30 +483,13 @@ where /// original wire bytes (never a re-encoding), the block/JSON body from the /// parsed record. fn run_hex_records( - records: impl Iterator< - Item = ( - bgpkit_parser::RawMrtRecord, - Option, - ), - >, + records: impl Iterator, output_format: OutputFormat, ) -> Result<(), String> { let mut stdout = std::io::stdout(); - for (raw, parsed) in records { + for (raw, record) in records { let hex = bgpkit_parser::render::hex::encode(raw.raw_bytes().as_ref()); - // The iterator parsed the record for filtering when filters are - // active; reuse that parse instead of a second pass. - let record = match parsed { - Some(record) => record, - None => match raw.parse() { - Ok(record) => record, - Err(error) => { - eprintln!("warning: skipping unparseable record: {error}"); - continue; - } - }, - }; let output = format_record(&record, output_format, Some(&hex)); if !write_output(&mut stdout, &output)? { return Ok(()); diff --git a/src/parser/iters/mod.rs b/src/parser/iters/mod.rs index 2449e58..ee21eab 100644 --- a/src/parser/iters/mod.rs +++ b/src/parser/iters/mod.rs @@ -182,9 +182,10 @@ impl BgpkitParser { /// while filters are active, and the `PeerIndexTable` always passes). /// The yielded records carry their original wire bytes — no /// re-encoding — which is what byte-exact consumers (hex output, - /// re-dissection) need. When filters required a parse for matching, - /// the parsed record is yielded alongside so consumers do not parse - /// the bytes twice; the no-filter fast path yields `None` for it. + /// re-dissection) need. Every record body is parsed once inside the + /// iterator and the parsed record is yielded alongside, so consumers + /// do not parse the bytes twice; parse failures follow the same + /// variant-aware diagnostics as the record iterator. /// /// # Example /// ```no_run diff --git a/src/parser/iters/raw.rs b/src/parser/iters/raw.rs index 31add07..de66bb7 100644 --- a/src/parser/iters/raw.rs +++ b/src/parser/iters/raw.rs @@ -108,10 +108,14 @@ impl Iterator for RawRecordIterator { /// /// The yielded [`RawMrtRecord`]s carry the *original* wire bytes — no /// re-encoding is involved — which is what byte-exact outputs (hex dumps, -/// re-dissection) require. When filters required a parse for matching, -/// the parsed record is yielded alongside (`Some`), so consumers that -/// render the record do not need to parse the bytes a second time; the -/// no-filter fast path skips parsing and yields `None`. +/// re-dissection) require. Every record body is parsed once inside the +/// iterator (for filter matching and for uniform error diagnostics), and +/// the parsed record is yielded alongside so consumers never parse the +/// bytes twice. Parse failures go through the same variant-aware policy +/// as [`RecordIterator`](crate::RecordIterator) — warnings honor +/// `disable_warnings()`, core dumps are written where applicable, and +/// fatal classes stop the iterator — so this pipeline cannot continue +/// past errors the record pipeline would stop for. /// /// Shortened Zebra BGP4MP records are detected and warned about once, /// matching the record iterator's data-quality diagnostic. @@ -142,22 +146,15 @@ impl FilteredRawRecordIterator { } impl Iterator for FilteredRawRecordIterator { - type Item = (RawMrtRecord, Option); + type Item = (RawMrtRecord, MrtRecord); fn next(&mut self) -> Option { - if self.filters.is_empty() { - // No filtering needed: yield the raw chunks untouched without - // paying for a parse. - let raw = self.inner.next()?; - self.warn_zebra_once(&raw); - return Some((raw, None)); - } loop { let raw = self.inner.next()?; self.warn_zebra_once(&raw); // Body-parse failures share the record iterator's // variant-aware error policy (warnings vs. errors, core - // dumps, stop-after-dump). + // dumps, stop-after-dump) — on every path, filtered or not. let record = match raw.clone().parse() { Ok(record) => record, Err(error) => { @@ -171,8 +168,10 @@ impl Iterator for FilteredRawRecordIterator { continue; } }; - if record_matches_filters(&record, &self.filters, &mut self.elementor) { - return Some((raw, Some(record))); + if self.filters.is_empty() + || record_matches_filters(&record, &self.filters, &mut self.elementor) + { + return Some((raw, record)); } } } @@ -259,15 +258,13 @@ mod filtered_tests { let parser = BgpkitParser::from_reader(Cursor::new(input)) .add_filter("prefix", "198.51.100.0/24") .unwrap(); - let yielded: Vec<(Vec, bool)> = parser + let yielded: Vec> = parser .into_filtered_raw_record_iter() - .map(|(raw, record)| (raw.raw_bytes().to_vec(), record.is_some())) + .map(|(raw, _)| raw.raw_bytes().to_vec()) .collect(); // the keepalive is dropped; the update passes with byte-exact bytes - // and its parse carried along (no re-parse needed downstream) assert_eq!(yielded.len(), 1); - assert_eq!(yielded[0].0, upd); - assert!(yielded[0].1); + assert_eq!(yielded[0], upd); } }