feat: RFC 7606 non-fatal NLRI parsing and treat-as-withdrawal support by digizeph · Pull Request #309 · bgpkit/bgpkit-parser

digizeph · 2026-07-27T20:48:30Z

Closes #303

Summary

Implements RFC 7606 revised error handling for BGP UPDATE messages, enabling treat-as-withdrawal emulation. Two coupled changes:

1. Non-fatal NLRI parsing (`parse_bgp_update_message`)

Previously, any malformed byte in the withdrawn or announced NLRI caused the entire UPDATE to fail — destroying all attributes, all other prefixes, and returning Err. This made RFC 7606 §5.3 treat-as-withdrawal impossible.

Now, NLRI parse errors are caught inside parse_bgp_update_message and converted to BgpValidationWarning::MalformedNlri. The UPDATE is returned with partial prefix data (empty for the failed NLRI section, intact for the other). Attributes are fully preserved.

// Before: any NLRI error kills the whole UPDATE
let withdrawn = read_nlri(...)?;     // fail = lose everything
let attributes = parse_attributes(...)?;
let announced = read_nlri(...)?;     // fail = lose withdrawn + attributes

// After: each NLRI section fails independently
let (withdrawn, warn1) = read_nlri(...).ok_or_warning();
let attributes = parse_attributes(...)?;  // framing errors still fatal
let (announced, warn2) = read_nlri(...).ok_or_warning();

Attribute framing errors (wrong length fields, truncated data) remain fatal, as they indicate structural corruption that makes the entire message untrustworthy.

2. Raw byte preservation (`parse_mrt_record`)

Body-parse failures now attach the original MRT record bytes to ParserErrorWithBytes.bytes. Previously this was always None for body-level failures, making forensic analysis and record export impossible from the fallible iterator.

API surface

BgpValidationWarning marked #[non_exhaustive] — exhaustive matches must add _ =>. This is the only breaking change and is expected at 0.x maturity.
New variant: BgpValidationWarning::MalformedNlri { nlri_type: &'static str, reason: String, raw_bytes: Vec<u8> }
No struct signature changes. No iterator type changes. Existing iterators are unchanged.

Example

examples/treat_as_withdrawal.rs demonstrates scanning an MRT file, classifying RFC 7606 validation issues by error-handling category, and extracting raw malformed NLRI bytes.

Validation

cargo fmt --check ✅
cargo clippy --all-targets --all-features -- -D warnings ✅
cargo test --all-features: 664 existing tests + 12 new RFC 7606 fixture tests, all passing ✅
cargo readme diff check ✅

New tests cover: malformed announced/withdrawn NLRI, raw byte preservation in warnings, attribute survival across NLRI failure, partial prefix recovery, valid UPDATE still clean, framing errors still fatal, MRT raw byte preservation, Qrator OTC incident scenario, treat-as-withdrawal emulation pattern.

- Malformed NLRI (withdrawn or announced) no longer causes the entire BGP UPDATE to be discarded. The parser returns partial data with a BgpValidationWarning::MalformedNlri containing the raw NLRI bytes. - parse_mrt_record now preserves raw MRT record bytes in ParserErrorWithBytes when the message body fails to parse. - BgpValidationWarning marked #[non_exhaustive] for forward compatibility. - Added examples/treat_as_withdrawal.rs demonstrating RFC 7606 error classification (attribute discard vs treat-as-withdrawal). - Added 12 fixture tests covering malformed NLRI recovery, raw byte preservation, the Qrator OTC incident scenario, and treat-as-withdrawal emulation. Closes #303

Copilot

Pull request overview

This PR implements RFC 7606 revised error handling for BGP UPDATE parsing by making standard NLRI parsing non-fatal (emitting structured validation warnings instead) and by preserving raw MRT record bytes on body-parse failures to enable downstream diagnostics/forensics.

Changes:

Convert withdrawn/announced NLRI parse failures in parse_bgp_update_message into BgpValidationWarning::MalformedNlri while preserving attributes and the other NLRI section.
Preserve raw MRT record bytes in ParserErrorWithBytes when parse_mrt_record fails while parsing the body.
Add fixture-style tests plus a runnable example demonstrating treat-as-withdrawal emulation and warning classification.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_rfc7606_error_handling.rs	Adds RFC 7606-focused regression tests for non-fatal NLRI parsing and raw-byte preservation.
src/parser/mrt/mrt_record.rs	Attaches raw MRT record bytes to `ParserErrorWithBytes` on body-parse errors.
src/parser/bgp/messages.rs	Implements non-fatal withdrawn/announced NLRI parsing with `MalformedNlri` warnings.
src/error.rs	Marks `BgpValidationWarning` as `#[non_exhaustive]` and adds `MalformedNlri` variant + Display.
examples/treat_as_withdrawal.rs	Demonstrates scanning MRT files and applying RFC 7606 treat-as-withdrawal logic using warnings.
examples/README.md	Indexes the new treat-as-withdrawal example.
CHANGELOG.md	Documents the new RFC 7606 NLRI handling and MRT raw-byte preservation behavior.

Comments suppressed due to low confidence (1)

src/parser/bgp/messages.rs:507

Same as withdrawn NLRI: a 1-byte announced NLRI currently becomes Ok(vec![]) inside read_nlri (no error), so no MalformedNlri warning will be recorded. That prevents callers from reliably detecting malformed announced NLRI via warnings.

    let (announced_prefixes, announced_nlri_error) = match read_nlri(input.clone(), &afi, add_path)
    {
        Ok(pfxs) => (pfxs, None),
        Err(e) => (
            Vec::new(),
            Some(BgpValidationWarning::MalformedNlri {
                nlri_type: "announced",
                reason: e.to_string(),
                raw_bytes: input.to_vec(),
            }),
        ),
    };

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    let (withdrawn_prefixes, withdrawn_nlri_error) =
+        match read_nlri(withdrawn_bytes.clone(), &afi, add_path) {
+            Ok(pfxs) => (pfxs, None),
+            Err(e) => (
+                Vec::new(),
+                Some(BgpValidationWarning::MalformedNlri {
+                    nlri_type: "withdrawn",
+                    reason: e.to_string(),
+                    raw_bytes: withdrawn_bytes.to_vec(),
+                }),
+            ),
+        };


 pub fn parse_mrt_record(input: &mut impl Read) -> Result<MrtRecord, ParserErrorWithBytes> {
    let raw_record = chunk_mrt_record(input)?;
+    let raw_bytes = raw_record.raw_bytes();
    match raw_record.parse() {
        Ok(record) => Ok(record),
        Err(e) => Err(ParserErrorWithBytes {
            error: e,
-            bytes: None,
+            bytes: Some(raw_bytes.to_vec()),
        }),
    }


codecov · 2026-07-27T20:52:28Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.15%. Comparing base (130b82e) to head (462faf6).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #309      +/-   ##
==========================================
+ Coverage   90.07%   90.15%   +0.08%     
==========================================
  Files          91       91              
  Lines       19136    19164      +28     
==========================================
+ Hits        17236    17277      +41     
+ Misses       1900     1887      -13

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Classify each validation warning into Notification / Treat-as-withdrawal / Attribute discard tiers following the Junos bgp-error-tolerance per-attribute mapping table. - Apply the 'most severe wins' rule when multiple attributes are malformed in a single UPDATE. - Show the router action (session reset, routes withdrawn, attribute discarded) for each finding.

- read_nlri: 1-byte NLRI field now returns Err instead of silently returning Ok(vec![]), so MalformedNlri warnings are correctly emitted for this edge case (both withdrawn and announced sections). - parse_mrt_record: build raw bytes Vec directly from header_bytes + message_bytes instead of going through raw_bytes() → to_vec(), avoiding an unnecessary intermediate Bytes allocation on the error path. - Added regression test for the 1-byte NLRI case.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/parser/bgp/messages.rs:514

Mandatory-attribute validation currently keys off announced_prefixes.is_empty(). When announced NLRI bytes are present but malformed (converted to a MalformedNlri warning), announced_prefixes becomes empty and check_mandatory_attributes may be skipped, missing warnings for missing ORIGIN/AS_PATH/NEXT_HOP. Use the presence of announced NLRI bytes (or the NLRI error) rather than parsed prefixes to decide whether this UPDATE is an announcement / has standard NLRI.

    // validate mandatory attributes
    let is_announcement =
        !announced_prefixes.is_empty() || attributes.has_attr(AttrType::MP_REACHABLE_NLRI);
    let has_standard_nlri = !announced_prefixes.is_empty();
    attributes.check_mandatory_attributes(is_announcement, has_standard_nlri);

    if length == 1 {
-        // 1 byte does not make sense
+        // 1 byte does not make a valid NLRI encoding
        warn!("seeing strange one-byte NLRI field (parsing NLRI in BGP UPDATE message)");
-        input.advance(1); // skip the byte
-        return Ok(vec![]);
+        return Err(ParserError::ParseError(
+            "one-byte NLRI field is not a valid encoding".to_string(),


+    // A 1-byte NLRI field in the announced section — previously this was
+    // silently skipped (Ok(vec![])), now it produces a MalformedNlri warning.
+    let one_byte_nlri = vec![0xFF];


Copilot correctly identified that a 1-byte NLRI with value 0x00 encodes the default route (prefix length 0, no prefix octets) — a valid encoding. The previous blanket rejection of all 1-byte NLRI would have incorrectly dropped valid 0.0.0.0/0 announcements. Now only 1-byte NLRI with a non-zero value is treated as malformed. Added test for both cases: invalid (0xFF) and valid default route (0x00).

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

    // validate mandatory attributes
    let is_announcement =
        !announced_prefixes.is_empty() || attributes.has_attr(AttrType::MP_REACHABLE_NLRI);
    let has_standard_nlri = !announced_prefixes.is_empty();
    attributes.check_mandatory_attributes(is_announcement, has_standard_nlri);


 pub fn parse_mrt_record(input: &mut impl Read) -> Result<MrtRecord, ParserErrorWithBytes> {
    let raw_record = chunk_mrt_record(input)?;
+    // Capture raw bytes before parse() consumes raw_record.
+    let header_bytes = raw_record.header_bytes.clone();
+    let message_bytes = raw_record.message_bytes.clone();
    match raw_record.parse() {
        Ok(record) => Ok(record),
-        Err(e) => Err(ParserErrorWithBytes {
-            error: e,
-            bytes: None,
-        }),
+        Err(e) => {
+            // Build the raw bytes Vec directly from header + message bytes.
+            let mut bytes = Vec::with_capacity(header_bytes.len() + message_bytes.len());
+            bytes.extend_from_slice(&header_bytes);
+            bytes.extend_from_slice(&message_bytes);
+            Err(ParserErrorWithBytes {
+                error: e,
+                bytes: Some(bytes),
+            })
+        }
    }


Copilot AI review requested due to automatic review settings July 27, 2026 20:48

Copilot started reviewing on behalf of digizeph July 27, 2026 20:49 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

digizeph added 2 commits July 27, 2026 14:43

digizeph requested a review from Copilot July 27, 2026 21:57

Copilot started reviewing on behalf of digizeph July 27, 2026 21:58 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

digizeph requested a review from Copilot July 27, 2026 22:37

Copilot started reviewing on behalf of digizeph July 27, 2026 22:37 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: RFC 7606 non-fatal NLRI parsing and treat-as-withdrawal support - #309

feat: RFC 7606 non-fatal NLRI parsing and treat-as-withdrawal support#309
digizeph wants to merge 4 commits into
mainfrom
feat/rfc7606-error-handling

digizeph commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

codecov Bot commented Jul 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

digizeph commented Jul 27, 2026

Summary

1. Non-fatal NLRI parsing (parse_bgp_update_message)

2. Raw byte preservation (parse_mrt_record)

API surface

Example

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. Non-fatal NLRI parsing (`parse_bgp_update_message`)

2. Raw byte preservation (`parse_mrt_record`)

codecov Bot commented Jul 27, 2026 •

edited

Loading