diff --git a/CHANGELOG.md b/CHANGELOG.md index ef558e5..2aff342 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,37 @@ All notable changes to this project will be documented in this file. ### Breaking changes +* **Encoding is now fallible and sink-based**: all `encode()` methods now return `Result` instead of `Bytes`, and most types also expose an `encode_to(&self, buf: &mut BytesMut) -> Result<(), EncodingError>` sink method for zero-copy composition. Arbitrary input data (e.g. a round-tripped OPEN with an oversized raw capability, an AS_PATH segment with >255 ASes, or a RIB entry with >65535 bytes of attributes) previously crashed the process via `.expect()` or silently truncated length fields with `as u8`/`as u16` casts; it now surfaces as an `Err`. ([#313](https://github.com/bgpkit/bgpkit-parser/issues/313)) + + Migration guide: + + | Before | After | + | --- | --- | + | `msg.encode()` → `Bytes` | `msg.encode()` → `Result` | + | `msg.encode(asn_len)` → `Bytes` | `msg.encode(asn_len)` → `Result` | + | `mrt_message.encode(sub_type)` → `Bytes` | `mrt_message.encode(sub_type)` → `Result` | + | `encoder.export_bytes()` → `Bytes` | `encoder.export_bytes()` → `Result` | + | `encoder.process_elem(&elem)` → `()` | `encoder.process_elem(&elem)` → `Result<(), EncodingError>` | + | `table.add_peer(peer)` → `u16` | `table.add_peer(peer)` → `Result` | + + Callers that want the old semantics write `.unwrap()` — the panic is visibly theirs. + * **`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`. +* **`Tlv::length()` and `SubTlv::length()` removed**: these were saturating casts that duplicated what the encode paths now check properly. Use `value.len()` and the fallible encoders. +* **`PeerIndexTable::add_peer` returns `Result`**: errors on the 65537th distinct peer instead of silently aliasing it to id 65535 and corrupting the table. + +### Added + +* **`EncodingError` type**: `ValueTooLarge` for values exceeding their wire-format field capacity, and `Unencodable` for values that cannot be represented on the wire at all (e.g. `ATTR_SET` encoding not yet implemented, OPEN `param_type == 255` in non-extended framing, NLRI with an empty label stack). Exported as `bgpkit_parser::EncodingError` and `bgpkit_parser::error::EncodingError`. ([#313](https://github.com/bgpkit/bgpkit-parser/issues/313)) +* **Sink-style encoding helpers**: internal `with_u8_len`/`with_u16_len` back-patching helpers centralize all length-prefix writes, replacing 24 hand-rolled `try_from(..).map_err(..)` copies. Buffers are rolled back to their pre-call state if encoding fails. ### Fixed -* **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. +* **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 (documented auto-upgrade behavior). +* **Encoding crash and silent truncation**: replaced `.expect()` panics and unchecked `as u8`/`as u16` casts throughout the encoding layer with checked conversions. ([#313](https://github.com/bgpkit/bgpkit-parser/issues/313)) +* **MP_REACH/MP_UNREACH NLRI encoding failures** were silently replaced with empty attribute bytes; they now return `EncodingError::Unencodable`. +* **`ATTR_SET` encoding** returned `Ok` with empty bytes; it now returns `EncodingError::Unencodable` until the encoding is implemented. +* **FlowSpec NLRI length bound**: the check used `u16::MAX`, but the wire length field is 12-bit per RFC 8955/8956 — now bounded at 4095 (0x0FFF). ## v0.19.0 - 2026-07-28 diff --git a/README.md b/README.md index ecea648..1021800 100644 --- a/README.md +++ b/README.md @@ -361,7 +361,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/examples/filter_export_rib.rs b/examples/filter_export_rib.rs index 73a5be2..31d58fa 100644 --- a/examples/filter_export_rib.rs +++ b/examples/filter_export_rib.rs @@ -18,12 +18,14 @@ fn main() { info!("processing rib {}", RIB_URL); for elem in parser { - encoder.process_elem(&elem); + encoder.process_elem(&elem).unwrap(); } 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 be3b859..4927a8b 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 1dbb37f..6b63f91 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 d6637fa..376ad0f 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 ae1a1fc..a8aac67 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/mod.rs b/src/encoder/mod.rs index 8ffafb9..02410b2 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/rib_encoder.rs b/src/encoder/rib_encoder.rs index 1d38826..d9c379e 100644 --- a/src/encoder/rib_encoder.rs +++ b/src/encoder/rib_encoder.rs @@ -4,6 +4,7 @@ //! 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, @@ -48,7 +49,12 @@ impl MrtRibEncoder { /// # Arguments /// /// * `elem` - A reference to a BgpElem that contains the information to be processed. - pub fn process_elem(&mut self, elem: &BgpElem) { + /// + /// # Errors + /// + /// Returns [`EncodingError::ValueTooLarge`] if the peer index table is full + /// (more than 65536 distinct peers). + pub fn process_elem(&mut self, elem: &BgpElem) -> Result<(), EncodingError> { if self.timestamp == 0.0 { self.timestamp = elem.timestamp; } @@ -57,7 +63,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; @@ -69,6 +75,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. @@ -78,8 +85,8 @@ 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 `Result` containing the exported data as a byte array. + pub fn export_bytes(&mut self) -> Result { let mut bytes = BytesMut::new(); // encode peer-index-table @@ -88,7 +95,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 +127,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 +142,7 @@ impl MrtRibEncoder { self.reset(); - bytes.freeze() + Ok(bytes.freeze()) } } @@ -156,10 +163,10 @@ 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); - let bytes = encoder.export_bytes(); + encoder.process_elem(&elem).unwrap(); + let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes.clone()); while cursor.has_remaining() { @@ -175,8 +182,8 @@ mod tests { }; // ipv6 prefix elem.prefix.prefix = "2001:db8::/32".parse().unwrap(); - encoder.process_elem(&elem); - let bytes = encoder.export_bytes(); + encoder.process_elem(&elem).unwrap(); + let bytes = encoder.export_bytes().unwrap(); let mut cursor = Cursor::new(bytes.clone()); while cursor.has_remaining() { @@ -193,9 +200,9 @@ 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(); + 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/sink.rs b/src/encoder/sink.rs new file mode 100644 index 0000000..c517333 --- /dev/null +++ b/src/encoder/sink.rs @@ -0,0 +1,163 @@ +//! Sink-style encoding helpers. +//! +//! All "write a length prefix, then the payload" logic in the crate funnels +//! through these helpers. They write a placeholder length, encode the payload +//! into the same buffer, then back-patch the measured length — checking it +//! against the field's capacity. If the payload encoder fails, or the measured +//! length overflows, the buffer is rolled back to its pre-call state so a +//! failed encode never leaves dirty bytes behind. + +use crate::error::EncodingError; +use bytes::{BufMut, BytesMut}; + +/// Check that `n` fits within `max`, returning [`EncodingError::ValueTooLarge`] +/// otherwise. Use for element counts and non-power-of-two byte bounds that are +/// written at a known position (not back-patched). +pub(crate) fn check_max(field: &'static str, n: usize, max: usize) -> Result { + if n > max { + Err(EncodingError::too_large(field, n, max)) + } else { + Ok(n) + } +} + +/// Encode `f`'s payload with a 1-octet length prefix, back-patched after +/// encoding. Errors if the payload exceeds 255 bytes or `f` fails; in both +/// cases `buf` is rolled back to its pre-call length. +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); // placeholder + if let Err(e) = f(buf) { + buf.truncate(at); + return Err(e); + } + let len = buf.len() - at - 1; + let Ok(len) = u8::try_from(len) else { + let actual = buf.len() - at - 1; + buf.truncate(at); + return Err(EncodingError::too_large(field, actual, u8::MAX as usize)); + }; + buf[at] = len; + Ok(()) +} + +/// Encode `f`'s payload with a 2-octet length prefix, back-patched after +/// encoding. Errors if the payload exceeds 65535 bytes or `f` fails; in both +/// cases `buf` is rolled back to its pre-call length. +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); // placeholder + if let Err(e) = f(buf) { + buf.truncate(at); + return Err(e); + } + let len = buf.len() - at - 2; + let Ok(len) = u16::try_from(len) else { + let actual = buf.len() - at - 2; + buf.truncate(at); + return Err(EncodingError::too_large(field, actual, u16::MAX as usize)); + }; + buf[at..at + 2].copy_from_slice(&len.to_be_bytes()); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use bytes::Bytes; + + #[test] + fn test_with_u8_len_roundtrip() { + let mut buf = BytesMut::new(); + with_u8_len(&mut buf, "test", |b| { + b.extend_from_slice(&[1, 2, 3]); + Ok(()) + }) + .unwrap(); + assert_eq!(buf.freeze(), Bytes::from_static(&[3, 1, 2, 3])); + } + + #[test] + fn test_with_u8_len_boundary() { + let mut buf = BytesMut::new(); + // 255 bytes fits exactly + with_u8_len(&mut buf, "test", |b| { + b.extend_from_slice(&[0u8; 255]); + Ok(()) + }) + .unwrap(); + assert_eq!(buf[0], 255); + + // 256 bytes overflows and rolls back + let before = buf.len(); + let err = with_u8_len(&mut buf, "test", |b| { + b.extend_from_slice(&[0u8; 256]); + Ok(()) + }) + .unwrap_err(); + assert!(matches!(err, EncodingError::ValueTooLarge { .. })); + assert_eq!(buf.len(), before, "buffer must roll back on overflow"); + } + + #[test] + fn test_with_u16_len_boundary() { + let mut buf = BytesMut::new(); + with_u16_len(&mut buf, "test", |b| { + b.extend_from_slice(&[0u8; 65535]); + Ok(()) + }) + .unwrap(); + assert_eq!(&buf[..2], &[0xFF, 0xFF]); + + let before = buf.len(); + let err = with_u16_len(&mut buf, "test", |b| { + b.extend_from_slice(&[0u8; 65536]); + Ok(()) + }) + .unwrap_err(); + assert!(matches!(err, EncodingError::ValueTooLarge { .. })); + assert_eq!(buf.len(), before); + } + + #[test] + fn test_child_error_rolls_back() { + let mut buf = BytesMut::from(&b"prefix"[..]); + let err = with_u16_len(&mut buf, "test", |b| { + b.extend_from_slice(&[1, 2, 3]); + Err(EncodingError::unencodable("test", "boom")) + }) + .unwrap_err(); + assert!(matches!(err, EncodingError::Unencodable { .. })); + assert_eq!(&buf[..], b"prefix", "buffer must roll back on child error"); + } + + #[test] + fn test_nested_prefixes() { + let mut buf = BytesMut::new(); + with_u8_len(&mut buf, "outer", |b| { + b.put_u8(0xAA); + with_u8_len(b, "inner", |b2| { + b2.extend_from_slice(&[1, 2]); + Ok(()) + }) + }) + .unwrap(); + assert_eq!(buf.freeze(), Bytes::from_static(&[4, 0xAA, 2, 1, 2])); + } + + #[test] + fn test_check_max() { + assert_eq!(check_max("count", 10, 255).unwrap(), 10); + assert_eq!(check_max("count", 255, 255).unwrap(), 255); + assert!(check_max("count", 256, 255).is_err()); + assert!(check_max("FlowSpec length", 4096, 0x0FFF).is_err()); + } +} diff --git a/src/encoder/updates_encoder.rs b/src/encoder/updates_encoder.rs index e503829..a0fa803 100644 --- a/src/encoder/updates_encoder.rs +++ b/src/encoder/updates_encoder.rs @@ -1,6 +1,7 @@ use std::net::IpAddr; use std::str::FromStr; +use crate::error::EncodingError; use crate::models::{ Asn, Bgp4MpEnum, Bgp4MpMessage, Bgp4MpType, BgpMessage, BgpUpdateMessage, CommonHeader, EntryType, MrtMessage, @@ -27,7 +28,10 @@ impl MrtUpdatesEncoder { self.cached_elems.push(elem.clone()); } - pub fn export_bytes(&mut self) -> Bytes { + /// Export all cached elements as MRT BGP4MP records. + /// + /// Returns [`EncodingError`] if any element fails to encode. + pub fn export_bytes(&mut self) -> Result { let mut bytes = BytesMut::new(); for elem in &self.cached_elems { @@ -55,7 +59,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 +74,7 @@ impl MrtUpdatesEncoder { self.reset(); - bytes.freeze() + Ok(bytes.freeze()) } } @@ -94,7 +98,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 +118,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/error.rs b/src/error.rs index 2c1bcf9..f84b266 100644 --- a/src/error.rs +++ b/src/error.rs @@ -38,6 +38,66 @@ pub enum ParserError { impl Error for ParserError {} +/// Errors that can occur during encoding of BGP/MRT messages to wire format. +/// +/// These arise when in-memory data structures contain values that are too large +/// for their wire-format length fields (e.g. an AS_PATH segment with more than +/// 255 ASes, or an attribute value exceeding 65535 bytes), or values that cannot +/// be represented on the wire at all. All such conditions were previously +/// handled by panicking or silently truncating — see issue #313. +#[derive(Debug)] +#[non_exhaustive] +pub enum EncodingError { + /// A value exceeded the maximum size that fits in its wire-format length + /// field. + /// + /// `field` names the wire field (e.g. `"AS_PATH segment count"`, + /// `"attribute value length"`, `"BGP message total 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 not implemented, a labeled NLRI with an empty label stack, or + /// an OPEN optional parameter of type 255 in non-extended framing. + Unencodable { field: &'static str, reason: String }, +} + +impl EncodingError { + /// Construct a [`ValueTooLarge`](EncodingError::ValueTooLarge) error. + pub(crate) fn too_large(field: &'static str, actual: usize, max: usize) -> Self { + EncodingError::ValueTooLarge { field, actual, max } + } + + /// Construct an [`Unencodable`](EncodingError::Unencodable) error. + pub(crate) fn unencodable(field: &'static str, reason: impl Into) -> Self { + EncodingError::Unencodable { + field, + reason: reason.into(), + } + } +} + +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}: {reason}") + } + } + } +} + +impl Error for EncodingError {} + #[derive(Debug)] pub struct ParserErrorWithBytes { pub error: ParserError, diff --git a/src/lib.rs b/src/lib.rs index 59afdfd..50ff194 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); ``` @@ -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" )] +// Encoding results must be consumed: dropping an encode_to/encode Result +// without handling it is a compile error, not a silent data loss. +#![deny(unused_must_use)] #[cfg(feature = "parser")] pub mod encoder; diff --git a/src/models/bgp/flowspec/nlri.rs b/src/models/bgp/flowspec/nlri.rs index ac16714..0e8a0c1 100644 --- a/src/models/bgp/flowspec/nlri.rs +++ b/src/models/bgp/flowspec/nlri.rs @@ -1,4 +1,6 @@ use super::*; +use crate::encoder::sink::check_max; +use crate::error::EncodingError; use crate::models::NetworkPrefix; use ipnet::IpNet; @@ -55,7 +57,7 @@ pub fn parse_flowspec_nlri(data: &[u8]) -> Result { } /// 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 +90,13 @@ pub fn encode_flowspec_nlri(nlri: &FlowSpecNlri) -> Vec { } } - // Prepend length + // Prepend length. RFC 8955/8956: the wire length field is 12-bit + // (0x0FFF = 4095), not 16-bit — check_max makes the bound explicit. let mut result = Vec::new(); - encode_length(data.len() as u16, &mut result); + let nlri_len = check_max("FlowSpec NLRI total length", data.len(), 0x0FFF)?; + encode_length(nlri_len as u16, &mut result); result.extend(data); - result + Ok(result) } /// Parse length field (1 or 2 octets) @@ -502,9 +506,17 @@ 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); } + + #[test] + fn test_encode_flowspec_nlri_oversize() { + // Exceed u16::MAX total length → EncodingError + let ops = vec![NumericOperator::equal_to(0); 40000]; + let nlri = FlowSpecNlri::new(vec![FlowSpecComponent::IpProtocol(ops)]); + assert!(encode_flowspec_nlri(&nlri).is_err()); + } } diff --git a/src/models/bgp/flowspec/tests.rs b/src/models/bgp/flowspec/tests.rs index 02d32c5..158e2a9 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 5ef4f96..e1c25e5 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,7 @@ 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); + assert_eq!(tlv.value.len(), 3); } #[test] diff --git a/src/models/bgp/tunnel_encap.rs b/src/models/bgp/tunnel_encap.rs index 7e2e57d..300694a 100644 --- a/src/models/bgp/tunnel_encap.rs +++ b/src/models/bgp/tunnel_encap.rs @@ -124,10 +124,6 @@ impl SubTlv { value, } } - - pub fn length(&self) -> u16 { - self.value.len() as u16 - } } /// Tunnel Encapsulation TLV @@ -338,7 +334,7 @@ mod tests { #[test] fn test_sub_tlv_length() { let sub_tlv = SubTlv::new(SubTlvType::Color, vec![0x00, 0x00, 0x00, 0x64]); - assert_eq!(sub_tlv.length(), 4); + 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 dd598d6..0b874c7 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,5 @@ +use crate::encoder::sink::check_max; +use crate::error::EncodingError; use crate::models::*; use crate::parser::ReadUtils; use crate::ParserError; @@ -54,33 +56,25 @@ fn parse_as_path_segment( } } -pub fn encode_as_path(path: &AsPath, asn_len: AsnLength) -> Bytes { - let mut output = BytesMut::with_capacity(1024); +/// Append the wire representation of `path` to `buf`. +pub fn encode_as_path_to( + path: &AsPath, + asn_len: AsnLength, + buf: &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 (seg_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), + }; + let count = check_max("AS_PATH segment AS count", asns.len(), u8::MAX as usize)?; + buf.put_u8(seg_type); + buf.put_u8(count as u8); + write_asns(asns, asn_len, buf); } - output.freeze() + Ok(()) } fn write_asns(asns: &[Asn], asn_len: AsnLength, output: &mut BytesMut) { @@ -222,7 +216,9 @@ 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 buf = BytesMut::new(); + encode_as_path_to(&path, AsnLength::Bits16, &mut buf).unwrap(); + let encoded_bytes = buf.freeze(); assert_eq!(data, encoded_bytes); let data = Bytes::from(vec![ @@ -233,7 +229,9 @@ 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 buf = BytesMut::new(); + encode_as_path_to(&path, AsnLength::Bits32, &mut buf).unwrap(); + let encoded_bytes = buf.freeze(); assert_eq!(data, encoded_bytes); } @@ -245,7 +243,9 @@ 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 buf = BytesMut::new(); + encode_as_path_to(&path, AsnLength::Bits16, &mut buf).unwrap(); + let encoded_bytes = buf.freeze(); assert_eq!(data, encoded_bytes); let data = Bytes::from(vec![ @@ -254,7 +254,9 @@ 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 buf = BytesMut::new(); + encode_as_path_to(&path, AsnLength::Bits16, &mut buf).unwrap(); + let encoded_bytes = buf.freeze(); assert_eq!(data, encoded_bytes); } diff --git a/src/parser/bgp/attributes/attr_23_tunnel_encap.rs b/src/parser/bgp/attributes/attr_23_tunnel_encap.rs index 5d49b96..eacf503 100644 --- a/src/parser/bgp/attributes/attr_23_tunnel_encap.rs +++ b/src/parser/bgp/attributes/attr_23_tunnel_encap.rs @@ -2,7 +2,8 @@ use bytes::{Buf, BufMut, Bytes, BytesMut}; -use crate::error::ParserError; +use crate::encoder::sink::{with_u16_len, with_u8_len}; +use crate::error::{EncodingError, ParserError}; use crate::models::*; use crate::parser::ReadUtils; @@ -81,39 +82,41 @@ fn parse_tunnel_tlv(tunnel_type: u16, mut data: Bytes) -> Result Bytes { - let mut bytes = BytesMut::new(); +pub fn encode_tunnel_encapsulation_attribute( + attr: &TunnelEncapAttribute, +) -> Result { + let mut buf = BytesMut::new(); 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); + buf.put_u16(tunnel_tlv.tunnel_type as u16); + + // Encode sub-TLVs with back-patched tunnel length + with_u16_len(&mut buf, "Tunnel Encap tunnel total length", |b| { + for sub_tlv in &tunnel_tlv.sub_tlvs { + let sub_tlv_type = sub_tlv.sub_tlv_type as u16; + + // Encode sub-TLV type (common to both branches) + b.put_u8(sub_tlv_type as u8); + + // Encode sub-TLV length+value: u8 for type < 128, u16 for type >= 128 + if sub_tlv_type < 128 { + with_u8_len(b, "Tunnel Encap sub-TLV value length", |b2| { + b2.extend_from_slice(&sub_tlv.value); + Ok(()) + })?; + } else { + with_u16_len(b, "Tunnel Encap sub-TLV value length", |b2| { + b2.extend_from_slice(&sub_tlv.value); + Ok(()) + })?; + } } - - // 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(buf.freeze()) } #[cfg(test)] @@ -231,7 +234,7 @@ mod tests { attr.add_tunnel_tlv(tunnel_tlv); - let encoded = encode_tunnel_encapsulation_attribute(&attr); + let encoded = encode_tunnel_encapsulation_attribute(&attr).unwrap(); // Should encode back to the same format we can parse let parsed = parse_tunnel_encapsulation_attribute(encoded).unwrap(); diff --git a/src/parser/bgp/attributes/attr_29_linkstate.rs b/src/parser/bgp/attributes/attr_29_linkstate.rs index 7350db5..4ec762b 100644 --- a/src/parser/bgp/attributes/attr_29_linkstate.rs +++ b/src/parser/bgp/attributes/attr_29_linkstate.rs @@ -1,5 +1,7 @@ //! BGP Link-State attribute parsing - RFC 7752 +use crate::encoder::sink::with_u16_len; +use crate::error::EncodingError; use bytes::{Buf, BufMut, Bytes, BytesMut}; use std::net::{Ipv4Addr, Ipv6Addr}; @@ -432,41 +434,54 @@ fn parse_ip_prefix_from_bytes(data: &[u8]) -> Result } /// Encode BGP Link-State attribute -pub fn encode_link_state_attribute(attr: &LinkStateAttribute) -> Bytes { +pub fn encode_link_state_attribute(attr: &LinkStateAttribute) -> Result { 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); + bytes.put_u16(u16::from(*attr_type)); + with_u16_len(&mut bytes, "Link-State node attribute value length", |b| { + b.extend_from_slice(value); + Ok(()) + })?; } // 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); + bytes.put_u16(u16::from(*attr_type)); + with_u16_len(&mut bytes, "Link-State link attribute value length", |b| { + b.extend_from_slice(value); + Ok(()) + })?; } // Encode prefix attributes for (attr_type, value) in &attr.prefix_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); + bytes.put_u16(u16::from(*attr_type)); + with_u16_len( + &mut bytes, + "Link-State prefix attribute value length", + |b| { + b.extend_from_slice(value); + Ok(()) + }, + )?; } // 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); + with_u16_len( + &mut bytes, + "Link-State unknown attribute value length", + |b| { + b.extend_from_slice(&tlv.value); + Ok(()) + }, + )?; } - bytes.freeze() + Ok(bytes.freeze()) } #[cfg(test)] @@ -531,7 +546,7 @@ 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 encoded = encode_link_state_attribute(&attr).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 e792b0d..b28185e 100644 --- a/src/parser/bgp/attributes/attr_37_sfp.rs +++ b/src/parser/bgp/attributes/attr_37_sfp.rs @@ -1,3 +1,5 @@ +use crate::encoder::sink::with_u16_len; +use crate::error::EncodingError; use crate::models::*; use crate::parser::ReadUtils; use crate::ParserError; @@ -27,14 +29,16 @@ pub fn parse_sfp(mut input: Bytes) -> Result { Ok(AttributeValue::Sfp(SfpAttribute { tlvs })) } -pub fn encode_sfp(attr: &SfpAttribute) -> Bytes { +pub fn encode_sfp(attr: &SfpAttribute) -> Result { let mut buf = BytesMut::new(); 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); + with_u16_len(&mut buf, "SFP TLV value length", |b| { + b.extend_from_slice(&tlv.value); + Ok(()) + })?; } - buf.freeze() + Ok(buf.freeze()) } #[cfg(test)] @@ -53,7 +57,7 @@ mod tests { attr.tlvs[0].value, Bytes::from_static(&[0xaa, 0xbb, 0xcc, 0xdd]) ); - assert_eq!(encode_sfp(&attr), input); + assert_eq!(encode_sfp(&attr).unwrap(), input); } value => panic!("expected SFP, got {value:?}"), } @@ -67,7 +71,7 @@ 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); + assert_eq!(encode_sfp(&attr).unwrap(), input); } value => panic!("expected SFP, got {value:?}"), } @@ -79,7 +83,7 @@ mod tests { match value { AttributeValue::Sfp(attr) => { assert!(attr.tlvs.is_empty()); - assert_eq!(encode_sfp(&attr), Bytes::new()); + assert_eq!(encode_sfp(&attr).unwrap(), Bytes::new()); } 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 7ccf748..4ab844a 100644 --- a/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs +++ b/src/parser/bgp/attributes/attr_38_bfd_discriminator.rs @@ -1,3 +1,5 @@ +use crate::encoder::sink::with_u8_len; +use crate::error::EncodingError; use crate::models::*; use crate::parser::ReadUtils; use crate::ParserError; @@ -41,16 +43,18 @@ pub fn parse_bfd_discriminator(mut input: Bytes) -> Result Bytes { +pub fn encode_bfd_discriminator(attr: &BfdDiscriminatorAttribute) -> Result { let mut buf = BytesMut::new(); 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); + with_u8_len(&mut buf, "BFD Discriminator TLV value length", |b| { + b.extend_from_slice(&tlv.value); + Ok(()) + })?; } - buf.freeze() + Ok(buf.freeze()) } #[cfg(test)] @@ -71,7 +75,7 @@ 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); + assert_eq!(encode_bfd_discriminator(&attr).unwrap(), input); } value => panic!("expected BFD Discriminator, got {value:?}"), } @@ -86,7 +90,7 @@ mod tests { assert_eq!(attr.mode, 1); assert_eq!(attr.discriminator, 0x01020304); assert!(attr.tlvs.is_empty()); - assert_eq!(encode_bfd_discriminator(&attr), input); + assert_eq!(encode_bfd_discriminator(&attr).unwrap(), 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 b65b6a9..b28accd 100644 --- a/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs +++ b/src/parser/bgp/attributes/attr_40_bgp_prefix_sid.rs @@ -1,3 +1,5 @@ +use crate::encoder::sink::with_u16_len; +use crate::error::EncodingError; use crate::models::*; use crate::parser::ReadUtils; use crate::ParserError; @@ -27,14 +29,16 @@ pub fn parse_bgp_prefix_sid(mut input: Bytes) -> Result Bytes { +pub fn encode_bgp_prefix_sid(attr: &BgpPrefixSidAttribute) -> Result { let mut buf = BytesMut::new(); 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); + with_u16_len(&mut buf, "BGP Prefix-SID TLV value length", |b| { + b.extend_from_slice(&tlv.value); + Ok(()) + })?; } - buf.freeze() + Ok(buf.freeze()) } #[cfg(test)] @@ -55,7 +59,7 @@ 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); + assert_eq!(encode_bgp_prefix_sid(&attr).unwrap(), input); } value => panic!("expected Prefix-SID, got {value:?}"), } @@ -69,7 +73,7 @@ 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); + assert_eq!(encode_bgp_prefix_sid(&attr).unwrap(), input); } value => panic!("expected Prefix-SID, got {value:?}"), } @@ -81,7 +85,7 @@ mod tests { match value { AttributeValue::BgpPrefixSid(attr) => { assert!(attr.tlvs.is_empty()); - assert_eq!(encode_bgp_prefix_sid(&attr), Bytes::new()); + assert_eq!(encode_bgp_prefix_sid(&attr).unwrap(), Bytes::new()); } 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 d00d565..b5bf436 100644 --- a/src/parser/bgp/attributes/attr_41_bier.rs +++ b/src/parser/bgp/attributes/attr_41_bier.rs @@ -1,3 +1,5 @@ +use crate::encoder::sink::with_u16_len; +use crate::error::EncodingError; use crate::models::*; use crate::parser::ReadUtils; use crate::ParserError; @@ -27,14 +29,16 @@ pub fn parse_bier(mut input: Bytes) -> Result { Ok(AttributeValue::Bier(BierAttribute { tlvs })) } -pub fn encode_bier(attr: &BierAttribute) -> Bytes { +pub fn encode_bier(attr: &BierAttribute) -> Result { let mut buf = BytesMut::new(); 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); + with_u16_len(&mut buf, "BIER TLV value length", |b| { + b.extend_from_slice(&tlv.value); + Ok(()) + })?; } - buf.freeze() + Ok(buf.freeze()) } #[cfg(test)] @@ -54,7 +58,7 @@ 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); + assert_eq!(encode_bier(&attr).unwrap(), input); } value => panic!("expected BIER, got {value:?}"), } @@ -68,7 +72,7 @@ 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); + assert_eq!(encode_bier(&attr).unwrap(), input); } value => panic!("expected BIER, got {value:?}"), } @@ -80,7 +84,7 @@ mod tests { match value { AttributeValue::Bier(attr) => { assert!(attr.tlvs.is_empty()); - assert_eq!(encode_bier(&attr), Bytes::new()); + assert_eq!(encode_bier(&attr).unwrap(), Bytes::new()); } value => panic!("expected BIER, got {value:?}"), } diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index 054b346..ea264d0 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -25,9 +25,10 @@ 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; +use crate::parser::bgp::attributes::attr_02_17_as_path::encode_as_path_to; pub(crate) use crate::parser::bgp::attributes::attr_02_17_as_path::parse_as_path; use crate::parser::bgp::attributes::attr_03_next_hop::{encode_next_hop, parse_next_hop}; use crate::parser::bgp::attributes::attr_04_med::{encode_med, parse_med}; @@ -499,98 +500,129 @@ 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`. + /// + /// Returns [`EncodingError`] when a value is too large for its wire-format + /// field, or cannot be encoded at all (e.g. `AttrSet`). + 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.extend_from_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_to(path, four_byte, b)?; + } + AttributeValue::NextHop(v) => b.extend_from_slice(&encode_next_hop(v)), + AttributeValue::MultiExitDiscriminator(v) => b.extend_from_slice(&encode_med(*v)), + AttributeValue::LocalPreference(v) => b.extend_from_slice(&encode_local_pref(*v)), + AttributeValue::OnlyToCustomer(v) => { + b.extend_from_slice(&encode_only_to_customer(v.into())) + } + AttributeValue::AtomicAggregate => {} + AttributeValue::Aggregator { asn, id, is_as4: _ } => { + b.extend_from_slice(&encode_aggregator(asn, &IpAddr::from(*id))) + } + AttributeValue::Communities(v) => { + b.extend_from_slice(&encode_regular_communities(v)) + } + AttributeValue::ExtendedCommunities(v) => { + b.extend_from_slice(&encode_extended_communities(v)) + } + AttributeValue::LargeCommunities(v) => { + b.extend_from_slice(&encode_large_communities(v)) + } + AttributeValue::Ipv6AddressSpecificExtendedCommunities(v) => { + b.extend_from_slice(&encode_ipv6_extended_communities(v)) + } + AttributeValue::OriginatorId(v) => { + b.extend_from_slice(&encode_originator_id(&IpAddr::from(*v))) + } + AttributeValue::Clusters(v) => b.extend_from_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())); + let encoded = encode_nlri(v, true, add_path) + .map_err(|e| EncodingError::unencodable("MP_REACH_NLRI", e.to_string()))?; + b.extend_from_slice(&encoded); + } + AttributeValue::MpUnreachNlri(v) => { + // Withdrawals don't use ADD-PATH encoding per RFC 8277 + let encoded = encode_nlri(v, false, false).map_err(|e| { + EncodingError::unencodable("MP_UNREACH_NLRI", e.to_string()) + })?; + b.extend_from_slice(&encoded); + } + AttributeValue::LinkState(v) => { + b.extend_from_slice(&encode_link_state_attribute(v)?) + } + AttributeValue::TunnelEncapsulation(v) => { + b.extend_from_slice(&encode_tunnel_encapsulation_attribute(v)?) + } + AttributeValue::BfdDiscriminator(v) => { + b.extend_from_slice(&encode_bfd_discriminator(v)?) + } + AttributeValue::BgpPrefixSid(v) => b.extend_from_slice(&encode_bgp_prefix_sid(v)?), + AttributeValue::Bier(v) => b.extend_from_slice(&encode_bier(v)?), + AttributeValue::Sfp(v) => b.extend_from_slice(&encode_sfp(v)?), + AttributeValue::Development(v) => b.extend_from_slice(v), + AttributeValue::Raw(v) => b.extend_from_slice(&v.bytes), + AttributeValue::Deprecated(v) => b.extend_from_slice(&v.bytes), + AttributeValue::Unknown(v) => b.extend_from_slice(&v.bytes), + AttributeValue::Aigp(v) => b.extend_from_slice(&encode_aigp(v)), + AttributeValue::AttrSet(_v) => { + return Err(EncodingError::unencodable( + "ATTR_SET", + "encoding not yet 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 (non-extended)", + write_value, + ), + true => with_u16_len(buf, "BGP attribute value length (extended)", write_value), } - bytes.extend(value_bytes); - bytes.freeze() + } + + /// Convenience: encode 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(()) + } + + /// Convenience: encode 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 +860,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 +881,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 +902,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 +931,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), Bytes::from(wire)); + assert_eq!( + attributes.encode(AsnLength::Bits16).unwrap(), + Bytes::from(wire) + ); } } @@ -926,7 +961,10 @@ 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 +998,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 6ac41a2..9591b0a 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::{check_max, with_u16_len, with_u8_len}; +use crate::error::{BgpValidationWarning, EncodingError, ParserError}; use crate::models::capabilities::{ AddPathCapability, BgpCapabilityType, BgpExtendedMessageCapability, BgpRoleCapability, ExtendedNextHopCapability, FourOctetAsCapability, GracefulRestartCapability, @@ -174,12 +175,18 @@ pub fn parse_bgp_notification_message( } impl BgpNotificationMessage { - pub fn encode(&self) -> Bytes { - let mut buf = BytesMut::new(); + /// Notification messages are fixed-shape and cannot fail to encode. + pub fn encode_to(&self, buf: &mut BytesMut) { let (code, subcode) = self.error.get_codes(); buf.put_u8(code); buf.put_u8(subcode); - buf.put_slice(&self.data); + buf.extend_from_slice(&self.data); + } + + /// Convenience: encode into a fresh buffer. + pub fn encode(&self) -> Bytes { + let mut buf = BytesMut::new(); + self.encode_to(&mut buf); buf.freeze() } } @@ -382,57 +389,81 @@ pub fn parse_bgp_open_message(input: &mut Bytes) -> Result Bytes { - let mut buf = BytesMut::new(); +fn encode_bgp_open_param_value_to( + param: &OptParam, + buf: &mut BytesMut, +) -> Result<(), EncodingError> { match ¶m.param_value { ParamValue::Capacities(capacities) => { for cap in capacities { buf.put_u8(cap.ty.into()); - let encoded_value = match &cap.value { - CapabilityValue::MultiprotocolExtensions(mp) => mp.encode(), - CapabilityValue::RouteRefresh(rr) => rr.encode(), - CapabilityValue::ExtendedNextHop(enh) => enh.encode(), - CapabilityValue::GracefulRestart(gr) => gr.encode(), - CapabilityValue::FourOctetAs(foa) => foa.encode(), - CapabilityValue::AddPath(ap) => ap.encode(), - CapabilityValue::BgpRole(br) => br.encode(), - 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); + with_u8_len(buf, "BGP capability value length", |b| { + let encoded_value = match &cap.value { + CapabilityValue::MultiprotocolExtensions(mp) => mp.encode(), + CapabilityValue::RouteRefresh(rr) => rr.encode(), + CapabilityValue::ExtendedNextHop(enh) => enh.encode(), + CapabilityValue::GracefulRestart(gr) => gr.encode(), + CapabilityValue::FourOctetAs(foa) => foa.encode(), + CapabilityValue::AddPath(ap) => ap.encode(), + CapabilityValue::BgpRole(br) => br.encode(), + CapabilityValue::BgpExtendedMessage(bem) => bem.encode(), + CapabilityValue::Raw(raw) => Bytes::from(raw.clone()), + }; + b.extend_from_slice(&encoded_value); + Ok(()) + })?; } } - ParamValue::Raw(bytes) => buf.put_slice(bytes), + ParamValue::Raw(bytes) => buf.extend_from_slice(bytes), } - buf.freeze() + Ok(()) } impl BgpOpenMessage { - pub fn encode(&self) -> Bytes { - let encoded_params: Vec<(u8, Bytes)> = self - .opt_params - .iter() - .map(|param| (param.param_type, encode_bgp_open_param_value(param))) - .collect(); + /// Append the wire representation of this OPEN message to `buf`. + /// + /// If `extended_length` is false but the optional parameters do not fit in + /// the single-octet aggregate length field, the encoder automatically + /// upgrades to RFC 9072 extended framing (documented behavior). A + /// non-extended OPEN with `param_type == 255` cannot be expressed + /// unambiguously and returns [`EncodingError::Unencodable`]. + pub fn encode_to(&self, buf: &mut BytesMut) -> Result<(), EncodingError> { + for param in &self.opt_params { + // RFC 9072: param_type 255 in the first position is the extended-length + // marker. A non-extended OPEN with a real parameter of type 255 would be + // ambiguous on the wire — the parser would misread it as extended framing. + if param.param_type == 255 && !self.extended_length { + return Err(EncodingError::unencodable( + "BGP OPEN optional parameter type 255", + "non-extended OPEN cannot use parameter type 255 (reserved by RFC 9072)", + )); + } + } + + // Measure the params section to decide framing. We encode each param + // twice only on the (rare) extended path; for the common non-extended + // path we compute sizes from the values without re-encoding by + // pre-checking the aggregate against u8::MAX using a scratch pass. + // + // Compute values_len cheaply: encode params into buf after the header + // and back-patch both the aggregate and per-param lengths depending on + // the framing we end up needing. To keep one pass, we first encode all + // params into a scratch buffer to learn their sizes. + let mut scratch = BytesMut::new(); + let mut per_param_value_lens = Vec::with_capacity(self.opt_params.len()); + for param in &self.opt_params { + let start = scratch.len(); + encode_bgp_open_param_value_to(param, &mut scratch)?; + per_param_value_lens.push(scratch.len() - start); + } + let values_len = scratch.len(); - let values_len: usize = encoded_params.iter().map(|(_, value)| value.len()).sum(); - // Non-extended framing spends 2 header octets (type + 1-octet length) per - // parameter; if that would overflow the single-octet aggregate length field - // we must switch to RFC 9072 extended framing (3 header octets each). - let non_extended_params_len = 2 * encoded_params.len() + values_len; + let non_extended_params_len = 2 * self.opt_params.len() + values_len; let use_extended_length = self.extended_length || non_extended_params_len > u8::MAX as usize; let per_param_header = if use_extended_length { 3 } else { 2 }; - let encoded_params_len = per_param_header * encoded_params.len() + values_len; + let encoded_params_len = per_param_header * self.opt_params.len() + values_len; - let mut buf = BytesMut::with_capacity( - size_of::() - + encoded_params_len - + if use_extended_length { 3 } else { 0 }, - ); let raw_header = RawBgpOpenHeader { version: self.version, asn: U16::new(self.asn.into()), @@ -441,6 +472,8 @@ impl BgpOpenMessage { opt_params_len: if use_extended_length { u8::MAX } else { + // Guaranteed: non_extended_params_len <= u8::MAX on this path, + // and encoded_params_len == non_extended_params_len here. encoded_params_len as u8 }, }; @@ -449,33 +482,38 @@ 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); } - for (param_type, value) in encoded_params { - buf.put_u8(param_type); + let mut scratch = scratch.freeze(); + for (param, value_len) in self.opt_params.iter().zip(per_param_value_lens) { + buf.put_u8(param.param_type); + let value = scratch.split_to(value_len); 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() - ); - buf.put_u16(value.len() as u16); + // Guaranteed by the aggregate check above. + 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 - // non-extended framing (2 + value.len() per param) would exceed u8::MAX. - buf.put_u8(value.len() as u8); + // Guaranteed by the framing decision above. + debug_assert!(value_len <= u8::MAX as usize); + buf.put_u8(value_len as u8); } - buf.put_slice(&value); + buf.extend_from_slice(&value); } - buf.freeze() + Ok(()) + } + + /// Convenience: encode into a fresh buffer. + pub fn encode(&self) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(&mut buf)?; + Ok(buf.freeze()) } } @@ -589,22 +627,29 @@ pub fn parse_bgp_update_message( } impl BgpUpdateMessage { - pub fn encode(&self, asn_len: AsnLength) -> Bytes { - let mut bytes = BytesMut::new(); - + /// Append the wire representation of this UPDATE message to `buf`. + pub fn encode_to(&self, asn_len: AsnLength, buf: &mut BytesMut) -> Result<(), EncodingError> { // withdrawn prefixes - let withdrawn_bytes = encode_nlri_prefixes(&self.withdrawn_prefixes); - bytes.put_u16(withdrawn_bytes.len() as u16); - bytes.put_slice(&withdrawn_bytes); + with_u16_len(buf, "BGP UPDATE withdrawn prefixes length", |b| { + b.extend_from_slice(&encode_nlri_prefixes(&self.withdrawn_prefixes)); + Ok(()) + })?; // attributes - let attr_bytes = self.attributes.encode(asn_len); + with_u16_len(buf, "BGP UPDATE path attributes length", |b| { + self.attributes.encode_to(asn_len, b) + })?; - bytes.put_u16(attr_bytes.len() as u16); - bytes.put_slice(&attr_bytes); + // announced prefixes (no length prefix — runs to end of message) + buf.extend_from_slice(&encode_nlri_prefixes(&self.announced_prefixes)); + Ok(()) + } - bytes.extend(encode_nlri_prefixes(&self.announced_prefixes)); - bytes.freeze() + /// Convenience: encode 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()) } /// Check if this is an end-of-rib message. @@ -654,23 +699,59 @@ 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 { - let mut bytes = BytesMut::new(); - // RFC 4271: Marker is 16 bytes of 0xFF - bytes.put_slice(&Self::MARKER); + /// Append the wire representation of this BGP message to `buf`. + pub fn encode_to(&self, asn_len: AsnLength, buf: &mut BytesMut) -> Result<(), EncodingError> { + // RFC 4271: Marker is 16 bytes of 0xFF, then a back-patched total + // length covering marker + length field + type + body. + let at = buf.len(); + buf.put_slice(&Self::MARKER); + buf.put_u16(0); // total-length placeholder + match self { + BgpMessage::Open(msg) => { + buf.put_u8(BgpMessageType::OPEN as u8); + if let Err(e) = msg.encode_to(buf) { + buf.truncate(at); + return Err(e); + } + } + BgpMessage::Update(msg) => { + buf.put_u8(BgpMessageType::UPDATE as u8); + if let Err(e) = msg.encode_to(asn_len, buf) { + buf.truncate(at); + return Err(e); + } + } + BgpMessage::Notification(msg) => { + buf.put_u8(BgpMessageType::NOTIFICATION as u8); + let (code, subcode) = msg.error.get_codes(); + buf.put_u8(code); + buf.put_u8(subcode); + buf.extend_from_slice(&msg.data); + } + BgpMessage::KeepAlive => { + buf.put_u8(BgpMessageType::KEEPALIVE as u8); + } + } - let (msg_type, msg_bytes) = match self { - 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()), + let total = buf.len() - at; + let Ok(total) = u16::try_from(total) else { + let actual = buf.len() - at; + buf.truncate(at); + return Err(EncodingError::too_large( + "BGP message total length", + actual, + u16::MAX as usize, + )); }; + buf[at + 16..at + 18].copy_from_slice(&total.to_be_bytes()); + Ok(()) + } - // 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); - bytes.put_u8(msg_type as u8); - bytes.put_slice(&msg_bytes); - bytes.freeze() + /// Convenience: encode 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()) } } @@ -895,7 +976,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!( @@ -1026,7 +1107,7 @@ mod tests { extended_length: false, opt_params: vec![], }; - let bytes = msg.encode(); + let bytes = msg.encode().unwrap(); assert_eq!( bytes, Bytes::from_static(&[ @@ -1084,7 +1165,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}"); } @@ -1104,16 +1185,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}; @@ -1140,7 +1220,17 @@ mod tests { }], }; - msg.encode(); + // encode returns Err instead of panicking + let result = msg.encode(); + assert!(result.is_err(), "encode should reject oversized capability"); + match result.unwrap_err() { + crate::error::EncodingError::ValueTooLarge { field, actual, max } => { + assert!(field.contains("capability value length"), "field: {field}"); + assert_eq!(max, 255); + assert!(actual > 255, "actual={actual}"); + } + other => panic!("expected ValueTooLarge, got {other:?}"), + } } #[test] @@ -1157,7 +1247,7 @@ mod tests { }], }; - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); assert_eq!( &encoded[9..], @@ -1165,11 +1255,13 @@ 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] fn test_bgp_open_automatically_uses_extended_parameter_encoding() { + // Documented behavior: a non-extended OPEN whose params exceed 255 bytes + // is automatically upgraded to RFC 9072 extended framing. let msg = BgpOpenMessage { version: 4, asn: Asn::new_16bit(64512), @@ -1182,13 +1274,108 @@ 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] + fn test_fallible_encoding_as_path_overflow() { + // An AS_PATH segment with >255 ASes overflows the 1-octet segment-length + // field. Before the fix this silently truncated; now try_encode returns Err. + let path = AsPath::from_sequence((1u32..=300).collect::>()); + let attr = Attribute { + flag: AttrFlags::TRANSITIVE, + value: AttributeValue::AsPath { path, is_as4: true }, + }; + + let result = attr.encode(AsnLength::Bits32); + assert!( + result.is_err(), + "encode should reject AS_PATH with >255 ASes" + ); + } + + #[test] + fn test_fallible_encoding_open_raw_capability_oversize() { + // A CapabilityValue::Raw with >255 bytes should fail gracefully via + // fallible encode, not panic. + 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: 2, + param_value: ParamValue::Capacities(vec![Capability { + ty: BgpCapabilityType::Unknown(99), + value: CapabilityValue::Raw(vec![0xAA; 300]), + }]), + }], + }; + + assert!(msg.encode().is_err()); + } + + #[test] + fn test_fallible_encoding_open_extended_param_oversize() { + // An OPEN with extended-length params that exceed u16::MAX should fail. + let msg = BgpOpenMessage { + version: 4, + asn: Asn::new_16bit(64512), + hold_time: 90, + bgp_identifier: Ipv4Addr::new(192, 0, 2, 1), + extended_length: true, + opt_params: vec![OptParam { + param_type: 254, + param_value: ParamValue::Raw(vec![0xBB; 70000]), + }], + }; + + let result = msg.encode(); + assert!( + result.is_err(), + "encode should reject oversized extended param" + ); + } + + #[test] + fn test_fallible_encoding_update_attributes_oversize() { + // An UPDATE whose total attributes exceed u16::MAX should fail. + use crate::models::{AttrFlags, AttrRaw, Attribute, AttributeValue}; + + // Each Raw attribute is 4 bytes header + 1000 bytes value = 1004 bytes. + // 70 of them ≈ 70280 bytes > 65535. + let attrs: Vec = (0..70) + .map(|_| Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::Raw(AttrRaw { + code: 200, + bytes: vec![0; 1000].into(), + }), + }) + .collect(); + + let msg = BgpUpdateMessage { + withdrawn_prefixes: vec![], + attributes: Attributes { + inner: attrs, + validation_warnings: vec![], + attr_mask: [0; 4], + }, + announced_prefixes: vec![], + }; + + let result = msg.encode(AsnLength::Bits32); + assert!( + result.is_err(), + "encode should reject oversized UPDATE attributes" + ); } #[test] @@ -1197,7 +1384,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, @@ -1452,7 +1639,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 { @@ -1475,7 +1662,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) @@ -1548,7 +1735,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(); @@ -1606,7 +1793,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(); @@ -1671,7 +1858,7 @@ mod tests { }; // Encode the message - let encoded = msg.encode(); + let encoded = msg.encode().unwrap(); // Parse it back let mut encoded_bytes = encoded.clone(); @@ -1752,7 +1939,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(); @@ -1781,4 +1968,249 @@ mod tests { panic!("Expected Capacities in second parameter"); } } + + #[test] + fn test_encoding_error_tunnel_encap_subtlv_oversize() { + use crate::models::tunnel_encap::{ + SubTlv, SubTlvType, TunnelEncapAttribute, TunnelEncapTlv, + }; + + let encap = TunnelEncapAttribute { + tunnel_tlvs: vec![TunnelEncapTlv { + tunnel_type: crate::models::tunnel_encap::TunnelType::Vxlan, + sub_tlvs: vec![SubTlv { + sub_tlv_type: SubTlvType::Color, + value: vec![0; 300], + }], + }], + }; + let attr = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::TunnelEncapsulation(encap), + }; + assert!(attr.encode(AsnLength::Bits32).is_err()); + } + + #[test] + fn test_encoding_error_tunnel_encap_ext_subtlv_oversize() { + use crate::models::tunnel_encap::{ + SubTlv, SubTlvType, TunnelEncapAttribute, TunnelEncapTlv, + }; + + let encap = TunnelEncapAttribute { + tunnel_tlvs: vec![TunnelEncapTlv { + tunnel_type: crate::models::tunnel_encap::TunnelType::Vxlan, + sub_tlvs: vec![SubTlv { + sub_tlv_type: SubTlvType::SegmentList, + value: vec![0; 70000], + }], + }], + }; + let attr = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::TunnelEncapsulation(encap), + }; + assert!(attr.encode(AsnLength::Bits32).is_err()); + } + + #[test] + fn test_encoding_error_tunnel_encap_tunnel_total_oversize() { + use crate::models::tunnel_encap::{ + SubTlv, SubTlvType, TunnelEncapAttribute, TunnelEncapTlv, + }; + + let encap = TunnelEncapAttribute { + tunnel_tlvs: vec![TunnelEncapTlv { + tunnel_type: crate::models::tunnel_encap::TunnelType::Vxlan, + sub_tlvs: vec![ + SubTlv { + sub_tlv_type: SubTlvType::SegmentList, + value: vec![0; 40000], + }; + 2 + ], + }], + }; + let attr = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::TunnelEncapsulation(encap), + }; + assert!(attr.encode(AsnLength::Bits32).is_err()); + } + + #[test] + fn test_encoding_error_linkstate_oversize() { + use crate::models::linkstate::{LinkStateAttribute, NodeAttributeType}; + + let mut ls = LinkStateAttribute::new(); + ls.add_node_attribute(NodeAttributeType::NodeName, vec![0; 70000]); + let attr = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::LinkState(ls), + }; + assert!(attr.encode(AsnLength::Bits32).is_err()); + + let mut ls2 = LinkStateAttribute::new(); + ls2.add_unknown_attribute(crate::models::linkstate::Tlv::new(1, vec![0; 70000])); + let attr2 = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::LinkState(ls2), + }; + assert!(attr2.encode(AsnLength::Bits32).is_err()); + } + + #[test] + fn test_encoding_error_bfd_discriminator_oversize() { + use crate::models::{BfdDiscriminatorAttribute, RawTlv8}; + + let attr_val = BfdDiscriminatorAttribute { + mode: 0, + discriminator: 0, + tlvs: vec![RawTlv8 { + tlv_type: 1, + value: vec![0; 300].into(), + }], + }; + let attr = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::BfdDiscriminator(attr_val), + }; + assert!(attr.encode(AsnLength::Bits32).is_err()); + } + + #[test] + fn test_encoding_error_bgp_prefix_sid_oversize() { + use crate::models::{BgpPrefixSidAttribute, RawTlv8Ext}; + + let attr_val = BgpPrefixSidAttribute { + tlvs: vec![RawTlv8Ext { + tlv_type: 1, + value: vec![0; 70000].into(), + }], + }; + let attr = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::BgpPrefixSid(attr_val), + }; + assert!(attr.encode(AsnLength::Bits32).is_err()); + } + + #[test] + fn test_encoding_error_bier_oversize() { + use crate::models::{BierAttribute, RawTlv16}; + + let attr_val = BierAttribute { + tlvs: vec![RawTlv16 { + tlv_type: 1, + value: vec![0; 70000].into(), + }], + }; + let attr = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::Bier(attr_val), + }; + assert!(attr.encode(AsnLength::Bits32).is_err()); + } + + #[test] + fn test_encoding_error_sfp_oversize() { + use crate::models::{RawTlv8Ext, SfpAttribute}; + + let attr_val = SfpAttribute { + tlvs: vec![RawTlv8Ext { + tlv_type: 1, + value: vec![0; 70000].into(), + }], + }; + let attr = Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::Sfp(attr_val), + }; + assert!(attr.encode(AsnLength::Bits32).is_err()); + } + + #[test] + fn test_encoding_error_mrt_table_dump_oversize() { + use crate::models::{AttrFlags, AttrRaw, Attribute, AttributeValue, TableDumpMessage}; + + let attrs: Vec = (0..70) + .map(|_| Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::Raw(AttrRaw { + code: 200, + bytes: vec![0; 1000].into(), + }), + }) + .collect(); + + let msg = TableDumpMessage { + view_number: 0, + sequence_number: 0, + prefix: "10.0.0.0/24".parse().unwrap(), + status: 0, + originated_time: 0, + peer_ip: std::net::IpAddr::V4(std::net::Ipv4Addr::new(10, 0, 0, 1)), + peer_asn: Asn::new_16bit(65000), + attributes: Attributes { + inner: attrs, + validation_warnings: vec![], + attr_mask: [0; 4], + }, + }; + assert!(msg.encode().is_err()); + } + + #[test] + fn test_encoding_error_rib_entry_oversize() { + use crate::models::{AttrFlags, AttrRaw, Attribute, AttributeValue, RibEntry}; + + let attrs: Vec = (0..70) + .map(|_| Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::Raw(AttrRaw { + code: 200, + bytes: vec![0; 1000].into(), + }), + }) + .collect(); + + let entry = RibEntry { + peer_index: 0, + originated_time: 0, + path_id: None, + attributes: Attributes { + inner: attrs, + validation_warnings: vec![], + attr_mask: [0; 4], + }, + }; + assert!(entry.encode().is_err()); + } + + #[test] + fn test_encoding_error_peer_index_table_oversize() { + use crate::models::PeerIndexTable; + + let table = PeerIndexTable { + collector_bgp_id: std::net::Ipv4Addr::new(0, 0, 0, 0), + view_name: "x".repeat(70000), + id_peer_map: std::collections::HashMap::new(), + peer_ip_id_map: std::collections::HashMap::new(), + }; + assert!(table.encode().is_err()); + } + + #[test] + fn test_encoding_error_geo_peer_table_oversize() { + use crate::models::GeoPeerTable; + + let table = GeoPeerTable { + collector_bgp_id: std::net::Ipv4Addr::new(0, 0, 0, 0), + view_name: "x".repeat(70000), + collector_latitude: 0.0, + collector_longitude: 0.0, + geo_peers: vec![], + }; + assert!(table.encode().is_err()); + } } diff --git a/src/parser/bmp/messages/peer_up_notification.rs b/src/parser/bmp/messages/peer_up_notification.rs index e2391da..5e4ffaa 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 0457aa3..ef5f3c7 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 98b98c2..2f24cda 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 8d44cac..2f9782e 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()); @@ -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 } @@ -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 { @@ -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.extend(&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); @@ -1150,6 +1150,7 @@ mod tests { }), ) .encode() + .unwrap() .to_vec(); let routes = BgpkitParser::from_reader(Cursor::new(bytes)) @@ -1202,6 +1203,7 @@ mod tests { }), ) .encode() + .unwrap() .to_vec(); let routes = assert_route_projection(bytes); @@ -1234,7 +1236,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()); @@ -1251,6 +1253,7 @@ mod tests { }), ) .encode() + .unwrap() .to_vec(); let routes = assert_route_projection(bytes); @@ -1260,7 +1263,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")], @@ -1303,7 +1306,7 @@ mod tests { ); let attrs = parse_route_attributes( - attributes.encode(AsnLength::Bits16), + attributes.encode(AsnLength::Bits16).unwrap(), &AsnLength::Bits16, false, RouteAttributeContext { @@ -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), + route_attributes([64500, 64501]) + .encode(AsnLength::Bits16) + .unwrap(), &AsnLength::Bits16, false, RouteAttributeContext { @@ -1356,7 +1361,7 @@ mod tests { ); let attrs = parse_route_attributes( - attributes.encode(AsnLength::Bits16), + attributes.encode(AsnLength::Bits16).unwrap(), &AsnLength::Bits16, false, RouteAttributeContext { @@ -1459,7 +1464,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 +1473,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!( @@ -1501,7 +1506,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 { @@ -1534,8 +1539,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 +1574,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 +1695,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 +1704,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(), @@ -1729,7 +1734,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, @@ -1741,7 +1746,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); @@ -1837,6 +1842,7 @@ mod tests { let routes = BgpkitParser::from_reader(Cursor::new( table_dump_v2_rib_without_peer_table_record() .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().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); @@ -1863,6 +1870,7 @@ mod tests { let mut iter = BgpkitParser::from_reader(Cursor::new( table_dump_v2_rib_without_peer_table_record() .encode() + .unwrap() .to_vec(), )) .into_fallible_route_iter(); @@ -1872,7 +1880,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/mod.rs b/src/parser/mod.rs index c15ab06..7ccf9d1 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -22,7 +22,7 @@ pub use mrt::mrt_elem::{BgpUpdateElemIter, ElemError, Elementor, RecordElemIter} #[cfg(feature = "oneio")] use oneio::{get_cache_reader, get_reader}; -pub use crate::error::{ParserError, ParserErrorWithBytes}; +pub use crate::error::{EncodingError, ParserError, ParserErrorWithBytes}; pub use bmp::{parse_bmp_msg, parse_openbmp_header, parse_openbmp_msg}; pub use filter::*; pub use iters::*; diff --git a/src/parser/mrt/messages/bgp4mp.rs b/src/parser/mrt/messages/bgp4mp.rs index e76734f..e4d25ff 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,16 +171,22 @@ pub fn parse_bgp4mp_message( } impl Bgp4MpMessage { - pub fn encode(&self, asn_len: AsnLength) -> Bytes { - let mut bytes = BytesMut::new(); - bytes.extend(encode_asn(&self.peer_asn, &asn_len)); - bytes.extend(encode_asn(&self.local_asn, &asn_len)); - bytes.put_u16(self.interface_index); - 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() + /// Append the wire representation of this BGP4MP message to `buf`. + pub fn encode_to(&self, asn_len: AsnLength, buf: &mut BytesMut) -> Result<(), EncodingError> { + buf.extend_from_slice(&encode_asn(&self.peer_asn, &asn_len)); + buf.extend_from_slice(&encode_asn(&self.local_asn, &asn_len)); + buf.put_u16(self.interface_index); + buf.put_u16(address_family(&self.peer_ip)); + buf.extend_from_slice(&encode_ipaddr(&self.peer_ip)); + buf.extend_from_slice(&encode_ipaddr(&self.local_ip)); + self.bgp_message.encode_to(asn_len, buf) + } + + /// Convenience: encode 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()) } } @@ -242,17 +248,23 @@ pub fn parse_bgp4mp_state_change( } impl Bgp4MpStateChange { + /// Append the wire representation of this state-change message to `buf`. + pub fn encode_to(&self, asn_len: AsnLength, buf: &mut BytesMut) { + buf.extend_from_slice(&encode_asn(&self.peer_asn, &asn_len)); + buf.extend_from_slice(&encode_asn(&self.local_asn, &asn_len)); + buf.put_u16(self.interface_index); + buf.put_u16(address_family(&self.peer_ip)); + buf.extend_from_slice(&encode_ipaddr(&self.peer_ip)); + buf.extend_from_slice(&encode_ipaddr(&self.local_addr)); + buf.put_u16(self.old_state as u16); + buf.put_u16(self.new_state as u16); + } + + /// Convenience: encode into a fresh buffer. pub fn encode(&self, asn_len: AsnLength) -> Bytes { - let mut bytes = BytesMut::new(); - bytes.extend(encode_asn(&self.peer_asn, &asn_len)); - bytes.extend(encode_asn(&self.local_asn, &asn_len)); - bytes.put_u16(self.interface_index); - bytes.put_u16(address_family(&self.peer_ip)); - bytes.extend(encode_ipaddr(&self.peer_ip)); - bytes.extend(encode_ipaddr(&self.local_addr)); - bytes.put_u16(self.old_state as u16); - bytes.put_u16(self.new_state as u16); - bytes.freeze() + let mut buf = BytesMut::new(); + self.encode_to(asn_len, &mut buf); + buf.freeze() } } @@ -274,7 +286,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 +309,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.extend(&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 3f0d238..370b71a 100644 --- a/src/parser/mrt/messages/mod.rs +++ b/src/parser/mrt/messages/mod.rs @@ -1,24 +1,32 @@ +use crate::error::EncodingError; use crate::models::{AsnLength, Bgp4MpEnum, Bgp4MpType, MrtMessage, TableDumpV2Message}; -use bytes::Bytes; +use bytes::{Bytes, BytesMut}; pub(crate) mod bgp4mp; pub(crate) mod table_dump; pub(crate) mod table_dump_v2; impl MrtMessage { - pub fn encode(&self, sub_type: u16) -> Bytes { - let msg_bytes: Bytes = match self { - MrtMessage::TableDumpMessage(m) => m.encode(), + /// Append the wire representation of this MRT message body to `buf`. + pub fn encode_to(&self, sub_type: u16, buf: &mut BytesMut) -> Result<(), EncodingError> { + match self { + MrtMessage::TableDumpMessage(m) => m.encode_to(buf), MrtMessage::TableDumpV2Message(m) => match m { - TableDumpV2Message::PeerIndexTable(p) => p.encode(), - TableDumpV2Message::RibAfi(r) => r.encode(), - TableDumpV2Message::RibGeneric(_) => { - todo!("RibGeneric message is not supported yet"); - } - TableDumpV2Message::GeoPeerTable(g) => g.encode(), + TableDumpV2Message::PeerIndexTable(p) => p.encode_to(buf), + TableDumpV2Message::RibAfi(r) => r.encode_to(buf), + TableDumpV2Message::RibGeneric(_) => Err(EncodingError::unencodable( + "TABLE_DUMP_V2 RIB_GENERIC", + "RIB_GENERIC encoding is not supported", + )), + TableDumpV2Message::GeoPeerTable(g) => g.encode_to(buf), }, MrtMessage::Bgp4Mp(m) => { - let msg_type = Bgp4MpType::try_from(sub_type).unwrap(); + let msg_type = Bgp4MpType::try_from(sub_type).map_err(|_| { + EncodingError::unencodable( + "BGP4MP subtype", + format!("unknown subtype {sub_type}"), + ) + })?; match m { Bgp4MpEnum::StateChange(msg) => { @@ -26,7 +34,8 @@ impl MrtMessage { true => AsnLength::Bits32, false => AsnLength::Bits16, }; - msg.encode(asn_len) + msg.encode_to(asn_len, buf); + Ok(()) } Bgp4MpEnum::Message(msg) => { let asn_len = match matches!( @@ -39,13 +48,18 @@ impl MrtMessage { true => AsnLength::Bits32, false => AsnLength::Bits16, }; - msg.encode(asn_len) + msg.encode_to(asn_len, buf) } } } - }; + } + } - msg_bytes + /// Convenience: encode into a fresh buffer. + pub fn encode(&self, sub_type: u16) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(sub_type, &mut buf)?; + Ok(buf.freeze()) } } @@ -70,7 +84,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 4d20185..f65e978 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,45 +119,44 @@ pub fn parse_table_dump_message( } impl TableDumpMessage { - pub fn encode(&self) -> Bytes { - let mut bytes = BytesMut::new(); - bytes.put_u16(self.view_number); - bytes.put_u16(self.sequence_number); + /// Append the wire representation of this TABLE_DUMP entry to `buf`. + pub fn encode_to(&self, buf: &mut BytesMut) -> Result<(), EncodingError> { + buf.put_u16(self.view_number); + buf.put_u16(self.sequence_number); match &self.prefix.prefix { IpNet::V4(p) => { - bytes.put_u32(p.addr().into()); - bytes.put_u8(p.prefix_len()); + buf.put_u32(p.addr().into()); + buf.put_u8(p.prefix_len()); } IpNet::V6(p) => { - bytes.put_u128(p.addr().into()); - bytes.put_u8(p.prefix_len()); + buf.put_u128(p.addr().into()); + buf.put_u8(p.prefix_len()); } } - bytes.put_u8(self.status); - bytes.put_u32(self.originated_time as u32); + buf.put_u8(self.status); + buf.put_u32(self.originated_time as u32); // peer address and peer asn match self.peer_ip { IpAddr::V4(a) => { - bytes.put_u32(a.into()); + buf.put_u32(a.into()); } IpAddr::V6(a) => { - bytes.put_u128(a.into()); + buf.put_u128(a.into()); } } - bytes.put_u16(self.peer_asn.into()); + buf.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); + with_u16_len(buf, "TABLE_DUMP attribute length", |b| { + self.attributes.encode_to(AsnLength::Bits16, b) + }) + } - bytes.freeze() + /// Convenience: encode into a fresh buffer. + pub fn encode(&self) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(&mut buf)?; + Ok(buf.freeze()) } } @@ -213,7 +213,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 +252,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); } 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 a906487..af42163 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::{check_max, with_u16_len}; +use crate::error::{EncodingError, ParserError}; use crate::models::*; use crate::parser::ReadUtils; use bytes::{Buf, BufMut, Bytes, BytesMut}; @@ -136,23 +137,28 @@ impl GeoPeerTable { /// /// let encoded = geo_table.encode(); /// ``` - pub fn encode(&self) -> Bytes { - let mut buf = BytesMut::new(); - + /// Append the wire representation of this geo peer table to `buf`. + pub fn encode_to(&self, buf: &mut BytesMut) -> Result<(), EncodingError> { // 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); + // Encode view name with back-patched length + with_u16_len(buf, "GEO_PEER_TABLE view name length", |b| { + b.extend_from_slice(self.view_name.as_bytes()); + Ok(()) + })?; // Encode collector coordinates (4 bytes each, 32-bit float) buf.put_f32(self.collector_latitude); buf.put_f32(self.collector_longitude); // Encode peer count - buf.put_u16(self.geo_peers.len() as u16); + let peer_count = check_max( + "GEO_PEER_TABLE peer count", + self.geo_peers.len(), + u16::MAX as usize, + )?; + buf.put_u16(peer_count as u16); // Encode each peer entry for geo_peer in &self.geo_peers { @@ -188,7 +194,14 @@ impl GeoPeerTable { buf.put_f32(geo_peer.peer_longitude); } - buf.freeze() + Ok(()) + } + + /// Convenience: encode into a fresh buffer. + pub fn encode(&self) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(&mut buf)?; + Ok(buf.freeze()) } } @@ -344,7 +357,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 +436,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 bd3a84a..3afbc66 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,3 +1,5 @@ +use crate::encoder::sink::{check_max, with_u16_len}; +use crate::error::EncodingError; use crate::models::{Afi, AsnLength, Peer, PeerIndexTable, PeerType}; use crate::parser::ReadUtils; use crate::ParserError; @@ -65,15 +67,24 @@ pub fn parse_peer_index_table(data: &mut Bytes) -> Result u16 { + /// Add peer to peer index table and return peer id. + /// + /// Errors with [`EncodingError::ValueTooLarge`] when the table already has + /// 65536 peers — the u16 peer index cannot address a 65537th. + 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 peer_id = u16::try_from(self.peer_ip_id_map.len()).map_err(|_| { + EncodingError::too_large( + "PeerIndexTable peer count", + self.peer_ip_id_map.len() + 1, + u16::MAX as usize + 1, + ) + })?; self.peer_ip_id_map.insert(peer.peer_ip, peer_id); self.id_peer_map.insert(peer_id, peer); - peer_id + Ok(peer_id) } } } @@ -138,22 +149,24 @@ impl PeerIndexTable { /// /// let encoded = data.encode(); /// ``` - pub fn encode(&self) -> Bytes { - let mut buf = BytesMut::new(); - + /// Append the wire representation of this peer index table to `buf`. + pub fn encode_to(&self, buf: &mut BytesMut) -> Result<(), EncodingError> { // 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 with back-patched length + with_u16_len(buf, "PeerIndexTable view name length", |b| { + b.extend_from_slice(self.view_name.as_bytes()); + Ok(()) + })?; // Encode peer_count - let peer_count = self.id_peer_map.len() as u16; - buf.put_u16(peer_count); + let peer_count = check_max( + "PeerIndexTable peer count", + self.id_peer_map.len(), + u16::MAX as usize, + )?; + buf.put_u16(peer_count as u16); // Encode peers let mut peer_ids: Vec<_> = self.id_peer_map.keys().collect(); @@ -183,8 +196,14 @@ impl PeerIndexTable { }; } - // Return Bytes - buf.freeze() + Ok(()) + } + + /// Convenience: encode into a fresh buffer. + pub fn encode(&self) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(&mut buf)?; + Ok(buf.freeze()) } } @@ -203,22 +222,64 @@ 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(); + 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); } + #[test] + fn test_add_peer_overflow_at_65537() { + // 65536 peers fit (ids 0..=65535); the 65537th must error, and the + // table must remain uncorrupted afterwards. + let mut table = PeerIndexTable::default(); + for i in 0..65536u32 { + let peer = Peer::new( + Ipv4Addr::from(1), + IpAddr::V4(Ipv4Addr::from(i)), + Asn::new_32bit(i), + ); + let id = table.add_peer(peer).unwrap(); + assert_eq!(id as u32, i); + } + let overflow_peer = Peer::new( + Ipv4Addr::from(1), + IpAddr::V4(Ipv4Addr::from(65536)), + Asn::new_32bit(65536), + ); + let err = table.add_peer(overflow_peer).unwrap_err(); + assert!(matches!( + err, + crate::error::EncodingError::ValueTooLarge { .. } + )); + // Table uncorrupted: the original peer at id 65535 still resolves correctly + let p65535 = table.get_peer_by_id(&65535).unwrap(); + assert_eq!(p65535.peer_asn.to_u32(), 65535); + // Adding the same overflow peer again still errors (no partial insert) + assert!(table.add_peer(overflow_peer).is_err()); + // Re-adding an existing peer still returns its id + let existing = Peer::new( + Ipv4Addr::from(1), + IpAddr::V4(Ipv4Addr::from(42)), + Asn::new_32bit(42), + ); + assert_eq!(table.add_peer(existing).unwrap(), 42); + } + #[test] fn test_get_peer_by_id() { let mut index_table = PeerIndexTable { @@ -239,8 +300,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), 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 02a354f..eaa3848 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,4 +1,6 @@ use crate::bgp::attributes::parse_attributes; +use crate::encoder::sink::{check_max, with_u16_len}; +use crate::error::EncodingError; use crate::models::{ Afi, AsnLength, NetworkPrefix, RibAfiEntries, RibEntry, Safi, TableDumpV2Type, }; @@ -164,42 +166,63 @@ pub fn parse_rib_entry( } impl RibAfiEntries { - pub fn encode(&self) -> Bytes { - let mut bytes = BytesMut::new(); + /// Append the wire representation of these RIB entries to `buf`. + pub fn encode_to(&self, buf: &mut BytesMut) -> Result<(), EncodingError> { let is_add_path = is_add_path_rib_type(self.rib_type); - bytes.put_u32(self.sequence_number); - bytes.extend(self.prefix.encode()); + buf.put_u32(self.sequence_number); + buf.extend_from_slice(&self.prefix.encode()); - let entry_count = self.rib_entries.len(); - bytes.put_u16(entry_count as u16); + let entry_count = check_max( + "RIB AFI entry count", + self.rib_entries.len(), + u16::MAX as usize, + )?; + buf.put_u16(entry_count as u16); for entry in &self.rib_entries { - bytes.extend(entry.encode_for_rib_type(is_add_path)); + entry.encode_to_for_rib_type(is_add_path, buf)?; } - bytes.freeze() + Ok(()) + } + + /// Convenience: encode into a fresh buffer. + pub fn encode(&self) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(&mut buf)?; + Ok(buf.freeze()) } } impl RibEntry { - pub fn encode(&self) -> Bytes { - self.encode_for_rib_type(self.path_id.is_some()) + /// Append the wire representation of this RIB entry to `buf`. + pub fn encode_to(&self, buf: &mut BytesMut) -> Result<(), EncodingError> { + self.encode_to_for_rib_type(self.path_id.is_some(), buf) + } + + /// Convenience: encode into a fresh buffer. + pub fn encode(&self) -> Result { + let mut buf = BytesMut::new(); + self.encode_to(&mut buf)?; + Ok(buf.freeze()) } - fn encode_for_rib_type(&self, include_path_id: bool) -> Bytes { - let mut bytes = BytesMut::new(); - bytes.put_u16(self.peer_index); - bytes.put_u32(self.originated_time); + fn encode_to_for_rib_type( + &self, + include_path_id: bool, + buf: &mut BytesMut, + ) -> Result<(), EncodingError> { + buf.put_u16(self.peer_index); + buf.put_u32(self.originated_time); if include_path_id { if let Some(path_id) = self.path_id { - bytes.put_u32(path_id); + buf.put_u32(path_id); } } - let attr_bytes = self.attributes.encode(AsnLength::Bits32); - bytes.put_u16(attr_bytes.len() as u16); - bytes.extend(attr_bytes); - bytes.freeze() + with_u16_len(buf, "RIB AFI entry attribute length", |b| { + self.attributes.encode_to(AsnLength::Bits32, b) + }) } } @@ -270,7 +293,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); @@ -297,7 +320,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); @@ -311,4 +334,73 @@ mod tests { rib.rib_entries[0].attributes.inner ); } + + /// Regression test for the `bytes.extend(Result)` silent-drop bug found in + /// review (issue #1): an oversized entry inside RibAfiEntries must surface + /// as an `Err` from `encode`, never as a truncated record. + #[test] + fn test_rib_afi_entries_oversized_entry_errors() { + use crate::models::{AttrFlags, AttrRaw, Attribute, AttributeValue, Attributes}; + + // 70 Raw attributes × ~1000 bytes = >65535 total attribute bytes + let attrs: Vec = (0..70) + .map(|_| Attribute { + flag: AttrFlags::OPTIONAL | AttrFlags::PARTIAL, + value: AttributeValue::Raw(AttrRaw { + code: 200, + bytes: vec![0; 1000].into(), + }), + }) + .collect(); + + let rib = RibAfiEntries { + rib_type: TableDumpV2Type::RibIpv4Unicast, + sequence_number: 1, + prefix: NetworkPrefix::from_str("10.0.0.0/24").unwrap(), + rib_entries: vec![RibEntry { + peer_index: 1, + originated_time: 0, + path_id: None, + attributes: Attributes::from(attrs), + }], + }; + + let result = rib.encode(); + assert!( + result.is_err(), + "oversized entry must return Err, not a truncated record" + ); + match result.unwrap_err() { + crate::error::EncodingError::ValueTooLarge { field, .. } => { + assert!(field.contains("attribute"), "field: {field}"); + } + other => panic!("expected ValueTooLarge, got {other:?}"), + } + } + + /// Regression: entry count beyond u16 must error instead of wrapping. + #[test] + fn test_rib_afi_entries_count_overflow() { + use crate::models::{AttributeValue, Attributes, Origin}; + + let mut attributes = Attributes::default(); + attributes.add_attr(AttributeValue::Origin(Origin::IGP).into()); + + let rib = RibAfiEntries { + rib_type: TableDumpV2Type::RibIpv4Unicast, + sequence_number: 1, + prefix: NetworkPrefix::from_str("10.0.0.0/24").unwrap(), + rib_entries: vec![ + RibEntry { + peer_index: 1, + originated_time: 0, + path_id: None, + attributes, + }; + 65536 + ], + }; + + assert!(rib.encode().is_err()); + } } diff --git a/src/parser/mrt/mrt_record.rs b/src/parser/mrt/mrt_record.rs index b14ad76..efc44df 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::{EncodingError, ParserError}; use crate::models::*; use crate::parser::{ parse_bgp4mp, parse_table_dump_message, parse_table_dump_v2_message, ParserErrorWithBytes, @@ -227,8 +227,11 @@ pub fn parse_mrt_body( } impl MrtRecord { - pub fn encode(&self) -> Bytes { - let message_bytes = self.message.encode(self.common_header.entry_subtype); + /// Encode the full MRT record (header + body). + /// + /// Returns [`EncodingError`] if the message body fails to encode. + 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!( @@ -240,20 +243,10 @@ impl MrtRecord { new_header.length = message_bytes.len() as u32; let header_bytes = new_header.encode(); - // // debug begins - // let parsed_body = parse_mrt_body( - // self.common_header.entry_type as u16, - // self.common_header.entry_subtype, - // message_bytes.clone(), - // ) - // .unwrap(); - // assert!(self.message == parsed_body); - // // debug ends - 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 +287,16 @@ impl TryFrom<&BmpMessage> for MrtRecord { let (seconds, microseconds) = convert_timestamp(bmp_header.timestamp); let subtype = Bgp4MpType::MessageAs4 as u16; + let body_len = mrt_message + .encode(subtype) + .map_err(|e| format!("failed to encode BGP4MP message: {e}"))? + .len() as u32; 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: body_len, }; Ok(MrtRecord { @@ -501,12 +498,13 @@ 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) + .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 7f4ceb3..12d121e 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);