read exact size of packets, fixing networking buf

Cargo.lock

@@ -84,11 +84,13 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
 name = "classics"
 version = "0.1.0"
 dependencies = [
+ "bytes",
  "flate2",
  "half",
  "internment",
  "parking_lot",
  "rand",
+ "safer-bytes",
  "serde",
  "serde_json",
  "tokio",
@@ -268,6 +270,12 @@ dependencies = [
  "windows-targets 0.48.5",
 ]
 
+[[package]]
+name = "paste"
+version = "1.0.14"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c"
+
 [[package]]
 name = "pin-project-lite"
 version = "0.2.14"
@@ -349,6 +357,17 @@ version = "1.0.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e86697c916019a8588c99b5fac3cead74ec0b4b819707a682fd4d23fa0ce1ba1"
 
+[[package]]
+name = "safer-bytes"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9814c78d534f27a438fcb57091d6deed0634b60e4e500fee28fe5990adf5ea54"
+dependencies = [
+ "bytes",
+ "paste",
+ "thiserror",
+]
+
 [[package]]
 name = "scopeguard"
 version = "1.2.0"
@@ -422,6 +441,26 @@ dependencies = [
  "unicode-ident",
 ]
 
+[[package]]
+name = "thiserror"
+version = "1.0.59"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f0126ad08bff79f29fc3ae6a55cc72352056dfff61e3ff8bb7129476d44b23aa"
+dependencies = [
+ "thiserror-impl",
+]
+
+[[package]]
+name = "thiserror-impl"
+version = "1.0.59"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d1cd413b5d558b4c5bf3680e324a6fa5014e7b7c067a51e69dbdf47eb7148b66"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "tokio"
 version = "1.37.0"

Cargo.toml

@@ -4,11 +4,13 @@ name = "classics"
 version = "0.1.0"
 
 [dependencies]
+bytes = "1.6.0"
 flate2 = "1"
 half = "2"
 internment = { version = "0.8", features = ["serde"] }
 parking_lot = "0.12.1"
 rand = "0.8"
+safer-bytes = "0.2"
 serde = {version = "1", features = ["derive"]}
 serde_json = "1"
 tokio = {version = "1", features = ["full"]}

src/packet.rs

@@ -1,4 +1,5 @@
 use half::f16;
+use safer_bytes::{error::Truncated, SafeBuf};
 
 pub mod client;
 pub mod server;
@@ -10,79 +11,30 @@ pub const ARRAY_LENGTH: usize = 1024;
 /// units in an f16 unit
 pub const F16_UNITS: f32 = 32.0;
 
-/// helper for reading packets
+/// trait extending the `SafeBuf` type
-#[derive(Debug)]
+pub trait SafeBufExtension: SafeBuf {
-pub struct PacketReader<'p> {
+	/// tries to get the next f16 in the buffer
-	raw_packet: &'p [u8],
+	fn try_get_f16(&mut self) -> Result<f16, Truncated>;
-	cursor: usize,
+	/// tries to get the next string in the buffer
+	fn try_get_string(&mut self) -> Result<String, Truncated>;
 }
 
-impl<'p> PacketReader<'p> {
+impl<T> SafeBufExtension for T
-	/// creates a new packet reader from the given packet data
+where
-	pub fn new(raw_packet: &'p [u8]) -> Self {
+	T: SafeBuf,
-		Self {
+{
-			raw_packet,
+	fn try_get_f16(&mut self) -> Result<f16, Truncated> {
-			cursor: 0,
+		self.try_get_i16()
-		}
+			.map(|v| f16::from_f32(v as f32 / F16_UNITS))
 	}
 
-	/// gets the next u8 in the packet, if any
+	fn try_get_string(&mut self) -> Result<String, Truncated> {
-	fn next_u8(&mut self) -> Option<u8> {
-		let r = self.raw_packet.get(self.cursor).copied();
-		self.cursor = self.cursor.checked_add(1).unwrap_or(self.cursor);
-		r
-	}
-
-	/// gets the next i8 in the packet, if any
-	fn next_i8(&mut self) -> Option<i8> {
-		self.next_u8().map(|b| b as i8)
-	}
-
-	/// gets the next u16 in the packet, if any
-	fn next_u16(&mut self) -> Option<u16> {
-		Some(u16::from_be_bytes([self.next_u8()?, self.next_u8()?]))
-	}
-
-	/// gets the next i16 in the packet, if any
-	fn next_i16(&mut self) -> Option<i16> {
-		self.next_u16().map(|s| s as i16)
-	}
-
-	/// gets the next f16 in the packet, if any
-	fn next_f16(&mut self) -> Option<f16> {
-		self.next_i16().map(|v| f16::from_f32(v as f32 / F16_UNITS))
-	}
-
-	/// gets the next string in the packet, if any
-	fn next_string(&mut self) -> Option<String> {
 		let mut chars: Vec<char> = Vec::new();
 		for _ in 0..STRING_LENGTH {
-			chars.push(self.next_u8()? as char);
+			chars.push(self.try_get_u8()? as char);
 		}
-		Some(String::from_iter(chars).trim().to_string())
+		Ok(String::from_iter(chars).trim().to_string())
 	}
-
-	// /// gets the next array of the given length in the packet, if any
-	// fn next_array_of_length(&mut self, len: usize) -> Option<Vec<u8>> {
-	// 	let mut bytes: Vec<u8> = Vec::new();
-	// 	let mut append = true;
-	// 	for _ in 0..len {
-	// 		let b = self.next_u8()?;
-	// 		if append {
-	// 			if b == 0 {
-	// 				append = false;
-	// 			} else {
-	// 				bytes.push(b);
-	// 			}
-	// 		}
-	// 	}
-	// 	Some(bytes)
-	// }
-
-	// /// gets the next array of default size in the packet, if any
-	// fn next_array(&mut self) -> Option<Vec<u8>> {
-	// 	self.next_array_of_length(ARRAY_LENGTH)
-	// }
 }
 
 /// helper for writing a packet

src/packet/client.rs

@@ -1,5 +1,7 @@
 use half::f16;
 
+use super::{SafeBufExtension, STRING_LENGTH};
+
 /// enum for a packet which can be received by the client
 #[derive(Debug, Clone)]
 pub enum ClientPacket {
@@ -53,35 +55,52 @@ impl ClientPacket {
 	// 	}
 	// }
 
+	/// gets the size of the packet from the given id (minus one byte for the id)
+	pub const fn get_size_from_id(id: u8) -> Option<usize> {
+		Some(match id {
+			0x00 => 1 + STRING_LENGTH + STRING_LENGTH + 1,
+			0x05 => 2 + 2 + 2 + 1 + 1,
+			0x08 => 1 + 2 + 2 + 2 + 1 + 1,
+			0x0d => 1 + STRING_LENGTH,
+			_ => return None,
+		})
+	}
+
 	/// reads the packet
-	pub fn read(id: u8, packet: &mut super::PacketReader) -> Option<Self> {
+	pub fn read<B>(id: u8, buf: &mut B) -> Option<Self>
+	where
+		B: SafeBufExtension,
+	{
 		Some(match id {
 			0x00 => Self::PlayerIdentification {
-				protocol_version: packet.next_u8()?,
+				protocol_version: buf.try_get_u8().ok()?,
-				username: packet.next_string()?,
+				username: buf.try_get_string().ok()?,
-				verification_key: packet.next_string()?,
+				verification_key: buf.try_get_string().ok()?,
-				_unused: packet.next_u8()?,
+				_unused: buf.try_get_u8().ok()?,
 			},
 			0x05 => Self::SetBlock {
-				x: packet.next_i16()?,
+				x: buf.try_get_i16().ok()?,
-				y: packet.next_i16()?,
+				y: buf.try_get_i16().ok()?,
-				z: packet.next_i16()?,
+				z: buf.try_get_i16().ok()?,
-				mode: packet.next_u8()?,
+				mode: buf.try_get_u8().ok()?,
-				block_type: packet.next_u8()?,
+				block_type: buf.try_get_u8().ok()?,
 			},
 			0x08 => Self::PositionOrientation {
-				_player_id: packet.next_i8()?,
+				_player_id: buf.try_get_i8().ok()?,
-				x: packet.next_f16()?,
+				x: buf.try_get_f16().ok()?,
-				y: packet.next_f16()?,
+				y: buf.try_get_f16().ok()?,
-				z: packet.next_f16()?,
+				z: buf.try_get_f16().ok()?,
-				yaw: packet.next_u8()?,
+				yaw: buf.try_get_u8().ok()?,
-				pitch: packet.next_u8()?,
+				pitch: buf.try_get_u8().ok()?,
 			},
 			0x0d => Self::Message {
-				player_id: packet.next_i8()?,
+				player_id: buf.try_get_i8().ok()?,
-				message: packet.next_string()?,
+				message: buf.try_get_string().ok()?,
 			},
-			_ => return None,
+			id => {
+				println!("unknown packet id: {id:0x}");
+				return None;
+			}
 		})
 	}
 

src/server/network.rs

@@ -1,18 +1,17 @@
 use std::{collections::VecDeque, io::Write, net::SocketAddr, sync::Arc};
 
+use bytes::BytesMut;
 use flate2::{write::GzEncoder, Compression};
 use half::f16;
 use tokio::{
-	io::{AsyncWriteExt, Interest},
+	io::{AsyncReadExt, AsyncWriteExt, Interest},
 	net::TcpStream,
 	sync::RwLock,
 };
 
 use crate::{
 	level::{block::BLOCK_INFO, Level},
-	packet::{
+	packet::{client::ClientPacket, server::ServerPacket, PacketWriter, ARRAY_LENGTH},
-		client::ClientPacket, server::ServerPacket, PacketReader, PacketWriter, ARRAY_LENGTH,
-	},
 	player::{Player, PlayerType},
 	server::config::ServerProtectionMode,
 };
@@ -69,11 +68,9 @@ async fn handle_stream_inner(
 	data: Arc<RwLock<ServerData>>,
 	own_id: &mut i8,
 ) -> std::io::Result<Option<String>> {
-	const BUF_SIZE: usize = 130;
-
 	let mut reply_queue: VecDeque<ServerPacket> = VecDeque::new();
-	let mut packet_buf = [0u8];
 	let mut read_buf;
+	let mut id_buf;
 
 	loop {
 		let ready = stream
@@ -86,20 +83,18 @@ async fn handle_stream_inner(
 		}
 
 		if ready.is_readable() {
-			match stream.try_read(&mut packet_buf) {
+			id_buf = [0u8];
+			match stream.try_read(&mut id_buf) {
 				Ok(n) => {
 					if n == 1 {
-						read_buf = [0; BUF_SIZE];
+						if let Some(size) = ClientPacket::get_size_from_id(id_buf[0]) {
-						match stream.try_read(&mut read_buf) {
+							read_buf = BytesMut::zeroed(size);
-							Ok(_n) => {}
-							Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => continue,
-							Err(e) => return Err(e),
-						}
 
-						let mut reader = PacketReader::new(&read_buf);
+							stream.read_exact(&mut read_buf).await?;
 
-						if let Some(packet) = ClientPacket::read(packet_buf[0], &mut reader) {
+							match ClientPacket::read(id_buf[0], &mut read_buf)
-							match packet {
+								.expect("should never fail: id already checked")
+							{
 								ClientPacket::PlayerIdentification {
 									protocol_version,
 									username,
@@ -331,7 +326,7 @@ async fn handle_stream_inner(
 								}
 							}
 						} else {
-							println!("unknown packet id: {:0x}", packet_buf[0]);
+							println!("unknown packet id: {}", id_buf[0]);
 						}
 					}
 				}