improve error handling

- replace anyhow with thiserror
- implement own error types
- Security: Begin to haarding Server, Prevent Crashes when Fail to receive/send Client Packet

Cargo.lock

@@ -19,12 +19,6 @@ dependencies = [
  "cpufeatures",
 ]
 
-[[package]]
-name = "anyhow"
-version = "1.0.86"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b3d1d046238990b9cf5bcde22a3fb3584ee5cf65fb2765f454ed428c7a0063da"
-
 [[package]]
 name = "autocfg"
 version = "1.3.0"
@@ -290,9 +284,9 @@ dependencies = [
 
 [[package]]
 name = "indexmap"
-version = "2.2.6"
+version = "2.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26"
+checksum = "de3fc2e30ba82dd1b3911c8de1ffc143c74a914a14e99514d7637e3099df5ea0"
 dependencies = [
  "equivalent",
  "hashbrown",
@@ -529,7 +523,6 @@ name = "pumpkin"
 version = "0.1.0"
 dependencies = [
  "aes",
- "anyhow",
  "base64",
  "bytes",
  "cfb8",
@@ -546,6 +539,7 @@ dependencies = [
  "serde",
  "serde_json",
  "simple_logger",
+ "thiserror",
  "toml",
  "uuid",
 ]
@@ -808,9 +802,9 @@ dependencies = [
 
 [[package]]
 name = "toml"
-version = "0.8.17"
+version = "0.8.19"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7a44eede9b727419af8095cb2d72fab15487a541f54647ad4414b34096ee4631"
+checksum = "a1ed1f98e3fdc28d6d910e6737ae6ab1a93bf1985935a1193e68f93eeb68d24e"
 dependencies = [
  "serde",
  "serde_spanned",
@@ -829,9 +823,9 @@ dependencies = [
 
 [[package]]
 name = "toml_edit"
-version = "0.22.18"
+version = "0.22.20"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1490595c74d930da779e944f5ba2ecdf538af67df1a9848cbd156af43c1b7cf0"
+checksum = "583c44c02ad26b0c3f3066fe629275e50627026c51ac2e595cca4c230ce1ce1d"
 dependencies = [
  "indexmap",
  "serde",
@@ -1011,9 +1005,9 @@ checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec"
 
 [[package]]
 name = "winnow"
-version = "0.6.16"
+version = "0.6.18"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b480ae9340fc261e6be3e95a1ba86d54ae3f9171132a73ce8d4bbaf68339507c"
+checksum = "68a9bda4691f099d435ad181000724da8e5899daa10713c2d432552b9ccd3a6f"
 dependencies = [
  "memchr",
 ]

README.md

@@ -24,5 +24,8 @@ A Minecraft server written in pure Rust
 - [ ] Compression
 - [x] Configuration
 
+## Contributions
+Contributions are welcome!. Feel free to open an issue or pull request.
+
 ### Thanks
 A big thank you to https://wiki.vg/ for providing valuable information used in the development of this project.

pumpkin/Cargo.toml

@@ -18,7 +18,7 @@ cfb8 = "0.8.1"
 flate2 = "1.0.30"
 bytes = "1.7"
 
-anyhow = "1.0.86"
+thiserror = "1.0.63"
 
 base64 = "0.22.1"
 image = { version = "0.25", default-features = false, features = ["png"]}

pumpkin/src/client/client_packet.rs

@@ -18,6 +18,8 @@ use crate::{
 
 use super::{Client, PlayerConfig};
 
+/// Processes incoming Packets from the Client to the Server
+/// Implements the `Client` Packets, So everything before the Play state, then will use the `PlayerPacketProcessor`
 pub trait ClientPacketProcessor {
     // Handshake
     fn handle_handshake(&mut self, server: &mut Server, handshake: SHandShake);
@@ -65,14 +67,18 @@ impl ClientPacketProcessor for Client {
     fn handle_status_request(&mut self, server: &mut Server, _status_request: SStatusRequest) {
         dbg!("sending status");
 
-        let response = serde_json::to_string(&server.status_response).unwrap();
+        if let Ok(response) = serde_json::to_string(&server.status_response) {
-
+            self.send_packet(CStatusResponse::new(response))
-        self.send_packet(CStatusResponse::new(response));
+                .unwrap_or_else(|e| self.kick(&e.to_string()));
+        } else {
+            log::error!("Failed to parse Status response to JSON")
+        }
     }
 
     fn handle_ping_request(&mut self, _server: &mut Server, ping_request: SPingRequest) {
         dbg!("ping");
-        self.send_packet(CPingResponse::new(ping_request.payload));
+        self.send_packet(CPingResponse::new(ping_request.payload))
+            .unwrap_or_else(|e| self.kick(&e.to_string()));
         self.close();
     }
 
@@ -88,7 +94,8 @@ impl ClientPacketProcessor for Client {
             &verify_token,
             false, // TODO
         );
-        self.send_packet(packet);
+        self.send_packet(packet)
+            .unwrap_or_else(|e| self.kick(&e.to_string()));
     }
 
     fn handle_encryption_response(
@@ -97,16 +104,20 @@ impl ClientPacketProcessor for Client {
         encryption_response: SEncryptionResponse,
     ) {
         dbg!("encryption response");
-        // should be impossible
-        if self.uuid.is_none() || self.name.is_none() {
-            self.kick("UUID or Name is none");
-            return;
-        }
         self.enable_encryption(server, encryption_response.shared_secret)
-            .unwrap();
+            .unwrap_or_else(|e| self.kick(&e.to_string()));
 
-        let packet = CLoginSuccess::new(self.uuid.unwrap(), self.name.clone().unwrap(), 0, false);
+        if let Some(uuid) = self.uuid {
-        self.send_packet(packet);
+            if let Some(name) = &self.name {
+                let packet = CLoginSuccess::new(uuid, name.clone(), 0, false);
+                self.send_packet(packet)
+                    .unwrap_or_else(|e| self.kick(&e.to_string()));
+            } else {
+                self.kick("Name is none");
+            }
+        } else {
+            self.kick("UUID is none");
+        }
     }
 
     fn handle_plugin_response(
@@ -122,13 +133,14 @@ impl ClientPacketProcessor for Client {
         _login_acknowledged: SLoginAcknowledged,
     ) {
         self.connection_state = ConnectionState::Config;
-        Server::send_brand(self);
+        Server::send_brand(self).unwrap_or_else(|e| self.kick(&e.to_string()));
         // known data packs
         self.send_packet(CKnownPacks::new(&[KnownPack {
             namespace: "minecraft",
             id: "core",
             version: "1.21",
-        }]));
+        }]))
+        .unwrap_or_else(|e| self.kick(&e.to_string()));
         dbg!("login achnowlaged");
     }
     fn handle_client_information(
@@ -169,11 +181,13 @@ impl ClientPacketProcessor for Client {
                     has_data: true,
                     }, */
             ],
-        ));
+        ))
+        .unwrap_or_else(|e| self.kick(&e.to_string()));
 
         // We are done with configuring
         dbg!("finish config");
-        self.send_packet(CFinishConfig::new());
+        self.send_packet(CFinishConfig::new())
+            .unwrap_or_else(|e| self.kick(&e.to_string()));
     }
 
     fn handle_config_acknowledged(

pumpkin/src/client/mod.rs

@@ -20,14 +20,15 @@ use crate::{
 };
 
 use crate::protocol::ConnectionState;
-use anyhow::Context;
 use mio::{event::Event, net::TcpStream, Token};
 use packet_decoder::PacketDecoder;
 use packet_encoder::PacketEncoder;
 use rsa::Pkcs1v15Encrypt;
 use std::io::Read;
+use thiserror::Error;
 
 mod client_packet;
+mod player_packet;
 
 mod packet_decoder;
 mod packet_encoder;
@@ -90,25 +91,30 @@ impl Client {
         &mut self,
         server: &mut Server,
         shared_secret: Vec<u8>,
-    ) -> anyhow::Result<()> {
+    ) -> Result<(), EncryptionError> {
         self.encrytion = true;
         let shared_secret = server
             .private_key
             .decrypt(Pkcs1v15Encrypt, &shared_secret)
-            .context("failed to decrypt shared secret")?;
+            .map_err(|_| EncryptionError::FailedDecrypt)?;
         let crypt_key: [u8; 16] = shared_secret
             .as_slice()
             .try_into()
-            .context("shared secret has the wrong length")?;
+            .map_err(|_| EncryptionError::SharedWrongLength)?;
         self.dec.enable_encryption(&crypt_key);
         self.enc.enable_encryption(&crypt_key);
         Ok(())
     }
 
-    pub fn send_packet<P: ClientPacket>(&mut self, packet: P) {
+    /// Im many cases we want to kick the Client when an Packet Error occours, But especially in the Client state we will not try to kick when not important packets
+    /// e.g Postion, Rotation... has not been send
+    pub fn send_packet<P: ClientPacket>(&mut self, packet: P) -> Result<(), PacketError> {
         dbg!("sending packet");
-        self.enc.append_packet(packet).unwrap();
+        self.enc.append_packet(packet)?;
-        self.connection.write_all(&self.enc.take()).unwrap();
+        self.connection
+            .write_all(&self.enc.take())
+            .map_err(|_| PacketError::ConnectionWrite)?;
+        Ok(())
     }
 
     pub fn process_packets(&mut self, server: &mut Server) {
@@ -180,8 +186,8 @@ impl Client {
         }
     }
 
-    /// Returns `true` if the connection is done.
+    /// Returns `true` if the connection is closed.
-    pub fn poll(&mut self, server: &mut Server, event: &Event) -> anyhow::Result<bool> {
+    pub fn poll(&mut self, server: &mut Server, event: &Event) -> Result<bool, io::Error> {
         if event.is_readable() {
             let mut received_data = vec![0; 4096];
             let mut bytes_read = 0;
@@ -205,16 +211,21 @@ impl Client {
                     Err(ref err) if would_block(err) => break,
                     Err(ref err) if interrupted(err) => continue,
                     // Other errors we'll consider fatal.
-                    Err(err) => return anyhow::bail!(err),
+                    Err(err) => return Err(err),
                 }
             }
 
             if bytes_read != 0 {
                 self.dec.reserve(4096);
                 self.dec.queue_slice(&received_data[..bytes_read]);
-                if let Some(packet) = self.dec.decode()? {
+                match self.dec.decode() {
-                    self.add_packet(packet);
+                    Ok(packet) => {
-                    self.process_packets(server);
+                        if let Some(packet) = packet {
+                            self.add_packet(packet);
+                            self.process_packets(server);
+                        }
+                    }
+                    Err(err) => self.kick(&err.to_string()),
                 }
                 self.dec.clear();
             }
@@ -226,10 +237,12 @@ impl Client {
         // Todo
         match self.connection_state {
             ConnectionState::Login => {
-                self.send_packet(CLoginDisconnect::new(reason));
+                self.send_packet(CLoginDisconnect::new(reason))
+                    .unwrap_or_else(|_| self.close());
             }
             ConnectionState::Config => {
-                self.send_packet(CConfigDisconnect::new(reason));
+                self.send_packet(CConfigDisconnect::new(reason))
+                    .unwrap_or_else(|_| self.close());
             }
             _ => {
                 log::warn!("Cant't kick in {:?} State", self.connection_state)
@@ -244,6 +257,32 @@ impl Client {
     }
 }
 
+#[derive(Error, Debug)]
+pub enum EncryptionError {
+    #[error("failed to decrypt shared secret")]
+    FailedDecrypt,
+    #[error("shared secret has the wrong length")]
+    SharedWrongLength,
+}
+
+#[derive(Error, Debug)]
+pub enum PacketError {
+    #[error("failed to decode packet ID")]
+    DecodeID,
+    #[error("failed to encode packet ID")]
+    EncodeID,
+    #[error("failed to write encoded packet")]
+    EncodeFailedWrite,
+    #[error("failed to write encoded packet to connection")]
+    ConnectionWrite,
+    #[error("packet exceeds maximum length")]
+    TooLong,
+    #[error("packet length is out of bounds")]
+    OutOfBounds,
+    #[error("malformed packet length VarInt")]
+    MailformedLength,
+}
+
 fn would_block(err: &io::Error) -> bool {
     err.kind() == io::ErrorKind::WouldBlock
 }

pumpkin/src/client/packet_decoder.rs

@@ -1,9 +1,8 @@
 use aes::cipher::{generic_array::GenericArray, BlockDecryptMut, BlockSizeUser, KeyIvInit};
-use anyhow::{bail, ensure, Context};
 use bytes::{Buf, BytesMut};
 
 use crate::{
-    client::MAX_PACKET_SIZE,
+    client::{PacketError, MAX_PACKET_SIZE},
     protocol::{bytebuf::ByteBuffer, RawPacket, VarInt32, VarIntDecodeError},
 };
 
@@ -18,19 +17,18 @@ pub struct PacketDecoder {
 }
 
 impl PacketDecoder {
-    pub fn decode(&mut self) -> anyhow::Result<Option<RawPacket>> {
+    pub fn decode(&mut self) -> Result<Option<RawPacket>, PacketError> {
         let mut r = &self.buf[..];
 
         let packet_len = match VarInt32::decode_partial(&mut r) {
             Ok(len) => len,
             Err(VarIntDecodeError::Incomplete) => return Ok(None),
-            Err(VarIntDecodeError::TooLarge) => bail!("malformed packet length VarInt"),
+            Err(VarIntDecodeError::TooLarge) => Err(PacketError::MailformedLength)?,
         };
 
-        ensure!(
+        if !(0..=MAX_PACKET_SIZE).contains(&packet_len) {
-            (0..=MAX_PACKET_SIZE).contains(&packet_len),
+            Err(PacketError::OutOfBounds)?
-            "packet length of {packet_len} is out of bounds"
+        }
-        );
 
         if r.len() < packet_len as usize {
             // Not enough data arrived yet.
@@ -48,7 +46,7 @@ impl PacketDecoder {
 
         r = &data[..];
         let packet_id = VarInt32::decode(&mut r)
-            .context("failed to decode packet ID")?
+            .map_err(|_| PacketError::DecodeID)?
             .0;
 
         data.advance(data.len() - r.len());

pumpkin/src/client/packet_encoder.rs

@@ -1,7 +1,6 @@
 use std::io::Write;
 
 use aes::cipher::{generic_array::GenericArray, BlockEncryptMut, BlockSizeUser, KeyIvInit};
-use anyhow::{ensure, Context};
 use bytes::{BufMut, BytesMut};
 
 use crate::{
@@ -9,6 +8,8 @@ use crate::{
     protocol::{bytebuf::ByteBuffer, ClientPacket, VarInt32},
 };
 
+use super::PacketError;
+
 type Cipher = cfb8::Encryptor<aes::Aes128>;
 
 // Encoder: Server -> Client
@@ -20,7 +21,7 @@ pub struct PacketEncoder {
 }
 
 impl PacketEncoder {
-    pub fn append_packet<P: ClientPacket>(&mut self, packet: P) -> anyhow::Result<()> {
+    pub fn append_packet<P: ClientPacket>(&mut self, packet: P) -> Result<(), PacketError> {
         let start_len = self.buf.len();
 
         let mut writer = (&mut self.buf).writer();
@@ -28,10 +29,12 @@ impl PacketEncoder {
         let mut packet_buf = ByteBuffer::empty();
         VarInt32(P::PACKET_ID)
             .encode(&mut writer)
-            .context("failed to encode packet ID")?;
+            .map_err(|_| PacketError::EncodeID)?;
         packet.write(&mut packet_buf);
 
-        writer.write(packet_buf.buf()).unwrap();
+        writer
+            .write(packet_buf.buf())
+            .map_err(|_| PacketError::EncodeFailedWrite)?;
 
         let data_len = self.buf.len() - start_len;
 
@@ -39,10 +42,9 @@ impl PacketEncoder {
         }
         let packet_len = data_len;
 
-        ensure!(
+        if packet_len >= MAX_PACKET_SIZE as usize {
-            packet_len <= MAX_PACKET_SIZE as usize,
+            Err(PacketError::TooLong)?
-            "packet exceeds maximum length"
+        }
-        );
 
         let packet_len_size = VarInt32(packet_len as i32).written_size();
 
@@ -51,7 +53,9 @@ impl PacketEncoder {
             .copy_within(start_len..start_len + data_len, start_len + packet_len_size);
 
         let front = &mut self.buf[start_len..];
-        VarInt32(packet_len as i32).encode(front)?;
+        VarInt32(packet_len as i32)
+            .encode(front)
+            .map_err(|_| PacketError::EncodeID)?;
         Ok(())
     }
 

pumpkin/src/client/player_packet.rs

@@ -0,0 +1,2 @@
+// implement player packets
+pub trait PlayerPacketProcessor {}

pumpkin/src/protocol/mod.rs

@@ -1,9 +1,9 @@
-use std::io::{Read, Write};
+use std::io::{self, Write};
 
-use anyhow::bail;
 use bytebuf::ByteBuffer;
-use bytes::{Buf, BytesMut};
+use bytes::Buf;
-use serde::{Deserialize, Serialize};
+use serde::Serialize;
+use thiserror::Error;
 
 pub mod bytebuf;
 mod registry;
@@ -42,7 +42,7 @@ impl VarInt32 {
         Err(VarIntDecodeError::TooLarge)
     }
 
-    pub fn encode(&self, mut w: impl Write) -> anyhow::Result<()> {
+    pub fn encode(&self, mut w: impl Write) -> Result<(), io::Error> {
         let x = self.0 as u64;
         let stage1 = (x & 0x000000000000007f)
             | ((x & 0x0000000000003f80) << 1)
@@ -63,11 +63,10 @@ impl VarInt32 {
         let bytes = merged.to_le_bytes();
 
         w.write_all(unsafe { bytes.get_unchecked(..bytes_needed as usize) })?;
-
         Ok(())
     }
 
-    pub fn decode(r: &mut &[u8]) -> anyhow::Result<Self> {
+    pub fn decode(r: &mut &[u8]) -> Result<Self, VarIntDecodeError> {
         let mut val = 0;
         for i in 0..Self::MAX_SIZE {
             let byte = r.get_u8();
@@ -76,13 +75,15 @@ impl VarInt32 {
                 return Ok(VarInt32(val));
             }
         }
-        bail!("VarInt is too large")
+        Err(VarIntDecodeError::TooLarge)
     }
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[derive(Copy, Clone, PartialEq, Eq, Debug, Error)]
 pub enum VarIntDecodeError {
+    #[error("incomplete VarInt decode")]
     Incomplete,
+    #[error("VarInt is too large")]
     TooLarge,
 }
 

pumpkin/src/server.rs

@@ -1,5 +1,5 @@
 use std::{
-    io::Cursor,
+    io::{self, Cursor},
     sync::atomic::{AtomicI32, Ordering},
 };
 
@@ -9,7 +9,7 @@ use rsa::{rand_core::OsRng, traits::PublicKeyParts, RsaPrivateKey, RsaPublicKey}
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    client::Client,
+    client::{Client, PacketError},
     configuration::{AdvancedConfiguration, BasicConfiguration},
     entity::{
         player::{GameMode, Player},
@@ -77,10 +77,9 @@ impl Server {
     pub fn poll(
         &mut self,
         client: &mut Client,
-        poll: &Poll,
+        _poll: &Poll,
         event: &Event,
-    ) -> anyhow::Result<bool> {
+    ) -> Result<bool, io::Error> {
-        let _ = poll;
         // todo, Poll players in every world
         client.poll(self, event)
     }
@@ -92,30 +91,32 @@ impl Server {
             },
         };
 
-        client.send_packet(CLogin::new(
+        client
-            player.entity_id(),
+            .send_packet(CLogin::new(
-            self.difficulty == Difficulty::Hard,
+                player.entity_id(),
-            1,
+                self.difficulty == Difficulty::Hard,
-            vec!["minecraft:overworld".into()],
+                1,
-            self.max_players as VarInt,
+                vec!["minecraft:overworld".into()],
-            8, //  view distance todo
+                self.max_players as VarInt,
-            8, // sim view dinstance todo
+                8, //  view distance todo
-            false,
+                8, // sim view dinstance todo
-            false,
+                false,
-            false,
+                false,
-            1,
+                false,
-            "minecraft:overworld".into(),
+                1,
-            0, // seed
+                "minecraft:overworld".into(),
-            GameMode::Survival,
+                0, // seed
-            GameMode::Undefined,
+                GameMode::Survival,
-            false,
+                GameMode::Undefined,
-            false,
+                false,
-            false, // deth loc
+                false,
-            None,
+                false, // deth loc
-            None,
+                None,
-            0,
+                None,
-            false,
+                0,
-        ));
+                false,
+            ))
+            .unwrap_or_else(|e| client.kick(&e.to_string()));
 
         client.player = Some(player);
     }
@@ -125,7 +126,7 @@ impl Server {
         self.entity_id.fetch_add(1, Ordering::SeqCst)
     }
 
-    pub fn send_brand(client: &mut Client) {
+    pub fn send_brand(client: &mut Client) -> Result<(), PacketError> {
         // send server brand
         let brand = "pumpkin";
         let mut buf = vec![];