From a6cd1650d6ffd141fbf5370201b2478e12353e8f Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Mon, 23 Mar 2026 16:48:30 -0700 Subject: [PATCH 1/3] Fix OTC attribute handling in WASM and withdrawal messages - Add test to verify only_to_customer serializes as null (not 0) for messages without OTC attribute - Fix withdrawal elements to not inherit only_to_customer from UPDATE message - Update CHANGELOG with bug fixes Fixes #269 --- CHANGELOG.md | 5 ++++ src/parser/mrt/mrt_elem.rs | 2 +- src/wasm.rs | 57 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbb933d..bff9ea5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. ## Unreleased +### Bug fixes + +* **WASM `only_to_customer` serialization**: Added test to verify that messages without the OTC (Only To Customer) attribute correctly serialize `only_to_customer` as `null` (not `0`) in JSON output. This ensures JavaScript consumers can properly distinguish between "no OTC attribute" (`null`) and "OTC with AS 0" (`0`). +* **OTC attribute on withdrawal messages**: Withdrawal elements no longer inherit the `only_to_customer` value from their associated UPDATE message. Since the OTC attribute (RFC 9234) is used for route leak detection on route advertisements, it is not semantically meaningful for withdrawals. Withdrawal elements now correctly have `only_to_customer: null`. + ### Breaking changes * **`BgpOpenMessage::sender_ip` renamed to `bgp_identifier`**: The field type remains `Ipv4Addr` (aliased as `BgpIdentifier`), but the name now correctly reflects RFC 4271 terminology — this is the BGP Identifier, not necessarily the sender's IP address. diff --git a/src/parser/mrt/mrt_elem.rs b/src/parser/mrt/mrt_elem.rs index b8e533d..2f1b295 100644 --- a/src/parser/mrt/mrt_elem.rs +++ b/src/parser/mrt/mrt_elem.rs @@ -402,7 +402,7 @@ impl Iterator for BgpUpdateElemIter { atomic: false, aggr_asn: None, aggr_ip: None, - only_to_customer: self.only_to_customer, + only_to_customer: None, unknown: None, deprecated: None, }) diff --git a/src/wasm.rs b/src/wasm.rs index c099c4e..bcb7998 100644 --- a/src/wasm.rs +++ b/src/wasm.rs @@ -630,4 +630,61 @@ mod tests { assert!(!header.is_post_policy); assert!(!header.is_adj_rib_out); } + + #[test] + fn test_bgp_update_without_otc_serializes_null() { + // Create a BGP UPDATE without OTC attribute and verify it serializes to null + let data = make_bgp_update_announce([10, 0, 0, 0], 24, [192, 168, 1, 1]); + let json = parse_bgp_update_core(&data).unwrap(); + let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + let elems = v.as_array().unwrap(); + assert_eq!(elems.len(), 1); + + // Verify only_to_customer is null (not 0) + let otc = &elems[0]["only_to_customer"]; + assert!( + otc.is_null(), + "only_to_customer should be null for messages without OTC, got: {:?}", + otc + ); + } +} + +#[cfg(test)] +mod otc_tests { + use super::*; + + #[test] + fn test_option_asn_serialization() { + // Test that Option serializes correctly for WASM + let some_asn: Option = Some(Asn::new_32bit(12345)); + let json = serde_json::to_string(&some_asn).unwrap(); + assert_eq!(json, "12345", "Some(Asn) should serialize as numeric value"); + + let none_asn: Option = None; + let json = serde_json::to_string(&none_asn).unwrap(); + assert_eq!(json, "null", "None should serialize as null"); + + // Test BgpElem with and without OTC + let elem_with_otc = BgpElem { + only_to_customer: Some(Asn::new_32bit(12345)), + ..Default::default() + }; + let json = serde_json::to_string(&elem_with_otc).unwrap(); + let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(v.get("only_to_customer").unwrap(), 12345); + + let elem_without_otc = BgpElem { + only_to_customer: None, + ..Default::default() + }; + let json = serde_json::to_string(&elem_without_otc).unwrap(); + let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + let otc_field = v.get("only_to_customer").unwrap(); + assert!( + otc_field.is_null(), + "BgpElem without OTC should have null, got: {:?}", + otc_field + ); + } } From 52387976c17832282d5e42895046935f291a3e1a Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Mon, 23 Mar 2026 17:07:30 -0700 Subject: [PATCH 2/3] Fixed rustdoc warnings and consolidated GitHub Actions workflow Documentation fixes: - Fixed broken Elementor links in bgp/mod.rs with proper crate:: prefix - Escaped brackets in ris_subscribe.rs example path pattern - Fixed 5 bare URL warnings by adding angle brackets GitHub Actions improvements: - Consolidated format.yml into build.yml as parallel jobs - Added rust-cache for faster builds - Added documentation check with RUSTDOCFLAGS=-D warnings - Changed MSRV check to use taiki-e/install-action@cargo-msrv - Split monolithic job into parallel: format, doc, clippy, build-and-test, msrv --- .github/workflows/build.yml | 50 ++++++++++++++++--- .github/workflows/format.yml | 23 --------- src/models/bgp/mod.rs | 4 +- src/parser/bgp/messages.rs | 2 +- src/parser/bmp/messages/headers.rs | 4 +- src/parser/bmp/messages/initiation_message.rs | 2 +- .../bmp/messages/peer_up_notification.rs | 2 +- .../bmp/messages/termination_message.rs | 2 +- .../rislive/messages/client/ris_subscribe.rs | 2 +- 9 files changed, 52 insertions(+), 39 deletions(-) delete mode 100644 .github/workflows/format.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4b6bd5b..8207841 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -14,12 +14,43 @@ env: CARGO_TERM_COLOR: always jobs: - build: + format: + name: Format Check runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + - name: Run format check + run: cargo fmt --check - - name: Build + doc: + name: Documentation + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + - name: Check documentation + env: + RUSTDOCFLAGS: "-D warnings" + run: cargo doc --all-features --no-deps + + clippy: + name: Clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + - name: Run clippy + run: cargo clippy --all-targets --all-features -- -D warnings + + build-and-test: + name: Build & Test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + + - name: Build default run: cargo build - name: Build cli @@ -34,8 +65,13 @@ jobs: - name: Run tests run: cargo test --all-features - - name: Run clippy - run: cargo clippy --all-targets --all-features -- -D warnings - - - name: Check MSRV - run: cargo install cargo-msrv --locked && cargo msrv verify \ No newline at end of file + msrv: + name: Check MSRV + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + - name: Install cargo-msrv + uses: taiki-e/install-action@cargo-msrv + - name: Verify MSRV + run: cargo msrv verify diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml deleted file mode 100644 index db6bf2a..0000000 --- a/.github/workflows/format.yml +++ /dev/null @@ -1,23 +0,0 @@ -name: formatting - -on: - push: - branches: [ main ] - paths: - - '**.rs' - pull_request: - branches: [ main ] - paths-ignore: - - '**.rs' - -env: - CARGO_TERM_COLOR: always - -jobs: - cargo-fmt-check: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - name: Run format check - run: cargo fmt --check \ No newline at end of file diff --git a/src/models/bgp/mod.rs b/src/models/bgp/mod.rs index 50523ee..03186b2 100644 --- a/src/models/bgp/mod.rs +++ b/src/models/bgp/mod.rs @@ -156,7 +156,7 @@ pub struct BgpUpdateMessage { /// and will **not** be included here. Accessing this field directly may cause you to miss /// IPv6 or multi-protocol prefixes. /// - /// Instead, use [`Elementor::bgp_update_to_elems`] to reliably extract all withdrawn prefixes from the update, + /// Instead, use [`Elementor::bgp_update_to_elems`](crate::Elementor::bgp_update_to_elems) to reliably extract all withdrawn prefixes from the update, /// or combine this field with prefixes found in the `MpUnreachNlri` attribute manually. /// /// See @@ -174,7 +174,7 @@ pub struct BgpUpdateMessage { /// and will **not** be included here. Accessing this field directly may cause you to miss /// IPv6 or multi-protocol prefixes. /// - /// Instead, use [`Elementor::bgp_update_to_elems`] to reliably extract all announced prefixes from the update, + /// Instead, use [`Elementor::bgp_update_to_elems`](crate::Elementor::bgp_update_to_elems) to reliably extract all announced prefixes from the update, /// or combine this field with prefixes found in the `MpReachNlri` attribute manually. /// /// See diff --git a/src/parser/bgp/messages.rs b/src/parser/bgp/messages.rs index e111342..f72ac88 100644 --- a/src/parser/bgp/messages.rs +++ b/src/parser/bgp/messages.rs @@ -173,7 +173,7 @@ impl BgpNotificationMessage { /// /// The parsing of BGP OPEN message also includes decoding the BGP capabilities. /// -/// RFC 4271: https://datatracker.ietf.org/doc/html/rfc4271 +/// RFC 4271: /// ```text /// 0 1 2 3 /// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 diff --git a/src/parser/bmp/messages/headers.rs b/src/parser/bmp/messages/headers.rs index 18c8224..51dc8aa 100644 --- a/src/parser/bmp/messages/headers.rs +++ b/src/parser/bmp/messages/headers.rs @@ -187,8 +187,8 @@ impl BmpPerPeerHeader { /// Peer type /// -/// - RFC7854: https://datatracker.ietf.org/doc/html/rfc7854#section-4.2 -/// - RFC9069: https://datatracker.ietf.org/doc/html/rfc9069 +/// - RFC7854: +/// - RFC9069: #[derive(Debug, Copy, TryFromPrimitive, IntoPrimitive, PartialEq, Eq, Hash, Clone)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[repr(u8)] diff --git a/src/parser/bmp/messages/initiation_message.rs b/src/parser/bmp/messages/initiation_message.rs index 217f320..5761a52 100644 --- a/src/parser/bmp/messages/initiation_message.rs +++ b/src/parser/bmp/messages/initiation_message.rs @@ -20,7 +20,7 @@ pub struct InitiationTlv { ///Type-Length-Value Type /// -/// https://www.iana.org/assignments/bmp-parameters/bmp-parameters.xhtml#initiation-peer-up-tlvs +/// #[derive(Debug, TryFromPrimitive, IntoPrimitive, PartialEq, Clone, Copy)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[repr(u16)] diff --git a/src/parser/bmp/messages/peer_up_notification.rs b/src/parser/bmp/messages/peer_up_notification.rs index 073d780..e3bcc69 100644 --- a/src/parser/bmp/messages/peer_up_notification.rs +++ b/src/parser/bmp/messages/peer_up_notification.rs @@ -22,7 +22,7 @@ pub struct PeerUpNotification { ///Type-Length-Value Type /// -/// https://www.iana.org/assignments/bmp-parameters/bmp-parameters.xhtml#initiation-peer-up-tlvs +/// #[derive(Debug, TryFromPrimitive, IntoPrimitive, PartialEq, Clone, Copy)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[repr(u16)] diff --git a/src/parser/bmp/messages/termination_message.rs b/src/parser/bmp/messages/termination_message.rs index a2e0c28..7f317ab 100644 --- a/src/parser/bmp/messages/termination_message.rs +++ b/src/parser/bmp/messages/termination_message.rs @@ -38,7 +38,7 @@ pub enum TerminationReason { ///Type-Length-Value Type /// -/// For more, see: https://datatracker.ietf.org/doc/html/rfc1213 +/// For more, see: #[derive(Debug, TryFromPrimitive, IntoPrimitive, PartialEq, Clone, Copy)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[repr(u16)] diff --git a/src/parser/rislive/messages/client/ris_subscribe.rs b/src/parser/rislive/messages/client/ris_subscribe.rs index 1e59f4d..6ce2e3e 100644 --- a/src/parser/rislive/messages/client/ris_subscribe.rs +++ b/src/parser/rislive/messages/client/ris_subscribe.rs @@ -67,7 +67,7 @@ pub struct RisSubscribe { /// /// Examples: /// * "789$" - /// * "^123,456,789,[789,10111]$" + /// * "^123,456,789,\[789,10111\]$" /// * "!6666$" /// * "!^3333" #[serde(skip_serializing_if = "Option::is_none")] From 76177c5eefe877b88402d29b9cf51caa072d3066 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Mon, 23 Mar 2026 17:08:17 -0700 Subject: [PATCH 3/3] Fixed MSRV action - use cargo install with cache instead of taiki-e/install-action --- .github/workflows/build.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8207841..03ff3a6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -71,7 +71,9 @@ jobs: steps: - uses: actions/checkout@v4 - uses: Swatinem/rust-cache@v2 + with: + cache-directories: ~/.cargo/bin - name: Install cargo-msrv - uses: taiki-e/install-action@cargo-msrv + run: cargo install cargo-msrv --locked - name: Verify MSRV run: cargo msrv verify