feat: --hex flag augments record-level output with raw record bytes - #334
Conversation
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.
There was a problem hiding this comment.
Pull request overview
Adds record-level hexadecimal output for CLI text and JSON formats.
Changes:
- Adds an MRT record hex renderer and tests.
- Introduces
--hexvalidation and output integration. - Documents the feature and adds text rendering support.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/render/hex.rs |
Encodes records as lowercase hexadecimal. |
src/render/text.rs |
Appends HEX: lines to text blocks. |
src/render/mod.rs |
Exposes the hex renderer. |
src/bin/main.rs |
Adds and processes the --hex option. |
CHANGELOG.md |
Documents the new flag. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ``` | ||
| */ | ||
|
|
||
| pub mod hex; |
| pub fn format_record(record: &MrtRecord) -> Result<String, EncodingError> { | ||
| let bytes = record.encode()?; |
| let mut out = String::with_capacity(bytes.len() * 2); | ||
| for byte in bytes.iter() { | ||
| out.push_str(&format!("{byte:02x}")); | ||
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #334 +/- ##
==========================================
- Coverage 90.82% 90.82% -0.01%
==========================================
Files 102 103 +1
Lines 25567 25720 +153
==========================================
+ Hits 23222 23360 +138
- Misses 2345 2360 +15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- 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
|
All 3 review comments addressed in 3d1aca3 — and comment 2 drove a design change, not just a patch:
Also: Gates: |
| if let Ok(record) = raw.parse() { | ||
| elems_count += elementor.record_to_elems(record).len(); | ||
| } |
| let record = match raw.clone().parse() { | ||
| Ok(record) => record, | ||
| Err(_) => continue, // skip unparseable, like the record iterator |
…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
|
Both follow-up comments addressed in d19b0de:
Gates: 958 tests / 0 failed, clippy |
|
|
||
| for raw in records { | ||
| let hex = bgpkit_parser::render::hex::encode(raw.raw_bytes().as_ref()); | ||
| let record = match raw.parse() { |
| // 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) |
| //! 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_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<MrtRecord>): 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
|
All 4 comments from the third round addressed in 30a0ade:
Gates: 958 tests / 0 failed, clippy |
| warned_zebra_compat: bool, | ||
| } | ||
|
|
||
| impl<R> FilteredRawRecordIterator<R> { | ||
| pub(crate) fn new(parser: BgpkitParser<R>) -> 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, | ||
| } | ||
| } | ||
|
|
||
| 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; | ||
| } | ||
| } | ||
| } |
| 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())); | ||
| continue; |
- 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
|
Both fourth-round comments addressed in 2261364:
Gates: 958 tests / 0 failed, clippy |
| None => match raw.parse() { | ||
| Ok(record) => record, | ||
| Err(error) => { | ||
| eprintln!("warning: skipping unparseable record: {error}"); | ||
| continue; |
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.
|
Fifth round addressed in 2470b8e:
Gates: 958 tests / 0 failed, clippy |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/parser/iters/raw.rs:158
- The advertised empty-filter fast path is missing: the iterator always parses the body before checking
self.filters, and its item type cannot represent the promised(raw, Option<MrtRecord>). As a result, unfiltered library consumers pay for full parsing and malformed body records are skipped instead of receiving the original raw record. Return(raw, None)before parsing when filters are empty, and adapt the CLI consumer to parse only when it needs a rendered record (or update the stated API/design if eager parsing is intentional).
type Item = (RawMrtRecord, MrtRecord);
fn next(&mut self) -> Option<Self::Item> {
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) — on every path, filtered or not.
let record = match raw.clone().parse() {
Summary
Adds a
--hexflag that augments record-level output with the record's original wire bytes as hex. Hex is deliberately not a standalone format — the unit is the record and the meaning is "attach the original bytes", so it rides on the formats that already emit records:--format text --hex— aHEX:line closes each block--format json/json-pretty--hex— ahexfield is injected into each record objectThe intended workflow is the pipe from filtered CLI output into a byte-level dissector (wirescope's hex paste input): copy the
HEX:line, paste, get the layered field tree.Design notes
BgpkitParser::into_filtered_raw_record_iter(), yields raw MRT records with the same record-level filter semantics asinto_record_iter(no-elem records drop under filters,PeerIndexTablepasses through for RIB resolution, empty-filter fast path skips parsing). The CLI hex-encodesRawMrtRecord::raw_bytes()directly. Re-encoding the parsed model was considered and rejected in review: BGP-LS attributes re-encode inHashMapiteration order, which is non-deterministic between runs. Byte-exactness is verified against a real rrc00 fixture: thehexfield equals that record's bytes in the decompressed file exactly.(raw, Option<MrtRecord>)— the parse performed for filter matching is reused by the renderer; the no-filter fast path skips parsing entirely.--level records; requires text/json/json-pretty; mutually exclusive with--recover; ignored entirely by count-only runs (-e/-r), which keep the normal counting pipelines and per-elem filter semantics (verified:-e -p Xand-e --hex -p Xreport identical totals).Testing
render::hex: golden encodings, all-256-byte lowercase sweep.FilteredRawRecordIterator: original-bytes-exact yields (no-filter and filtered paths), no-elem record dropping under filters, parse carried alongside when filtering.cargo test --all-features958 passed / 0 failed, clippy-D warningsclean,--no-default-featuresbuild clean, strict rustdoc clean, fmt clean, MSRV 1.87 verified.