Fixed gamepacket parsing (#987)

* grg

* ef

* gg

* refwd

---------

Co-authored-by: unschlagbar <adrian@kuhlmann@gmx.de>

pumpkin-protocol/src/bedrock/frame_set.rs

@@ -51,7 +51,7 @@ impl Frame {
     pub fn read(mut read: impl std::io::Read) -> Result<Vec<Self>, crate::ser::ReadingError> {
         let mut frames = Vec::new();
 
-        while let Ok(header) = read.get_u8_be() {
+        while let Ok(header) = read.get_u8() {
             let mut frame = Self::default();
             let reliability_id = (header & 0xE0) >> 5;
             let reliability = match RakReliability::from_id(reliability_id) {
@@ -75,7 +75,7 @@ impl Frame {
 
             if reliability.is_ordered() {
                 frame.order_index = read.get_u24()?.0;
-                frame.order_channel = read.get_u8_be()?;
+                frame.order_channel = read.get_u8()?;
             }
 
             if split {

pumpkin-protocol/src/bedrock/mod.rs

@@ -95,3 +95,11 @@ impl RakReliability {
         }
     }
 }
+
+#[repr(u16)]
+pub enum SubClient {
+    Main = 0,
+    SubClient0 = 1,
+    SubClient1 = 2,
+    SubClietn2 = 3,
+}

pumpkin-protocol/src/bedrock/packet_decoder.rs

@@ -5,9 +5,8 @@ use bytes::Bytes;
 use tokio::io::{AsyncRead, AsyncReadExt, BufReader};
 
 use crate::{
-    Aes128Cfb8Dec, CompressionThreshold, MAX_PACKET_SIZE, PacketDecodeError, RawPacket,
-    StreamDecryptor,
-    codec::var_int::VarInt,
+    Aes128Cfb8Dec, CompressionThreshold, PacketDecodeError, RawPacket, StreamDecryptor,
+    codec::var_uint::VarUInt,
     ser::{NetworkReadExt, ReadingError},
 };
 
@@ -119,11 +118,12 @@ impl UDPNetworkDecoder {
         &mut self,
         mut reader: Cursor<Vec<u8>>,
     ) -> Result<RawPacket, PacketDecodeError> {
-        let compression = reader.get_u8_be()?;
-        dbg!(compression);
+        //compression is only included after the network settings packet is sent
+        //let compression = reader.get_u8()?;
+        //dbg!(compression);
 
         // TODO: compression & encryption
-        let packet_len = VarInt::decode_async(&mut reader)
+        let packet_len = VarUInt::decode_async(&mut reader)
             .await
             .map_err(|err| match err {
                 ReadingError::CleanEOF(_) => PacketDecodeError::ConnectionClosed,
@@ -133,11 +133,12 @@ impl UDPNetworkDecoder {
         let packet_len = packet_len.0 as u64;
         dbg!(packet_len);
 
-        if !(0..=MAX_PACKET_SIZE).contains(&packet_len) {
+        // This is the default MTU size
+        if !(0..=1492).contains(&packet_len) {
             Err(PacketDecodeError::OutOfBounds)?
         }
 
-        let header = VarInt::decode_async(&mut reader).await?;
+        let header = VarUInt::decode_async(&mut reader).await?;
 
         let header_value = header.0;
 
@@ -151,16 +152,16 @@ impl UDPNetworkDecoder {
         let fourteen_bit_header = header_value & 0x3FFF; // Mask to get the lower 14 bits (2^14 - 1)
 
         // SubClient Target ID: Lowest 2 bits
-        let _sub_client_target_id = (fourteen_bit_header & 0b11) as u8;
+        let _sub_client_target = (fourteen_bit_header & 0b11) as u8;
 
         // SubClient Sender ID: Next 2 bits (bits 2 and 3)
-        let _sub_client_sender_id = ((fourteen_bit_header >> 2) & 0b11) as u8;
+        let _sub_client_sender = ((fourteen_bit_header >> 2) & 0b11) as u8;
 
         // Gamepacket ID: Remaining 10 bits (bits 4 to 13)
         let gamepacket_id = ((fourteen_bit_header >> 4) & 0x3FF) as u16; // 0x3FF is 10 bits set to 1
 
         let payload = reader
-            .read_boxed_slice(packet_len as usize)
+            .read_boxed_slice(packet_len as usize - header.written_size())
             .map_err(|err| PacketDecodeError::FailedDecompression(err.to_string()))?;
 
         Ok(RawPacket {

pumpkin-protocol/src/bedrock/packet_encoder.rs

@@ -6,7 +6,7 @@ use tokio::{io::AsyncWrite, net::UdpSocket};
 
 use crate::{
     Aes128Cfb8Enc, CompressionLevel, CompressionThreshold, PacketEncodeError, StreamEncryptor,
-    codec::var_int::VarInt, ser::NetworkWriteExt,
+    bedrock::SubClient, codec::var_uint::VarUInt, ser::NetworkWriteExt,
 };
 
 // raw -> compress -> encrypt
@@ -110,9 +110,9 @@ impl UDPNetworkEncoder {
 
     pub async fn write_game_packet(
         &mut self,
-        packet_id: i32,
-        sub_client_sender_id: i32,
-        sub_client_target_id: i32,
+        packet_id: u16,
+        sub_client_sender: SubClient,
+        sub_client_target: SubClient,
         packet_payload: Bytes,
         mut writer: impl Write,
     ) -> Result<(), PacketEncodeError> {
@@ -125,8 +125,8 @@ impl UDPNetworkEncoder {
         // SubClient Sender ID (2 bits) << 2 (offset by 2 bits for target)
         // SubClient Target ID (2 bits)
         let header_value: u32 = ((packet_id as u32) << 4)
-            | ((sub_client_sender_id as u32) << 2)
-            | (sub_client_target_id as u32);
+            | ((sub_client_sender as u32) << 2)
+            | (sub_client_target as u32);
 
         // Ensure the combined header doesn't exceed 14 bits (just a sanity check, should be handled by above shifts)
         let fourteen_bit_header = header_value & 0x3FFF; // Mask to ensure it fits in 14 bits
@@ -134,7 +134,7 @@ impl UDPNetworkEncoder {
         // 2. Calculate total packet_len
         // This is where `VarInt::encoded_len` is crucial.
         // We need to know the byte length of the header's VarInt *before* we write the packet_len.
-        let header_byte_len = VarInt(fourteen_bit_header as i32).written_size();
+        let header_byte_len = VarUInt(fourteen_bit_header).written_size();
 
         let packet_payload_len = packet_payload.len() as u32;
         // total_content_length is the length of the header VarInt bytes + payload bytes.
@@ -145,12 +145,12 @@ impl UDPNetworkEncoder {
         // Ensure consistency in your actual `VarInt` definition.
         // For this example, I'll cast `total_content_length` to `i32`.
         writer
-            .write_var_int(&VarInt(total_content_length as i32))
+            .write_var_uint(&VarUInt(total_content_length))
             .unwrap();
 
         // 4. Write the combined 14-bit header_value as VarInt
         writer
-            .write_var_int(&VarInt(fourteen_bit_header as i32))
+            .write_var_uint(&VarUInt(fourteen_bit_header))
             .unwrap();
 
         // 5. Write the Packet ID + payload

pumpkin-protocol/src/codec/mod.rs

@@ -5,3 +5,4 @@ pub mod socket_address;
 pub mod u24;
 pub mod var_int;
 pub mod var_long;
+pub mod var_uint;

pumpkin-protocol/src/codec/u24.rs

@@ -7,9 +7,9 @@ pub struct U24(pub u32);
 
 impl U24 {
     pub fn decode(read: &mut impl Read) -> Result<Self, ReadingError> {
-        let a = read.get_u8_le()?;
-        let b = read.get_u8_le()?;
-        let c = read.get_u8_le()?;
+        let a = read.get_u8()?;
+        let b = read.get_u8()?;
+        let c = read.get_u8()?;
         Ok(U24(u32::from_le_bytes([a, b, c, 0])))
     }
 

pumpkin-protocol/src/codec/var_int.rs

@@ -51,7 +51,7 @@ impl VarInt {
     pub fn decode(read: &mut impl Read) -> Result<Self, ReadingError> {
         let mut val = 0;
         for i in 0..Self::MAX_SIZE.get() {
-            let byte = read.get_u8_be()?;
+            let byte = read.get_u8()?;
             val |= (i32::from(byte) & 0x7F) << (i * 7);
             if byte & 0x80 == 0 {
                 return Ok(VarInt(val));

pumpkin-protocol/src/codec/var_long.rs

@@ -54,7 +54,7 @@ impl VarLong {
     pub fn decode(read: &mut impl Read) -> Result<Self, ReadingError> {
         let mut val = 0;
         for i in 0..Self::MAX_SIZE.get() {
-            let byte = read.get_u8_be()?;
+            let byte = read.get_u8()?;
             val |= (i64::from(byte) & 0b01111111) << (i * 7);
             if byte & 0b10000000 == 0 {
                 return Ok(VarLong(val));

pumpkin-protocol/src/codec/var_uint.rs

@@ -0,0 +1,191 @@
+use std::{
+    io::{ErrorKind, Read, Write},
+    num::NonZeroUsize,
+};
+
+use bytes::BufMut;
+use serde::{
+    Deserialize, Deserializer, Serialize, Serializer,
+    de::{SeqAccess, Visitor},
+};
+use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
+
+use crate::ser::{NetworkReadExt, NetworkWriteExt, ReadingError, WritingError};
+
+pub type VarUIntType = u32;
+
+/**
+ * A variable-length integer type used by the Minecraft network protocol.
+ */
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct VarUInt(pub VarUIntType);
+
+impl VarUInt {
+    /// The maximum number of bytes a `VarUInt` can occupy.
+    const MAX_SIZE: NonZeroUsize = NonZeroUsize::new(5).unwrap();
+
+    /// Returns the exact number of bytes this VarUInt will write when
+    /// [`Encode::encode`] is called, assuming no error occurs.
+    pub fn written_size(&self) -> usize {
+        (32 - self.0.leading_zeros() as usize).max(1).div_ceil(7)
+    }
+
+    pub fn encode(&self, write: &mut impl Write) -> Result<(), WritingError> {
+        let mut val = self.0;
+        loop {
+            let mut byte = (val & 0x7F) as u8;
+            val >>= 7;
+            if val != 0 {
+                byte |= 0x80;
+            }
+            write.write_u8(byte)?;
+            if val == 0 {
+                break;
+            }
+        }
+        Ok(())
+    }
+
+    // TODO: Validate that the first byte will not overflow a i32
+    pub fn decode(read: &mut impl Read) -> Result<Self, ReadingError> {
+        let mut val = 0;
+        for i in 0..Self::MAX_SIZE.get() {
+            let byte = read.get_u8()?;
+            val |= (u32::from(byte) & 0x7F) << (i * 7);
+            if byte & 0x80 == 0 {
+                return Ok(VarUInt(val));
+            }
+        }
+        Err(ReadingError::TooLarge("VarInt".to_string()))
+    }
+}
+
+impl VarUInt {
+    pub async fn decode_async(read: &mut (impl AsyncRead + Unpin)) -> Result<Self, ReadingError> {
+        let mut val = 0;
+        for i in 0..Self::MAX_SIZE.get() {
+            let byte = read.read_u8().await.map_err(|err| {
+                if i == 0 && matches!(err.kind(), ErrorKind::UnexpectedEof) {
+                    ReadingError::CleanEOF("VarInt".to_string())
+                } else {
+                    ReadingError::Incomplete(err.to_string())
+                }
+            })?;
+            val |= (u32::from(byte) & 0x7F) << (i * 7);
+            if byte & 0x80 == 0 {
+                return Ok(VarUInt(val));
+            }
+        }
+        Err(ReadingError::TooLarge("VarInt".to_string()))
+    }
+
+    pub async fn encode_async(
+        &self,
+        write: &mut (impl AsyncWrite + Unpin),
+    ) -> Result<(), WritingError> {
+        let mut val = self.0;
+        for _ in 0..Self::MAX_SIZE.get() {
+            let b: u8 = val as u8 & 0b01111111;
+            val >>= 7;
+            write
+                .write_u8(if val == 0 { b } else { b | 0b10000000 })
+                .await
+                .map_err(WritingError::IoError)?;
+            if val == 0 {
+                break;
+            }
+        }
+        Ok(())
+    }
+}
+
+// Macros are needed because traits over generics succccccccccck
+macro_rules! gen_from {
+    ($ty: ty) => {
+        impl From<$ty> for VarUInt {
+            fn from(value: $ty) -> Self {
+                VarUInt(value as u32)
+            }
+        }
+    };
+}
+
+gen_from!(i8);
+gen_from!(u8);
+gen_from!(i16);
+gen_from!(u16);
+gen_from!(u32);
+
+macro_rules! gen_try_from {
+    ($ty: ty) => {
+        impl TryFrom<$ty> for VarUInt {
+            type Error = <i32 as TryFrom<$ty>>::Error;
+
+            fn try_from(value: $ty) -> Result<Self, Self::Error> {
+                Ok(VarUInt(value as u32))
+            }
+        }
+    };
+}
+
+gen_try_from!(i32);
+gen_try_from!(i64);
+gen_try_from!(u64);
+gen_try_from!(isize);
+gen_try_from!(usize);
+
+impl Serialize for VarUInt {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        let mut value = self.0;
+        let mut buf = Vec::with_capacity(5);
+
+        while value > 0x7F {
+            buf.put_u8(value as u8 | 0x80);
+            value >>= 7;
+        }
+
+        buf.put_u8(value as u8);
+
+        serializer.serialize_bytes(&buf)
+    }
+}
+
+impl<'de> Deserialize<'de> for VarUInt {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        struct VarIntVisitor;
+
+        impl<'de> Visitor<'de> for VarIntVisitor {
+            type Value = VarUInt;
+
+            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
+                formatter.write_str("a valid VarInt encoded in a byte sequence")
+            }
+
+            fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
+            where
+                A: SeqAccess<'de>,
+            {
+                let mut val = 0;
+                for i in 0..VarUInt::MAX_SIZE.get() {
+                    if let Some(byte) = seq.next_element::<u8>()? {
+                        val |= (u32::from(byte) & 0b01111111) << (i * 7);
+                        if byte & 0b10000000 == 0 {
+                            return Ok(VarUInt(val));
+                        }
+                    } else {
+                        break;
+                    }
+                }
+                Err(serde::de::Error::custom("VarInt was too large"))
+            }
+        }
+
+        deserializer.deserialize_seq(VarIntVisitor)
+    }
+}

pumpkin-protocol/src/java/server/play/chat_message.rs

@@ -32,7 +32,7 @@ impl ServerPacket for SChatMessage {
             signature: read.get_option(|v| v.read_boxed_slice(256))?,
             message_count: read.get_var_int()?,
             acknowledged: read.get_fixed_bitset(20)?,
-            checksum: read.get_u8_be()?,
+            checksum: read.get_u8()?,
         })
     }
 }

pumpkin-protocol/src/ser/deserializer.rs

@@ -45,7 +45,7 @@ impl<'de, R: Read> de::Deserializer<'de> for &mut Deserializer<R> {
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_i8(self.inner.get_i8_be()?)
+        visitor.visit_i8(self.inner.get_i8()?)
     }
 
     fn deserialize_i16<V>(self, visitor: V) -> Result<V::Value, Self::Error>
@@ -73,7 +73,7 @@ impl<'de, R: Read> de::Deserializer<'de> for &mut Deserializer<R> {
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_u8(self.inner.get_u8_be()?)
+        visitor.visit_u8(self.inner.get_u8()?)
     }
 
     fn deserialize_u16<V>(self, visitor: V) -> Result<V::Value, Self::Error>

pumpkin-protocol/src/ser/mod.rs

@@ -3,7 +3,7 @@ use std::io::{Read, Write};
 
 use crate::{
     FixedBitSet,
-    codec::{bit_set::BitSet, u24::U24, var_int::VarInt, var_long::VarLong},
+    codec::{bit_set::BitSet, u24::U24, var_int::VarInt, var_long::VarLong, var_uint::VarUInt},
 };
 
 pub mod deserializer;
@@ -46,9 +46,8 @@ pub enum WritingError {
 }
 
 pub trait NetworkReadExt {
-    fn get_i8_be(&mut self) -> Result<i8, ReadingError>;
-    fn get_u8_be(&mut self) -> Result<u8, ReadingError>;
-    fn get_u8_le(&mut self) -> Result<u8, ReadingError>;
+    fn get_i8(&mut self) -> Result<i8, ReadingError>;
+    fn get_u8(&mut self) -> Result<u8, ReadingError>;
 
     fn get_i16_be(&mut self) -> Result<i16, ReadingError>;
     fn get_u16_be(&mut self) -> Result<u16, ReadingError>;
@@ -87,28 +86,20 @@ pub trait NetworkReadExt {
 
 impl<R: Read> NetworkReadExt for R {
     //TODO: Macroize this
-    fn get_i8_be(&mut self) -> Result<i8, ReadingError> {
+    fn get_i8(&mut self) -> Result<i8, ReadingError> {
         let mut buf = [0u8];
         self.read_exact(&mut buf)
             .map_err(|err| ReadingError::Incomplete(err.to_string()))?;
 
-        Ok(i8::from_be_bytes(buf))
+        Ok(buf[0] as i8)
     }
 
-    fn get_u8_be(&mut self) -> Result<u8, ReadingError> {
+    fn get_u8(&mut self) -> Result<u8, ReadingError> {
         let mut buf = [0u8];
         self.read_exact(&mut buf)
             .map_err(|err| ReadingError::Incomplete(err.to_string()))?;
 
-        Ok(u8::from_be_bytes(buf))
-    }
-
-    fn get_u8_le(&mut self) -> Result<u8, ReadingError> {
-        let mut buf = [0u8];
-        self.read_exact(&mut buf)
-            .map_err(|err| ReadingError::Incomplete(err.to_string()))?;
-
-        Ok(u8::from_le_bytes(buf))
+        Ok(buf[0])
     }
 
     fn get_i16_be(&mut self) -> Result<i16, ReadingError> {
@@ -226,7 +217,7 @@ impl<R: Read> NetworkReadExt for R {
     }
 
     fn get_bool(&mut self) -> Result<bool, ReadingError> {
-        let byte = self.get_u8_be()?;
+        let byte = self.get_u8()?;
         Ok(byte != 0)
     }
 
@@ -321,6 +312,7 @@ pub trait NetworkWriteExt {
         }
     }
     fn write_var_int(&mut self, data: &VarInt) -> Result<(), WritingError>;
+    fn write_var_uint(&mut self, data: &VarUInt) -> Result<(), WritingError>;
     fn write_var_long(&mut self, data: &VarLong) -> Result<(), WritingError>;
     fn write_string_bounded(&mut self, data: &str, bound: usize) -> Result<(), WritingError>;
     fn write_string(&mut self, data: &str) -> Result<(), WritingError>;
@@ -429,6 +421,10 @@ impl<W: Write> NetworkWriteExt for W {
         data.encode(self)
     }
 
+    fn write_var_uint(&mut self, data: &VarUInt) -> Result<(), WritingError> {
+        data.encode(self)
+    }
+
     fn write_var_long(&mut self, data: &VarLong) -> Result<(), WritingError> {
         data.encode(self)
     }

pumpkin/src/net/bedrock/mod.rs

@@ -10,7 +10,7 @@ use bytes::Bytes;
 use pumpkin_protocol::{
     ClientPacket, PacketDecodeError, PacketEncodeError, RawPacket, ServerPacket,
     bedrock::{
-        RAKNET_ACK, RAKNET_GAME_PACKET, RAKNET_NACK, RAKNET_VALID, RakReliability,
+        RAKNET_ACK, RAKNET_GAME_PACKET, RAKNET_NACK, RAKNET_VALID, RakReliability, SubClient,
         ack::Ack,
         frame_set::{Frame, FrameSet},
         packet_decoder::UDPNetworkDecoder,
@@ -101,7 +101,13 @@ impl BedrockClientPlatform {
         self.network_writer
             .lock()
             .await
-            .write_game_packet(P::PACKET_ID, 0, 0, packet_payload.into(), write)
+            .write_game_packet(
+                P::PACKET_ID as u16,
+                SubClient::Main,
+                SubClient::Main,
+                packet_payload.into(),
+                write,
+            )
             .await
             .unwrap();
         Ok(())
@@ -230,7 +236,7 @@ impl BedrockClientPlatform {
     ) -> Result<(), ReadingError> {
         let mut payload = &packet[..];
 
-        let Ok(id) = payload.get_u8_be() else {
+        let Ok(id) = payload.get_u8() else {
             return Err(ReadingError::CleanEOF(String::new()));
         };
 
@@ -290,7 +296,7 @@ impl BedrockClientPlatform {
         dbg!(frame.reliability);
 
         let mut payload = &frame.payload[..];
-        let id = payload.get_u8_be()?;
+        let id = payload.get_u8()?;
         self.handle_raknet_packet(client, server, i32::from(id), payload)
             .await
     }