Merge pull request 'Fix reencoding of demos recorded on older protocols' (#10) from pet0r/demostf-parser:fix-reencode-protocol-version into master

Reviewed-on: https://codeberg.org/demostf/parser/pulls/10
Reviewed-by: Robin Appelman <icewind@noreply.codeberg.org>

src/bin/reencode.rs

@@ -45,8 +45,15 @@ fn main() -> Result<(), MainError> {
         }
         header.write(&mut out_stream)?;
 
+        // ParserState defaults to protocol 24 (DemoHandler::default ->
+        // ParserState::new(24, ..)); without this the demo's own protocol is
+        // never applied and older demos are parsed with the wrong field widths.
+        // DemoParser::ticker does this for us, but this binary drives the
+        // DemoHandler directly.
         let mut handler = DemoHandler::default();
+        handler.handle_header(&header);
         let mut encode_handler = DemoHandler::default();
+        encode_handler.handle_header(&header);
 
         let mut has_stop = false;
         let mut last_tick = DemoTick::default();

src/demo/message/stringtable.rs

@@ -164,7 +164,7 @@ impl Encode for CreateStringTableMessage<'_> {
     fn encode(
         &self,
         stream: &mut BitWriteStream<LittleEndian>,
-        _state: &ParserState,
+        state: &ParserState,
     ) -> Result<()> {
         let table = &self.table;
         table.name.write(stream)?;
@@ -172,7 +172,14 @@ impl Encode for CreateStringTableMessage<'_> {
         let encode_bits = log_base2(table.max_entries) as usize;
         (table.entries.len() as u16).write_sized(stream, encode_bits + 1)?;
 
-        stream.reserve_int::<ParseError, _>(40, |stream| {
+        // threshold and both widths are the ones used by the read side above
+        let (length_bits, encode_length): (usize, fn(u32) -> u64) =
+            if state.protocol_version > 23 {
+                (40, encode_var_int_fixed)
+            } else {
+                (20, |length| length as u64)
+            };
+        stream.reserve_int::<ParseError, _>(length_bits, |stream| {
             table.fixed_user_data_size.is_some().write(stream)?;
             if let Some(fixed_size) = table.fixed_user_data_size {
                 fixed_size.write(stream)?;
@@ -188,7 +195,7 @@ impl Encode for CreateStringTableMessage<'_> {
             write_string_table_update(&table.entries, stream, &table_meta)?;
 
             let end = stream.bit_len();
-            Ok(encode_var_int_fixed((end - start) as u32))
+            Ok(encode_length((end - start) as u32))
         })?;
 
         Ok(())

src/demo/message/tempentities.rs

@@ -109,7 +109,14 @@ impl Encode for TempEntitiesMessage {
         };
         count.write(stream)?;
 
-        stream.reserve_int::<ParseError, _>(40, |stream| {
+        // threshold and both widths are the ones used by the read side above
+        let (length_bits, encode_length): (usize, fn(u32) -> u64) =
+            if state.protocol_version > 23 {
+                (40, encode_var_int_fixed)
+            } else {
+                (17, |length| length as u64)
+            };
+        stream.reserve_int::<ParseError, _>(length_bits, |stream| {
             let start = stream.bit_len();
             let mut last_class_id = u16::MAX.into();
 
@@ -138,7 +145,7 @@ impl Encode for TempEntitiesMessage {
                 PacketEntitiesMessage::write_update(&event.props, stream, send_table, 0u32.into())?;
             }
             let end = stream.bit_len();
-            Ok(encode_var_int_fixed((end - start) as u32))
+            Ok(encode_length((end - start) as u32))
         })?;
         Ok(())
     }

tests/reencode.rs

@@ -32,6 +32,7 @@ use tf_demo_parser::{Demo, Parse};
 #[test_case("test_data/decal.dem"; "decal.dem")]
 #[test_case("test_data/saytext2.dem"; "saytext2.dem")]
 #[test_case("test_data/emptysaytext.dem"; "emptysaytext.dem")]
+#[test_case("test_data/protocol23.dem"; "protocol23.dem")]
 fn re_encode_test(input_file: &str) {
     let file = fs::read(input_file).expect("Unable to read file");
     let demo = Demo::new(&file);
@@ -44,6 +45,7 @@ fn re_encode_test(input_file: &str) {
 
     let mut packets = RawPacketStream::new(stream);
     let mut handler = DemoHandler::parse_all_with_analyser(NullHandler);
+    handler.handle_header(&header);
 
     {
         let mut out_stream = BitWriteStream::new(&mut out_buffer, LittleEndian);
@@ -150,4 +152,36 @@ fn re_encode_test(input_file: &str) {
 
         handler.handle_packet(packet).unwrap();
     }
+
+    // a desync shows up as the packet loop quietly stopping early, so the
+    // round trip above passes while most of the demo is never looked at
+    assert!(!packets.incomplete, "{} was not read to the end", input_file);
+}
+
+/// A demo recorded on an older protocol must be parsed with that protocol, not
+/// the protocol 24 that `ParserState` starts out with. Getting it wrong shifts
+/// the width of a handful of fields (`ServerInfo`'s map hash, `PreFetch`), the
+/// message stream desyncs, and `RawPacketStream` quietly stops early rather
+/// than returning an error - so this checks the whole demo was actually read.
+#[cfg(feature = "write")]
+#[test]
+fn parse_with_header_protocol() {
+    let file = fs::read("test_data/protocol23.dem").expect("Unable to read file");
+    let demo = Demo::new(&file);
+    let mut stream = demo.get_stream();
+    let header = Header::read(&mut stream).unwrap();
+    assert_eq!(23, header.protocol);
+
+    let mut packets = RawPacketStream::new(stream);
+    let mut handler = DemoHandler::parse_all_with_analyser(NullHandler);
+    handler.handle_header(&header);
+
+    let mut count = 0;
+    while let Some(packet) = packets.next(&handler.state_handler).unwrap() {
+        handler.handle_packet(packet).unwrap();
+        count += 1;
+    }
+
+    assert!(!packets.incomplete, "demo was not read to the end");
+    assert_eq!(128235, count);
 }