From a0925cf47051fb807ac37f8e3ca37a6570d8cf5a Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Wed, 29 Jul 2026 21:35:20 +0200 Subject: [PATCH 1/7] Add EncodingError and back-patching length-prefix sink helpers EncodingError carries ValueTooLarge {field, actual, max} and Unencodable {field, reason}. All length-prefixed wire fields will be written through with_u8_len/with_u16_len (back-patched prefixes) or put_u8/u16_len_slice, so every capacity check lives in one place and non-power-of-two bounds are explicit via check_max. Groundwork for fallible encoding (issue #313). Assisted-by: Claude Code --- src/encoder/mod.rs | 1 + src/encoder/sink.rs | 200 ++++++++++++++++++++++++++++++++++++++++++++ src/error.rs | 73 ++++++++++++++++ 3 files changed, 274 insertions(+) create mode 100644 src/encoder/sink.rs diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 8ffafb9c..02410b2d 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -1,4 +1,5 @@ mod rib_encoder; +pub(crate) mod sink; mod updates_encoder; pub use rib_encoder::MrtRibEncoder; diff --git a/src/encoder/sink.rs b/src/encoder/sink.rs new file mode 100644 index 00000000..aea20462 --- /dev/null +++ b/src/encoder/sink.rs @@ -0,0 +1,200 @@ +/*! +Back-patching helpers for length-prefixed wire fields. + +All "write a length, then the payload" encoding in this crate goes through +these helpers: a placeholder length is written, the payload is encoded +directly into the shared buffer, and the length is patched in afterwards with +a checked conversion. This centralizes every wire-capacity check and avoids +per-node intermediate buffers. +*/ +use crate::error::{check_max, EncodingError}; +use bytes::{BufMut, BytesMut}; + +/// Encode a payload with a 1-byte length prefix. +/// +/// Runs `f` to append the payload to `buf`, then back-patches the length. +/// Returns `EncodingError::ValueTooLarge` (naming `field`) if the payload +/// exceeds 255 bytes. +pub(crate) fn with_u8_len( + buf: &mut BytesMut, + field: &'static str, + f: impl FnOnce(&mut BytesMut) -> Result<(), EncodingError>, +) -> Result<(), EncodingError> { + let at = buf.len(); + buf.put_u8(0); + f(buf)?; + let len = buf.len() - at - 1; + check_max(field, len, u8::MAX as usize)?; + buf[at] = len as u8; + Ok(()) +} + +/// Encode a payload with a 2-byte big-endian length prefix. +/// +/// Runs `f` to append the payload to `buf`, then back-patches the length. +/// Returns `EncodingError::ValueTooLarge` (naming `field`) if the payload +/// exceeds 65535 bytes. +pub(crate) fn with_u16_len( + buf: &mut BytesMut, + field: &'static str, + f: impl FnOnce(&mut BytesMut) -> Result<(), EncodingError>, +) -> Result<(), EncodingError> { + let at = buf.len(); + buf.put_u16(0); + f(buf)?; + let len = buf.len() - at - 2; + check_max(field, len, u16::MAX as usize)?; + buf[at..at + 2].copy_from_slice(&(len as u16).to_be_bytes()); + Ok(()) +} + +/// Write `slice` preceded by its length as 1 byte. +/// +/// Returns `EncodingError::ValueTooLarge` (naming `field`) if the slice +/// exceeds 255 bytes; `buf` is not modified in that case. +pub(crate) fn put_u8_len_slice( + buf: &mut BytesMut, + field: &'static str, + slice: &[u8], +) -> Result<(), EncodingError> { + check_max(field, slice.len(), u8::MAX as usize)?; + buf.put_u8(slice.len() as u8); + buf.put_slice(slice); + Ok(()) +} + +/// Write `slice` preceded by its length as 2 big-endian bytes. +/// +/// Returns `EncodingError::ValueTooLarge` (naming `field`) if the slice +/// exceeds 65535 bytes; `buf` is not modified in that case. +pub(crate) fn put_u16_len_slice( + buf: &mut BytesMut, + field: &'static str, + slice: &[u8], +) -> Result<(), EncodingError> { + check_max(field, slice.len(), u16::MAX as usize)?; + buf.put_u16(slice.len() as u16); + buf.put_slice(slice); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::error::check_max; + + #[test] + fn test_with_u8_len_basic() { + let mut buf = BytesMut::new(); + with_u8_len(&mut buf, "test", |b| { + b.put_slice(&[1, 2, 3]); + Ok(()) + }) + .unwrap(); + assert_eq!(&buf[..], &[3, 1, 2, 3]); + } + + #[test] + fn test_with_u8_len_empty() { + let mut buf = BytesMut::new(); + with_u8_len(&mut buf, "test", |_| Ok(())).unwrap(); + assert_eq!(&buf[..], &[0]); + } + + #[test] + fn test_with_u8_len_at_max() { + let mut buf = BytesMut::new(); + with_u8_len(&mut buf, "test", |b| { + b.put_slice(&[0xAA; 255]); + Ok(()) + }) + .unwrap(); + assert_eq!(buf[0], 255); + assert_eq!(buf.len(), 256); + } + + #[test] + fn test_with_u8_len_overflow() { + let mut buf = BytesMut::new(); + let err = with_u8_len(&mut buf, "test field", |b| { + b.put_slice(&[0xAA; 256]); + Ok(()) + }) + .unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "test field", + actual: 256, + max: 255 + } + ); + } + + #[test] + fn test_with_u16_len_basic() { + let mut buf = BytesMut::new(); + buf.put_u8(0xFF); // pre-existing content must be preserved + with_u16_len(&mut buf, "test", |b| { + b.put_slice(&[1, 2, 3]); + Ok(()) + }) + .unwrap(); + assert_eq!(&buf[..], &[0xFF, 0, 3, 1, 2, 3]); + } + + #[test] + fn test_with_u16_len_overflow() { + let mut buf = BytesMut::new(); + let err = with_u16_len(&mut buf, "test field", |b| { + b.put_slice(&vec![0xAA; 65536]); + Ok(()) + }) + .unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "test field", + actual: 65536, + max: 65535 + } + ); + } + + #[test] + fn test_nested_prefixes() { + let mut buf = BytesMut::new(); + with_u16_len(&mut buf, "outer", |b| { + with_u8_len(b, "inner", |b| { + b.put_slice(&[7, 8]); + Ok(()) + }) + }) + .unwrap(); + // outer len = 3 (inner prefix byte + 2 payload bytes) + assert_eq!(&buf[..], &[0, 3, 2, 7, 8]); + } + + #[test] + fn test_inner_error_propagates() { + let mut buf = BytesMut::new(); + let err = with_u16_len(&mut buf, "outer", |_| { + Err(EncodingError::too_large("inner", 1, 0)) + }) + .unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "inner", + actual: 1, + max: 0 + } + ); + } + + #[test] + fn test_check_max_custom_bound() { + assert_eq!(check_max("f", 4095, 0x0FFF), Ok(4095)); + assert!(check_max("f", 4096, 0x0FFF).is_err()); + } +} diff --git a/src/error.rs b/src/error.rs index 2c1bcf96..da6d3b1b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -38,6 +38,79 @@ pub enum ParserError { impl Error for ParserError {} +/// Errors that can occur when encoding BGP/MRT messages to wire format. +/// +/// These arise when in-memory data structures contain values that cannot be +/// represented in their wire-format fields (e.g. an AS_PATH segment with more +/// than 255 ASes, or an attribute value exceeding 65535 bytes). All such +/// conditions were previously handled by silently truncating — see issue #313. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum EncodingError { + /// A value exceeded the maximum size that fits in its wire-format field. + /// + /// `field` names the wire field (e.g. `"AS_PATH segment count"`, + /// `"BGP attribute value length"`). `actual` is the byte/element count + /// that overflowed; `max` is the field's capacity. + ValueTooLarge { + field: &'static str, + actual: usize, + max: usize, + }, + /// The value cannot be represented in wire format at all (e.g. ATTR_SET + /// encoding is not implemented, a labeled NLRI has an empty label stack, + /// or a BGP OPEN optional parameter uses the reserved type 255). + Unencodable { + field: &'static str, + reason: String, + }, +} + +impl EncodingError { + pub(crate) fn too_large(field: &'static str, actual: usize, max: usize) -> Self { + EncodingError::ValueTooLarge { field, actual, max } + } + + pub(crate) fn unencodable(field: &'static str, reason: impl Into) -> Self { + EncodingError::Unencodable { + field, + reason: reason.into(), + } + } +} + +/// Check that `actual` fits within `max`, returning `actual` unchanged. +/// +/// This is the single place where wire-format capacity checks happen, so +/// non-power-of-two bounds (e.g. the 12-bit FlowSpec NLRI length) are an +/// explicit, greppable decision at the call site. +pub(crate) fn check_max( + field: &'static str, + actual: usize, + max: usize, +) -> Result { + if actual > max { + return Err(EncodingError::too_large(field, actual, max)); + } + Ok(actual) +} + +impl Display for EncodingError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + EncodingError::ValueTooLarge { field, actual, max } => write!( + f, + "encoding error: {field} ({actual}) exceeds maximum ({max})" + ), + EncodingError::Unencodable { field, reason } => { + write!(f, "encoding error: {field} cannot be encoded: {reason}") + } + } + } +} + +impl Error for EncodingError {} + #[derive(Debug)] pub struct ParserErrorWithBytes { pub error: ParserError, From 9b13874b9f339bd5a00c48ff7fb0041344d1031a Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Wed, 29 Jul 2026 21:44:55 +0200 Subject: [PATCH 2/7] Make attribute encoders fallible via sink-based encode_to Leaf attribute encoders that can overflow a wire length field now write into a shared BytesMut and return Result<(), EncodingError>: - AS_PATH rejects segments with >255 ASes (was: silent u8 truncation) - Tunnel Encap, BGP-LS, SFP, BFD Discriminator, Prefix-SID and BIER TLV value lengths are checked (was: silent u8/u16 truncation) - FlowSpec NLRI length is bounded at 0x0FFF, matching the 12-bit wire field that parse_length reads (was: unchecked cast to u16, corrupting any NLRI of 4096..=65535 bytes) - encode_nlri propagates labeled-prefix errors and now encodes flowspec_nlris (was: silently dropped) Attribute::encode_to writes the value through with_u8_len/with_u16_len according to the attribute's EXTENDED_LENGTH flag, so an oversized value yields ValueTooLarge instead of a truncated length byte. ATTR_SET returns Unencodable instead of Ok + empty value. Attributes::encode now returns Result; the two call sites in BgpUpdateMessage and RibEntry carry temporary expect()s until they are converted next. Tlv::length()/SubTlv::length() (silently saturating, dead on production paths) are removed. Assisted-by: Claude Code --- src/models/bgp/flowspec/nlri.rs | 18 +- src/models/bgp/flowspec/tests.rs | 6 +- src/models/bgp/linkstate.rs | 5 - src/models/bgp/tunnel_encap.rs | 8 +- .../bgp/attributes/attr_02_17_as_path.rs | 71 ++++--- src/parser/bgp/attributes/attr_14_15_nlri.rs | 41 +++- .../bgp/attributes/attr_23_tunnel_encap.rs | 54 +++-- .../bgp/attributes/attr_29_linkstate.rs | 56 +++--- src/parser/bgp/attributes/attr_37_sfp.rs | 22 +- .../attributes/attr_38_bfd_discriminator.rs | 21 +- .../bgp/attributes/attr_40_bgp_prefix_sid.rs | 25 ++- src/parser/bgp/attributes/attr_41_bier.rs | 22 +- src/parser/bgp/attributes/mod.rs | 190 ++++++++++-------- src/parser/bgp/messages.rs | 3 +- src/parser/iters/route.rs | 6 +- .../messages/table_dump_v2/rib_afi_entries.rs | 6 +- 16 files changed, 316 insertions(+), 238 deletions(-) diff --git a/src/models/bgp/flowspec/nlri.rs b/src/models/bgp/flowspec/nlri.rs index ac16714f..50d3a54f 100644 --- a/src/models/bgp/flowspec/nlri.rs +++ b/src/models/bgp/flowspec/nlri.rs @@ -1,4 +1,5 @@ use super::*; +use crate::error::{check_max, EncodingError}; use crate::models::NetworkPrefix; use ipnet::IpNet; @@ -54,8 +55,12 @@ pub fn parse_flowspec_nlri(data: &[u8]) -> Result { Ok(FlowSpecNlri { components }) } +/// Maximum encodable FlowSpec NLRI length: the wire length field is 12 bits +/// (RFC 8955 §4.1 — a 2-octet length has its high nibble fixed to 0xF). +pub(crate) const FLOWSPEC_NLRI_MAX_LEN: usize = 0x0FFF; + /// Encode Flow-Spec NLRI to byte data -pub fn encode_flowspec_nlri(nlri: &FlowSpecNlri) -> Vec { +pub fn encode_flowspec_nlri(nlri: &FlowSpecNlri) -> Result, EncodingError> { let mut data = Vec::new(); // Encode each component @@ -88,11 +93,16 @@ pub fn encode_flowspec_nlri(nlri: &FlowSpecNlri) -> Vec { } } - // Prepend length + // Prepend length; the wire length field is 12-bit, not a full u16 + check_max( + "FlowSpec NLRI total length", + data.len(), + FLOWSPEC_NLRI_MAX_LEN, + )?; let mut result = Vec::new(); encode_length(data.len() as u16, &mut result); result.extend(data); - result + Ok(result) } /// Parse length field (1 or 2 octets) @@ -502,7 +512,7 @@ mod tests { FlowSpecComponent::DestinationPort(vec![NumericOperator::equal_to(80)]), ]); - let encoded = encode_flowspec_nlri(&original_nlri); + let encoded = encode_flowspec_nlri(&original_nlri).unwrap(); let parsed_nlri = parse_flowspec_nlri(&encoded).unwrap(); assert_eq!(original_nlri, parsed_nlri); diff --git a/src/models/bgp/flowspec/tests.rs b/src/models/bgp/flowspec/tests.rs index 02d32c55..158e2a9a 100644 --- a/src/models/bgp/flowspec/tests.rs +++ b/src/models/bgp/flowspec/tests.rs @@ -63,7 +63,7 @@ mod rfc_examples { } // Test round-trip encoding - let encoded = encode_flowspec_nlri(&nlri); + let encoded = encode_flowspec_nlri(&nlri).unwrap(); assert_eq!(encoded, data); } @@ -726,7 +726,7 @@ mod nlri_parsing_tests { prefix, }]); - let encoded = encode_flowspec_nlri(&nlri); + let encoded = encode_flowspec_nlri(&nlri).unwrap(); // Should start with length, then type 1, then prefix len, then offset assert!(encoded.len() > 4); @@ -789,7 +789,7 @@ mod nlri_parsing_tests { FlowSpecComponent::TcpFlags(vec![bm_op1, bm_op2]), ]); - let encoded = encode_flowspec_nlri(&nlri); + let encoded = encode_flowspec_nlri(&nlri).unwrap(); let parsed = parse_flowspec_nlri(&encoded).unwrap(); // Verify round-trip encoding worked diff --git a/src/models/bgp/linkstate.rs b/src/models/bgp/linkstate.rs index 5ef4f965..ead21ad4 100644 --- a/src/models/bgp/linkstate.rs +++ b/src/models/bgp/linkstate.rs @@ -183,10 +183,6 @@ impl Tlv { pub fn new(tlv_type: u16, value: Vec) -> Self { Self { tlv_type, value } } - - pub fn length(&self) -> u16 { - self.value.len() as u16 - } } /// Node Descriptor TLVs @@ -609,7 +605,6 @@ mod tests { let tlv = Tlv::new(1024, vec![0x01, 0x02, 0x03]); assert_eq!(tlv.tlv_type, 1024); assert_eq!(tlv.value, vec![0x01, 0x02, 0x03]); - assert_eq!(tlv.length(), 3); } #[test] diff --git a/src/models/bgp/tunnel_encap.rs b/src/models/bgp/tunnel_encap.rs index 7e2e57de..f757548f 100644 --- a/src/models/bgp/tunnel_encap.rs +++ b/src/models/bgp/tunnel_encap.rs @@ -125,9 +125,6 @@ impl SubTlv { } } - pub fn length(&self) -> u16 { - self.value.len() as u16 - } } /// Tunnel Encapsulation TLV @@ -336,9 +333,10 @@ mod tests { } #[test] - fn test_sub_tlv_length() { + fn test_sub_tlv_creation() { let sub_tlv = SubTlv::new(SubTlvType::Color, vec![0x00, 0x00, 0x00, 0x64]); - assert_eq!(sub_tlv.length(), 4); + assert_eq!(sub_tlv.sub_tlv_type, SubTlvType::Color); + assert_eq!(sub_tlv.value.len(), 4); } #[test] diff --git a/src/parser/bgp/attributes/attr_02_17_as_path.rs b/src/parser/bgp/attributes/attr_02_17_as_path.rs index dd598d66..90c25519 100644 --- a/src/parser/bgp/attributes/attr_02_17_as_path.rs +++ b/src/parser/bgp/attributes/attr_02_17_as_path.rs @@ -1,3 +1,4 @@ +use crate::error::{check_max, EncodingError}; use crate::models::*; use crate::parser::ReadUtils; use crate::ParserError; @@ -54,33 +55,24 @@ fn parse_as_path_segment( } } -pub fn encode_as_path(path: &AsPath, asn_len: AsnLength) -> Bytes { - let mut output = BytesMut::with_capacity(1024); +pub fn encode_as_path( + path: &AsPath, + asn_len: AsnLength, + output: &mut BytesMut, +) -> Result<(), EncodingError> { for segment in path.segments.iter() { - match segment { - AsPathSegment::AsSet(asns) => { - output.put_u8(AS_PATH_AS_SET); - output.put_u8(asns.len() as u8); - write_asns(asns, asn_len, &mut output); - } - AsPathSegment::AsSequence(asns) => { - output.put_u8(AS_PATH_AS_SEQUENCE); - output.put_u8(asns.len() as u8); - write_asns(asns, asn_len, &mut output); - } - AsPathSegment::ConfedSequence(asns) => { - output.put_u8(AS_PATH_CONFED_SEQUENCE); - output.put_u8(asns.len() as u8); - write_asns(asns, asn_len, &mut output); - } - AsPathSegment::ConfedSet(asns) => { - output.put_u8(AS_PATH_CONFED_SET); - output.put_u8(asns.len() as u8); - write_asns(asns, asn_len, &mut output); - } - } + let (segment_type, asns) = match segment { + AsPathSegment::AsSet(asns) => (AS_PATH_AS_SET, asns), + AsPathSegment::AsSequence(asns) => (AS_PATH_AS_SEQUENCE, asns), + AsPathSegment::ConfedSequence(asns) => (AS_PATH_CONFED_SEQUENCE, asns), + AsPathSegment::ConfedSet(asns) => (AS_PATH_CONFED_SET, asns), + }; + check_max("AS_PATH segment count", asns.len(), u8::MAX as usize)?; + output.put_u8(segment_type); + output.put_u8(asns.len() as u8); + write_asns(asns, asn_len, output); } - output.freeze() + Ok(()) } fn write_asns(asns: &[Asn], asn_len: AsnLength, output: &mut BytesMut) { @@ -222,7 +214,8 @@ mod tests { 0, 3, // AS3 ]); let path = parse_as_path(data.clone(), &AsnLength::Bits16).unwrap(); - let encoded_bytes = encode_as_path(&path, AsnLength::Bits16); + let mut encoded_bytes = BytesMut::new(); + encode_as_path(&path, AsnLength::Bits16, &mut encoded_bytes).unwrap(); assert_eq!(data, encoded_bytes); let data = Bytes::from(vec![ @@ -233,10 +226,28 @@ mod tests { 0, 0, 0, 3, // AS3 ]); let path = parse_as_path(data.clone(), &AsnLength::Bits32).unwrap(); - let encoded_bytes = encode_as_path(&path, AsnLength::Bits32); + let mut encoded_bytes = BytesMut::new(); + encode_as_path(&path, AsnLength::Bits32, &mut encoded_bytes).unwrap(); assert_eq!(data, encoded_bytes); } + #[test] + fn test_encode_as_path_oversized_segment() { + let path = AsPath::from_segments(vec![AsPathSegment::AsSequence( + (0..256u32).map(Asn::from).collect(), + )]); + let mut buf = BytesMut::new(); + let err = encode_as_path(&path, AsnLength::Bits32, &mut buf).unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "AS_PATH segment count", + actual: 256, + max: 255 + } + ); + } + #[test] fn test_encode_confed() { let data = Bytes::from(vec![ @@ -245,7 +256,8 @@ mod tests { 0, 1, ]); let path = parse_as_path(data.clone(), &AsnLength::Bits16).unwrap(); - let encoded_bytes = encode_as_path(&path, AsnLength::Bits16); + let mut encoded_bytes = BytesMut::new(); + encode_as_path(&path, AsnLength::Bits16, &mut encoded_bytes).unwrap(); assert_eq!(data, encoded_bytes); let data = Bytes::from(vec![ @@ -254,7 +266,8 @@ mod tests { 0, 1, ]); let path = parse_as_path(data.clone(), &AsnLength::Bits16).unwrap(); - let encoded_bytes = encode_as_path(&path, AsnLength::Bits16); + let mut encoded_bytes = BytesMut::new(); + encode_as_path(&path, AsnLength::Bits16, &mut encoded_bytes).unwrap(); assert_eq!(data, encoded_bytes); } diff --git a/src/parser/bgp/attributes/attr_14_15_nlri.rs b/src/parser/bgp/attributes/attr_14_15_nlri.rs index 83eca5c9..35ca1f94 100644 --- a/src/parser/bgp/attributes/attr_14_15_nlri.rs +++ b/src/parser/bgp/attributes/attr_14_15_nlri.rs @@ -1,3 +1,5 @@ +use crate::encoder::sink::put_u8_len_slice; +use crate::error::EncodingError; use crate::models::*; use crate::parser::bgp::attributes::attr_03_next_hop::parse_mp_next_hop; use crate::parser::bgp::attributes::attr_29_linkstate::parse_link_state_nlri; @@ -152,9 +154,12 @@ pub fn parse_nlri( /// * `reachable` - Whether this is MP_REACH_NLRI (true) or MP_UNREACH_NLRI (false) /// * `add_path` - Whether ADD-PATH capability was negotiated (RFC 7911). If true, /// path_id will be encoded; if false, prefixes with path_id will cause an error. -pub fn encode_nlri(nlri: &Nlri, reachable: bool, add_path: bool) -> Result { - let mut bytes = BytesMut::new(); - +pub fn encode_nlri( + nlri: &Nlri, + reachable: bool, + add_path: bool, + bytes: &mut BytesMut, +) -> Result<(), EncodingError> { // encode address family bytes.put_u16(nlri.afi as u16); bytes.put_u8(nlri.safi as u8); @@ -187,8 +192,8 @@ pub fn encode_nlri(nlri: &Nlri, reachable: bool, add_path: bool) -> Result Result Result Result Bytes { - let mut bytes = BytesMut::new(); - +pub fn encode_tunnel_encapsulation_attribute( + attr: &TunnelEncapAttribute, + bytes: &mut BytesMut, +) -> Result<(), EncodingError> { for tunnel_tlv in &attr.tunnel_tlvs { // Encode tunnel type bytes.put_u16(tunnel_tlv.tunnel_type as u16); - // Encode sub-TLVs first to calculate total length - let mut sub_tlv_bytes = BytesMut::new(); - for sub_tlv in &tunnel_tlv.sub_tlvs { - let sub_tlv_type = sub_tlv.sub_tlv_type as u16; - - // Encode sub-TLV type - if sub_tlv_type < 128 { - sub_tlv_bytes.put_u8(sub_tlv_type as u8); - sub_tlv_bytes.put_u8(sub_tlv.value.len() as u8); - } else { - sub_tlv_bytes.put_u8(sub_tlv_type as u8); - sub_tlv_bytes.put_u16(sub_tlv.value.len() as u16); + // Encode tunnel length, then sub-TLVs + with_u16_len(bytes, "Tunnel Encap TLV length", |b| { + for sub_tlv in &tunnel_tlv.sub_tlvs { + let sub_tlv_type = sub_tlv.sub_tlv_type as u16; + b.put_u8(sub_tlv_type as u8); + + // RFC 9012: sub-TLV types < 128 use a 1-octet length field, + // types >= 128 use a 2-octet length field + if sub_tlv_type < 128 { + put_u8_len_slice(b, "Tunnel Encap sub-TLV value length", &sub_tlv.value)?; + } else { + put_u16_len_slice(b, "Tunnel Encap sub-TLV value length", &sub_tlv.value)?; + } } - - // Encode sub-TLV value - sub_tlv_bytes.extend_from_slice(&sub_tlv.value); - } - - // Encode tunnel length - bytes.put_u16(sub_tlv_bytes.len() as u16); - - // Append sub-TLV data - bytes.extend_from_slice(&sub_tlv_bytes); + Ok(()) + })?; } - bytes.freeze() + Ok(()) } #[cfg(test)] @@ -231,10 +226,11 @@ mod tests { attr.add_tunnel_tlv(tunnel_tlv); - let encoded = encode_tunnel_encapsulation_attribute(&attr); + let mut encoded = BytesMut::new(); + encode_tunnel_encapsulation_attribute(&attr, &mut encoded).unwrap(); // Should encode back to the same format we can parse - let parsed = parse_tunnel_encapsulation_attribute(encoded).unwrap(); + let parsed = parse_tunnel_encapsulation_attribute(encoded.freeze()).unwrap(); if let AttributeValue::TunnelEncapsulation(parsed_attr) = parsed { assert_eq!(parsed_attr.tunnel_tlvs.len(), 1); diff --git a/src/parser/bgp/attributes/attr_29_linkstate.rs b/src/parser/bgp/attributes/attr_29_linkstate.rs index 7350db53..f2dcd9de 100644 --- a/src/parser/bgp/attributes/attr_29_linkstate.rs +++ b/src/parser/bgp/attributes/attr_29_linkstate.rs @@ -3,7 +3,8 @@ use bytes::{Buf, BufMut, Bytes, BytesMut}; use std::net::{Ipv4Addr, Ipv6Addr}; -use crate::error::ParserError; +use crate::encoder::sink::put_u16_len_slice; +use crate::error::{EncodingError, ParserError}; use crate::models::*; use crate::parser::ReadUtils; @@ -432,41 +433,37 @@ fn parse_ip_prefix_from_bytes(data: &[u8]) -> Result } /// Encode BGP Link-State attribute -pub fn encode_link_state_attribute(attr: &LinkStateAttribute) -> Bytes { - let mut bytes = BytesMut::new(); - - // Encode node attributes - for (attr_type, value) in &attr.node_attributes { - let type_code = u16::from(*attr_type); - bytes.put_u16(type_code); - bytes.put_u16(value.len() as u16); - bytes.extend_from_slice(value); - } - - // Encode link attributes - for (attr_type, value) in &attr.link_attributes { - let type_code = u16::from(*attr_type); - bytes.put_u16(type_code); - bytes.put_u16(value.len() as u16); - bytes.extend_from_slice(value); - } - - // Encode prefix attributes - for (attr_type, value) in &attr.prefix_attributes { - let type_code = u16::from(*attr_type); +pub fn encode_link_state_attribute( + attr: &LinkStateAttribute, + bytes: &mut BytesMut, +) -> Result<(), EncodingError> { + let typed_tlvs = attr + .node_attributes + .iter() + .map(|(attr_type, value)| (u16::from(*attr_type), value)) + .chain( + attr.link_attributes + .iter() + .map(|(attr_type, value)| (u16::from(*attr_type), value)), + ) + .chain( + attr.prefix_attributes + .iter() + .map(|(attr_type, value)| (u16::from(*attr_type), value)), + ); + + for (type_code, value) in typed_tlvs { bytes.put_u16(type_code); - bytes.put_u16(value.len() as u16); - bytes.extend_from_slice(value); + put_u16_len_slice(bytes, "BGP-LS TLV value length", value)?; } // Encode unknown attributes for tlv in &attr.unknown_attributes { bytes.put_u16(tlv.tlv_type); - bytes.put_u16(tlv.length()); - bytes.extend_from_slice(&tlv.value); + put_u16_len_slice(bytes, "BGP-LS TLV value length", &tlv.value)?; } - bytes.freeze() + Ok(()) } #[cfg(test)] @@ -531,7 +528,8 @@ mod tests { let mut attr = LinkStateAttribute::new(); attr.add_node_attribute(NodeAttributeType::NodeName, b"router1".to_vec()); - let encoded = encode_link_state_attribute(&attr); + let mut encoded = BytesMut::new(); + encode_link_state_attribute(&attr, &mut encoded).unwrap(); assert!(!encoded.is_empty()); // Should contain the node name TLV diff --git a/src/parser/bgp/attributes/attr_37_sfp.rs b/src/parser/bgp/attributes/attr_37_sfp.rs index e792b0d1..4243a866 100644 --- a/src/parser/bgp/attributes/attr_37_sfp.rs +++ b/src/parser/bgp/attributes/attr_37_sfp.rs @@ -1,5 +1,7 @@ use crate::models::*; use crate::parser::ReadUtils; +use crate::encoder::sink::put_u16_len_slice; +use crate::error::EncodingError; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; @@ -27,14 +29,12 @@ pub fn parse_sfp(mut input: Bytes) -> Result { Ok(AttributeValue::Sfp(SfpAttribute { tlvs })) } -pub fn encode_sfp(attr: &SfpAttribute) -> Bytes { - let mut buf = BytesMut::new(); +pub fn encode_sfp(attr: &SfpAttribute, buf: &mut BytesMut) -> Result<(), EncodingError> { for tlv in &attr.tlvs { buf.put_u8(tlv.tlv_type); - buf.put_u16(tlv.value.len() as u16); - buf.extend_from_slice(&tlv.value); + put_u16_len_slice(buf, "SFP TLV value length", &tlv.value)?; } - buf.freeze() + Ok(()) } #[cfg(test)] @@ -53,7 +53,9 @@ mod tests { attr.tlvs[0].value, Bytes::from_static(&[0xaa, 0xbb, 0xcc, 0xdd]) ); - assert_eq!(encode_sfp(&attr), input); + let mut buf = BytesMut::new(); + encode_sfp(&attr, &mut buf).unwrap(); + assert_eq!(buf.freeze(), input); } value => panic!("expected SFP, got {value:?}"), } @@ -67,7 +69,9 @@ mod tests { AttributeValue::Sfp(attr) => { assert_eq!(attr.tlvs[0].tlv_type, 0x7f); assert_eq!(attr.tlvs[0].value, Bytes::from_static(&[0xde, 0xad])); - assert_eq!(encode_sfp(&attr), input); + let mut buf = BytesMut::new(); + encode_sfp(&attr, &mut buf).unwrap(); + assert_eq!(buf.freeze(), input); } value => panic!("expected SFP, got {value:?}"), } @@ -79,7 +83,9 @@ mod tests { match value { AttributeValue::Sfp(attr) => { assert!(attr.tlvs.is_empty()); - assert_eq!(encode_sfp(&attr), Bytes::new()); + let mut buf = BytesMut::new(); + encode_sfp(&attr, &mut buf).unwrap(); + assert!(buf.is_empty()); } value => panic!("expected SFP, got {value:?}"), } diff --git a/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs b/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs index 7ccf7482..3dbcf76e 100644 --- a/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs +++ b/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs @@ -1,5 +1,7 @@ use crate::models::*; use crate::parser::ReadUtils; +use crate::encoder::sink::put_u8_len_slice; +use crate::error::EncodingError; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; @@ -41,16 +43,17 @@ pub fn parse_bfd_discriminator(mut input: Bytes) -> Result Bytes { - let mut buf = BytesMut::new(); +pub fn encode_bfd_discriminator( + attr: &BfdDiscriminatorAttribute, + buf: &mut BytesMut, +) -> Result<(), EncodingError> { buf.put_u8(attr.mode); buf.put_u32(attr.discriminator); for tlv in &attr.tlvs { buf.put_u8(tlv.tlv_type); - buf.put_u8(tlv.value.len() as u8); - buf.extend_from_slice(&tlv.value); + put_u8_len_slice(buf, "BFD Discriminator TLV value length", &tlv.value)?; } - buf.freeze() + Ok(()) } #[cfg(test)] @@ -71,7 +74,9 @@ mod tests { assert_eq!(attr.tlvs.len(), 1); assert_eq!(attr.tlvs[0].tlv_type, 1); assert_eq!(attr.tlvs[0].value, Bytes::from_static(&[192, 0, 2, 1])); - assert_eq!(encode_bfd_discriminator(&attr), input); + let mut buf = BytesMut::new(); + encode_bfd_discriminator(&attr, &mut buf).unwrap(); + assert_eq!(buf.freeze(), input); } value => panic!("expected BFD Discriminator, got {value:?}"), } @@ -86,7 +91,9 @@ mod tests { assert_eq!(attr.mode, 1); assert_eq!(attr.discriminator, 0x01020304); assert!(attr.tlvs.is_empty()); - assert_eq!(encode_bfd_discriminator(&attr), input); + let mut buf = BytesMut::new(); + encode_bfd_discriminator(&attr, &mut buf).unwrap(); + assert_eq!(buf.freeze(), input); } value => panic!("expected BFD Discriminator, got {value:?}"), } diff --git a/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs b/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs index b65b6a93..2e43f6d4 100644 --- a/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs +++ b/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs @@ -1,5 +1,7 @@ use crate::models::*; use crate::parser::ReadUtils; +use crate::encoder::sink::put_u16_len_slice; +use crate::error::EncodingError; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; @@ -27,14 +29,15 @@ pub fn parse_bgp_prefix_sid(mut input: Bytes) -> Result Bytes { - let mut buf = BytesMut::new(); +pub fn encode_bgp_prefix_sid( + attr: &BgpPrefixSidAttribute, + buf: &mut BytesMut, +) -> Result<(), EncodingError> { for tlv in &attr.tlvs { buf.put_u8(tlv.tlv_type); - buf.put_u16(tlv.value.len() as u16); - buf.extend_from_slice(&tlv.value); + put_u16_len_slice(buf, "BGP Prefix-SID TLV value length", &tlv.value)?; } - buf.freeze() + Ok(()) } #[cfg(test)] @@ -55,7 +58,9 @@ mod tests { assert_eq!(attr.tlvs.len(), 1); assert_eq!(attr.tlvs[0].tlv_type, 1); assert_eq!(attr.tlvs[0].value.len(), 7); - assert_eq!(encode_bgp_prefix_sid(&attr), input); + let mut buf = BytesMut::new(); + encode_bgp_prefix_sid(&attr, &mut buf).unwrap(); + assert_eq!(buf.freeze(), input); } value => panic!("expected Prefix-SID, got {value:?}"), } @@ -69,7 +74,9 @@ mod tests { AttributeValue::BgpPrefixSid(attr) => { assert_eq!(attr.tlvs[0].tlv_type, 0x7f); assert_eq!(attr.tlvs[0].value, Bytes::from_static(&[0xaa, 0xbb])); - assert_eq!(encode_bgp_prefix_sid(&attr), input); + let mut buf = BytesMut::new(); + encode_bgp_prefix_sid(&attr, &mut buf).unwrap(); + assert_eq!(buf.freeze(), input); } value => panic!("expected Prefix-SID, got {value:?}"), } @@ -81,7 +88,9 @@ mod tests { match value { AttributeValue::BgpPrefixSid(attr) => { assert!(attr.tlvs.is_empty()); - assert_eq!(encode_bgp_prefix_sid(&attr), Bytes::new()); + let mut buf = BytesMut::new(); + encode_bgp_prefix_sid(&attr, &mut buf).unwrap(); + assert!(buf.is_empty()); } value => panic!("expected Prefix-SID, got {value:?}"), } diff --git a/src/parser/bgp/attributes/attr_41_bier.rs b/src/parser/bgp/attributes/attr_41_bier.rs index d00d5652..bb1b69a3 100644 --- a/src/parser/bgp/attributes/attr_41_bier.rs +++ b/src/parser/bgp/attributes/attr_41_bier.rs @@ -1,5 +1,7 @@ use crate::models::*; use crate::parser::ReadUtils; +use crate::encoder::sink::put_u16_len_slice; +use crate::error::EncodingError; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; @@ -27,14 +29,12 @@ pub fn parse_bier(mut input: Bytes) -> Result { Ok(AttributeValue::Bier(BierAttribute { tlvs })) } -pub fn encode_bier(attr: &BierAttribute) -> Bytes { - let mut buf = BytesMut::new(); +pub fn encode_bier(attr: &BierAttribute, buf: &mut BytesMut) -> Result<(), EncodingError> { for tlv in &attr.tlvs { buf.put_u16(tlv.tlv_type); - buf.put_u16(tlv.value.len() as u16); - buf.extend_from_slice(&tlv.value); + put_u16_len_slice(buf, "BIER TLV value length", &tlv.value)?; } - buf.freeze() + Ok(()) } #[cfg(test)] @@ -54,7 +54,9 @@ mod tests { assert_eq!(attr.tlvs.len(), 1); assert_eq!(attr.tlvs[0].tlv_type, 1); assert_eq!(attr.tlvs[0].value, Bytes::from_static(&[0xaa, 0xbb, 0xcc])); - assert_eq!(encode_bier(&attr), input); + let mut buf = BytesMut::new(); + encode_bier(&attr, &mut buf).unwrap(); + assert_eq!(buf.freeze(), input); } value => panic!("expected BIER, got {value:?}"), } @@ -68,7 +70,9 @@ mod tests { AttributeValue::Bier(attr) => { assert_eq!(attr.tlvs[0].tlv_type, 0x1234); assert_eq!(attr.tlvs[0].value, Bytes::from_static(&[0xde, 0xad])); - assert_eq!(encode_bier(&attr), input); + let mut buf = BytesMut::new(); + encode_bier(&attr, &mut buf).unwrap(); + assert_eq!(buf.freeze(), input); } value => panic!("expected BIER, got {value:?}"), } @@ -80,7 +84,9 @@ mod tests { match value { AttributeValue::Bier(attr) => { assert!(attr.tlvs.is_empty()); - assert_eq!(encode_bier(&attr), Bytes::new()); + let mut buf = BytesMut::new(); + encode_bier(&attr, &mut buf).unwrap(); + assert!(buf.is_empty()); } value => panic!("expected BIER, got {value:?}"), } diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index 054b3468..1a1ea036 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -25,7 +25,8 @@ use std::net::IpAddr; use crate::models::*; -use crate::error::{BgpValidationWarning, ParserError}; +use crate::encoder::sink::{with_u16_len, with_u8_len}; +use crate::error::{BgpValidationWarning, EncodingError, ParserError}; use crate::parser::bgp::attributes::attr_01_origin::{encode_origin, parse_origin}; use crate::parser::bgp::attributes::attr_02_17_as_path::encode_as_path; pub(crate) use crate::parser::bgp::attributes::attr_02_17_as_path::parse_as_path; @@ -499,98 +500,113 @@ pub fn parse_attributes( } impl Attribute { - pub fn encode(&self, asn_len: AsnLength) -> Bytes { - let mut bytes = BytesMut::new(); - - let flag = self.flag.bits(); - let type_code = self.value.attr_code(); - - bytes.put_u8(flag); - bytes.put_u8(type_code); - - let value_bytes = match &self.value { - AttributeValue::Origin(v) => encode_origin(v), - AttributeValue::AsPath { path, is_as4 } => { - let four_byte = match is_as4 { - true => AsnLength::Bits32, - false => match asn_len.is_four_byte() { + /// Append the wire representation of this attribute to `buf`. + /// + /// The length-field width follows the attribute's EXTENDED_LENGTH flag; a + /// value that does not fit the resulting field yields + /// [`EncodingError::ValueTooLarge`] instead of being truncated. + pub fn encode_to(&self, asn_len: AsnLength, buf: &mut BytesMut) -> Result<(), EncodingError> { + buf.put_u8(self.flag.bits()); + buf.put_u8(self.value.attr_code()); + + let write_value = |b: &mut BytesMut| -> Result<(), EncodingError> { + match &self.value { + AttributeValue::Origin(v) => b.put_slice(&encode_origin(v)), + AttributeValue::AsPath { path, is_as4 } => { + let four_byte = match is_as4 { true => AsnLength::Bits32, - false => AsnLength::Bits16, - }, - }; - encode_as_path(path, four_byte) - } - AttributeValue::NextHop(v) => encode_next_hop(v), - AttributeValue::MultiExitDiscriminator(v) => encode_med(*v), - AttributeValue::LocalPreference(v) => encode_local_pref(*v), - AttributeValue::OnlyToCustomer(v) => encode_only_to_customer(v.into()), - AttributeValue::AtomicAggregate => Bytes::default(), - AttributeValue::Aggregator { asn, id, is_as4: _ } => { - encode_aggregator(asn, &IpAddr::from(*id)) - } - AttributeValue::Communities(v) => encode_regular_communities(v), - AttributeValue::ExtendedCommunities(v) => encode_extended_communities(v), - AttributeValue::LargeCommunities(v) => encode_large_communities(v), - AttributeValue::Ipv6AddressSpecificExtendedCommunities(v) => { - encode_ipv6_extended_communities(v) - } - AttributeValue::OriginatorId(v) => encode_originator_id(&IpAddr::from(*v)), - AttributeValue::Clusters(v) => encode_clusters(v), - AttributeValue::MpReachNlri(v) => { - // Infer ADD-PATH from presence of path_id in any labeled prefix - let add_path = v - .labeled_prefixes - .as_ref() - .is_some_and(|prefixes| prefixes.iter().any(|p| p.path_id.is_some())); - encode_nlri(v, true, add_path).unwrap_or_else(|e| { - log::warn!("Failed to encode MP_REACH_NLRI: {}", e); - Bytes::new() - }) - } - AttributeValue::MpUnreachNlri(v) => { - // Withdrawals don't use ADD-PATH encoding per RFC 8277 - encode_nlri(v, false, false).unwrap_or_else(|e| { - log::warn!("Failed to encode MP_UNREACH_NLRI: {}", e); - Bytes::new() - }) - } - AttributeValue::LinkState(v) => encode_link_state_attribute(v), - AttributeValue::TunnelEncapsulation(v) => encode_tunnel_encapsulation_attribute(v), - AttributeValue::BfdDiscriminator(v) => encode_bfd_discriminator(v), - AttributeValue::BgpPrefixSid(v) => encode_bgp_prefix_sid(v), - AttributeValue::Bier(v) => encode_bier(v), - AttributeValue::Sfp(v) => encode_sfp(v), - AttributeValue::Development(v) => Bytes::copy_from_slice(v), - AttributeValue::Raw(v) => v.bytes.clone(), - AttributeValue::Deprecated(v) => v.bytes.clone(), - AttributeValue::Unknown(v) => v.bytes.clone(), - AttributeValue::Aigp(v) => encode_aigp(v), - AttributeValue::AttrSet(_v) => { - // ATTR_SET encoding not yet implemented - return empty bytes - Bytes::new() + false => match asn_len.is_four_byte() { + true => AsnLength::Bits32, + false => AsnLength::Bits16, + }, + }; + encode_as_path(path, four_byte, b)?; + } + AttributeValue::NextHop(v) => b.put_slice(&encode_next_hop(v)), + AttributeValue::MultiExitDiscriminator(v) => b.put_slice(&encode_med(*v)), + AttributeValue::LocalPreference(v) => b.put_slice(&encode_local_pref(*v)), + AttributeValue::OnlyToCustomer(v) => { + b.put_slice(&encode_only_to_customer(v.into())) + } + AttributeValue::AtomicAggregate => {} + AttributeValue::Aggregator { asn, id, is_as4: _ } => { + b.put_slice(&encode_aggregator(asn, &IpAddr::from(*id))) + } + AttributeValue::Communities(v) => b.put_slice(&encode_regular_communities(v)), + AttributeValue::ExtendedCommunities(v) => { + b.put_slice(&encode_extended_communities(v)) + } + AttributeValue::LargeCommunities(v) => b.put_slice(&encode_large_communities(v)), + AttributeValue::Ipv6AddressSpecificExtendedCommunities(v) => { + b.put_slice(&encode_ipv6_extended_communities(v)) + } + AttributeValue::OriginatorId(v) => { + b.put_slice(&encode_originator_id(&IpAddr::from(*v))) + } + AttributeValue::Clusters(v) => b.put_slice(&encode_clusters(v)), + AttributeValue::MpReachNlri(v) => { + // Infer ADD-PATH from presence of path_id in any labeled prefix + let add_path = v + .labeled_prefixes + .as_ref() + .is_some_and(|prefixes| prefixes.iter().any(|p| p.path_id.is_some())); + encode_nlri(v, true, add_path, b)?; + } + AttributeValue::MpUnreachNlri(v) => { + // Withdrawals don't use ADD-PATH encoding per RFC 8277 + encode_nlri(v, false, false, b)?; + } + AttributeValue::LinkState(v) => encode_link_state_attribute(v, b)?, + AttributeValue::TunnelEncapsulation(v) => { + encode_tunnel_encapsulation_attribute(v, b)? + } + AttributeValue::BfdDiscriminator(v) => encode_bfd_discriminator(v, b)?, + AttributeValue::BgpPrefixSid(v) => encode_bgp_prefix_sid(v, b)?, + AttributeValue::Bier(v) => encode_bier(v, b)?, + AttributeValue::Sfp(v) => encode_sfp(v, b)?, + AttributeValue::Development(v) => b.put_slice(v), + AttributeValue::Raw(v) => b.put_slice(&v.bytes), + AttributeValue::Deprecated(v) => b.put_slice(&v.bytes), + AttributeValue::Unknown(v) => b.put_slice(&v.bytes), + AttributeValue::Aigp(v) => b.put_slice(&encode_aigp(v)), + AttributeValue::AttrSet(_v) => { + return Err(EncodingError::unencodable( + "ATTR_SET attribute", + "encoding not implemented", + )); + } } + Ok(()) }; match self.is_extended() { - false => { - bytes.put_u8(value_bytes.len() as u8); - } - true => { - bytes.put_u16(value_bytes.len() as u16); - } + false => with_u8_len(buf, "BGP attribute value length", write_value), + true => with_u16_len(buf, "BGP attribute value length (extended)", write_value), } - bytes.extend(value_bytes); - bytes.freeze() + } + + /// Encode this attribute into a fresh buffer. + pub fn encode(&self, asn_len: AsnLength) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(asn_len, &mut buf)?; + Ok(buf.freeze()) } } impl Attributes { - pub fn encode(&self, asn_len: AsnLength) -> Bytes { - let mut bytes = BytesMut::new(); + /// Append the wire representation of all attributes to `buf`. + pub fn encode_to(&self, asn_len: AsnLength, buf: &mut BytesMut) -> Result<(), EncodingError> { for attr in &self.inner { - bytes.extend(attr.encode(asn_len)); + attr.encode_to(asn_len, buf)?; } - bytes.freeze() + Ok(()) + } + + /// Encode all attributes into a fresh buffer. + pub fn encode(&self, asn_len: AsnLength) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(asn_len, &mut buf)?; + Ok(buf.freeze()) } } @@ -828,7 +844,7 @@ mod tests { value => panic!("expected Raw, got {value:?}"), } assert_eq!( - attributes.encode(AsnLength::Bits16), + attributes.encode(AsnLength::Bits16).unwrap(), Bytes::from_static(&[0x80, 0x16, 0x03, 0xaa, 0xbb, 0xcc]) ); } @@ -849,7 +865,7 @@ mod tests { value => panic!("expected Deprecated, got {value:?}"), } assert_eq!( - attributes.encode(AsnLength::Bits16), + attributes.encode(AsnLength::Bits16).unwrap(), Bytes::from_static(&[0x80, 0x0d, 0x04, 0x01, 0x02, 0x03, 0x04]) ); } @@ -870,7 +886,7 @@ mod tests { value => panic!("expected Raw fallback, got {value:?}"), } assert_eq!( - attributes.encode(AsnLength::Bits16), + attributes.encode(AsnLength::Bits16).unwrap(), Bytes::from_static(&[0x40, 0x03, 0x03, 0x01, 0x02, 0x03]) ); } @@ -899,7 +915,7 @@ mod tests { value => panic!("expected Raw for code {code}, got {value:?}"), } assert!(attributes.has_attr(AttrType::from(code)), "code {code}"); - assert_eq!(attributes.encode(AsnLength::Bits16), Bytes::from(wire)); + assert_eq!(attributes.encode(AsnLength::Bits16).unwrap(), Bytes::from(wire)); } } @@ -926,7 +942,7 @@ mod tests { value => panic!("expected Unknown, got {value:?}"), } assert!(attributes.has_attr(AttrType::Unknown(0x7f))); - assert_eq!(attributes.encode(AsnLength::Bits16), Bytes::from(wire)); + assert_eq!(attributes.encode(AsnLength::Bits16).unwrap(), Bytes::from(wire)); } #[test] @@ -960,7 +976,7 @@ mod tests { (_, value) => panic!("unexpected value for {name}: {value:?}"), } assert_eq!( - attributes.encode(AsnLength::Bits16), + attributes.encode(AsnLength::Bits16).unwrap(), Bytes::from(wire), "{name}" ); diff --git a/src/parser/bgp/messages.rs b/src/parser/bgp/messages.rs index 6ac41a24..d2144f7b 100644 --- a/src/parser/bgp/messages.rs +++ b/src/parser/bgp/messages.rs @@ -598,7 +598,8 @@ impl BgpUpdateMessage { bytes.put_slice(&withdrawn_bytes); // attributes - let attr_bytes = self.attributes.encode(asn_len); + // TODO(fallible-encoding): temporary until BgpUpdateMessage::encode returns Result + let attr_bytes = self.attributes.encode(asn_len).expect("attribute encoding failed"); bytes.put_u16(attr_bytes.len() as u16); bytes.put_slice(&attr_bytes); diff --git a/src/parser/iters/route.rs b/src/parser/iters/route.rs index 8d44cac6..1fa77515 100644 --- a/src/parser/iters/route.rs +++ b/src/parser/iters/route.rs @@ -1303,7 +1303,7 @@ mod tests { ); let attrs = parse_route_attributes( - attributes.encode(AsnLength::Bits16), + attributes.encode(AsnLength::Bits16).unwrap(), &AsnLength::Bits16, false, RouteAttributeContext { @@ -1325,7 +1325,7 @@ mod tests { #[test] fn selective_attribute_parser_handles_as_path_without_as4_path() { let attrs = parse_route_attributes( - route_attributes([64500, 64501]).encode(AsnLength::Bits16), + route_attributes([64500, 64501]).encode(AsnLength::Bits16).unwrap(), &AsnLength::Bits16, false, RouteAttributeContext { @@ -1356,7 +1356,7 @@ mod tests { ); let attrs = parse_route_attributes( - attributes.encode(AsnLength::Bits16), + attributes.encode(AsnLength::Bits16).unwrap(), &AsnLength::Bits16, false, RouteAttributeContext { diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index 02a354fc..9bca83ee 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -196,7 +196,11 @@ impl RibEntry { bytes.put_u32(path_id); } } - let attr_bytes = self.attributes.encode(AsnLength::Bits32); + // TODO(fallible-encoding): temporary until RibEntry encoding returns Result + let attr_bytes = self + .attributes + .encode(AsnLength::Bits32) + .expect("attribute encoding failed"); bytes.put_u16(attr_bytes.len() as u16); bytes.extend(attr_bytes); bytes.freeze() From a22ea6b9a7d1d09e5cb04b57c19ede1c891e5edd Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Wed, 29 Jul 2026 22:03:56 +0200 Subject: [PATCH 3/7] Propagate fallible encoding through BGP and MRT message layers All encode() methods from BgpOpenMessage up to MrtRecord now return Result; no silent truncation and no panicking wrappers remain on any encode path: - BgpOpenMessage rejects optional parameter type 255 (reserved by RFC 9072 as the extended-length marker; emitting it round-trips to a structurally different message) and returns an error instead of debug_assert when extended parameters exceed u16::MAX - BgpUpdateMessage writes withdrawn-routes and path-attribute sections through checked length prefixes - BgpMessage checks the total message length against the u16 wire field - TableDumpMessage uses the shared Attributes::encode_to (removing a hand-rolled duplicate loop); RibAfiEntries checks the entry count (was: wrapping cast) and propagates per-entry errors - PeerIndexTable/GeoPeerTable check view-name length and peer count - MrtMessage::encode returns Unencodable for RIB_GENERIC instead of todo!(); Bgp4MpMessage, MrtRecord, and both MRT encoders' export_bytes() propagate errors Assisted-by: Claude Code --- examples/filter_export_rib.rs | 2 +- examples/mrt_filter_archiver.rs | 2 +- examples/parse_bmp_mpls.rs | 2 +- examples/raw_attributes.rs | 2 +- examples/real_time_routeviews_kafka_to_mrt.rs | 2 +- src/encoder/rib_encoder.rs | 18 +-- src/encoder/updates_encoder.rs | 11 +- src/lib.rs | 2 +- src/parser/bgp/messages.rs | 124 ++++++++++-------- .../bmp/messages/peer_up_notification.rs | 26 ++-- src/parser/bmp/messages/route_mirroring.rs | 2 +- src/parser/bmp/messages/route_monitoring.rs | 6 +- src/parser/iters/route.rs | 46 +++---- src/parser/mrt/messages/bgp4mp.rs | 12 +- src/parser/mrt/messages/mod.rs | 22 ++-- src/parser/mrt/messages/table_dump.rs | 26 ++-- .../messages/table_dump_v2/geo_peer_table.rs | 26 ++-- .../table_dump_v2/peer_index_table.rs | 27 ++-- .../messages/table_dump_v2/rib_afi_entries.rs | 37 +++--- src/parser/mrt/mrt_record.rs | 22 +++- tests/test_encoding.rs | 10 +- 21 files changed, 234 insertions(+), 193 deletions(-) diff --git a/examples/filter_export_rib.rs b/examples/filter_export_rib.rs index 73a5be20..45dea1aa 100644 --- a/examples/filter_export_rib.rs +++ b/examples/filter_export_rib.rs @@ -23,7 +23,7 @@ fn main() { info!("exporting filtered RIB..."); let mut writer = oneio::get_writer("filtered-13335.rib.gz").unwrap(); - writer.write_all(encoder.export_bytes().as_ref()).unwrap(); + writer.write_all(encoder.export_bytes().unwrap().as_ref()).unwrap(); drop(writer); info!("exporting filtered RIB...done"); diff --git a/examples/mrt_filter_archiver.rs b/examples/mrt_filter_archiver.rs index be3b859b..4927a8bf 100644 --- a/examples/mrt_filter_archiver.rs +++ b/examples/mrt_filter_archiver.rs @@ -27,7 +27,7 @@ fn main() { .unwrap() .into_record_iter() .for_each(|record| { - let bytes = record.encode(); + let bytes = record.encode().unwrap(); mrt_writer.write_all(&bytes).unwrap(); records_count += 1; let mut elementor = Elementor::new(); diff --git a/examples/parse_bmp_mpls.rs b/examples/parse_bmp_mpls.rs index 1dbb37f7..6b63f919 100644 --- a/examples/parse_bmp_mpls.rs +++ b/examples/parse_bmp_mpls.rs @@ -72,7 +72,7 @@ fn create_bmp_mpls_message() -> Vec { let bgp_msg = BgpMessage::Update(bgp_update); // Encode the BGP message - let bgp_bytes = bgp_msg.encode(AsnLength::Bits32); + let bgp_bytes = bgp_msg.encode(AsnLength::Bits32).unwrap(); // Now construct the BMP message // BMP Common Header (6 bytes) + Per-Peer Header (42 bytes) + BGP message diff --git a/examples/raw_attributes.rs b/examples/raw_attributes.rs index d6637fa9..376ad0f1 100644 --- a/examples/raw_attributes.rs +++ b/examples/raw_attributes.rs @@ -90,7 +90,7 @@ fn main() { ); // Encode and decode round-trip - let encoded = attributes.encode(AsnLength::Bits32); + let encoded = attributes.encode(AsnLength::Bits32).unwrap(); println!("\nEncoded size: {} bytes", encoded.len()); // Show raw access to the undecoded bytes diff --git a/examples/real_time_routeviews_kafka_to_mrt.rs b/examples/real_time_routeviews_kafka_to_mrt.rs index ae1a1fc3..a8aac67e 100644 --- a/examples/real_time_routeviews_kafka_to_mrt.rs +++ b/examples/real_time_routeviews_kafka_to_mrt.rs @@ -56,7 +56,7 @@ fn consume_and_archive( } }; - let bytes = mrt_record.encode(); + let bytes = mrt_record.encode().unwrap(); archive_writer.write_all(&bytes).unwrap(); archive_writer.flush().unwrap(); records_count += 1; diff --git a/src/encoder/rib_encoder.rs b/src/encoder/rib_encoder.rs index 1d388261..250014e8 100644 --- a/src/encoder/rib_encoder.rs +++ b/src/encoder/rib_encoder.rs @@ -8,6 +8,7 @@ use crate::models::{ Attributes, BgpElem, CommonHeader, EntryType, MrtMessage, NetworkPrefix, Peer, PeerIndexTable, RibAfiEntries, RibEntry, TableDumpV2Message, TableDumpV2Type, }; +use crate::error::EncodingError; use crate::utils::convert_timestamp; use bytes::{Bytes, BytesMut}; use ipnet::IpNet; @@ -78,8 +79,9 @@ impl MrtRibEncoder { /// The resulting `BytesMut` object is then converted to an immutable `Bytes` object using `freeze()` and returned. /// /// # Return - /// Returns a `Bytes` object containing the exported data as a byte array. - pub fn export_bytes(&mut self) -> Bytes { + /// Returns a `Bytes` object containing the exported data as a byte array, + /// or an [EncodingError] if any element cannot be represented in wire format. + pub fn export_bytes(&mut self) -> Result { let mut bytes = BytesMut::new(); // encode peer-index-table @@ -88,7 +90,7 @@ impl MrtRibEncoder { )); let (seconds, _microseconds) = convert_timestamp(self.timestamp); let subtype = TableDumpV2Type::PeerIndexTable as u16; - let data_bytes = mrt_message.encode(subtype); + let data_bytes = mrt_message.encode(subtype)?; let header = CommonHeader { timestamp: seconds, microsecond_timestamp: None, @@ -120,7 +122,7 @@ impl MrtRibEncoder { let (seconds, _microseconds) = convert_timestamp(self.timestamp); let subtype = rib_type as u16; - let data_bytes = mrt_message.encode(subtype); + let data_bytes = mrt_message.encode(subtype)?; let header_bytes = CommonHeader { timestamp: seconds, microsecond_timestamp: None, @@ -135,7 +137,7 @@ impl MrtRibEncoder { self.reset(); - bytes.freeze() + Ok(bytes.freeze()) } } @@ -159,7 +161,7 @@ mod tests { encoder.process_elem(&elem); elem.prefix.prefix = "10.251.0.0/24".parse().unwrap(); encoder.process_elem(&elem); - let bytes = encoder.export_bytes(); + let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes.clone()); while cursor.has_remaining() { @@ -176,7 +178,7 @@ mod tests { // ipv6 prefix elem.prefix.prefix = "2001:db8::/32".parse().unwrap(); encoder.process_elem(&elem); - let bytes = encoder.export_bytes(); + let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes.clone()); while cursor.has_remaining() { @@ -195,7 +197,7 @@ mod tests { elem.prefix = NetworkPrefix::new("10.250.0.0/24".parse().unwrap(), Some(42)); encoder.process_elem(&elem); - let bytes = encoder.export_bytes(); + let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes); let _peer_table = parse_mrt_record(&mut cursor).unwrap(); let parsed = parse_mrt_record(&mut cursor).unwrap(); diff --git a/src/encoder/updates_encoder.rs b/src/encoder/updates_encoder.rs index e5038290..473ea554 100644 --- a/src/encoder/updates_encoder.rs +++ b/src/encoder/updates_encoder.rs @@ -6,6 +6,7 @@ use crate::models::{ EntryType, MrtMessage, }; use crate::utils::convert_timestamp; +use crate::error::EncodingError; use crate::BgpElem; use bytes::{Bytes, BytesMut}; @@ -27,7 +28,7 @@ impl MrtUpdatesEncoder { self.cached_elems.push(elem.clone()); } - pub fn export_bytes(&mut self) -> Bytes { + pub fn export_bytes(&mut self) -> Result { let mut bytes = BytesMut::new(); for elem in &self.cached_elems { @@ -55,7 +56,7 @@ impl MrtUpdatesEncoder { let (seconds, microseconds) = convert_timestamp(elem.timestamp); let subtype = Bgp4MpType::MessageAs4 as u16; - let data_bytes = mrt_message.encode(subtype); + let data_bytes = mrt_message.encode(subtype)?; let header_bytes = CommonHeader { timestamp: seconds, microsecond_timestamp: Some(microseconds), @@ -70,7 +71,7 @@ impl MrtUpdatesEncoder { self.reset(); - bytes.freeze() + Ok(bytes.freeze()) } } @@ -94,7 +95,7 @@ mod tests { encoder.process_elem(&elem); elem.prefix.prefix = "10.251.0.0/24".parse().unwrap(); encoder.process_elem(&elem); - let bytes = encoder.export_bytes(); + let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes.clone()); while cursor.has_remaining() { @@ -114,7 +115,7 @@ mod tests { // ipv6 prefix elem.prefix = NetworkPrefix::from_str("2001:db8::/32").unwrap(); encoder.process_elem(&elem); - let bytes = encoder.export_bytes(); + let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes.clone()); while cursor.has_remaining() { let _parsed = parse_mrt_record(&mut cursor).unwrap(); diff --git a/src/lib.rs b/src/lib.rs index 59afdfd7..ee9f424c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -357,7 +357,7 @@ bgpkit_parser::BgpkitParser::new( }); let mut mrt_writer = oneio::get_writer("as3356_mrt.gz").unwrap(); -mrt_writer.write_all(updates_encoder.export_bytes().as_ref()).unwrap(); +mrt_writer.write_all(updates_encoder.export_bytes().unwrap().as_ref()).unwrap(); drop(mrt_writer); ``` diff --git a/src/parser/bgp/messages.rs b/src/parser/bgp/messages.rs index d2144f7b..dd397407 100644 --- a/src/parser/bgp/messages.rs +++ b/src/parser/bgp/messages.rs @@ -3,7 +3,8 @@ use bytes::{Buf, BufMut, Bytes, BytesMut}; use std::convert::TryFrom; use std::net::Ipv4Addr; -use crate::error::{BgpValidationWarning, ParserError}; +use crate::encoder::sink::{put_u16_len_slice, put_u8_len_slice, with_u16_len}; +use crate::error::{check_max, BgpValidationWarning, EncodingError, ParserError}; use crate::models::capabilities::{ AddPathCapability, BgpCapabilityType, BgpExtendedMessageCapability, BgpRoleCapability, ExtendedNextHopCapability, FourOctetAsCapability, GracefulRestartCapability, @@ -382,7 +383,7 @@ pub fn parse_bgp_open_message(input: &mut Bytes) -> Result Bytes { +fn encode_bgp_open_param_value(param: &OptParam) -> Result { let mut buf = BytesMut::new(); match ¶m.param_value { ParamValue::Capacities(capacities) => { @@ -399,24 +400,32 @@ fn encode_bgp_open_param_value(param: &OptParam) -> Bytes { CapabilityValue::BgpExtendedMessage(bem) => bem.encode(), CapabilityValue::Raw(raw) => Bytes::from(raw.clone()), }; - let capability_len = u8::try_from(encoded_value.len()) - .expect("BGP capability value length exceeds 255 octets"); - buf.put_u8(capability_len); - buf.put_slice(&encoded_value); + put_u8_len_slice(&mut buf, "BGP capability value length", &encoded_value)?; } } ParamValue::Raw(bytes) => buf.put_slice(bytes), } - buf.freeze() + Ok(buf.freeze()) } impl BgpOpenMessage { - pub fn encode(&self) -> Bytes { + pub fn encode(&self) -> Result { let encoded_params: Vec<(u8, Bytes)> = self .opt_params .iter() - .map(|param| (param.param_type, encode_bgp_open_param_value(param))) - .collect(); + .map(|param| { + // RFC 9072 reserves optional parameter type 255 as the + // extended-length marker; emitting it as a real parameter + // would be misparsed on the receiving side. + if param.param_type == u8::MAX { + return Err(EncodingError::unencodable( + "BGP OPEN optional parameter type", + "type 255 is reserved by RFC 9072 as the extended-length marker", + )); + } + Ok((param.param_type, encode_bgp_open_param_value(param)?)) + }) + .collect::>()?; let values_len: usize = encoded_params.iter().map(|(_, value)| value.len()).sum(); // Non-extended framing spends 2 header octets (type + 1-octet length) per @@ -449,11 +458,11 @@ impl BgpOpenMessage { if use_extended_length { // RFC 9072: type 255 signals a two-octet aggregate length and // two-octet lengths for each optional parameter. - debug_assert!( - encoded_params_len <= u16::MAX as usize, - "BGP OPEN optional parameters ({encoded_params_len} bytes) exceed the \ - two-octet extended length field; wire framing would be corrupt" - ); + check_max( + "BGP OPEN extended optional parameters total length", + encoded_params_len, + u16::MAX as usize, + )?; buf.put_u8(u8::MAX); buf.put_u16(encoded_params_len as u16); } @@ -461,12 +470,9 @@ impl BgpOpenMessage { for (param_type, value) in encoded_params { buf.put_u8(param_type); if use_extended_length { - debug_assert!( - value.len() <= u16::MAX as usize, - "BGP OPEN optional parameter ({} bytes) exceeds the two-octet \ - length field; wire framing would be corrupt", - value.len() - ); + // Cannot overflow: the aggregate check above bounds + // encoded_params_len, and each value.len() <= encoded_params_len - 3. + debug_assert!(value.len() <= u16::MAX as usize); buf.put_u16(value.len() as u16); } else { // Fits in a u8: use_extended_length is set above whenever the @@ -475,7 +481,7 @@ impl BgpOpenMessage { } buf.put_slice(&value); } - buf.freeze() + Ok(buf.freeze()) } } @@ -589,23 +595,24 @@ pub fn parse_bgp_update_message( } impl BgpUpdateMessage { - pub fn encode(&self, asn_len: AsnLength) -> Bytes { + pub fn encode(&self, asn_len: AsnLength) -> Result { let mut bytes = BytesMut::new(); // withdrawn prefixes let withdrawn_bytes = encode_nlri_prefixes(&self.withdrawn_prefixes); - bytes.put_u16(withdrawn_bytes.len() as u16); - bytes.put_slice(&withdrawn_bytes); + put_u16_len_slice( + &mut bytes, + "BGP UPDATE withdrawn routes length", + &withdrawn_bytes, + )?; // attributes - // TODO(fallible-encoding): temporary until BgpUpdateMessage::encode returns Result - let attr_bytes = self.attributes.encode(asn_len).expect("attribute encoding failed"); - - bytes.put_u16(attr_bytes.len() as u16); - bytes.put_slice(&attr_bytes); + with_u16_len(&mut bytes, "BGP UPDATE total path attribute length", |b| { + self.attributes.encode_to(asn_len, b) + })?; bytes.extend(encode_nlri_prefixes(&self.announced_prefixes)); - bytes.freeze() + Ok(bytes.freeze()) } /// Check if this is an end-of-rib message. @@ -655,23 +662,25 @@ impl BgpMessage { /// BGP marker value: 16 bytes of 0xFF (RFC 4271) const MARKER: [u8; 16] = [0xFF; 16]; - pub fn encode(&self, asn_len: AsnLength) -> Bytes { + pub fn encode(&self, asn_len: AsnLength) -> Result { let mut bytes = BytesMut::new(); // RFC 4271: Marker is 16 bytes of 0xFF bytes.put_slice(&Self::MARKER); let (msg_type, msg_bytes) = match self { - BgpMessage::Open(msg) => (BgpMessageType::OPEN, msg.encode()), - BgpMessage::Update(msg) => (BgpMessageType::UPDATE, msg.encode(asn_len)), + BgpMessage::Open(msg) => (BgpMessageType::OPEN, msg.encode()?), + BgpMessage::Update(msg) => (BgpMessageType::UPDATE, msg.encode(asn_len)?), BgpMessage::Notification(msg) => (BgpMessageType::NOTIFICATION, msg.encode()), BgpMessage::KeepAlive => (BgpMessageType::KEEPALIVE, Bytes::new()), }; // msg total bytes length = msg bytes + 16 bytes marker + 2 bytes length + 1 byte type - bytes.put_u16(msg_bytes.len() as u16 + 16 + 2 + 1); + let total_len = msg_bytes.len() + 16 + 2 + 1; + check_max("BGP message total length", total_len, u16::MAX as usize)?; + bytes.put_u16(total_len as u16); bytes.put_u8(msg_type as u8); bytes.put_slice(&msg_bytes); - bytes.freeze() + Ok(bytes.freeze()) } } @@ -896,7 +905,7 @@ mod tests { fn test_bgp_marker_encoding_rfc4271() { // Test that BgpMessage::encode produces correct RFC 4271 marker (all 0xFF) let msg = BgpMessage::KeepAlive; - let encoded = msg.encode(AsnLength::Bits16); + let encoded = msg.encode(AsnLength::Bits16).unwrap(); // First 16 bytes should be all 0xFF assert_eq!( @@ -1027,7 +1036,7 @@ mod tests { extended_length: false, opt_params: vec![], }; - let bytes = msg.encode(); + let bytes = msg.encode().unwrap(); assert_eq!( bytes, Bytes::from_static(&[ @@ -1085,7 +1094,7 @@ mod tests { parse_bgp_message(&mut input, false, &AsnLength::Bits16).unwrap_or_else(|error| { panic!("failed to parse {name}: {error}"); }); - let encoded = parsed.encode(AsnLength::Bits16); + let encoded = parsed.encode(AsnLength::Bits16).unwrap(); assert_eq!(encoded, wire, "{name}"); } @@ -1105,16 +1114,15 @@ mod tests { }], }; - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); assert_eq!(encoded[9], 5); assert_eq!(&encoded[10..], &[254, 3, 0xAA, 0xBB, 0xCC]); let parsed = parse_bgp_open_message(&mut encoded.clone()).unwrap(); - assert_eq!(parsed.encode(), encoded); + assert_eq!(parsed.encode().unwrap(), encoded); } #[test] - #[should_panic(expected = "BGP capability value length exceeds 255 octets")] fn test_bgp_open_encoding_rejects_oversized_add_path_capability() { use crate::models::capabilities::{AddPathAddressFamily, AddPathSendReceive}; @@ -1141,7 +1149,15 @@ mod tests { }], }; - msg.encode(); + let err = msg.encode().unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "BGP capability value length", + actual: 256, + max: 255 + } + ); } #[test] @@ -1158,7 +1174,7 @@ mod tests { }], }; - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); assert_eq!( &encoded[9..], @@ -1166,7 +1182,7 @@ mod tests { ); let parsed = parse_bgp_open_message(&mut encoded.clone()).unwrap(); assert!(parsed.extended_length); - assert_eq!(parsed.encode(), encoded); + assert_eq!(parsed.encode().unwrap(), encoded); } #[test] @@ -1183,13 +1199,13 @@ mod tests { }], }; - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); assert_eq!(encoded.len(), 272); assert_eq!(&encoded[9..16], &[0xFF, 0xFF, 0x01, 0x03, 254, 0x01, 0x00]); let parsed = parse_bgp_open_message(&mut encoded.clone()).unwrap(); assert!(parsed.extended_length); - assert_eq!(parsed.encode(), encoded); + assert_eq!(parsed.encode().unwrap(), encoded); } #[test] @@ -1198,7 +1214,7 @@ mod tests { error: BgpError::MessageHeaderError(MessageHeaderError::BAD_MESSAGE_LENGTH), data: vec![0x00, 0x00], }); - let bytes = bgp_message.encode(AsnLength::Bits16); + let bytes = bgp_message.encode(AsnLength::Bits16).unwrap(); // RFC 4271: Marker is 16 bytes of 0xFF assert_eq!( bytes, @@ -1453,7 +1469,7 @@ mod tests { use crate::models::capabilities::BgpExtendedMessageCapability; // Test that the encoding path for BgpExtendedMessage capability is covered - // This specifically tests the line: CapabilityValue::BgpExtendedMessage(bem) => bem.encode() + // This specifically tests the line: CapabilityValue::BgpExtendedMessage(bem) => bem.encode().unwrap() let capability_value = CapabilityValue::BgpExtendedMessage(BgpExtendedMessageCapability::new()); let capability = Capability { @@ -1476,7 +1492,7 @@ mod tests { }; // This will exercise the encoding path we need to test - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); assert!(!encoded.is_empty()); // Verify we can parse it back (exercises the parsing path too) @@ -1549,7 +1565,7 @@ mod tests { }], }; - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); // Parse the encoded message back and verify it matches let parsed = parse_bgp_open_message(&mut encoded.clone()).unwrap(); @@ -1607,7 +1623,7 @@ mod tests { }], }; - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); // Parse the encoded message back and verify it matches let parsed = parse_bgp_open_message(&mut encoded.clone()).unwrap(); @@ -1672,7 +1688,7 @@ mod tests { }; // Encode the message - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); // Parse it back let mut encoded_bytes = encoded.clone(); @@ -1753,7 +1769,7 @@ mod tests { }; // Encode and parse back - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); let mut encoded_bytes = encoded.clone(); let parsed = parse_bgp_open_message(&mut encoded_bytes).unwrap(); diff --git a/src/parser/bmp/messages/peer_up_notification.rs b/src/parser/bmp/messages/peer_up_notification.rs index e2391daa..5e4ffaa9 100644 --- a/src/parser/bmp/messages/peer_up_notification.rs +++ b/src/parser/bmp/messages/peer_up_notification.rs @@ -203,7 +203,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp_open_message_bytes = bgp_open_message.encode(AsnLength::Bits32); + let bgp_open_message_bytes = bgp_open_message.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_open_message_bytes); data.extend_from_slice(&bgp_open_message_bytes); @@ -322,7 +322,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp1_bytes = bgp1.encode(AsnLength::Bits32); + let bgp1_bytes = bgp1.encode(AsnLength::Bits32).unwrap(); // Second BGP OPEN message let bgp2 = crate::models::BgpMessage::Open(BgpOpenMessage { @@ -333,7 +333,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp2_bytes = bgp2.encode(AsnLength::Bits32); + let bgp2_bytes = bgp2.encode(AsnLength::Bits32).unwrap(); // Add both BGP messages consecutively data.extend_from_slice(&bgp1_bytes); @@ -441,7 +441,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); // Add incomplete second BGP message (only partial header) @@ -483,7 +483,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); @@ -537,7 +537,7 @@ mod tests { extended_length: false, opt_params: vec![], // No capabilities }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); @@ -586,7 +586,7 @@ mod tests { }]), }], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); @@ -635,7 +635,7 @@ mod tests { }]), }], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); @@ -689,7 +689,7 @@ mod tests { }]), }], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); @@ -751,7 +751,7 @@ mod tests { }]), }], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); @@ -804,7 +804,7 @@ mod tests { }]), }], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); @@ -846,7 +846,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); @@ -900,7 +900,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp_bytes = bgp_open.encode(AsnLength::Bits32); + let bgp_bytes = bgp_open.encode(AsnLength::Bits32).unwrap(); data.extend_from_slice(&bgp_bytes); data.extend_from_slice(&bgp_bytes); diff --git a/src/parser/bmp/messages/route_mirroring.rs b/src/parser/bmp/messages/route_mirroring.rs index 0457aa36..ef5f3c7c 100644 --- a/src/parser/bmp/messages/route_mirroring.rs +++ b/src/parser/bmp/messages/route_mirroring.rs @@ -85,7 +85,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp_message_bytes = bgp_message.encode(AsnLength::Bits32); + let bgp_message_bytes = bgp_message.encode(AsnLength::Bits32).unwrap(); let expected_asn_len = AsnLength::Bits32; let actual_info_len = bgp_message_bytes.len() as u16; diff --git a/src/parser/bmp/messages/route_monitoring.rs b/src/parser/bmp/messages/route_monitoring.rs index 98b98c25..2f24cda5 100644 --- a/src/parser/bmp/messages/route_monitoring.rs +++ b/src/parser/bmp/messages/route_monitoring.rs @@ -98,7 +98,7 @@ mod tests { attributes: Attributes::default(), announced_prefixes: vec![], }); - let bgp_bytes = bgp_update.encode(AsnLength::Bits16); + let bgp_bytes = bgp_update.encode(AsnLength::Bits16).unwrap(); let mut data = bgp_bytes; let asn_len = AsnLength::Bits16; // RFC 9069 violation @@ -121,7 +121,7 @@ mod tests { attributes: Attributes::default(), announced_prefixes: vec![], }); - let bgp_bytes = bgp_update.encode(AsnLength::Bits32); + let bgp_bytes = bgp_update.encode(AsnLength::Bits32).unwrap(); let mut data = bgp_bytes; let asn_len = AsnLength::Bits32; // RFC 9069 compliant @@ -144,7 +144,7 @@ mod tests { attributes: Attributes::default(), announced_prefixes: vec![], }); - let bgp_bytes = bgp_update.encode(AsnLength::Bits16); + let bgp_bytes = bgp_update.encode(AsnLength::Bits16).unwrap(); let mut data = bgp_bytes; let asn_len = AsnLength::Bits16; diff --git a/src/parser/iters/route.rs b/src/parser/iters/route.rs index 1fa77515..cd8151a9 100644 --- a/src/parser/iters/route.rs +++ b/src/parser/iters/route.rs @@ -1009,8 +1009,8 @@ mod tests { })), }; - let mut bytes = pit_record.encode().to_vec(); - bytes.extend_from_slice(&rib_record.encode()); + let mut bytes = pit_record.encode().unwrap().to_vec(); + bytes.extend_from_slice(&rib_record.encode().unwrap()); bytes } @@ -1047,7 +1047,7 @@ mod tests { rib_body.put_u32(1); rib_body.extend(NetworkPrefix::from_str("203.0.113.0/24").unwrap().encode()); rib_body.put_u16(2); - rib_body.extend(first_entry.encode()); + rib_body.extend(first_entry.encode().unwrap()); rib_body.put_u16(peer_index); rib_body.put_u32(1_699_999_998); rib_body.put_u16(32); @@ -1062,7 +1062,7 @@ mod tests { length: rib_body.len() as u32, }; - let mut bytes = pit_record.encode().to_vec(); + let mut bytes = pit_record.encode().unwrap().to_vec(); bytes.extend_from_slice(&rib_header.encode()); bytes.extend_from_slice(&rib_body); @@ -1112,7 +1112,7 @@ mod tests { data.put_u16(65001); data.put_u16(0); data.put_u16(Afi::LinkState as u16); - data.extend(&BgpMessage::KeepAlive.encode(AsnLength::Bits16)); + data.put_slice(&BgpMessage::KeepAlive.encode(AsnLength::Bits16).unwrap()); let error = match parse_bgp4mp_routes(Bgp4MpType::Message as u16, data.freeze(), 1_700_000_000.0) { @@ -1128,7 +1128,7 @@ mod tests { #[test] fn route_iterator_matches_elem_projection_for_update() { - let bytes = update_record().encode().to_vec(); + let bytes = update_record().encode().unwrap().to_vec(); let routes = assert_route_projection(bytes); assert_eq!(routes.len(), 2); assert_eq!(routes[0].elem_type, ElemType::ANNOUNCE); @@ -1149,7 +1149,7 @@ mod tests { ], }), ) - .encode() + .encode().unwrap() .to_vec(); let routes = BgpkitParser::from_reader(Cursor::new(bytes)) @@ -1201,7 +1201,7 @@ mod tests { announced_prefixes: vec![], }), ) - .encode() + .encode().unwrap() .to_vec(); let routes = assert_route_projection(bytes); @@ -1234,7 +1234,7 @@ mod tests { ]; let mut bytes = Vec::new(); for record in records { - bytes.extend_from_slice(&record.encode()); + bytes.extend_from_slice(&record.encode().unwrap()); } assert!(assert_route_projection(bytes).is_empty()); @@ -1250,7 +1250,7 @@ mod tests { announced_prefixes: vec![NetworkPrefix::from_str("203.0.113.0/24").unwrap()], }), ) - .encode() + .encode().unwrap() .to_vec(); let routes = assert_route_projection(bytes); @@ -1260,7 +1260,7 @@ mod tests { #[test] fn route_iterator_filters_match_elem_projection_for_update() { - let bytes = update_record().encode().to_vec(); + let bytes = update_record().encode().unwrap().to_vec(); let cases: &[&[(&str, &str)]] = &[ &[("peer_ip", "192.0.2.1")], &[("peer_ip", "192.0.2.99")], @@ -1459,7 +1459,7 @@ mod tests { #[test] fn route_iterator_matches_elem_projection_for_table_dump() { - let bytes = table_dump_record().encode().to_vec(); + let bytes = table_dump_record().encode().unwrap().to_vec(); let routes = assert_route_projection(bytes); assert_eq!(routes.len(), 1); assert_eq!(routes[0].timestamp, 1_699_999_998.0); @@ -1468,7 +1468,7 @@ mod tests { #[test] fn route_iterator_matches_elem_projection_for_table_dump_ipv6() { - let bytes = table_dump_ipv6_record().encode().to_vec(); + let bytes = table_dump_ipv6_record().encode().unwrap().to_vec(); let routes = assert_route_projection(bytes); assert_eq!(routes.len(), 1); assert_eq!( @@ -1534,8 +1534,8 @@ mod tests { })), }; - let mut bytes = pit_record.encode().to_vec(); - bytes.extend_from_slice(&rib_record.encode()); + let mut bytes = pit_record.encode().unwrap().to_vec(); + bytes.extend_from_slice(&rib_record.encode().unwrap()); let routes = assert_route_projection(bytes); assert_eq!(routes.len(), 1); assert_eq!( @@ -1569,7 +1569,7 @@ mod tests { })), }; - let routes = assert_route_projection(record.encode().to_vec()); + let routes = assert_route_projection(record.encode().unwrap().to_vec()); assert_eq!(routes.len(), 1); assert_eq!( routes[0].peer_ip, @@ -1690,7 +1690,7 @@ mod tests { let mut no_peer_table = None; assert!(parse_table_dump_v2_routes( TableDumpV2Type::RibIpv4Unicast as u16, - rib.encode(), + rib.encode().unwrap(), &mut no_peer_table, ) .is_err()); @@ -1699,7 +1699,7 @@ mod tests { let routes = collect_route_record_iter( parse_table_dump_v2_routes( TableDumpV2Type::RibIpv4Unicast as u16, - rib.encode(), + rib.encode().unwrap(), &mut empty_peer_table, ) .unwrap(), @@ -1741,7 +1741,7 @@ mod tests { add_path_truncated.put_u32(1); add_path_truncated.extend(NetworkPrefix::from_str("203.0.113.0/24").unwrap().encode()); add_path_truncated.put_u16(2); - add_path_truncated.extend(first_entry.encode()); + add_path_truncated.extend(first_entry.encode().unwrap()); add_path_truncated.put_u16(peer_index); add_path_truncated.put_u32(1_699_999_998); add_path_truncated.put_u32(5678); @@ -1836,7 +1836,7 @@ mod tests { fn route_iterator_skips_route_parse_errors() { let routes = BgpkitParser::from_reader(Cursor::new( table_dump_v2_rib_without_peer_table_record() - .encode() + .encode().unwrap() .to_vec(), )) .into_route_iter() @@ -1847,7 +1847,7 @@ mod tests { #[test] fn fallible_route_iterator_applies_filters_to_cached_routes() { - let routes = BgpkitParser::from_reader(Cursor::new(update_record().encode().to_vec())) + let routes = BgpkitParser::from_reader(Cursor::new(update_record().encode().unwrap().to_vec())) .add_filter("type", "w") .unwrap() .into_fallible_route_iter() @@ -1862,7 +1862,7 @@ mod tests { fn fallible_route_iterator_returns_route_parse_errors() { let mut iter = BgpkitParser::from_reader(Cursor::new( table_dump_v2_rib_without_peer_table_record() - .encode() + .encode().unwrap() .to_vec(), )) .into_fallible_route_iter(); @@ -1872,7 +1872,7 @@ mod tests { #[test] fn fallible_route_iterator_yields_routes() { - let bytes = update_record().encode().to_vec(); + let bytes = update_record().encode().unwrap().to_vec(); let routes = BgpkitParser::from_reader(Cursor::new(bytes)) .into_fallible_route_iter() .collect::, _>>() diff --git a/src/parser/mrt/messages/bgp4mp.rs b/src/parser/mrt/messages/bgp4mp.rs index e76734fd..57db802a 100644 --- a/src/parser/mrt/messages/bgp4mp.rs +++ b/src/parser/mrt/messages/bgp4mp.rs @@ -1,4 +1,4 @@ -use crate::error::ParserError; +use crate::error::{EncodingError, ParserError}; use crate::models::*; use crate::parser::bgp::messages::parse_bgp_message; use crate::parser::{encode_asn, encode_ipaddr, ReadUtils}; @@ -171,7 +171,7 @@ pub fn parse_bgp4mp_message( } impl Bgp4MpMessage { - pub fn encode(&self, asn_len: AsnLength) -> Bytes { + pub fn encode(&self, asn_len: AsnLength) -> Result { let mut bytes = BytesMut::new(); bytes.extend(encode_asn(&self.peer_asn, &asn_len)); bytes.extend(encode_asn(&self.local_asn, &asn_len)); @@ -179,8 +179,8 @@ impl Bgp4MpMessage { bytes.put_u16(address_family(&self.peer_ip)); bytes.extend(encode_ipaddr(&self.peer_ip)); bytes.extend(encode_ipaddr(&self.local_ip)); - bytes.extend(&self.bgp_message.encode(asn_len)); - bytes.freeze() + bytes.put_slice(&self.bgp_message.encode(asn_len)?); + Ok(bytes.freeze()) } } @@ -274,7 +274,7 @@ mod tests { bgp_message: BgpMessage::KeepAlive, }; - let encoded = message.encode(AsnLength::Bits16); + let encoded = message.encode(AsnLength::Bits16).unwrap(); let parsed = parse_bgp4mp(Bgp4MpType::Message as u16, encoded).unwrap(); match parsed { @@ -297,7 +297,7 @@ mod tests { data.put_u16(65001); data.put_u16(0); data.put_u16(Afi::LinkState as u16); - data.extend(&BgpMessage::KeepAlive.encode(AsnLength::Bits16)); + data.put_slice(&BgpMessage::KeepAlive.encode(AsnLength::Bits16).unwrap()); let error = match parse_bgp4mp(Bgp4MpType::Message as u16, data.freeze()) { Err(error) => error, diff --git a/src/parser/mrt/messages/mod.rs b/src/parser/mrt/messages/mod.rs index 3f0d2380..1f1e905c 100644 --- a/src/parser/mrt/messages/mod.rs +++ b/src/parser/mrt/messages/mod.rs @@ -1,3 +1,4 @@ +use crate::error::EncodingError; use crate::models::{AsnLength, Bgp4MpEnum, Bgp4MpType, MrtMessage, TableDumpV2Message}; use bytes::Bytes; @@ -6,16 +7,19 @@ pub(crate) mod table_dump; pub(crate) mod table_dump_v2; impl MrtMessage { - pub fn encode(&self, sub_type: u16) -> Bytes { + pub fn encode(&self, sub_type: u16) -> Result { let msg_bytes: Bytes = match self { - MrtMessage::TableDumpMessage(m) => m.encode(), + MrtMessage::TableDumpMessage(m) => m.encode()?, MrtMessage::TableDumpV2Message(m) => match m { - TableDumpV2Message::PeerIndexTable(p) => p.encode(), - TableDumpV2Message::RibAfi(r) => r.encode(), + TableDumpV2Message::PeerIndexTable(p) => p.encode()?, + TableDumpV2Message::RibAfi(r) => r.encode()?, TableDumpV2Message::RibGeneric(_) => { - todo!("RibGeneric message is not supported yet"); + return Err(EncodingError::unencodable( + "MRT TABLE_DUMP_V2 RIB_GENERIC message", + "encoding not implemented", + )); } - TableDumpV2Message::GeoPeerTable(g) => g.encode(), + TableDumpV2Message::GeoPeerTable(g) => g.encode()?, }, MrtMessage::Bgp4Mp(m) => { let msg_type = Bgp4MpType::try_from(sub_type).unwrap(); @@ -39,13 +43,13 @@ impl MrtMessage { true => AsnLength::Bits32, false => AsnLength::Bits16, }; - msg.encode(asn_len) + msg.encode(asn_len)? } } } }; - msg_bytes + Ok(msg_bytes) } } @@ -70,7 +74,7 @@ mod tests { MrtMessage::TableDumpV2Message(TableDumpV2Message::GeoPeerTable(geo_table)); let subtype = TableDumpV2Type::GeoPeerTable as u16; - let encoded = mrt_message.encode(subtype); + let encoded = mrt_message.encode(subtype).unwrap(); // Should produce some encoded bytes assert!(!encoded.is_empty()); diff --git a/src/parser/mrt/messages/table_dump.rs b/src/parser/mrt/messages/table_dump.rs index 4d201852..61d3f02b 100644 --- a/src/parser/mrt/messages/table_dump.rs +++ b/src/parser/mrt/messages/table_dump.rs @@ -1,3 +1,4 @@ +use crate::encoder::sink::with_u16_len; use crate::error::*; use crate::models::*; use crate::parser::bgp::attributes::parse_attributes; @@ -118,7 +119,7 @@ pub fn parse_table_dump_message( } impl TableDumpMessage { - pub fn encode(&self) -> Bytes { + pub fn encode(&self) -> Result { let mut bytes = BytesMut::new(); bytes.put_u16(self.view_number); bytes.put_u16(self.sequence_number); @@ -146,17 +147,12 @@ impl TableDumpMessage { } bytes.put_u16(self.peer_asn.into()); - // encode attributes - let mut attr_bytes = BytesMut::new(); - for attr in &self.attributes.inner { - // asn_len always 16 bites - attr_bytes.extend(attr.encode(AsnLength::Bits16)); - } - - bytes.put_u16(attr_bytes.len() as u16); - bytes.put_slice(&attr_bytes); + // encode attributes; asn_len is always 16-bit for TABLE_DUMP + with_u16_len(&mut bytes, "TABLE_DUMP attribute length", |b| { + self.attributes.encode_to(AsnLength::Bits16, b) + })?; - bytes.freeze() + Ok(bytes.freeze()) } } @@ -213,7 +209,7 @@ mod tests { "SEQUENCE_NUMBER mismatch" ); // Add more assertions here as per your actual requirements - let encoded = table_dump_message.encode(); + let encoded = table_dump_message.encode().unwrap(); assert_eq!(encoded, bytes); } #[test] @@ -252,7 +248,7 @@ mod tests { // Add more assertions here as per your actual requirements // test encoding - let encoded = table_dump_message.encode(); + let encoded = table_dump_message.encode().unwrap(); assert_eq!(encoded, bytes); } @@ -311,7 +307,7 @@ mod tests { attributes, }; - // This should exercise the attr.encode(AsnLength::Bits16) line - let _encoded = table_dump.encode(); + // This should exercise the attr.encode(AsnLength::Bits16).unwrap() line + let _encoded = table_dump.encode().unwrap(); } } diff --git a/src/parser/mrt/messages/table_dump_v2/geo_peer_table.rs b/src/parser/mrt/messages/table_dump_v2/geo_peer_table.rs index a906487e..ec222423 100644 --- a/src/parser/mrt/messages/table_dump_v2/geo_peer_table.rs +++ b/src/parser/mrt/messages/table_dump_v2/geo_peer_table.rs @@ -1,6 +1,7 @@ //! RFC 6397: GEO_PEER_TABLE parsing for MRT TABLE_DUMP_V2 format -use crate::error::ParserError; +use crate::encoder::sink::put_u16_len_slice; +use crate::error::{check_max, EncodingError, ParserError}; use crate::models::*; use crate::parser::ReadUtils; use bytes::{Buf, BufMut, Bytes, BytesMut}; @@ -134,24 +135,31 @@ impl GeoPeerTable { /// -0.1278, // London longitude /// ); /// - /// let encoded = geo_table.encode(); + /// let encoded = geo_table.encode().unwrap(); /// ``` - pub fn encode(&self) -> Bytes { + pub fn encode(&self) -> Result { let mut buf = BytesMut::new(); // Encode collector BGP ID (4 bytes) buf.put_u32(self.collector_bgp_id.into()); // Encode view name length and view name - let view_name_bytes = self.view_name.as_bytes(); - buf.put_u16(view_name_bytes.len() as u16); - buf.extend(view_name_bytes); + put_u16_len_slice( + &mut buf, + "GeoPeerTable view name length", + self.view_name.as_bytes(), + )?; // Encode collector coordinates (4 bytes each, 32-bit float) buf.put_f32(self.collector_latitude); buf.put_f32(self.collector_longitude); // Encode peer count + check_max( + "GeoPeerTable peer count", + self.geo_peers.len(), + u16::MAX as usize, + )?; buf.put_u16(self.geo_peers.len() as u16); // Encode each peer entry @@ -188,7 +196,7 @@ impl GeoPeerTable { buf.put_f32(geo_peer.peer_longitude); } - buf.freeze() + Ok(buf.freeze()) } } @@ -344,7 +352,7 @@ mod tests { original_table.add_geo_peer(geo_peer2); // Encode and then parse back - let encoded = original_table.encode(); + let encoded = original_table.encode().unwrap(); let mut encoded_bytes = encoded; let parsed_table = parse_geo_peer_table(&mut encoded_bytes).unwrap(); @@ -423,7 +431,7 @@ mod tests { geo_table.add_geo_peer(geo_peer2); // Encode the geo table - let encoded = geo_table.encode(); + let encoded = geo_table.encode().unwrap(); // Create expected bytes manually for comparison let mut expected = BytesMut::new(); diff --git a/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs b/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs index bd3a84a5..d0ccb6d1 100644 --- a/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs +++ b/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs @@ -1,5 +1,7 @@ use crate::models::{Afi, AsnLength, Peer, PeerIndexTable, PeerType}; use crate::parser::ReadUtils; +use crate::encoder::sink::put_u16_len_slice; +use crate::error::{check_max, EncodingError}; use crate::ParserError; use bytes::{BufMut, Bytes, BytesMut}; use std::collections::HashMap; @@ -136,24 +138,25 @@ impl PeerIndexTable { /// peer_ip_id_map: Default::default(), /// }; /// - /// let encoded = data.encode(); + /// let encoded = data.encode().unwrap(); /// ``` - pub fn encode(&self) -> Bytes { + pub fn encode(&self) -> Result { let mut buf = BytesMut::new(); // Encode collector_bgp_id buf.put_u32(self.collector_bgp_id.into()); - // Encode view_name_length - let view_name_bytes = self.view_name.as_bytes(); - buf.put_u16(view_name_bytes.len() as u16); - - // Encode view_name - buf.extend(view_name_bytes); + // Encode view_name_length and view_name + put_u16_len_slice( + &mut buf, + "PeerIndexTable view name length", + self.view_name.as_bytes(), + )?; // Encode peer_count - let peer_count = self.id_peer_map.len() as u16; - buf.put_u16(peer_count); + let peer_count = self.id_peer_map.len(); + check_max("PeerIndexTable peer count", peer_count, u16::MAX as usize)?; + buf.put_u16(peer_count as u16); // Encode peers let mut peer_ids: Vec<_> = self.id_peer_map.keys().collect(); @@ -184,7 +187,7 @@ impl PeerIndexTable { } // Return Bytes - buf.freeze() + Ok(buf.freeze()) } } @@ -214,7 +217,7 @@ mod tests { Asn::new_32bit(12345), )); - let encoded = index_table.encode(); + let encoded = index_table.encode().unwrap(); let parsed_index_table = parse_peer_index_table(&mut encoded.clone()).unwrap(); assert_eq!(index_table, parsed_index_table); } diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index 9bca83ee..d566ed9f 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -3,6 +3,8 @@ use crate::models::{ Afi, AsnLength, NetworkPrefix, RibAfiEntries, RibEntry, Safi, TableDumpV2Type, }; use crate::parser::ReadUtils; +use crate::encoder::sink::with_u16_len; +use crate::error::{check_max, EncodingError}; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; use log::warn; @@ -164,7 +166,7 @@ pub fn parse_rib_entry( } impl RibAfiEntries { - pub fn encode(&self) -> Bytes { + pub fn encode(&self) -> Result { let mut bytes = BytesMut::new(); let is_add_path = is_add_path_rib_type(self.rib_type); @@ -172,23 +174,29 @@ impl RibAfiEntries { bytes.extend(self.prefix.encode()); let entry_count = self.rib_entries.len(); + check_max("RIB entry count", entry_count, u16::MAX as usize)?; bytes.put_u16(entry_count as u16); for entry in &self.rib_entries { - bytes.extend(entry.encode_for_rib_type(is_add_path)); + entry.encode_for_rib_type(is_add_path, &mut bytes)?; } - bytes.freeze() + Ok(bytes.freeze()) } } impl RibEntry { - pub fn encode(&self) -> Bytes { - self.encode_for_rib_type(self.path_id.is_some()) + pub fn encode(&self) -> Result { + let mut bytes = BytesMut::new(); + self.encode_for_rib_type(self.path_id.is_some(), &mut bytes)?; + Ok(bytes.freeze()) } - fn encode_for_rib_type(&self, include_path_id: bool) -> Bytes { - let mut bytes = BytesMut::new(); + fn encode_for_rib_type( + &self, + include_path_id: bool, + bytes: &mut BytesMut, + ) -> Result<(), EncodingError> { bytes.put_u16(self.peer_index); bytes.put_u32(self.originated_time); if include_path_id { @@ -196,14 +204,9 @@ impl RibEntry { bytes.put_u32(path_id); } } - // TODO(fallible-encoding): temporary until RibEntry encoding returns Result - let attr_bytes = self - .attributes - .encode(AsnLength::Bits32) - .expect("attribute encoding failed"); - bytes.put_u16(attr_bytes.len() as u16); - bytes.extend(attr_bytes); - bytes.freeze() + with_u16_len(bytes, "RIB entry attribute length", |b| { + self.attributes.encode_to(AsnLength::Bits32, b) + }) } } @@ -274,7 +277,7 @@ mod tests { attributes, }; - let mut encoded = rib_entry.encode(); + let mut encoded = rib_entry.encode().unwrap(); assert_eq!(encoded.read_u16().unwrap(), 1); assert_eq!(encoded.read_u32().unwrap(), 12345); assert_eq!(encoded.read_u32().unwrap(), 42); @@ -301,7 +304,7 @@ mod tests { }], }; - let encoded = rib.encode(); + let encoded = rib.encode().unwrap(); let parsed = parse_rib_afi_entries(&mut encoded.clone(), rib.rib_type).unwrap(); assert_eq!(parsed.rib_type, rib.rib_type); assert_eq!(parsed.sequence_number, rib.sequence_number); diff --git a/src/parser/mrt/mrt_record.rs b/src/parser/mrt/mrt_record.rs index b14ad767..e8ea4370 100644 --- a/src/parser/mrt/mrt_record.rs +++ b/src/parser/mrt/mrt_record.rs @@ -1,6 +1,6 @@ use super::mrt_header::parse_common_header_with_bytes; use crate::bmp::messages::{BmpMessage, BmpMessageBody}; -use crate::error::ParserError; +use crate::error::{check_max, EncodingError, ParserError}; use crate::models::*; use crate::parser::{ parse_bgp4mp, parse_table_dump_message, parse_table_dump_v2_message, ParserErrorWithBytes, @@ -227,8 +227,8 @@ pub fn parse_mrt_body( } impl MrtRecord { - pub fn encode(&self) -> Bytes { - let message_bytes = self.message.encode(self.common_header.entry_subtype); + pub fn encode(&self) -> Result { + let message_bytes = self.message.encode(self.common_header.entry_subtype)?; let mut new_header = self.common_header; if message_bytes.len() != new_header.length as usize { warn!( @@ -237,6 +237,11 @@ impl MrtRecord { new_header.length ); } + check_max( + "MRT record message length", + message_bytes.len(), + u32::MAX as usize, + )?; new_header.length = message_bytes.len() as u32; let header_bytes = new_header.encode(); @@ -253,7 +258,7 @@ impl MrtRecord { let mut bytes = BytesMut::with_capacity(header_bytes.len() + message_bytes.len()); bytes.put_slice(&header_bytes); bytes.put_slice(&message_bytes); - bytes.freeze() + Ok(bytes.freeze()) } } @@ -294,12 +299,15 @@ impl TryFrom<&BmpMessage> for MrtRecord { let (seconds, microseconds) = convert_timestamp(bmp_header.timestamp); let subtype = Bgp4MpType::MessageAs4 as u16; + let encoded_message = mrt_message + .encode(subtype) + .map_err(|e| format!("cannot encode MRT message: {e}"))?; let mrt_header = CommonHeader { timestamp: seconds, microsecond_timestamp: Some(microseconds), entry_type: EntryType::BGP4MP_ET, entry_subtype: Bgp4MpType::MessageAs4 as u16, - length: mrt_message.encode(subtype).len() as u32, + length: encoded_message.len() as u32, }; Ok(MrtRecord { @@ -501,12 +509,12 @@ mod tests { })), }; - let encoded = record.encode(); + let encoded = record.encode().unwrap(); let mut cursor = Cursor::new(encoded); let parsed = parse_mrt_record(&mut cursor).unwrap(); let expected_len = parsed .message - .encode(parsed.common_header.entry_subtype) + .encode(parsed.common_header.entry_subtype).unwrap() .len() as u32; assert_eq!(parsed.common_header.length, expected_len); diff --git a/tests/test_encoding.rs b/tests/test_encoding.rs index 7f4ceb3d..12d121e7 100644 --- a/tests/test_encoding.rs +++ b/tests/test_encoding.rs @@ -9,7 +9,7 @@ mod tests { let url = "https://spaces.bgpkit.org/parser/update-example.gz"; let parser = BgpkitParser::new(url).unwrap(); for record in parser.into_record_iter() { - let bytes = record.encode(); + let bytes = record.encode().unwrap(); let parsed_record = parse_mrt_record(&mut Cursor::new(bytes)).unwrap(); assert_eq!(record, parsed_record); } @@ -17,7 +17,7 @@ mod tests { let url = "http://archive.routeviews.org/bgpdata/2023.10/UPDATES/updates.20231029.2015.bz2"; let parser = BgpkitParser::new(url).unwrap(); for record in parser.into_record_iter() { - let bytes = record.encode(); + let bytes = record.encode().unwrap(); let parsed_record = parse_mrt_record(&mut Cursor::new(bytes)).unwrap(); assert_eq!(record, parsed_record); } @@ -28,7 +28,7 @@ mod tests { let url = "http://archive.routeviews.org/route-views6/bgpdata/2023.10/UPDATES/updates.20231029.2115.bz2"; let parser = BgpkitParser::new(url).unwrap(); for record in parser.into_record_iter() { - let bytes = record.encode(); + let bytes = record.encode().unwrap(); let parsed_record = match parse_mrt_record(&mut Cursor::new(bytes.clone())) { Ok(r) => r, Err(_) => { @@ -48,7 +48,7 @@ mod tests { let url = "http://archive.routeviews.org/route-views.amsix/bgpdata/2023.05/UPDATES/updates.20230505.0330.bz2"; let parser = BgpkitParser::new(url).unwrap(); for record in parser.into_record_iter() { - let bytes = record.encode(); + let bytes = record.encode().unwrap(); let parsed_record = parse_mrt_record(&mut Cursor::new(bytes)).unwrap(); assert_eq!(record, parsed_record); } @@ -71,7 +71,7 @@ mod tests { let mut writer = oneio::get_writer(tempfile.as_str()).unwrap(); for record in input_records.iter() { - let bytes = record.encode(); + let bytes = record.encode().unwrap(); writer.write_all(&bytes).unwrap(); let parsed_record = parse_mrt_record(&mut Cursor::new(bytes)).unwrap(); assert_eq!(*record, parsed_record); From 2b644c735c5fa2c666a7832ac0465bcf760e8224 Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Wed, 29 Jul 2026 22:06:50 +0200 Subject: [PATCH 4/7] Reject peer-table overflow at insertion time PeerIndexTable::add_peer returns Result and refuses the 65536th distinct peer (the wire Peer Count field is 16-bit), leaving the table unmodified instead of silently wrapping the id and aliasing routes onto an existing peer. MrtRibEncoder::process_elem propagates the error so a writer learns about the overflow when it happens, not after the in-memory table is already corrupted. Assisted-by: Claude Code --- examples/filter_export_rib.rs | 2 +- src/encoder/rib_encoder.rs | 17 ++-- src/parser/iters/route.rs | 8 +- .../table_dump_v2/peer_index_table.rs | 84 +++++++++++++++---- 4 files changed, 83 insertions(+), 28 deletions(-) diff --git a/examples/filter_export_rib.rs b/examples/filter_export_rib.rs index 45dea1aa..7d7279da 100644 --- a/examples/filter_export_rib.rs +++ b/examples/filter_export_rib.rs @@ -18,7 +18,7 @@ fn main() { info!("processing rib {}", RIB_URL); for elem in parser { - encoder.process_elem(&elem); + encoder.process_elem(&elem).unwrap(); } info!("exporting filtered RIB..."); diff --git a/src/encoder/rib_encoder.rs b/src/encoder/rib_encoder.rs index 250014e8..be56e0d8 100644 --- a/src/encoder/rib_encoder.rs +++ b/src/encoder/rib_encoder.rs @@ -46,10 +46,14 @@ impl MrtRibEncoder { /// Processes a BgpElem and updates the internal data structures. /// + /// Returns [`EncodingError::ValueTooLarge`] if the element's peer would + /// exceed the 65535-peer capacity of the PEER_INDEX_TABLE; the encoder's + /// state is left unchanged in that case. + /// /// # Arguments /// /// * `elem` - A reference to a BgpElem that contains the information to be processed. - pub fn process_elem(&mut self, elem: &BgpElem) { + pub fn process_elem(&mut self, elem: &BgpElem) -> Result<(), EncodingError> { if self.timestamp == 0.0 { self.timestamp = elem.timestamp; } @@ -58,7 +62,7 @@ impl MrtRibEncoder { IpAddr::V6(_ip) => Ipv4Addr::from(0), }; let peer = Peer::new(bgp_identifier, elem.peer_ip, elem.peer_asn); - let peer_index = self.index_table.add_peer(peer); + let peer_index = self.index_table.add_peer(peer)?; let path_id = elem.prefix.path_id; let prefix = elem.prefix.prefix; @@ -70,6 +74,7 @@ impl MrtRibEncoder { attributes: Attributes::from(elem), }; entries_map.insert(peer_index, entry); + Ok(()) } /// Export the data stored in the struct to a byte array. @@ -158,9 +163,9 @@ mod tests { ..Default::default() }; elem.prefix.prefix = "10.250.0.0/24".parse().unwrap(); - encoder.process_elem(&elem); + encoder.process_elem(&elem).unwrap(); elem.prefix.prefix = "10.251.0.0/24".parse().unwrap(); - encoder.process_elem(&elem); + encoder.process_elem(&elem).unwrap(); let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes.clone()); @@ -177,7 +182,7 @@ mod tests { }; // ipv6 prefix elem.prefix.prefix = "2001:db8::/32".parse().unwrap(); - encoder.process_elem(&elem); + encoder.process_elem(&elem).unwrap(); let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes.clone()); @@ -195,7 +200,7 @@ mod tests { ..Default::default() }; elem.prefix = NetworkPrefix::new("10.250.0.0/24".parse().unwrap(), Some(42)); - encoder.process_elem(&elem); + encoder.process_elem(&elem).unwrap(); let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes); diff --git a/src/parser/iters/route.rs b/src/parser/iters/route.rs index cd8151a9..40c16ecb 100644 --- a/src/parser/iters/route.rs +++ b/src/parser/iters/route.rs @@ -964,7 +964,7 @@ mod tests { Asn::new_32bit(64496), ); let mut peer_table = PeerIndexTable::default(); - let peer_index = peer_table.add_peer(peer); + let peer_index = peer_table.add_peer(peer).unwrap(); let mut attributes = Attributes::default(); attributes.add_attr(AttributeValue::Origin(Origin::IGP).into()); @@ -1021,7 +1021,7 @@ mod tests { Asn::new_32bit(64496), ); let mut peer_table = PeerIndexTable::default(); - let peer_index = peer_table.add_peer(peer); + let peer_index = peer_table.add_peer(peer).unwrap(); let pit_record = MrtRecord { common_header: CommonHeader { @@ -1501,7 +1501,7 @@ mod tests { Asn::new_32bit(64496), ); let mut peer_table = PeerIndexTable::default(); - let peer_index = peer_table.add_peer(peer); + let peer_index = peer_table.add_peer(peer).unwrap(); let pit_record = MrtRecord { common_header: CommonHeader { @@ -1729,7 +1729,7 @@ mod tests { Asn::new_32bit(64496), ); let mut peer_table = PeerIndexTable::default(); - let peer_index = peer_table.add_peer(peer); + let peer_index = peer_table.add_peer(peer).unwrap(); let first_entry = RibEntry { peer_index, diff --git a/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs b/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs index d0ccb6d1..6d653274 100644 --- a/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs +++ b/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs @@ -67,15 +67,22 @@ pub fn parse_peer_index_table(data: &mut Bytes) -> Result u16 { + /// Add peer to peer index table and return peer id. + /// + /// The PEER_INDEX_TABLE wire format uses a 16-bit peer count, so at most + /// 65535 distinct peers can be stored. Adding a peer beyond that returns + /// [`EncodingError::ValueTooLarge`] and leaves the table unmodified — + /// previously the id silently wrapped, aliasing routes to the wrong peer. + pub fn add_peer(&mut self, peer: Peer) -> Result { match self.peer_ip_id_map.get(&peer.peer_ip) { - Some(id) => *id, + Some(id) => Ok(*id), None => { - let peer_id = self.peer_ip_id_map.len() as u16; + let next_id = self.peer_ip_id_map.len(); + check_max("PeerIndexTable peer count", next_id + 1, u16::MAX as usize)?; + let peer_id = next_id as u16; self.peer_ip_id_map.insert(peer.peer_ip, peer_id); self.id_peer_map.insert(peer_id, peer); - peer_id + Ok(peer_id) } } } @@ -206,16 +213,20 @@ mod tests { peer_ip_id_map: Default::default(), }; - index_table.add_peer(Peer::new( - Ipv4Addr::from(1234), - IpAddr::from_str("192.168.1.1").unwrap(), - Asn::new_32bit(1234), - )); - index_table.add_peer(Peer::new( - Ipv4Addr::from(12345), - IpAddr::from_str("192.168.1.2").unwrap(), - Asn::new_32bit(12345), - )); + index_table + .add_peer(Peer::new( + Ipv4Addr::from(1234), + IpAddr::from_str("192.168.1.1").unwrap(), + Asn::new_32bit(1234), + )) + .unwrap(); + index_table + .add_peer(Peer::new( + Ipv4Addr::from(12345), + IpAddr::from_str("192.168.1.2").unwrap(), + Asn::new_32bit(12345), + )) + .unwrap(); let encoded = index_table.encode().unwrap(); let parsed_index_table = parse_peer_index_table(&mut encoded.clone()).unwrap(); @@ -242,8 +253,8 @@ mod tests { Asn::new_32bit(12345), ); - let peer1_id = index_table.add_peer(peer1); - let peer2_id = index_table.add_peer(peer2); + let peer1_id = index_table.add_peer(peer1).unwrap(); + let peer2_id = index_table.add_peer(peer2).unwrap(); assert_eq!( index_table.get_peer_by_id(&peer1_id), @@ -262,4 +273,43 @@ mod tests { )) ); } + + #[test] + fn test_add_peer_rejects_overflow_without_corruption() { + let mut index_table = PeerIndexTable::default(); + + // fill the table to its 16-bit wire capacity of 65535 peers + for i in 0..(u16::MAX as u32) { + let ip = IpAddr::from(Ipv4Addr::from(i + 1)); + index_table + .add_peer(Peer::new(Ipv4Addr::from(1), ip, Asn::new_32bit(i))) + .unwrap(); + } + assert_eq!(index_table.id_peer_map.len(), u16::MAX as usize); + + // the 65536th peer must be rejected, not aliased onto an existing id + let overflow_ip = IpAddr::from(Ipv4Addr::from(u16::MAX as u32 + 1)); + let overflow_peer = Peer::new(Ipv4Addr::from(1), overflow_ip, Asn::new_32bit(65536)); + let err = index_table.add_peer(overflow_peer).unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "PeerIndexTable peer count", + actual: u16::MAX as usize + 1, + max: u16::MAX as usize + } + ); + + // the failed insert must not have modified the table + assert_eq!(index_table.id_peer_map.len(), u16::MAX as usize); + assert_eq!(index_table.get_peer_id_by_addr(&overflow_ip), None); + + // adding an existing peer still returns its id without error + let existing_ip = IpAddr::from(Ipv4Addr::from(1u32)); + let existing = Peer::new(Ipv4Addr::from(1), existing_ip, Asn::new_32bit(0)); + assert_eq!(index_table.add_peer(existing).unwrap(), 0); + + // the full table still encodes successfully + index_table.encode().unwrap(); + } } From 93483fefbd33a16cd10ad1511439f4ee9b58edd9 Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Wed, 29 Jul 2026 22:15:05 +0200 Subject: [PATCH 5/7] Harden fallible encoding: deny dropped Results, add error-path tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deny(unused_must_use) at the crate root so a dropped Result from an encode_to() call — which would silently skip wire data — fails the build. New tests cover one error path per wire bound: non-extended and extended attribute value lengths, ATTR_SET (Unencodable), MP_REACH labeled prefix with an empty label stack (previously swallowed with a log line), RIB entry attribute overflow propagation (previously the entry vanished from the record), RIB entry count, OPEN parameter type 255, BGP message total length, tunnel-encap sub-TLV length, and the FlowSpec 4096..=65535 range that fits a u16 but not the 12-bit wire length field. Assisted-by: Claude Code --- src/lib.rs | 3 + src/models/bgp/flowspec/tests.rs | 36 +++++ tests/test_encoding_errors.rs | 221 +++++++++++++++++++++++++++++++ 3 files changed, 260 insertions(+) create mode 100644 tests/test_encoding_errors.rs diff --git a/src/lib.rs b/src/lib.rs index ee9f424c..dcf5945b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -858,6 +858,9 @@ Additional known attribute type codes are raw-retained (`AttributeValue::Raw`) a html_logo_url = "https://raw.githubusercontent.com/bgpkit/assets/main/logos/icon-transparent.png", html_favicon_url = "https://raw.githubusercontent.com/bgpkit/assets/main/logos/favicon.ico" )] +// A dropped `Result` from an encode_to() call would silently skip wire data — +// exactly the corruption class the fallible encoding API exists to prevent. +#![deny(unused_must_use)] #[cfg(feature = "parser")] pub mod encoder; diff --git a/src/models/bgp/flowspec/tests.rs b/src/models/bgp/flowspec/tests.rs index 158e2a9a..911fcf43 100644 --- a/src/models/bgp/flowspec/tests.rs +++ b/src/models/bgp/flowspec/tests.rs @@ -326,6 +326,42 @@ mod error_handling { let parsed_length = parse_length(&data, &mut offset).unwrap(); assert_eq!(parsed_length, 4095); } + + #[test] + fn test_encode_rejects_length_beyond_12_bit_field() { + use crate::error::EncodingError; + + // Component data in the 4096..=65535 range fits a u16 but NOT the + // 12-bit wire length field: encoding it would corrupt the length byte + // (e.g. 5000 = 0x1388 encodes as 0xF3 0x88, decoding back as 904). + // 2500 numeric operators encode to 2 bytes each, plus the type byte. + let nlri = FlowSpecNlri::new(vec![FlowSpecComponent::Port(vec![ + NumericOperator::equal_to( + 80 + ); + 2500 + ])]); + let err = encode_flowspec_nlri(&nlri).unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "FlowSpec NLRI total length", + actual: 5001, + max: 0x0FFF + } + ); + + // 2000 operators (4001 bytes) still fit within the 12-bit bound + let nlri = FlowSpecNlri::new(vec![FlowSpecComponent::Port(vec![ + NumericOperator::equal_to( + 80 + ); + 2000 + ])]); + let encoded = encode_flowspec_nlri(&nlri).unwrap(); + let mut offset = 0; + assert_eq!(parse_length(&encoded, &mut offset).unwrap(), 4001); + } } /// IPv6-specific test cases from RFC 8956 diff --git a/tests/test_encoding_errors.rs b/tests/test_encoding_errors.rs new file mode 100644 index 00000000..72629a72 --- /dev/null +++ b/tests/test_encoding_errors.rs @@ -0,0 +1,221 @@ +//! Error-path tests for the fallible encoding API (issue #313). +//! +//! Every wire-format capacity bound must surface as an `EncodingError` +//! instead of silently truncating, wrapping, or dropping data. + +use bgpkit_parser::error::EncodingError; +use bgpkit_parser::models::*; +use std::net::Ipv4Addr; +use std::str::FromStr; + +fn oversized_attribute() -> Attribute { + // 30 large communities = 360 bytes, exceeding the 255-byte limit of a + // non-extended attribute length field + let communities = vec![LargeCommunity::new(1, [2, 3]); 30]; + Attribute { + value: AttributeValue::LargeCommunities(communities), + flag: AttrFlags::OPTIONAL | AttrFlags::TRANSITIVE, + } +} + +#[test] +fn test_oversized_attribute_value_non_extended() { + let attr = oversized_attribute(); + assert!(!attr.is_extended()); + let err = attr.encode(AsnLength::Bits32).unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "BGP attribute value length", + actual: 360, + max: 255 + } + ); +} + +#[test] +fn test_oversized_attribute_value_extended() { + // 6000 large communities = 72000 bytes, exceeding even the extended + // 2-byte attribute length field + let communities = vec![LargeCommunity::new(1, [2, 3]); 6000]; + let attr = Attribute { + value: AttributeValue::LargeCommunities(communities), + flag: AttrFlags::OPTIONAL | AttrFlags::TRANSITIVE | AttrFlags::EXTENDED, + }; + let err = attr.encode(AsnLength::Bits32).unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "BGP attribute value length (extended)", + actual: 72000, + max: 65535 + } + ); +} + +#[test] +fn test_attr_set_is_unencodable() { + let attr = Attribute::from(AttributeValue::AttrSet(AttrSet { + origin_as: Asn::from(65000), + attributes: Attributes::default(), + })); + let err = attr.encode(AsnLength::Bits32).unwrap_err(); + assert!(matches!( + err, + EncodingError::Unencodable { + field: "ATTR_SET attribute", + .. + } + )); +} + +#[test] +fn test_mp_reach_empty_label_stack_propagates() { + // Previously this error was swallowed (log + empty attribute value); + // it must now surface through Attribute::encode. + let nlri = Nlri { + afi: Afi::Ipv4, + safi: Safi::MplsLabel, + next_hop: Some(NextHopAddress::Ipv4(Ipv4Addr::new(192, 0, 2, 1))), + prefixes: vec![], + labeled_prefixes: Some(vec![LabeledNetworkPrefix { + prefix: "203.0.113.0/24".parse().unwrap(), + labels: Default::default(), // empty label stack cannot be encoded + path_id: None, + }]), + link_state_nlris: None, + flowspec_nlris: None, + }; + let attr = Attribute::from(AttributeValue::MpReachNlri(nlri)); + let err = attr.encode(AsnLength::Bits32).unwrap_err(); + assert!(matches!( + err, + EncodingError::Unencodable { + field: "MP NLRI labeled prefix", + .. + } + )); +} + +#[test] +fn test_rib_entry_oversized_attributes_propagate() { + // Regression guard: an entry whose attributes cannot be encoded must fail + // the whole RibAfiEntries::encode, not be silently dropped from the record. + let entry = RibEntry { + peer_index: 0, + originated_time: 0, + path_id: None, + attributes: Attributes::from(vec![oversized_attribute()]), + }; + let rib = RibAfiEntries { + rib_type: TableDumpV2Type::RibIpv4Unicast, + sequence_number: 1, + prefix: NetworkPrefix::from_str("203.0.113.0/24").unwrap(), + rib_entries: vec![entry], + }; + let err = rib.encode().unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "BGP attribute value length", + actual: 360, + max: 255 + } + ); +} + +#[test] +fn test_rib_entry_count_overflow() { + let entry = RibEntry { + peer_index: 0, + originated_time: 0, + path_id: None, + attributes: Attributes::default(), + }; + let rib = RibAfiEntries { + rib_type: TableDumpV2Type::RibIpv4Unicast, + sequence_number: 1, + prefix: NetworkPrefix::from_str("203.0.113.0/24").unwrap(), + rib_entries: vec![entry; 65536], + }; + let err = rib.encode().unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "RIB entry count", + actual: 65536, + max: 65535 + } + ); +} + +#[test] +fn test_open_message_rejects_reserved_param_type_255() { + // RFC 9072 reserves parameter type 255 as the extended-length marker; + // encoding it as a real parameter would be misparsed on round trip. + let msg = BgpOpenMessage { + version: 4, + asn: Asn::new_16bit(64512), + hold_time: 90, + bgp_identifier: Ipv4Addr::new(192, 0, 2, 1), + extended_length: false, + opt_params: vec![OptParam { + param_type: 255, + param_value: ParamValue::Raw(vec![0xAA, 0xBB]), + }], + }; + let err = msg.encode().unwrap_err(); + assert!(matches!( + err, + EncodingError::Unencodable { + field: "BGP OPEN optional parameter type", + .. + } + )); +} + +#[test] +fn test_bgp_message_total_length_overflow() { + // 17000 distinct /24 prefixes at 4 bytes each push the UPDATE body past + // the 16-bit BGP message length field. + let prefixes: Vec = (0..17000u32) + .map(|i| { + NetworkPrefix::from_str(&format!("10.{}.{}.0/24", (i >> 8) & 0xFF, i & 0xFF)).unwrap() + }) + .collect(); + let update = BgpUpdateMessage { + withdrawn_prefixes: vec![], + attributes: Attributes::default(), + announced_prefixes: prefixes, + }; + let err = BgpMessage::Update(update) + .encode(AsnLength::Bits32) + .unwrap_err(); + assert!(matches!( + err, + EncodingError::ValueTooLarge { + field: "BGP message total length", + max: 65535, + .. + } + )); +} + +#[test] +fn test_tunnel_encap_oversized_sub_tlv() { + // Sub-TLV types < 128 carry a 1-octet length field + let mut tlv = TunnelEncapTlv::new(TunnelType::Vxlan); + tlv.add_sub_tlv(SubTlv::new(SubTlvType::Color, vec![0u8; 300])); + let mut attr = TunnelEncapAttribute::new(); + attr.add_tunnel_tlv(tlv); + let attribute = Attribute::from(AttributeValue::TunnelEncapsulation(attr)); + let err = attribute.encode(AsnLength::Bits32).unwrap_err(); + assert_eq!( + err, + EncodingError::ValueTooLarge { + field: "Tunnel Encap sub-TLV value length", + actual: 300, + max: 255 + } + ); +} From 4efebdc7c60aff11dd177895843c3eeb4c0f0275 Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Wed, 29 Jul 2026 22:19:01 +0200 Subject: [PATCH 6/7] Document fallible encoding API in CHANGELOG with migration table Assisted-by: Claude Code --- CHANGELOG.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef558e5e..83ea381c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,35 @@ All notable changes to this project will be documented in this file. ### Breaking changes +* **Fallible encoding** ([#313](https://github.com/bgpkit/bgpkit-parser/issues/313)): all encode paths now return `Result<_, EncodingError>` instead of silently truncating values that do not fit their wire-format fields. There are no panicking wrappers; callers that want the old "just give me bytes" ergonomics can `.unwrap()`. Migration table: + + | Before | After | + | --- | --- | + | `Attribute::encode(asn_len) -> Bytes` | `-> Result` (also `encode_to(asn_len, &mut BytesMut)`) | + | `Attributes::encode(asn_len) -> Bytes` | `-> Result` (also `encode_to`) | + | `BgpOpenMessage::encode() -> Bytes` | `-> Result` | + | `BgpUpdateMessage::encode(asn_len) -> Bytes` | `-> Result` | + | `BgpMessage::encode(asn_len) -> Bytes` | `-> Result` | + | `TableDumpMessage::encode() -> Bytes` | `-> Result` | + | `RibEntry::encode()` / `RibAfiEntries::encode() -> Bytes` | `-> Result` | + | `PeerIndexTable::encode()` / `GeoPeerTable::encode() -> Bytes` | `-> Result` | + | `PeerIndexTable::add_peer(peer) -> u16` | `-> Result` (rejects the 65,536th peer instead of wrapping the id) | + | `MrtMessage::encode(sub_type)` / `Bgp4MpMessage::encode(asn_len)` / `MrtRecord::encode() -> Bytes` | `-> Result` | + | `MrtRibEncoder::process_elem(&elem)` | `-> Result<(), EncodingError>` | + | `MrtRibEncoder::export_bytes()` / `MrtUpdatesEncoder::export_bytes() -> Bytes` | `-> Result` | + + `EncodingError` has two variants: `ValueTooLarge { field, actual, max }` for wire-capacity overflows and `Unencodable { field, reason }` for values with no valid wire representation (ATTR_SET, RIB_GENERIC, OPEN parameter type 255, labeled NLRI with an empty label stack). +* **`Tlv::length()` and `SubTlv::length()` removed**: both silently saturated at `u16::MAX`; encoders now compute checked lengths internally. * **`OptParam` no longer has a `param_len` field**: the field was redundant now that the encoder always derives the wire length from `param_value`, and the parser recomputes it on read. Construct `OptParam` with just `param_type` and `param_value`. ### Fixed +* **Silent truncation on encode eliminated** ([#313](https://github.com/bgpkit/bgpkit-parser/issues/313)): AS_PATH segments with >255 ASes, attribute values exceeding their (extended or non-extended) length field, TLV values in Tunnel Encapsulation / BGP-LS / SFP / BFD Discriminator / Prefix-SID / BIER attributes, RIB entry counts, peer counts, view names, and BGP message total lengths now return `EncodingError` instead of writing corrupt length fields. +* **FlowSpec NLRI length bound**: the wire length field is 12-bit (RFC 8955); NLRI data of 4096..=65535 bytes previously passed an unchecked `u16` cast and encoded a corrupt length byte. It is now rejected with `ValueTooLarge`. +* **MP_REACH/MP_UNREACH encoding errors are no longer swallowed**: labeled-NLRI failures previously logged a warning and emitted a zero-length (RFC-invalid) attribute value; they now propagate as `EncodingError`. +* **FlowSpec NLRI entries are now encoded**: `Nlri::flowspec_nlris` was previously silently dropped when encoding MP_REACH/MP_UNREACH attributes. +* **Peer index aliasing**: `PeerIndexTable::add_peer` previously wrapped the peer id at 65,536 peers, silently attributing routes to the wrong peer; it now returns an error and leaves the table unmodified. +* **BGP OPEN parameter type 255 rejected**: RFC 9072 reserves type 255 as the extended-length marker; encoding it as a real parameter produced output that round-tripped to a structurally different message. * **BGP OPEN optional-parameter encoding**: Encode the Optional Parameters Length as the total byte length required by RFC 4271 instead of the number of parameters. OPEN messages now also use the extended length format from RFC 9072 when requested or required. ## v0.19.0 - 2026-07-28 From b44eccc3a52ac9a5e8d328aa014f6314c695eb0f Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Wed, 29 Jul 2026 22:35:31 +0200 Subject: [PATCH 7/7] fmt --- examples/filter_export_rib.rs | 4 ++- src/encoder/rib_encoder.rs | 2 +- src/encoder/updates_encoder.rs | 2 +- src/error.rs | 5 +-- src/models/bgp/tunnel_encap.rs | 1 - .../bgp/attributes/attr_23_tunnel_encap.rs | 2 +- src/parser/bgp/attributes/attr_37_sfp.rs | 4 +-- .../attributes/attr_38_bfd_discriminator.rs | 4 +-- .../bgp/attributes/attr_40_bgp_prefix_sid.rs | 4 +-- src/parser/bgp/attributes/attr_41_bier.rs | 4 +-- src/parser/bgp/attributes/mod.rs | 10 ++++-- src/parser/iters/route.rs | 32 ++++++++++++------- .../table_dump_v2/peer_index_table.rs | 4 +-- .../messages/table_dump_v2/rib_afi_entries.rs | 4 +-- src/parser/mrt/mrt_record.rs | 3 +- 15 files changed, 49 insertions(+), 36 deletions(-) diff --git a/examples/filter_export_rib.rs b/examples/filter_export_rib.rs index 7d7279da..31d58fa6 100644 --- a/examples/filter_export_rib.rs +++ b/examples/filter_export_rib.rs @@ -23,7 +23,9 @@ fn main() { info!("exporting filtered RIB..."); let mut writer = oneio::get_writer("filtered-13335.rib.gz").unwrap(); - writer.write_all(encoder.export_bytes().unwrap().as_ref()).unwrap(); + writer + .write_all(encoder.export_bytes().unwrap().as_ref()) + .unwrap(); drop(writer); info!("exporting filtered RIB...done"); diff --git a/src/encoder/rib_encoder.rs b/src/encoder/rib_encoder.rs index be56e0d8..14d77d8a 100644 --- a/src/encoder/rib_encoder.rs +++ b/src/encoder/rib_encoder.rs @@ -4,11 +4,11 @@ //! difficulty part of this process is the handling of TableDumpV2 RIB dumps, which requires //! reconstructing the peer index table before encoding all other contents. +use crate::error::EncodingError; use crate::models::{ Attributes, BgpElem, CommonHeader, EntryType, MrtMessage, NetworkPrefix, Peer, PeerIndexTable, RibAfiEntries, RibEntry, TableDumpV2Message, TableDumpV2Type, }; -use crate::error::EncodingError; use crate::utils::convert_timestamp; use bytes::{Bytes, BytesMut}; use ipnet::IpNet; diff --git a/src/encoder/updates_encoder.rs b/src/encoder/updates_encoder.rs index 473ea554..fee1054b 100644 --- a/src/encoder/updates_encoder.rs +++ b/src/encoder/updates_encoder.rs @@ -1,12 +1,12 @@ use std::net::IpAddr; use std::str::FromStr; +use crate::error::EncodingError; use crate::models::{ Asn, Bgp4MpEnum, Bgp4MpMessage, Bgp4MpType, BgpMessage, BgpUpdateMessage, CommonHeader, EntryType, MrtMessage, }; use crate::utils::convert_timestamp; -use crate::error::EncodingError; use crate::BgpElem; use bytes::{Bytes, BytesMut}; diff --git a/src/error.rs b/src/error.rs index da6d3b1b..606d2f6a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -60,10 +60,7 @@ pub enum EncodingError { /// The value cannot be represented in wire format at all (e.g. ATTR_SET /// encoding is not implemented, a labeled NLRI has an empty label stack, /// or a BGP OPEN optional parameter uses the reserved type 255). - Unencodable { - field: &'static str, - reason: String, - }, + Unencodable { field: &'static str, reason: String }, } impl EncodingError { diff --git a/src/models/bgp/tunnel_encap.rs b/src/models/bgp/tunnel_encap.rs index f757548f..7204da7d 100644 --- a/src/models/bgp/tunnel_encap.rs +++ b/src/models/bgp/tunnel_encap.rs @@ -124,7 +124,6 @@ impl SubTlv { value, } } - } /// Tunnel Encapsulation TLV diff --git a/src/parser/bgp/attributes/attr_23_tunnel_encap.rs b/src/parser/bgp/attributes/attr_23_tunnel_encap.rs index 36d7323a..4a3e128b 100644 --- a/src/parser/bgp/attributes/attr_23_tunnel_encap.rs +++ b/src/parser/bgp/attributes/attr_23_tunnel_encap.rs @@ -2,7 +2,7 @@ use bytes::{Buf, BufMut, Bytes, BytesMut}; -use crate::encoder::sink::{put_u8_len_slice, put_u16_len_slice, with_u16_len}; +use crate::encoder::sink::{put_u16_len_slice, put_u8_len_slice, with_u16_len}; use crate::error::{EncodingError, ParserError}; use crate::models::*; use crate::parser::ReadUtils; diff --git a/src/parser/bgp/attributes/attr_37_sfp.rs b/src/parser/bgp/attributes/attr_37_sfp.rs index 4243a866..0ceb0dc5 100644 --- a/src/parser/bgp/attributes/attr_37_sfp.rs +++ b/src/parser/bgp/attributes/attr_37_sfp.rs @@ -1,7 +1,7 @@ -use crate::models::*; -use crate::parser::ReadUtils; use crate::encoder::sink::put_u16_len_slice; use crate::error::EncodingError; +use crate::models::*; +use crate::parser::ReadUtils; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; diff --git a/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs b/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs index 3dbcf76e..7323bb78 100644 --- a/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs +++ b/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs @@ -1,7 +1,7 @@ -use crate::models::*; -use crate::parser::ReadUtils; use crate::encoder::sink::put_u8_len_slice; use crate::error::EncodingError; +use crate::models::*; +use crate::parser::ReadUtils; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; diff --git a/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs b/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs index 2e43f6d4..61d9dda3 100644 --- a/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs +++ b/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs @@ -1,7 +1,7 @@ -use crate::models::*; -use crate::parser::ReadUtils; use crate::encoder::sink::put_u16_len_slice; use crate::error::EncodingError; +use crate::models::*; +use crate::parser::ReadUtils; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; diff --git a/src/parser/bgp/attributes/attr_41_bier.rs b/src/parser/bgp/attributes/attr_41_bier.rs index bb1b69a3..c4e4ae0d 100644 --- a/src/parser/bgp/attributes/attr_41_bier.rs +++ b/src/parser/bgp/attributes/attr_41_bier.rs @@ -1,7 +1,7 @@ -use crate::models::*; -use crate::parser::ReadUtils; use crate::encoder::sink::put_u16_len_slice; use crate::error::EncodingError; +use crate::models::*; +use crate::parser::ReadUtils; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index 1a1ea036..40041068 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -915,7 +915,10 @@ mod tests { value => panic!("expected Raw for code {code}, got {value:?}"), } assert!(attributes.has_attr(AttrType::from(code)), "code {code}"); - assert_eq!(attributes.encode(AsnLength::Bits16).unwrap(), Bytes::from(wire)); + assert_eq!( + attributes.encode(AsnLength::Bits16).unwrap(), + Bytes::from(wire) + ); } } @@ -942,7 +945,10 @@ mod tests { value => panic!("expected Unknown, got {value:?}"), } assert!(attributes.has_attr(AttrType::Unknown(0x7f))); - assert_eq!(attributes.encode(AsnLength::Bits16).unwrap(), Bytes::from(wire)); + assert_eq!( + attributes.encode(AsnLength::Bits16).unwrap(), + Bytes::from(wire) + ); } #[test] diff --git a/src/parser/iters/route.rs b/src/parser/iters/route.rs index 40c16ecb..56ff299c 100644 --- a/src/parser/iters/route.rs +++ b/src/parser/iters/route.rs @@ -1149,7 +1149,8 @@ mod tests { ], }), ) - .encode().unwrap() + .encode() + .unwrap() .to_vec(); let routes = BgpkitParser::from_reader(Cursor::new(bytes)) @@ -1201,7 +1202,8 @@ mod tests { announced_prefixes: vec![], }), ) - .encode().unwrap() + .encode() + .unwrap() .to_vec(); let routes = assert_route_projection(bytes); @@ -1250,7 +1252,8 @@ mod tests { announced_prefixes: vec![NetworkPrefix::from_str("203.0.113.0/24").unwrap()], }), ) - .encode().unwrap() + .encode() + .unwrap() .to_vec(); let routes = assert_route_projection(bytes); @@ -1325,7 +1328,9 @@ mod tests { #[test] fn selective_attribute_parser_handles_as_path_without_as4_path() { let attrs = parse_route_attributes( - route_attributes([64500, 64501]).encode(AsnLength::Bits16).unwrap(), + route_attributes([64500, 64501]) + .encode(AsnLength::Bits16) + .unwrap(), &AsnLength::Bits16, false, RouteAttributeContext { @@ -1836,7 +1841,8 @@ mod tests { fn route_iterator_skips_route_parse_errors() { let routes = BgpkitParser::from_reader(Cursor::new( table_dump_v2_rib_without_peer_table_record() - .encode().unwrap() + .encode() + .unwrap() .to_vec(), )) .into_route_iter() @@ -1847,12 +1853,13 @@ mod tests { #[test] fn fallible_route_iterator_applies_filters_to_cached_routes() { - let routes = BgpkitParser::from_reader(Cursor::new(update_record().encode().unwrap().to_vec())) - .add_filter("type", "w") - .unwrap() - .into_fallible_route_iter() - .collect::, _>>() - .unwrap(); + let routes = + BgpkitParser::from_reader(Cursor::new(update_record().encode().unwrap().to_vec())) + .add_filter("type", "w") + .unwrap() + .into_fallible_route_iter() + .collect::, _>>() + .unwrap(); assert_eq!(routes.len(), 1); assert_eq!(routes[0].elem_type, ElemType::WITHDRAW); @@ -1862,7 +1869,8 @@ mod tests { fn fallible_route_iterator_returns_route_parse_errors() { let mut iter = BgpkitParser::from_reader(Cursor::new( table_dump_v2_rib_without_peer_table_record() - .encode().unwrap() + .encode() + .unwrap() .to_vec(), )) .into_fallible_route_iter(); diff --git a/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs b/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs index 6d653274..838794cd 100644 --- a/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs +++ b/src/parser/mrt/messages/table_dump_v2/peer_index_table.rs @@ -1,7 +1,7 @@ -use crate::models::{Afi, AsnLength, Peer, PeerIndexTable, PeerType}; -use crate::parser::ReadUtils; use crate::encoder::sink::put_u16_len_slice; use crate::error::{check_max, EncodingError}; +use crate::models::{Afi, AsnLength, Peer, PeerIndexTable, PeerType}; +use crate::parser::ReadUtils; use crate::ParserError; use bytes::{BufMut, Bytes, BytesMut}; use std::collections::HashMap; diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index d566ed9f..369408b5 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -1,10 +1,10 @@ use crate::bgp::attributes::parse_attributes; +use crate::encoder::sink::with_u16_len; +use crate::error::{check_max, EncodingError}; use crate::models::{ Afi, AsnLength, NetworkPrefix, RibAfiEntries, RibEntry, Safi, TableDumpV2Type, }; use crate::parser::ReadUtils; -use crate::encoder::sink::with_u16_len; -use crate::error::{check_max, EncodingError}; use crate::ParserError; use bytes::{Buf, BufMut, Bytes, BytesMut}; use log::warn; diff --git a/src/parser/mrt/mrt_record.rs b/src/parser/mrt/mrt_record.rs index e8ea4370..259bc543 100644 --- a/src/parser/mrt/mrt_record.rs +++ b/src/parser/mrt/mrt_record.rs @@ -514,7 +514,8 @@ mod tests { let parsed = parse_mrt_record(&mut cursor).unwrap(); let expected_len = parsed .message - .encode(parsed.common_header.entry_subtype).unwrap() + .encode(parsed.common_header.entry_subtype) + .unwrap() .len() as u32; assert_eq!(parsed.common_header.length, expected_len);