From 00d0256bea147193cfd03ac8b42c2162fa33fa80 Mon Sep 17 00:00:00 2001 From: userFRM Date: Thu, 30 Jul 2026 11:51:40 +0200 Subject: [PATCH] tick decode: step over a tick too wide to read, rather than dropping the message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An extended entry states its width in a full byte, so it can name a value wider than this decoder returns. That entry is lost either way — but the decoder abandoned the whole message at that point, so every tick after it went with it, including the other server tags in the same 35=P. A quote update sitting behind one simply never arrived, and nothing recorded that it had. The field is now stepped over and decoding continues. The entry itself is still dropped rather than guessed at, and a genuinely truncated message still ends where it did. Stepping over needs a `skip` on the bit reader. Its width comes off the wire, so it is bounded by what is actually left rather than by an addition that wraps in release and rewinds the reader instead of stopping it. It is private: only this decoder steps. The test payload builder gained the ability to emit a field wider than a `u64`, which is what building one of these requires. Closes #302. --- src/protocol/tick_decoder.rs | 84 +++++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 2 deletions(-) diff --git a/src/protocol/tick_decoder.rs b/src/protocol/tick_decoder.rs index e3a3f012..e0db3c7e 100644 --- a/src/protocol/tick_decoder.rs +++ b/src/protocol/tick_decoder.rs @@ -30,6 +30,20 @@ impl<'a> BitReader<'a> { self.total_bits.saturating_sub(self.bit_pos) } + /// Advance past `n` bits without decoding them. Used to step over a field + /// this decoder cannot represent, so the rest of the message still decodes. + /// + /// Compares against `remaining`, which saturates, so a width read off the + /// wire cannot overflow the position. + #[inline] + fn skip(&mut self, n: usize) -> bool { + if n > self.remaining() { + return false; + } + self.bit_pos += n; + true + } + /// Read n bits as unsigned integer (MSB first). /// Uses word-aligned reads for performance (1-3 ops instead of n iterations). #[inline] @@ -254,6 +268,19 @@ pub fn decode_ticks_35p_into(body: &[u8], ticks: &mut Vec) { return; } + // An extended entry states its width in a full byte, so it can name + // a value wider than this decoder reads. That entry is lost either + // way; abandoning the message threw away every tick after it as + // well, including the other server tags in the same 35=P (ibx#302). + // Stepping over it keeps the rest. + if total_value_bits > 64 { + log::debug!("35=P: skipping a {}-byte tick value", byte_width); + if !reader.skip(total_value_bits) { + return; + } + continue; + } + let sign = match reader.read_unsigned(1) { Some(v) => v, None => return, @@ -659,9 +686,14 @@ mod tests { Self { bits: Vec::new() } } - /// Push `n` bits from the MSB side of `val`. + /// Push `n` bits from the MSB side of `val`. Widths above 64 are + /// zero-filled on the left, so a field wider than a `u64` can be built + /// — which is the point of the extended entries this exercises. fn push(&mut self, val: u64, n: usize) { - for i in (0..n).rev() { + for _ in 64..n { + self.bits.push(0); + } + for i in (0..n.min(64)).rev() { self.bits.push(((val >> i) & 1) as u8); } } @@ -726,6 +758,54 @@ mod tests { } } + /// The width comes off the wire, so the step has to be bounded by what is + /// actually left rather than trusted. An unchecked addition wraps in + /// release and rewinds the reader instead of stopping it. + #[test] + fn skipping_past_the_end_refuses_rather_than_wrapping() { + let mut reader = BitReader::new(&[0xFF, 0xFF], 16); + assert!(reader.read_unsigned(1).is_some()); + assert!(!reader.skip(usize::MAX), "an absurd width is refused"); + assert!(!reader.skip(16), "and so is one just past the end"); + assert_eq!(reader.remaining(), 15, "the position is untouched by either"); + assert!(reader.skip(15), "while a width that fits still advances"); + assert_eq!(reader.remaining(), 0); + } + + /// ibx#302: an extended entry states its width in a full byte, so it can + /// name a value wider than this decoder reads. That entry is lost either + /// way — but abandoning the message threw away every tick after it too, + /// including the other server tags in the same 35=P. A quote update sitting + /// behind one simply never arrived. + #[test] + fn a_tick_too_wide_to_read_does_not_discard_the_rest_of_the_message() { + let mut b = PayloadBuilder::new(); + b.server_tag(1, 100); + // Nine bytes: 72 value bits, which the reader cannot return. + b.tick_extended(1, 40, 9, 7, false); + b.tick(2, 0, 4, 12_345, false); + b.server_tag(0, 200); + b.tick(3, 0, 4, 678, false); + + let ticks = decode_ticks_35p(&b.build()); + + let seen: Vec<(u32, u64, i64)> = ticks.iter() + .map(|t| (t.server_tag, t.tick_type, t.magnitude)) + .collect(); + assert!( + seen.contains(&(100, 2, 12_345)), + "the tick after the wide one, under the same server tag: {seen:?}", + ); + assert!( + seen.contains(&(200, 3, 678)), + "and the whole server tag after it: {seen:?}", + ); + assert!( + !seen.iter().any(|(_, ty, _)| *ty == 40), + "the entry itself is still dropped, not guessed at: {seen:?}", + ); + } + // ── decode_ticks_35p tests ────────────────────────────────────────── #[test]