From 48da2c5783bb46e445e1b18ede22684965031211 Mon Sep 17 00:00:00 2001 From: Alan Egerton Date: Tue, 20 Jan 2026 15:32:33 +0000 Subject: [PATCH 1/4] Add length estimation for ext formats (cherry picked from commit c2dcea97ed63c5b8b6b46f50d74ee7658dd62eda) --- rmp/src/decode/est.rs | 16 ++++++++-------- rmp/tests/func/est.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/rmp/src/decode/est.rs b/rmp/src/decode/est.rs index 8782105e..dcf3f294 100644 --- a/rmp/src/decode/est.rs +++ b/rmp/src/decode/est.rs @@ -161,10 +161,10 @@ impl MessageLen { Marker::Array16 | Marker::Array32 | Marker::Map16 | - Marker::Map32 => self.read_marker_with_len(data, MarkerLen { marker, buf: [0; 4], has: 0 }), + Marker::Map32 | Marker::Ext8 | Marker::Ext16 | - Marker::Ext32 => todo!(), + Marker::Ext32 => self.read_marker_with_len(data, MarkerLen { marker, buf: [0; 4], has: 0 }), Marker::F32 => self.skip_data(data, 4), Marker::F64 => self.skip_data(data, 8), Marker::U8 => self.skip_data(data, 1), @@ -175,11 +175,11 @@ impl MessageLen { Marker::I16 => self.skip_data(data, 2), Marker::I32 => self.skip_data(data, 4), Marker::I64 => self.skip_data(data, 8), - Marker::FixExt1 | - Marker::FixExt2 | - Marker::FixExt4 | - Marker::FixExt8 | - Marker::FixExt16 => todo!(), + Marker::FixExt1 => self.skip_data(data, 2), + Marker::FixExt2 => self.skip_data(data, 3), + Marker::FixExt4 => self.skip_data(data, 5), + Marker::FixExt8 => self.skip_data(data, 9), + Marker::FixExt16 => self.skip_data(data, 17), Marker::FixNeg(_) => Some(()), } } @@ -217,7 +217,7 @@ impl MessageLen { Marker::Str32 => self.skip_data(data, len), Marker::Ext8 | Marker::Ext16 | - Marker::Ext32 => todo!(), + Marker::Ext32 => self.skip_data(data, len + 1), Marker::Array16 | Marker::Array32 => self.read_sequence(data, len), Marker::Map16 | diff --git a/rmp/tests/func/est.rs b/rmp/tests/func/est.rs index d5d484bc..a701e55c 100644 --- a/rmp/tests/func/est.rs +++ b/rmp/tests/func/est.rs @@ -115,3 +115,37 @@ fn nested() { assert!(MessageLen::with_limits(4, 1 << 16).incremental_len(out.as_slice()).is_err()); assert!(MessageLen::with_limits(14, 1 << 16).incremental_len(out.as_slice()).is_ok()); } + +#[test] +fn extensions() { + let mut out = Vec::with_capacity(263); + + let mut expected = Vec::with_capacity(264); + expected.push(1); + + for len in (0i32..=17).chain([256,257]) { + const TOO_BIG_FOR_U8: i32 = u8::MAX as i32 + 1; + const TOO_BIG_FOR_U16: i32 = u16::MAX as i32 + 1; + + let length_bytes = match len { + 1 | 2 | 4 | 8 | 16 => 0, + ..TOO_BIG_FOR_U8 => 1, + TOO_BIG_FOR_U8..TOO_BIG_FOR_U16 => 2, + _ => 4, + }; + + let len_with_prefix = |len| 1 + length_bytes + 1 + len; + let msg_len = len_with_prefix(len); + + expected.truncate(1); + expected.resize(1 + length_bytes as usize, 1 + length_bytes); + expected.resize(msg_len as usize, msg_len); + expected.push(-msg_len); + + out.clear(); + write_ext_meta(&mut out, len as u32, 0x67).unwrap(); + out.resize(out.len() + len as usize, 0xab); + + check_estimates(&out, &expected); + } +} From 306ac70403270dbbba13e194c7ff8518edb96eaa Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <4995967+baszalmstra@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:42:41 +0200 Subject: [PATCH 2/4] Test ext32 and ext nested in arrays/maps in MessageLen Extends the ext coverage from the previous commit with the ext32 format (both a small non-canonical length checked for every prefix, and a canonical 70k payload), the u8/u16 boundary lengths, and ext values inside arrays and maps so that resumption of an interrupted ext inside a sequence is exercised too. Claude-Session: https://claude.ai/code/session_011NAQi5b2wGJsRjiEcCHjZ8 --- rmp/tests/func/est.rs | 55 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/rmp/tests/func/est.rs b/rmp/tests/func/est.rs index a701e55c..20c2d2a1 100644 --- a/rmp/tests/func/est.rs +++ b/rmp/tests/func/est.rs @@ -123,7 +123,7 @@ fn extensions() { let mut expected = Vec::with_capacity(264); expected.push(1); - for len in (0i32..=17).chain([256,257]) { + for len in (0i32..=17).chain([255, 256, 257, 65535]) { const TOO_BIG_FOR_U8: i32 = u8::MAX as i32 + 1; const TOO_BIG_FOR_U16: i32 = u16::MAX as i32 + 1; @@ -149,3 +149,56 @@ fn extensions() { check_estimates(&out, &expected); } } + +#[test] +fn ext32() { + // Non-canonical but valid: ext32 marker with a small length, so the estimates can be + // checked exhaustively for every prefix. + let mut out = vec![Marker::Ext32.to_u8()]; + out.extend_from_slice(&3u32.to_be_bytes()); + out.push(7); + out.extend_from_slice(&[0xEE; 3]); + // marker + 4 len bytes + type byte + data + check_estimates(&out, &[1, 5, 5, 5, 5, 9, 9, 9, 9, -9]); + + // Canonical ext32 as produced by the encoder. + let len = 70_000u32; + let mut out = Vec::new(); + assert_eq!(Marker::Ext32, write_ext_meta(&mut out, len, 7).unwrap()); + out.extend(std::iter::repeat_n(0xEE, len as usize)); + assert_eq!(out.len(), MessageLen::len_of(&out).unwrap()); + assert_eq!(out.len(), MessageLen::len_of(&out[..out.len() - 1]).unwrap_err().len()); +} + +#[test] +fn ext_in_array() { + let mut out = Vec::new(); + write_array_len(&mut out, 3).unwrap(); + write_ext_meta(&mut out, 1, 7).unwrap(); // fixext1 + out.push(0xEE); + write_ext_meta(&mut out, 3, 7).unwrap(); // ext8 + out.extend_from_slice(&[0xEE; 3]); + write_nil(&mut out).unwrap(); + assert_eq!(11, out.len()); + + check_estimates(&out, &[1, 4, 4, 4, 6, 6, 10, 10, 10, 10, 11, -11]); +} + +#[test] +fn ext_in_map() { + let mut out = Vec::new(); + write_map_len(&mut out, 1).unwrap(); + write_ext_meta(&mut out, 8, 1).unwrap(); // fixext8 key + out.extend_from_slice(&[0xEE; 8]); + write_ext_meta(&mut out, 300, 2).unwrap(); // ext16 value + out.extend_from_slice(&[0xEE; 300]); + // 1 + (2 + 8) + (4 + 300) + assert_eq!(315, out.len()); + + let mut expected = vec![1, 3]; + expected.extend(std::iter::repeat_n(11, 9)); // key: type byte + 8 data bytes + expected.extend([12, 14, 14]); // value: marker, then 2 length bytes + expected.resize(315, 315); + expected.push(-315); + check_estimates(&out, &expected); +} From 4792295f8fda909116a8048ee696705b1a960433 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <4995967+baszalmstra@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:44:05 +0200 Subject: [PATCH 3/4] Report MessageLen limit violations and reserved marker as ParseError Exceeding the depth or length limit set the LimitExceeded state, but the call that hit it still reported `Truncated(lower_bound)`; only the next call returned `ParseError`. A caller using `len_of` on a complete message therefore saw "needs more data" for an over-limit message, and a streaming caller had to make one extra round trip to learn the message was unparseable. `incremental_len` now reports `ParseError` as soon as a sticky state is entered. A second sticky state, `Reserved`, is added for the reserved marker 0xc1, which every decoder in this repository rejects but MessageLen used to count as a one-byte value. Claude-Session: https://claude.ai/code/session_011NAQi5b2wGJsRjiEcCHjZ8 --- rmp/src/decode/est.rs | 31 +++++++++++++------ rmp/tests/func/est.rs | 70 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 9 deletions(-) diff --git a/rmp/src/decode/est.rs b/rmp/src/decode/est.rs index dcf3f294..30105d69 100644 --- a/rmp/src/decode/est.rs +++ b/rmp/src/decode/est.rs @@ -82,7 +82,8 @@ impl MessageLen { /// and it would need *at least* this many bytes to parse. /// The `len` is always the lower bound, and never exceeds actual message length. /// - /// `Err(LenError::ParseError)` — the end of the message is unknown. + /// `Err(LenError::ParseError)` — the message is malformed or exceeds the limits, so the end + /// of the message cannot be determined. /// /// Don't call this function in a loop. Use [`MessageLen::incremental_len`] instead. pub fn len_of(complete_message: &[u8]) -> Result { @@ -104,8 +105,9 @@ impl MessageLen { /// The `len` is always the lower bound, and never exceeds actual message length, /// so it's safe to read the additional bytes without overshooting the end of the message. /// - /// * `Err(LenError::ParseError)` — the end of the message cannot be determined, and this - /// is a non-recoverable error. Any further calls to this function may return nonsense. + /// * `Err(LenError::ParseError)` — the message is malformed or exceeds the limits, so the end + /// of the message cannot be determined. This is a non-recoverable error: any further calls + /// to this function will keep returning it until [`MessageLen::reset`] is called. pub fn incremental_len(&mut self, mut next_message_fragment: &[u8]) -> Result { let data = &mut next_message_fragment; let Some(wip) = self.wip.take() else { @@ -115,16 +117,16 @@ impl MessageLen { WIP::Data(Data { bytes_left }) => self.skip_data(data, bytes_left.get()), WIP::MarkerLen(wip) => self.read_marker_with_len(data, wip), WIP::NextMarker => self.read_one_item(data), - WIP::LimitExceeded => { - self.wip = Some(WIP::LimitExceeded); // put it back! + kind @ (WIP::LimitExceeded | WIP::Reserved) => { + self.wip = Some(kind); // put it back! return Err(LenError::ParseError); }, - }.ok_or(LenError::Truncated(self.max_position))?; + }.ok_or_else(|| self.interrupted())?; while let Some(seq) = self.sequences_wip.pop() { self.current_depth = seq.depth; debug_assert!(self.wip.is_none()); - self.read_sequence(data, seq.items_left.get() - 1).ok_or(LenError::Truncated(self.max_position))?; + self.read_sequence(data, seq.items_left.get() - 1).ok_or_else(|| self.interrupted())?; } debug_assert!(self.wip.is_none()); debug_assert!(self.max_position.get() <= self.position); @@ -149,9 +151,10 @@ impl MessageLen { Marker::FixArray(len) => self.read_sequence(data, u32::from(len)), Marker::FixStr(len) => self.skip_data(data, len.into()), Marker::Null | - Marker::Reserved | Marker::False | Marker::True => Some(()), + // 0xc1 is not a valid marker + Marker::Reserved => self.fail(WIP::Reserved), Marker::Str8 | Marker::Str16 | Marker::Str32 | @@ -297,11 +300,19 @@ impl MessageLen { WIP::NextMarker => self.position + 1, WIP::Data(Data { bytes_left }) => self.position + bytes_left.get() as usize, WIP::MarkerLen(m) => self.position + (m.size() - m.has) as usize, - WIP::LimitExceeded => 0, + WIP::LimitExceeded | WIP::Reserved => 0, }; self.set_max_position(pos); None } + + /// The error to report when an operation was interrupted (returned `None`) + fn interrupted(&self) -> LenError { + match self.wip { + Some(WIP::LimitExceeded | WIP::Reserved) => LenError::ParseError, + _ => LenError::Truncated(self.max_position), + } + } } enum WIP { @@ -309,6 +320,8 @@ enum WIP { Data(Data), MarkerLen(MarkerLen), LimitExceeded, + /// The message contains the reserved marker `0xc1`. Sticky, like `LimitExceeded`. + Reserved, } struct Seq { items_left: NonZeroU32, depth: u16 } diff --git a/rmp/tests/func/est.rs b/rmp/tests/func/est.rs index 20c2d2a1..c0c3382e 100644 --- a/rmp/tests/func/est.rs +++ b/rmp/tests/func/est.rs @@ -202,3 +202,73 @@ fn ext_in_map() { expected.push(-315); check_estimates(&out, &expected); } + +#[test] +fn limit_exceeded_is_parse_error() { + // Nesting limit, all data available in the first call. + let mut out = Vec::new(); + for _ in 0..5 { + write_array_len(&mut out, 1).unwrap(); + } + write_nil(&mut out).unwrap(); + let mut est = MessageLen::with_limits(4, 1 << 16); + assert!(matches!(est.incremental_len(&out), Err(LenError::ParseError))); + // The error is sticky. + assert!(matches!(est.incremental_len(&out), Err(LenError::ParseError))); + assert_eq!(6, MessageLen::with_limits(5, 1 << 16).incremental_len(&out).unwrap()); + + // Nesting limit hit while resuming from a truncated message. + let mut est = MessageLen::with_limits(2, 1 << 16); + assert_eq!(2, est.incremental_len(&[0x91]).unwrap_err().len()); + assert!(matches!(est.incremental_len(&[0x91, 0x91, 0xc0]), Err(LenError::ParseError))); + + // `len_of` has a fixed nesting limit of 1024. + let mut out = Vec::new(); + for _ in 0..1025 { + write_array_len(&mut out, 1).unwrap(); + } + write_nil(&mut out).unwrap(); + assert!(matches!(MessageLen::len_of(&out), Err(LenError::ParseError))); + + // Length limit is exclusive, for strings... + let mut out = Vec::new(); + write_str_len(&mut out, 40).unwrap(); // str8 + out.extend_from_slice(&[b'x'; 40]); + assert!(matches!(MessageLen::with_limits(1024, 40).incremental_len(&out), Err(LenError::ParseError))); + assert_eq!(42, MessageLen::with_limits(1024, 41).incremental_len(&out).unwrap()); + + // ...arrays (counted in items)... + let mut out = Vec::new(); + write_array_len(&mut out, 40).unwrap(); // array16 + out.extend(std::iter::repeat_n(0xc0, 40)); + assert!(matches!(MessageLen::with_limits(1024, 40).incremental_len(&out), Err(LenError::ParseError))); + assert_eq!(43, MessageLen::with_limits(1024, 41).incremental_len(&out).unwrap()); + + // ...maps (counted in keys + values)... + let mut out = Vec::new(); + write_map_len(&mut out, 20).unwrap(); // map16 + out.extend(std::iter::repeat_n(0xc0, 40)); + assert!(matches!(MessageLen::with_limits(1024, 40).incremental_len(&out), Err(LenError::ParseError))); + assert_eq!(43, MessageLen::with_limits(1024, 41).incremental_len(&out).unwrap()); + + // ...and ext payloads. + let mut out = Vec::new(); + write_ext_meta(&mut out, 40, 7).unwrap(); // ext8 + out.extend_from_slice(&[0xEE; 40]); + assert!(matches!(MessageLen::with_limits(1024, 40).incremental_len(&out), Err(LenError::ParseError))); + assert_eq!(43, MessageLen::with_limits(1024, 41).incremental_len(&out).unwrap()); +} + +#[test] +fn reserved_marker_is_parse_error() { + assert!(matches!(MessageLen::len_of(&[0xc1]), Err(LenError::ParseError))); + assert!(matches!(MessageLen::len_of(&[0x92, 0xc0, 0xc1]), Err(LenError::ParseError))); + // Truncation before the reserved byte is still just truncation. + assert_eq!(3, MessageLen::len_of(&[0x92, 0xc0]).unwrap_err().len()); + + // The error is sticky. + let mut est = MessageLen::new(); + assert_eq!(3, est.incremental_len(&[0x92, 0xc0]).unwrap_err().len()); + assert!(matches!(est.incremental_len(&[0xc1]), Err(LenError::ParseError))); + assert!(matches!(est.incremental_len(&[0xc0]), Err(LenError::ParseError))); +} From 698e2b6c32b55ab9cb676dd326df2dcdbd7b0dfa Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <4995967+baszalmstra@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:45:18 +0200 Subject: [PATCH 4/4] Tighten the MessageLen lower bound for ext formats While the length prefix of an ext8/16/32 value is still being read, the type byte is already known to follow it, so include it in the estimate. Also documents that `max_len` applies to ext payloads and why `len + 1` cannot overflow. Claude-Session: https://claude.ai/code/session_011NAQi5b2wGJsRjiEcCHjZ8 --- rmp/src/decode/est.rs | 18 +++++++++++++++--- rmp/tests/func/est.rs | 9 +++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/rmp/src/decode/est.rs b/rmp/src/decode/est.rs index 30105d69..685c9863 100644 --- a/rmp/src/decode/est.rs +++ b/rmp/src/decode/est.rs @@ -54,7 +54,7 @@ impl MessageLen { /// * `max_depth` limits nesting of arrays and maps /// - /// * `max_len` is maximum size of any string, byte string, map, or array. + /// * `max_len` is maximum size of any string, byte string, ext payload, map, or array. /// For maps and arrays this is the number of items, not bytes. /// /// Messages can be both deep and wide, being `max_depth` * `max_len` in size. @@ -220,7 +220,10 @@ impl MessageLen { Marker::Str32 => self.skip_data(data, len), Marker::Ext8 | Marker::Ext16 | - Marker::Ext32 => self.skip_data(data, len + 1), + Marker::Ext32 => { + // type byte + payload; `len < max_len <= u32::MAX`, so this can't overflow + self.skip_data(data, len + 1) + }, Marker::Array16 | Marker::Array32 => self.read_sequence(data, len), Marker::Map16 | @@ -299,7 +302,7 @@ impl MessageLen { let pos = match self.wip.insert(wip) { WIP::NextMarker => self.position + 1, WIP::Data(Data { bytes_left }) => self.position + bytes_left.get() as usize, - WIP::MarkerLen(m) => self.position + (m.size() - m.has) as usize, + WIP::MarkerLen(m) => self.position + (m.size() - m.has) as usize + m.min_trailing(), WIP::LimitExceeded | WIP::Reserved => 0, }; self.set_max_position(pos); @@ -347,4 +350,13 @@ impl MarkerLen { _ => unimplemented!(), } } + + /// Number of bytes that must follow the length, regardless of its value + fn min_trailing(&self) -> usize { + match self.marker { + // the type byte + Marker::Ext8 | Marker::Ext16 | Marker::Ext32 => 1, + _ => 0, + } + } } diff --git a/rmp/tests/func/est.rs b/rmp/tests/func/est.rs index c0c3382e..3faf19d3 100644 --- a/rmp/tests/func/est.rs +++ b/rmp/tests/func/est.rs @@ -138,7 +138,8 @@ fn extensions() { let msg_len = len_with_prefix(len); expected.truncate(1); - expected.resize(1 + length_bytes as usize, 1 + length_bytes); + // while the length is being read, the type byte is already known to follow it + expected.resize(1 + length_bytes as usize, 2 + length_bytes); expected.resize(msg_len as usize, msg_len); expected.push(-msg_len); @@ -159,7 +160,7 @@ fn ext32() { out.push(7); out.extend_from_slice(&[0xEE; 3]); // marker + 4 len bytes + type byte + data - check_estimates(&out, &[1, 5, 5, 5, 5, 9, 9, 9, 9, -9]); + check_estimates(&out, &[1, 6, 6, 6, 6, 9, 9, 9, 9, -9]); // Canonical ext32 as produced by the encoder. let len = 70_000u32; @@ -181,7 +182,7 @@ fn ext_in_array() { write_nil(&mut out).unwrap(); assert_eq!(11, out.len()); - check_estimates(&out, &[1, 4, 4, 4, 6, 6, 10, 10, 10, 10, 11, -11]); + check_estimates(&out, &[1, 4, 4, 4, 6, 7, 10, 10, 10, 10, 11, -11]); } #[test] @@ -197,7 +198,7 @@ fn ext_in_map() { let mut expected = vec![1, 3]; expected.extend(std::iter::repeat_n(11, 9)); // key: type byte + 8 data bytes - expected.extend([12, 14, 14]); // value: marker, then 2 length bytes + expected.extend([12, 15, 15]); // value: marker, then 2 length bytes + type byte expected.resize(315, 315); expected.push(-315); check_estimates(&out, &expected);