Minior Code cleanups

.github/workflows/rust.yml

@@ -70,6 +70,7 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: pumpkin-${{ matrix.os }}
+          compression-level: 9
           path: target/${{ matrix.target }}/release/pumpkin*
   clippy_release:
     name: Run lints in release mode

docs/developer/networking.md

@@ -266,6 +266,9 @@ For Players:
 }
 ```
 
+### Compression
+Minecraft Packets **can** use the ZLib compression for decoding/encoding there is usally a threshold set when compression is applied, This most often affects Chunk Packets.
+
 ### Porting
 
 To port to a new Minecraft version, You can compare difference in Protocol on wiki.vg https://wiki.vg/index.php?title=Protocol&action=history

fuzz/Cargo.toml

@@ -0,0 +1,21 @@
+[package]
+name = "pumpkin-fuzz"
+version = "0.0.0"
+publish = false
+edition = "2021"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+pumpkin = { path = "../"}
+
+libfuzzer-sys = "0.4"
+
+
+[[bin]]
+name = "fuzz_target_1"
+path = "fuzz_targets/fuzz_target_1.rs"
+test = false
+doc = false
+bench = false

fuzz/fuzz_targets/client.rs

@@ -0,0 +1,7 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+
+fuzz_target!(|data: &[u8]| {
+    // fuzzed code goes here
+});

pumpkin-world/src/cylindrical_chunk_iterator.rs

@@ -22,12 +22,10 @@ impl Cylindrical {
         just_removed: impl FnMut(Vector2<i32>),
         ignore: bool,
     ) {
-        let min_x = old_cylindrical.get_left().min(new_cylindrical.get_left());
-        let max_x = old_cylindrical.get_right().max(new_cylindrical.get_right());
-        let min_z = old_cylindrical
-            .get_bottom()
-            .min(new_cylindrical.get_bottom());
-        let max_z = old_cylindrical.get_top().max(new_cylindrical.get_top());
+        let min_x = old_cylindrical.left().min(new_cylindrical.left());
+        let max_x = old_cylindrical.right().max(new_cylindrical.right());
+        let min_z = old_cylindrical.bottom().min(new_cylindrical.bottom());
+        let max_z = old_cylindrical.top().max(new_cylindrical.top());
 
         for x in min_x..=max_x {
             for z in min_z..=max_z {
@@ -55,19 +53,19 @@ impl Cylindrical {
         }
     }
 
-    fn get_left(&self) -> i32 {
+    fn left(&self) -> i32 {
         self.center.x - self.view_distance - 1
     }
 
-    fn get_bottom(&self) -> i32 {
+    fn bottom(&self) -> i32 {
         self.center.z - self.view_distance - 1
     }
 
-    fn get_right(&self) -> i32 {
+    fn right(&self) -> i32 {
         self.center.x + self.view_distance + 1
     }
 
-    fn get_top(&self) -> i32 {
+    fn top(&self) -> i32 {
         self.center.z + self.view_distance + 1
     }
 

pumpkin/src/client/authentication.rs

@@ -1,4 +1,4 @@
-use std::{collections::HashMap, net::IpAddr, sync::Arc};
+use std::{collections::HashMap, net::IpAddr};
 
 use base64::{engine::general_purpose, Engine};
 use pumpkin_config::{auth::TextureConfig, ADVANCED_CONFIG};
@@ -9,8 +9,6 @@ use serde::Deserialize;
 use thiserror::Error;
 use uuid::Uuid;
 
-use crate::server::Server;
-
 #[derive(Deserialize, Clone, Debug)]
 #[expect(dead_code)]
 #[serde(rename_all = "camelCase")]
@@ -55,10 +53,9 @@ pub async fn authenticate(
     username: &str,
     server_hash: &str,
     ip: &IpAddr,
-    server: &Arc<Server>,
+    auth_client: &reqwest::Client,
 ) -> Result<GameProfile, AuthError> {
     assert!(ADVANCED_CONFIG.authentication.enabled);
-    assert!(server.auth_client.is_some());
     let address = if ADVANCED_CONFIG.authentication.prevent_proxy_connections {
         ADVANCED_CONFIG
             .authentication
@@ -73,10 +70,6 @@ pub async fn authenticate(
             .replace("{username}", username)
             .replace("{server_hash}", server_hash)
     };
-    let auth_client = server
-        .auth_client
-        .as_ref()
-        .ok_or(AuthError::MissingAuthClient)?;
 
     let response = auth_client
         .get(address)
@@ -92,7 +85,7 @@ pub async fn authenticate(
     Ok(profile)
 }
 
-pub fn unpack_textures(property: &Property, config: &TextureConfig) -> Result<(), TextureError> {
+pub fn validate_textures(property: &Property, config: &TextureConfig) -> Result<(), TextureError> {
     let from64 = general_purpose::STANDARD
         .decode(&property.value)
         .map_err(|e| TextureError::DecodeError(e.to_string()))?;
@@ -134,6 +127,12 @@ pub enum AuthError {
     FailedResponse,
     #[error("Failed to verify username")]
     UnverifiedUsername,
+    #[error("You are banned from Authentication servers")]
+    Banned,
+    #[error("Texture Error {0}")]
+    TextureError(TextureError),
+    #[error("You have disallowed actions from Authentication servers")]
+    DisallowedAction,
     #[error("Failed to parse JSON into Game Profile")]
     FailedParse,
     #[error("Unknown Status Code")]

pumpkin/src/client/client_packet.rs

@@ -1,5 +1,3 @@
-use std::sync::Arc;
-
 use num_traits::FromPrimitive;
 use pumpkin_config::{ADVANCED_CONFIG, BASIC_CONFIG};
 use pumpkin_core::text::TextComponent;
@@ -20,20 +18,20 @@ use pumpkin_protocol::{
 use uuid::Uuid;
 
 use crate::{
-    client::authentication::{self, GameProfile},
+    client::authentication::{self, validate_textures, GameProfile},
     entity::player::{ChatMode, Hand},
     proxy::velocity::velocity_login,
     server::{Server, CURRENT_MC_VERSION},
 };
 
-use super::{authentication::unpack_textures, Client, PlayerConfig};
+use super::{authentication::AuthError, Client, PlayerConfig};
 
 /// Processes incoming Packets from the Client to the Server
 /// Implements the `Client` Packets
 /// NEVER TRUST THE CLIENT. HANDLE EVERY ERROR, UNWRAP/EXPECT
 /// TODO: REMOVE ALL UNWRAPS
 impl Client {
-    pub fn handle_handshake(&self, _server: &Arc<Server>, handshake: SHandShake) {
+    pub fn handle_handshake(&self, handshake: SHandShake) {
         dbg!("handshake");
         let version = handshake.protocol_version.0;
         self.protocol_version
@@ -54,11 +52,11 @@ impl Client {
         }
     }
 
-    pub fn handle_status_request(&self, server: &Arc<Server>, _status_request: SStatusRequest) {
+    pub fn handle_status_request(&self, server: &Server, _status_request: SStatusRequest) {
         self.send_packet(&server.get_status());
     }
 
-    pub fn handle_ping_request(&self, _server: &Arc<Server>, ping_request: SStatusPingRequest) {
+    pub fn handle_ping_request(&self, ping_request: SStatusPingRequest) {
         dbg!("ping");
         self.send_packet(&CPingResponse::new(ping_request.payload));
         self.close();
@@ -71,7 +69,7 @@ impl Client {
                 .all(|c| c > 32_u8 as char && c < 127_u8 as char)
     }
 
-    pub fn handle_login_start(&self, server: &Arc<Server>, login_start: SLoginStart) {
+    pub fn handle_login_start(&self, server: &Server, login_start: SLoginStart) {
         log::debug!("login start, State {:?}", self.connection_state);
 
         if !Self::is_valid_player_name(&login_start.name) {
@@ -102,7 +100,7 @@ impl Client {
 
     pub async fn handle_encryption_response(
         &self,
-        server: &Arc<Server>,
+        server: &Server,
         encryption_response: SEncryptionResponse,
     ) {
         let shared_secret = server.decrypt(&encryption_response.shared_secret).unwrap();
@@ -113,49 +111,16 @@ impl Client {
         let mut gameprofile = self.gameprofile.lock();
 
         if BASIC_CONFIG.online_mode {
-            let hash = server.digest_secret(&shared_secret);
-            let ip = self.address.lock().ip();
-
-            match authentication::authenticate(
-                &gameprofile.as_ref().unwrap().name,
-                &hash,
-                &ip,
-                server,
-            )
-            .await
+            match self
+                .autenticate(server, &shared_secret, &gameprofile.as_ref().unwrap().name)
+                .await
             {
-                Ok(profile) => {
-                    // Check if player should join
-                    if let Some(actions) = &profile.profile_actions {
-                        if !ADVANCED_CONFIG
-                            .authentication
-                            .player_profile
-                            .allow_banned_players
-                        {
-                            if !actions.is_empty() {
-                                self.kick("Your account can't join");
-                            }
-                        } else {
-                            for allowed in &ADVANCED_CONFIG
-                                .authentication
-                                .player_profile
-                                .allowed_actions
-                            {
-                                if !actions.contains(allowed) {
-                                    self.kick("Your account can't join");
-                                }
-                            }
-                        }
-                    }
-                    *gameprofile = Some(profile);
+                Ok(profile) => *gameprofile = Some(profile),
+                Err(e) => {
+                    self.kick(&e.to_string());
                 }
-                Err(e) => self.kick(&e.to_string()),
             }
         }
-        for property in &gameprofile.as_ref().unwrap().properties {
-            unpack_textures(property, &ADVANCED_CONFIG.authentication.textures)
-                .unwrap_or_else(|e| self.kick(&e.to_string()));
-        }
 
         // enable compression
         if ADVANCED_CONFIG.packet_compression.enabled {
@@ -172,16 +137,54 @@ impl Client {
         }
     }
 
-    pub fn handle_plugin_response(
+    async fn autenticate(
         &self,
-        _server: &Arc<Server>,
-        _plugin_response: SLoginPluginResponse,
-    ) {
+        server: &Server,
+        shared_secret: &[u8],
+        username: &str,
+    ) -> Result<GameProfile, AuthError> {
+        if let Some(auth_client) = &server.auth_client {
+            let hash = server.digest_secret(shared_secret);
+            let ip = self.address.lock().ip();
+
+            let profile = authentication::authenticate(username, &hash, &ip, auth_client).await?;
+            // Check if player should join
+            if let Some(actions) = &profile.profile_actions {
+                if !ADVANCED_CONFIG
+                    .authentication
+                    .player_profile
+                    .allow_banned_players
+                {
+                    if !actions.is_empty() {
+                        self.kick("Your account can't join");
+                    }
+                } else {
+                    for allowed in &ADVANCED_CONFIG
+                        .authentication
+                        .player_profile
+                        .allowed_actions
+                    {
+                        if !actions.contains(allowed) {
+                            self.kick("Your account can't join");
+                        }
+                    }
+                }
+            }
+            // validate textures
+            for property in &profile.properties {
+                validate_textures(property, &ADVANCED_CONFIG.authentication.textures)
+                    .map_err(AuthError::TextureError)?;
+            }
+            return Ok(profile);
+        }
+        Err(AuthError::MissingAuthClient)
     }
 
+    pub fn handle_plugin_response(&self, _plugin_response: SLoginPluginResponse) {}
+
     pub fn handle_login_acknowledged(
         &self,
-        server: &Arc<Server>,
+        server: &Server,
         _login_acknowledged: SLoginAcknowledged,
     ) {
         self.connection_state.store(ConnectionState::Config);
@@ -215,11 +218,7 @@ impl Client {
         }]));
         dbg!("login acknowledged");
     }
-    pub fn handle_client_information_config(
-        &self,
-        _server: &Arc<Server>,
-        client_information: SClientInformationConfig,
-    ) {
+    pub fn handle_client_information_config(&self, client_information: SClientInformationConfig) {
         dbg!("got client settings");
         if let (Some(main_hand), Some(chat_mode)) = (
             Hand::from_i32(client_information.main_hand.into()),
@@ -240,7 +239,7 @@ impl Client {
         }
     }
 
-    pub fn handle_plugin_message(&self, _server: &Arc<Server>, plugin_message: SPluginMessage) {
+    pub fn handle_plugin_message(&self, plugin_message: SPluginMessage) {
         if plugin_message.channel.starts_with("minecraft:brand")
             || plugin_message.channel.starts_with("MC|Brand")
         {
@@ -252,7 +251,7 @@ impl Client {
         }
     }
 
-    pub fn handle_known_packs(&self, server: &Arc<Server>, _config_acknowledged: SKnownPacks) {
+    pub fn handle_known_packs(&self, server: &Server, _config_acknowledged: SKnownPacks) {
         for registry in &server.cached_registry {
             self.send_packet(&CRegistryData::new(
                 &registry.registry_id,
@@ -265,11 +264,7 @@ impl Client {
         self.send_packet(&CFinishConfig::new());
     }
 
-    pub async fn handle_config_acknowledged(
-        &self,
-        _server: &Arc<Server>,
-        _config_acknowledged: SAcknowledgeFinishConfig,
-    ) {
+    pub async fn handle_config_acknowledged(&self, _config_acknowledged: SAcknowledgeFinishConfig) {
         dbg!("config acknowledged");
         self.connection_state.store(ConnectionState::Play);
         self.make_player

pumpkin/src/client/container.rs

@@ -20,7 +20,7 @@ use pumpkin_world::item::ItemStack;
 use std::sync::Arc;
 
 impl Player {
-    pub fn open_container(&self, server: &Arc<Server>, minecraft_menu_id: &str) {
+    pub fn open_container(&self, server: &Server, minecraft_menu_id: &str) {
         let inventory = self.inventory.lock();
         inventory
             .state_id