diff --git a/pumpkin-protocol/src/bedrock/client/level_chunk.rs b/pumpkin-protocol/src/bedrock/client/level_chunk.rs index d7d36900e..ebaee3913 100644 --- a/pumpkin-protocol/src/bedrock/client/level_chunk.rs +++ b/pumpkin-protocol/src/bedrock/client/level_chunk.rs @@ -34,7 +34,12 @@ impl PacketWrite for CLevelChunk<'_> { let mut chunk_data = Vec::new(); let data_write = &mut chunk_data; - let block_sections = self.chunk.section.block_sections.read().unwrap(); + let block_sections = self + .chunk + .section + .block_sections + .read() + .map_err(|_| Error::other("block_sections read lock poisoned"))?; let min_y_section = (self.chunk.section.min_y >> 4) as i8; for (i, block_palette) in block_sections.iter().enumerate() { diff --git a/pumpkin-protocol/src/bedrock/packet_decoder.rs b/pumpkin-protocol/src/bedrock/packet_decoder.rs index 03868fa91..4bd830875 100644 --- a/pumpkin-protocol/src/bedrock/packet_decoder.rs +++ b/pumpkin-protocol/src/bedrock/packet_decoder.rs @@ -23,7 +23,7 @@ impl DecryptionReader { pub fn upgrade(self, cipher: Aes128Cfb8Dec) -> Self { match self { Self::None(stream) => Self::Decrypt(Box::new(StreamDecryptor::new(cipher, stream))), - Self::Decrypt(_) => panic!("Cannot upgrade a stream that already has a cipher!"), + Self::Decrypt(_) => self, } } } @@ -61,6 +61,12 @@ impl Default for UDPNetworkDecoder { } } +use thiserror::Error; + +#[derive(Debug, Error)] +#[error("Encryption already enabled")] +pub struct EncryptionAlreadyEnabledError; + impl UDPNetworkDecoder { #[must_use] pub const fn new() -> Self { @@ -72,12 +78,16 @@ impl UDPNetworkDecoder { } /// NOTE: Encryption can only be set; a minecraft stream cannot go back to being unencrypted - pub const fn set_encryption(&mut self, _key: &[u8; 16]) { + pub const fn set_encryption( + &mut self, + _key: &[u8; 16], + ) -> Result<(), EncryptionAlreadyEnabledError> { // if matches!(self.reader, DecryptionReader::Decrypt(_)) { - // panic!("Cannot upgrade a stream that already has a cipher!"); + // return Err(EncryptionAlreadyEnabledError); // } - // let cipher = Aes128Cfb8Dec::new_from_slices(key, key).expect("invalid key"); + // let cipher = Aes128Cfb8Dec::new_from_slices(key, key).map_err(|_| ()); // take_mut::take(&mut self.reader, |decoder| decoder.upgrade(cipher)); + Ok(()) } pub async fn get_packet_payload( diff --git a/pumpkin-protocol/src/bedrock/packet_encoder.rs b/pumpkin-protocol/src/bedrock/packet_encoder.rs index be2f86b67..965722fcd 100644 --- a/pumpkin-protocol/src/bedrock/packet_encoder.rs +++ b/pumpkin-protocol/src/bedrock/packet_encoder.rs @@ -103,12 +103,16 @@ impl UDPNetworkEncoder { } /// NOTE: Encryption can only be set; a minecraft stream cannot go back to being unencrypted - pub const fn set_encryption(&mut self, _key: &[u8; 16]) { + pub const fn set_encryption( + &mut self, + _key: &[u8; 16], + ) -> Result<(), crate::bedrock::packet_decoder::EncryptionAlreadyEnabledError> { // if matches!(self.writer, EncryptionWriter::Encrypt(_)) { - // panic!("Cannot upgrade a stream that already has a cipher!"); + // return Err(crate::bedrock::packet_decoder::EncryptionAlreadyEnabledError); // } - // let cipher = Aes128Cfb8Enc::new_from_slices(key, key).expect("invalid key"); + // let cipher = Aes128Cfb8Enc::new_from_slices(key, key).map_err(|_| ()); // take_mut::take(&mut self.writer, |encoder| encoder.upgrade(cipher)); + Ok(()) } pub fn write_game_packet( @@ -132,20 +136,26 @@ impl UDPNetworkEncoder { inner_buffer .write_var_uint(&VarUInt(total_content_length)) - .unwrap(); - inner_buffer.write_var_uint(&header_varint).unwrap(); + .map_err(|_| Error::other("Failed to write total content length"))?; + inner_buffer + .write_var_uint(&header_varint) + .map_err(|_| Error::other("Failed to write header varint"))?; inner_buffer.write_all(packet_payload)?; // Handle Outer Container - writer.write_u8(0xfe).unwrap(); // Bedrock Game Packet Header + writer + .write_u8(0xfe) + .map_err(|e| Error::other(e.to_string()))?; // Bedrock Game Packet Header if let Some((_threshold, level)) = self.compression { // Write Compression Method (0x00 for Zlib) - writer.write_u8(0x00).unwrap(); + writer + .write_u8(0x00) + .map_err(|e| Error::other(e.to_string()))?; let mut encoder = DeflateEncoder::new(Vec::new(), Compression::new(level)); - encoder.write_all(&inner_buffer).unwrap(); - let compressed_data = encoder.finish().unwrap(); + encoder.write_all(&inner_buffer)?; + let compressed_data = encoder.finish()?; writer.write_all(&compressed_data)?; } else { @@ -172,7 +182,7 @@ mod tests { use std::io::Cursor; #[tokio::test] - async fn bedrock_compression_cycle() { + async fn bedrock_compression_cycle() -> Result<(), Box> { let mut encoder = UDPNetworkEncoder::new(); encoder.set_compression((256, 6)); @@ -180,24 +190,23 @@ mod tests { let payload = b"Hello Bedrock Compression!"; let mut encoded_buf = Vec::new(); - encoder - .write_game_packet( - packet_id, - SubClient::Main, - SubClient::Main, - payload, - &mut encoded_buf, - ) - .unwrap(); + encoder.write_game_packet( + packet_id, + SubClient::Main, + SubClient::Main, + payload, + &mut encoded_buf, + )?; let mut decoder = UDPNetworkDecoder::new(); decoder.set_compression(256); - let decompressed_payload = decoder.get_packet_payload(encoded_buf).await.unwrap(); + let decompressed_payload = decoder.get_packet_payload(encoded_buf).await?; let mut cursor = Cursor::new(decompressed_payload); - let raw_packet = decoder.get_game_packet(&mut cursor).unwrap(); + let raw_packet = decoder.get_game_packet(&mut cursor)?; assert_eq!(raw_packet.id, packet_id as i32); assert_eq!(raw_packet.payload.as_ref(), payload); + Ok(()) } } diff --git a/pumpkin-protocol/src/codec/data_component.rs b/pumpkin-protocol/src/codec/data_component.rs index 9dc5b5b9e..9c57fd80c 100644 --- a/pumpkin-protocol/src/codec/data_component.rs +++ b/pumpkin-protocol/src/codec/data_component.rs @@ -31,15 +31,16 @@ pub fn data_to_proto_sound(id_or: &IdOr) -> crate::IdOr) -> IdOr { +pub fn proto_to_data_sound(id_or: &crate::IdOr) -> Option> { match id_or { crate::IdOr::Id(id) => { - IdOr::Id(Sound::from_name(Sound::NAMES[*id as usize]).expect("Invalid sound id")) + let name = Sound::NAMES.get(*id as usize)?; + Some(IdOr::Id(Sound::from_name(name)?)) } - crate::IdOr::Value(sound) => IdOr::Value(SoundEvent { + crate::IdOr::Value(sound) => Some(IdOr::Value(SoundEvent { sound_name: sound.sound_name.clone(), range: sound.range, - }), + })), } } @@ -168,14 +169,12 @@ fn serialize_status_effects( seq.serialize_field::("", &VarInt(effects.len() as i32))?; for effect in effects { - seq.serialize_field::( - "", - &VarInt( - StatusEffect::from_minecraft_name(&effect.effect_id) - .unwrap() - .registry_id() as i32, - ), - )?; + let effect_id = StatusEffect::from_minecraft_name(&effect.effect_id) + .ok_or_else(|| { + serde::ser::Error::custom(format!("Invalid status effect: {}", effect.effect_id)) + })? + .registry_id(); + seq.serialize_field::("", &VarInt(effect_id as i32))?; // Effect parameters seq.serialize_field::("", &VarInt::from(effect.amplifier))?; seq.serialize_field::("", &VarInt::from(effect.duration))?; @@ -222,9 +221,10 @@ fn deserialize_consume_effect<'a, A: SeqAccess<'a>>( .ok_or(de::Error::custom( "No sound IdOr in ConsumeEffect", ))?; - Ok(ConsumeEffect::PlaySound(proto_to_data_sound( - &proto_sound_event, - ))) + Ok(ConsumeEffect::PlaySound( + proto_to_data_sound(&proto_sound_event) + .ok_or(de::Error::custom("Invalid sound in ConsumeEffect"))?, + )) } _ => Err(de::Error::custom("Invalid effect_type in ConsumeEffect")), } @@ -399,7 +399,8 @@ impl DataComponentCodec for ConsumableImpl { "No ConsumableImpl consume_particles bool!", ))?; - let sound_event = proto_to_data_sound(&proto_sound_event); + let sound_event = proto_to_data_sound(&proto_sound_event) + .ok_or(de::Error::custom("Invalid sound in ConsumableImpl"))?; let effects_len = seq .next_element::()? .ok_or(de::Error::custom("No array_len VarInt in ConsumableImpl"))? @@ -461,7 +462,8 @@ impl DataComponentCodec for EquippableImpl { .ok_or(de::Error::custom( "No EquippableImpl equip_sound IdOr!", ))?, - ); + ) + .ok_or(de::Error::custom("Invalid sound in EquippableImpl"))?; let asset_id = seq.next_element::>>()? .ok_or(de::Error::custom( @@ -502,7 +504,10 @@ impl DataComponentCodec for EquippableImpl { .ok_or(de::Error::custom( "No EquippableImpl shearing_sound IdOr!", ))?, - ); + ) + .ok_or(de::Error::custom( + "Invalid shearing sound in EquippableImpl", + ))?; Ok(Self { slot, diff --git a/pumpkin-protocol/src/codec/lp_vector_3d.rs b/pumpkin-protocol/src/codec/lp_vector_3d.rs index d9ce9dc79..d8768c300 100644 --- a/pumpkin-protocol/src/codec/lp_vector_3d.rs +++ b/pumpkin-protocol/src/codec/lp_vector_3d.rs @@ -50,12 +50,12 @@ impl LpVector3d { // Write low 16 bits (Little Endian) writer .write_all(&(packed_data as u16).to_le_bytes()) - .unwrap(); + .map_err(WritingError::IoError)?; // Write next 32 bits (Big Endian) writer .write_all(&((packed_data >> 16) as i32).to_be_bytes()) - .unwrap(); + .map_err(WritingError::IoError)?; if is_extended { let scale_tail = VarInt((scale_factor >> 2) as i32); @@ -156,7 +156,8 @@ fn from_long(quantized: i64, scale: f64) -> f64 { impl Serialize for LpVector3d { fn serialize(&self, serializer: S) -> Result { let mut buf = Vec::new(); - self.write(&mut buf).unwrap(); + self.write(&mut buf) + .map_err(|e| serde::ser::Error::custom(e.to_string()))?; serializer.serialize_bytes(&buf) } } @@ -202,10 +203,11 @@ mod tests { } #[test] - fn write_legacy_writes_three_i16_be_components() { + fn write_legacy_writes_three_i16_be_components() -> Result<(), Box> { let velocity = LpVector3d(Vector3::new(0.5, -0.5, 0.0)); let mut buf = Vec::new(); - velocity.write_legacy(&mut buf).unwrap(); + velocity.write_legacy(&mut buf)?; assert_eq!(buf, vec![0x0F, 0xA0, 0xF0, 0x60, 0x00, 0x00]); + Ok(()) } } diff --git a/pumpkin-protocol/src/java/client/play/chunk_data.rs b/pumpkin-protocol/src/java/client/play/chunk_data.rs index 40007af23..dff8c18c0 100644 --- a/pumpkin-protocol/src/java/client/play/chunk_data.rs +++ b/pumpkin-protocol/src/java/client/play/chunk_data.rs @@ -32,7 +32,11 @@ impl ClientPacket for CChunkData<'_> { // Chunk Z write.write_i32_be(self.0.z)?; - let heightmaps = self.0.heightmap.lock().unwrap(); + let heightmaps = self + .0 + .heightmap + .lock() + .map_err(|_| WritingError::Message("heightmap lock poisoned".into()))?; if version <= &MinecraftVersion::V_1_21_4 { pumpkin_nbt::serializer::to_bytes_unnamed(&*heightmaps, &mut write) .map_err(|err| WritingError::Serde(err.to_string()))?; @@ -62,8 +66,14 @@ impl ClientPacket for CChunkData<'_> { { let mut blocks_and_biomes_buf = Vec::new(); - let block_sections = self.0.section.block_sections.read().unwrap(); - let biome_sections = self.0.section.biome_sections.read().unwrap(); + let block_sections = + self.0.section.block_sections.read().map_err(|_| { + WritingError::Message("block_sections read lock poisoned".into()) + })?; + let biome_sections = + self.0.section.biome_sections.read().map_err(|_| { + WritingError::Message("biome_sections read lock poisoned".into()) + })?; for (block_palette, biome_palette) in block_sections.iter().zip(biome_sections.iter()) { let non_empty_block_count = block_palette.non_air_block_count() as i16; @@ -183,7 +193,11 @@ impl ClientPacket for CChunkData<'_> { write.write_slice(&blocks_and_biomes_buf)?; }; - let block_entities = self.0.block_entities.lock().unwrap(); + let block_entities = self + .0 + .block_entities + .lock() + .map_err(|_| WritingError::Message("block_entities lock poisoned".into()))?; write.write_var_int(&VarInt(block_entities.len() as i32))?; for block_entity in block_entities.values() { let pos = block_entity.get_position(); @@ -203,7 +217,11 @@ impl ClientPacket for CChunkData<'_> { { // Light masks include sections from -1 (below world) to num_sections (above world) // This means we need to account for 2 extra sections in the bitset - let light_engine = self.0.light_engine.lock().unwrap(); + let light_engine = self + .0 + .light_engine + .lock() + .map_err(|_| WritingError::Message("light_engine lock poisoned".into()))?; let num_sections = light_engine.sky_light.len(); let mut sky_light_empty_mask = 0u64; @@ -237,19 +255,15 @@ impl ClientPacket for CChunkData<'_> { block_light_empty_mask |= 1 << (num_sections + 1); // Write Sky Light Mask - write.write_bitset(&BitSet(Box::new([sky_light_mask.try_into().unwrap()])))?; + write.write_bitset(&BitSet(Box::new([sky_light_mask as i64])))?; // Write Block Light Mask - write.write_bitset(&BitSet(Box::new([block_light_mask.try_into().unwrap()])))?; + write.write_bitset(&BitSet(Box::new([block_light_mask as i64])))?; // Write Empty Sky Light Mask - write.write_bitset(&BitSet(Box::new([sky_light_empty_mask - .try_into() - .unwrap()])))?; + write.write_bitset(&BitSet(Box::new([sky_light_empty_mask as i64])))?; // Write Empty Block Light Mask - write.write_bitset(&BitSet(Box::new([block_light_empty_mask - .try_into() - .unwrap()])))?; + write.write_bitset(&BitSet(Box::new([block_light_empty_mask as i64])))?; - let light_data_size: VarInt = LightContainer::ARRAY_SIZE.try_into().unwrap(); + let light_data_size: VarInt = VarInt(LightContainer::ARRAY_SIZE as i32); // Write Sky Light arrays write.write_var_int(&VarInt(sky_light_mask.count_ones() as i32))?; diff --git a/pumpkin-protocol/src/java/client/play/commands.rs b/pumpkin-protocol/src/java/client/play/commands.rs index 35bd3a284..da5fd3afe 100644 --- a/pumpkin-protocol/src/java/client/play/commands.rs +++ b/pumpkin-protocol/src/java/client/play/commands.rs @@ -161,7 +161,9 @@ impl ProtoNode<'_> { .. } => { // suggestion type - let suggestion_type = &override_suggestion_type.expect("ProtoNode::FLAG_HAS_SUGGESTION_TYPE should only be set if override_suggestion_type is not `None`."); + let suggestion_type = override_suggestion_type.as_ref().ok_or_else(|| { + WritingError::Message("ProtoNode::FLAG_HAS_SUGGESTION_TYPE set but override_suggestion_type is None".into()) + })?; write.write_string(suggestion_type.resource_location())?; } _ => unimplemented!( diff --git a/pumpkin-protocol/src/java/client/play/entity_velocity.rs b/pumpkin-protocol/src/java/client/play/entity_velocity.rs index fd3d95e4d..9a7e27655 100644 --- a/pumpkin-protocol/src/java/client/play/entity_velocity.rs +++ b/pumpkin-protocol/src/java/client/play/entity_velocity.rs @@ -61,14 +61,14 @@ mod tests { codec::lp_vector_3d::{LpVector3d, encode_legacy_velocity_component}, }; - fn encode_packet(version: MinecraftVersion) -> Vec { + fn encode_packet(version: MinecraftVersion) -> Result, Box> { let packet = CEntityVelocity::new( VarInt(1), pumpkin_util::math::vector3::Vector3::new(0.5, -0.5, 0.25), ); let mut out = Vec::new(); - packet.write_packet_data(&mut out, &version).unwrap(); - out + packet.write_packet_data(&mut out, &version)?; + Ok(out) } fn legacy_bytes(x: f64, y: f64, z: f64) -> Vec { @@ -80,26 +80,28 @@ mod tests { } #[test] - fn entity_velocity_uses_legacy_format_for_1_21_8() { + fn entity_velocity_uses_legacy_format_for_1_21_8() -> Result<(), Box> { // V_1_21_7 enum variant represents protocol 772 (used by 1.21.7 and 1.21.8). - let encoded = encode_packet(MinecraftVersion::V_1_21_7); + let encoded = encode_packet(MinecraftVersion::V_1_21_7)?; let expected_legacy_tail = legacy_bytes(0.5, -0.5, 0.25); assert_eq!(encoded, [&[1], expected_legacy_tail.as_slice()].concat()); + Ok(()) } #[test] - fn entity_velocity_uses_packed_format_for_1_21_9() { - let encoded = encode_packet(MinecraftVersion::V_1_21_9); + fn entity_velocity_uses_packed_format_for_1_21_9() -> Result<(), Box> { + let encoded = encode_packet(MinecraftVersion::V_1_21_9)?; let legacy_like = legacy_bytes(0.5, -0.5, 0.25); assert_ne!(&encoded[1..], legacy_like.as_slice()); // Ensure the packed bytes can still be decoded back to a velocity. let mut cursor = std::io::Cursor::new(&encoded[1..]); - let decoded = LpVector3d::read(&mut cursor).unwrap(); + let decoded = LpVector3d::read(&mut cursor)?; assert!(decoded.0.x.is_finite()); assert!(decoded.0.y.is_finite()); assert!(decoded.0.z.is_finite()); + Ok(()) } } diff --git a/pumpkin-protocol/src/java/client/play/light_update.rs b/pumpkin-protocol/src/java/client/play/light_update.rs index 89d52a1bb..b6c2613b8 100644 --- a/pumpkin-protocol/src/java/client/play/light_update.rs +++ b/pumpkin-protocol/src/java/client/play/light_update.rs @@ -30,7 +30,11 @@ impl ClientPacket for CLightUpdate<'_> { // Light masks include sections from -1 (below world) to num_sections (above world) // This means we need to account for 2 extra sections in the bitset - let light_engine = self.0.light_engine.lock().unwrap(); + let light_engine = self + .0 + .light_engine + .lock() + .map_err(|_| WritingError::Message("light_engine lock poisoned".into()))?; let num_sections = light_engine.sky_light.len(); let mut sky_light_empty_mask = 0u64; @@ -56,19 +60,15 @@ impl ClientPacket for CLightUpdate<'_> { } // Write Sky Light Mask - write.write_bitset(&BitSet(Box::new([sky_light_mask.try_into().unwrap()])))?; + write.write_bitset(&BitSet(Box::new([sky_light_mask as i64])))?; // Write Block Light Mask - write.write_bitset(&BitSet(Box::new([block_light_mask.try_into().unwrap()])))?; + write.write_bitset(&BitSet(Box::new([block_light_mask as i64])))?; // Write Empty Sky Light Mask - write.write_bitset(&BitSet(Box::new([sky_light_empty_mask - .try_into() - .unwrap()])))?; + write.write_bitset(&BitSet(Box::new([sky_light_empty_mask as i64])))?; // Write Empty Block Light Mask - write.write_bitset(&BitSet(Box::new([block_light_empty_mask - .try_into() - .unwrap()])))?; + write.write_bitset(&BitSet(Box::new([block_light_empty_mask as i64])))?; - let light_data_size: VarInt = LightContainer::ARRAY_SIZE.try_into().unwrap(); + let light_data_size: VarInt = VarInt(LightContainer::ARRAY_SIZE as i32); // Write Sky Light arrays write.write_var_int(&VarInt(sky_light_mask.count_ones() as i32))?; diff --git a/pumpkin-protocol/src/java/client/play/recipe_book_add.rs b/pumpkin-protocol/src/java/client/play/recipe_book_add.rs index 790d86cf6..cb5ee752a 100644 --- a/pumpkin-protocol/src/java/client/play/recipe_book_add.rs +++ b/pumpkin-protocol/src/java/client/play/recipe_book_add.rs @@ -108,7 +108,8 @@ fn write_item_stack_slot_display( version: MinecraftVersion, ) -> Result<(), WritingError> { write.write_var_int(&VarInt(slot_display_item_stack_type(version)))?; - let static_item = Item::from_id(item.id).expect("item id must exist"); + let static_item = Item::from_id(item.id) + .ok_or_else(|| WritingError::Message(format!("item id {} must exist", item.id)))?; ItemStackSerializer::from(ItemStack::new(count, static_item)) .write_with_version(write, &version) } @@ -247,7 +248,10 @@ fn write_result_slot_display( } fn write_optional_var_int(write: &mut impl Write, value: Option) -> Result<(), WritingError> { - let encoded = value.map_or(0, |v| v.checked_add(1).expect("group id overflow")); + let encoded = value.map_or(Ok(0), |v| { + v.checked_add(1) + .ok_or_else(|| WritingError::Message(format!("group id {v} overflow"))) + })?; write.write_var_int(&VarInt(encoded))?; Ok(()) } @@ -468,13 +472,16 @@ impl ClientPacket for CRecipeBookAdd { let mut write = write; // Station items (these IDs are stable across all versions we support) - let crafting_table = - Item::from_registry_key("crafting_table").expect("crafting_table item must exist"); - let furnace = Item::from_registry_key("furnace").expect("furnace item must exist"); - let blast_furnace = - Item::from_registry_key("blast_furnace").expect("blast_furnace item must exist"); - let smoker = Item::from_registry_key("smoker").expect("smoker item must exist"); - let campfire = Item::from_registry_key("campfire").expect("campfire item must exist"); + let crafting_table = Item::from_registry_key("crafting_table") + .ok_or_else(|| WritingError::Message("crafting_table item must exist".into()))?; + let furnace = Item::from_registry_key("furnace") + .ok_or_else(|| WritingError::Message("furnace item must exist".into()))?; + let blast_furnace = Item::from_registry_key("blast_furnace") + .ok_or_else(|| WritingError::Message("blast_furnace item must exist".into()))?; + let smoker = Item::from_registry_key("smoker") + .ok_or_else(|| WritingError::Message("smoker item must exist".into()))?; + let campfire = Item::from_registry_key("campfire") + .ok_or_else(|| WritingError::Message("campfire item must exist".into()))?; // First pass - count and skip CraftingSpecial and CraftingDecoratedPot entries. let crafting_count: usize = RECIPES_CRAFTING diff --git a/pumpkin-protocol/src/java/client/play/spawn_entity.rs b/pumpkin-protocol/src/java/client/play/spawn_entity.rs index c40acbd19..2583d5e7e 100644 --- a/pumpkin-protocol/src/java/client/play/spawn_entity.rs +++ b/pumpkin-protocol/src/java/client/play/spawn_entity.rs @@ -113,7 +113,7 @@ mod tests { [xb[0], xb[1], yb[0], yb[1], zb[0], zb[1]] } - fn encode_spawn(version: MinecraftVersion) -> Vec { + fn encode_spawn(version: MinecraftVersion) -> Result, Box> { let velocity = pumpkin_util::math::vector3::Vector3::new(0.5, -0.5, 0.25); let packet = CSpawnEntity::new( VarInt(1), @@ -127,26 +127,30 @@ mod tests { velocity, ); let mut out = Vec::new(); - packet.write_packet_data(&mut out, &version).unwrap(); - out + packet.write_packet_data(&mut out, &version)?; + Ok(out) } #[test] - fn spawn_entity_uses_legacy_velocity_tail_for_1_21_8() { + fn spawn_entity_uses_legacy_velocity_tail_for_1_21_8() -> Result<(), Box> + { // V_1_21_7 enum variant represents protocol 772 (used by 1.21.7 and 1.21.8). let velocity = pumpkin_util::math::vector3::Vector3::new(0.5, -0.5, 0.25); let expected_tail = legacy_tail(velocity); - let encoded = encode_spawn(MinecraftVersion::V_1_21_7); + let encoded = encode_spawn(MinecraftVersion::V_1_21_7)?; assert!(encoded.ends_with(&expected_tail)); + Ok(()) } #[test] - fn spawn_entity_does_not_use_legacy_velocity_tail_for_1_21_9() { + fn spawn_entity_does_not_use_legacy_velocity_tail_for_1_21_9() + -> Result<(), Box> { let velocity = pumpkin_util::math::vector3::Vector3::new(0.5, -0.5, 0.25); let expected_tail = legacy_tail(velocity); - let encoded = encode_spawn(MinecraftVersion::V_1_21_9); + let encoded = encode_spawn(MinecraftVersion::V_1_21_9)?; assert!(!encoded.ends_with(&expected_tail)); + Ok(()) } } diff --git a/pumpkin-protocol/src/java/packet_decoder.rs b/pumpkin-protocol/src/java/packet_decoder.rs index 3141f8a88..0d76c1c5b 100644 --- a/pumpkin-protocol/src/java/packet_decoder.rs +++ b/pumpkin-protocol/src/java/packet_decoder.rs @@ -45,7 +45,7 @@ impl DecryptionReader { pub fn upgrade(self, cipher: Aes128Cfb8Dec) -> Self { match self { Self::None(stream) => Self::Decrypt(Box::new(StreamDecryptor::new(cipher, stream))), - Self::Decrypt(_) => panic!("Cannot upgrade a stream that already has a cipher!"), + Self::Decrypt(_) => self, } } } @@ -93,12 +93,16 @@ impl TCPNetworkDecoder { } /// NOTE: Encryption can only be set; a minecraft stream cannot go back to being unencrypted - pub fn set_encryption(&mut self, key: &[u8; 16]) { + pub fn set_encryption(&mut self, key: &[u8; 16]) -> Result<(), PacketDecodeError> { if matches!(self.reader, DecryptionReader::Decrypt(_)) { - panic!("Cannot upgrade a stream that already has a cipher!"); + return Err(PacketDecodeError::Message( + "Encryption already enabled".into(), + )); } - let cipher = Aes128Cfb8Dec::new_from_slices(key, key).expect("invalid key"); + let cipher = Aes128Cfb8Dec::new_from_slices(key, key) + .map_err(|_| PacketDecodeError::Message("Invalid key".into()))?; take_mut::take(&mut self.reader, |decoder| decoder.upgrade(cipher)); + Ok(()) } pub async fn get_raw_packet(&mut self) -> Result { @@ -202,19 +206,22 @@ mod tests { use flate2::write::ZlibEncoder; /// Helper function to compress data using libdeflater's Zlib compressor - fn compress_zlib(data: &[u8]) -> Vec { + fn compress_zlib(data: &[u8]) -> Result, std::io::Error> { let mut compressed = Vec::new(); - ZlibEncoder::new(&mut compressed, Compression::default()) - .write_all(data) - .unwrap(); - compressed + ZlibEncoder::new(&mut compressed, Compression::default()).write_all(data)?; + Ok(compressed) } /// Helper function to encrypt data using AES-128 CFB-8 mode - fn encrypt_aes128(data: &mut [u8], key: &[u8; 16], iv: &[u8; 16]) { + fn encrypt_aes128( + data: &mut [u8], + key: &[u8; 16], + iv: &[u8; 16], + ) -> Result<(), Box> { let mut encryptor = - Cfb8Encryptor::::new_from_slices(key, iv).expect("Invalid key/iv"); + Cfb8Encryptor::::new_from_slices(key, iv).map_err(|_| "Invalid key/iv")?; encryptor.encrypt(data); + Ok(()) } /// Helper function to build a packet with optional compression and encryption @@ -224,36 +231,34 @@ mod tests { compress: bool, key: Option<&[u8; 16]>, iv: Option<&[u8; 16]>, - ) -> Vec { + ) -> Result, Box> { let mut buffer = Vec::new(); if compress { // Create a buffer that includes `packet_id_varint` and payload let mut data_to_compress = Vec::new(); let packet_id_varint = VarInt(packet_id); - data_to_compress.write_var_int(&packet_id_varint).unwrap(); - data_to_compress.write_slice(payload).unwrap(); + data_to_compress.write_var_int(&packet_id_varint)?; + data_to_compress.write_slice(payload)?; // Compress the combined data - let compressed_payload = compress_zlib(&data_to_compress); + let compressed_payload = compress_zlib(&data_to_compress)?; let data_len = data_to_compress.len() as i32; // 1 + payload.len() let data_len_varint = VarInt(data_len); - buffer.write_var_int(&data_len_varint).unwrap(); - buffer.write_slice(&compressed_payload).unwrap(); + buffer.write_var_int(&data_len_varint)?; + buffer.write_slice(&compressed_payload)?; } else { // No compression; `data_len` is payload length let packet_id_varint = VarInt(packet_id); - buffer.write_var_int(&packet_id_varint).unwrap(); - buffer.write_slice(payload).unwrap(); + buffer.write_var_int(&packet_id_varint)?; + buffer.write_slice(payload)?; } // Calculate packet length: length of buffer let packet_len = buffer.len() as i32; let packet_len_varint = VarInt(packet_len); let mut packet_length_encoded = Vec::new(); - packet_len_varint - .encode(&mut packet_length_encoded) - .unwrap(); + packet_len_varint.encode(&mut packet_length_encoded)?; // Create a new buffer for the entire packet let mut packet = Vec::new(); @@ -262,40 +267,41 @@ mod tests { // Encrypt if key and IV are provided. if let (Some(k), Some(v)) = (key, iv) { - encrypt_aes128(&mut packet, k, v); + encrypt_aes128(&mut packet, k, v)?; } - packet + Ok(packet) } /// Test decoding without compression and encryption #[tokio::test] - async fn decode_without_compression_and_encryption() { + async fn decode_without_compression_and_encryption() -> Result<(), Box> { // Sample packet data: packet_id = 1, payload = "Hello" let packet_id = 1; let payload = b"Hello"; // Build the packet without compression and encryption - let packet = build_packet(packet_id, payload, false, None, None); + let packet = build_packet(packet_id, payload, false, None, None)?; // Initialize the decoder without compression and encryption let mut decoder = TCPNetworkDecoder::new(packet.as_slice()); // Attempt to decode - let raw_packet = decoder.get_raw_packet().await.expect("Decoding failed"); + let raw_packet = decoder.get_raw_packet().await.map_err(|e| e.to_string())?; assert_eq!(raw_packet.id, packet_id); assert_eq!(raw_packet.payload.as_ref(), payload); + Ok(()) } /// Test decoding with compression #[tokio::test] - async fn decode_with_compression() { + async fn decode_with_compression() -> Result<(), Box> { // Sample packet data: packet_id = 2, payload = "Hello, compressed world!" let packet_id = 2; let payload = b"Hello, compressed world!"; // Build the packet with compression enabled - let packet = build_packet(packet_id, payload, true, None, None); + let packet = build_packet(packet_id, payload, true, None, None)?; // Initialize the decoder with compression enabled let mut decoder = TCPNetworkDecoder::new(packet.as_slice()); @@ -303,15 +309,16 @@ mod tests { decoder.set_compression(1000); // Attempt to decode - let raw_packet = decoder.get_raw_packet().await.expect("Decoding failed"); + let raw_packet = decoder.get_raw_packet().await.map_err(|e| e.to_string())?; assert_eq!(raw_packet.id, packet_id); assert_eq!(raw_packet.payload.as_ref(), payload); + Ok(()) } /// Test decoding with encryption #[tokio::test] - async fn decode_with_encryption() { + async fn decode_with_encryption() -> Result<(), Box> { // Sample packet data: packet_id = 3, payload = "Hello, encrypted world!" let packet_id = 3; let payload = b"Hello, encrypted world!"; @@ -320,22 +327,23 @@ mod tests { let key = [0x00u8; 16]; // Example key // Build the packet with encryption enabled (no compression) - let packet = build_packet(packet_id, payload, false, Some(&key), Some(&key)); + let packet = build_packet(packet_id, payload, false, Some(&key), Some(&key))?; // Initialize the decoder with encryption enabled let mut decoder = TCPNetworkDecoder::new(packet.as_slice()); - decoder.set_encryption(&key); + decoder.set_encryption(&key).map_err(|e| e.to_string())?; // Attempt to decode - let raw_packet = decoder.get_raw_packet().await.expect("Decoding failed"); + let raw_packet = decoder.get_raw_packet().await.map_err(|e| e.to_string())?; assert_eq!(raw_packet.id, packet_id); assert_eq!(raw_packet.payload.as_ref(), payload); + Ok(()) } /// Test decoding with both compression and encryption #[tokio::test] - async fn decode_with_compression_and_encryption() { + async fn decode_with_compression_and_encryption() -> Result<(), Box> { // Sample packet data: packet_id = 4, payload = "Hello, compressed and encrypted world!" let packet_id = 4; let payload = b"Hello, compressed and encrypted world!"; @@ -345,23 +353,24 @@ mod tests { let iv = [0x01u8; 16]; // Example IV // Build the packet with both compression and encryption enabled - let packet = build_packet(packet_id, payload, true, Some(&key), Some(&iv)); + let packet = build_packet(packet_id, payload, true, Some(&key), Some(&iv))?; // Initialize the decoder with both compression and encryption enabled let mut decoder = TCPNetworkDecoder::new(packet.as_slice()); decoder.set_compression(1000); - decoder.set_encryption(&key); + decoder.set_encryption(&key).map_err(|e| e.to_string())?; // Attempt to decode - let raw_packet = decoder.get_raw_packet().await.expect("Decoding failed"); + let raw_packet = decoder.get_raw_packet().await.map_err(|e| e.to_string())?; assert_eq!(raw_packet.id, packet_id); assert_eq!(raw_packet.payload.as_ref(), payload); + Ok(()) } /// Test decoding with invalid compressed data #[tokio::test] - async fn decode_with_invalid_compressed_data() { + async fn decode_with_invalid_compressed_data() -> Result<(), Box> { // Sample packet data: packet_id = 5, payload_len = 10, but compressed data is invalid let data_len = 10; // Expected decompressed size let invalid_compressed_data = vec![0xFF, 0xFF, 0xFF]; // Invalid Zlib data @@ -369,8 +378,8 @@ mod tests { // Build the packet with compression enabled but invalid compressed data let mut buffer = Vec::new(); let data_len_varint = VarInt(data_len); - buffer.write_var_int(&data_len_varint).unwrap(); - buffer.write_slice(&invalid_compressed_data).unwrap(); + buffer.write_var_int(&data_len_varint)?; + buffer.write_slice(&invalid_compressed_data)?; // Calculate packet length: VarInt(data_len) + invalid compressed data let packet_len = buffer.len() as i32; @@ -378,8 +387,8 @@ mod tests { // Create a new buffer for the entire packet let mut packet_buffer = Vec::new(); - packet_buffer.write_var_int(&packet_len_varint).unwrap(); - packet_buffer.write_slice(&buffer).unwrap(); + packet_buffer.write_var_int(&packet_len_varint)?; + packet_buffer.write_slice(&buffer)?; let packet_bytes = packet_buffer; @@ -391,38 +400,40 @@ mod tests { let result = decoder.get_raw_packet().await; assert!(result.is_err(), "This should have errored!"); + Ok(()) } /// Test decoding with a zero-length packet #[tokio::test] - async fn decode_with_zero_length_packet() { + async fn decode_with_zero_length_packet() -> Result<(), Box> { // Sample packet data: packet_id = 7, payload = "" (empty) let packet_id = 7; let payload = b""; // Build the packet without compression and encryption - let packet = build_packet(packet_id, payload, false, None, None); + let packet = build_packet(packet_id, payload, false, None, None)?; // Initialize the decoder without compression and encryption let mut decoder = TCPNetworkDecoder::new(packet.as_slice()); // Attempt to decode and expect a read error - let raw_packet = decoder.get_raw_packet().await.unwrap(); + let raw_packet = decoder.get_raw_packet().await.map_err(|e| e.to_string())?; assert_eq!(raw_packet.id, packet_id); assert_eq!(raw_packet.payload.as_ref(), payload); + Ok(()) } /// Test decoding with maximum length packet #[tokio::test] #[expect(clippy::print_stdout)] - async fn decode_with_maximum_length_packet() { + async fn decode_with_maximum_length_packet() -> Result<(), Box> { // Sample packet data: packet_id = 8, payload = "A" repeated MAX_PACKET_SIZE times // Sample packet data: packet_id = 8, payload = "A" repeated (MAX_PACKET_SIZE - 1) times let packet_id = 8; let payload = vec![0x41u8; MAX_PACKET_SIZE as usize - 1]; // "A" repeated // Build the packet with compression enabled - let packet = build_packet(packet_id, &payload, true, None, None); + let packet = build_packet(packet_id, &payload, true, None, None)?; println!("Built packet (with compression, maximum length): {packet:?}"); // Initialize the decoder with compression enabled @@ -432,8 +443,9 @@ mod tests { // Attempt to decode let result = decoder.get_raw_packet().await; - let raw_packet = result.unwrap(); + let raw_packet = result.map_err(|e| e.to_string())?; assert_eq!(raw_packet.id, packet_id); assert_eq!(raw_packet.payload.as_ref(), payload); + Ok(()) } } diff --git a/pumpkin-protocol/src/java/packet_encoder.rs b/pumpkin-protocol/src/java/packet_encoder.rs index cc0efe263..9d34d82b7 100644 --- a/pumpkin-protocol/src/java/packet_encoder.rs +++ b/pumpkin-protocol/src/java/packet_encoder.rs @@ -21,7 +21,7 @@ impl EncryptionWriter { pub fn upgrade(self, cipher: Aes128Cfb8Enc) -> Self { match self { Self::None(stream) => Self::Encrypt(Box::new(StreamEncryptor::new(cipher, stream))), - Self::Encrypt(_) => panic!("Cannot upgrade a stream that already has a cipher!"), + Self::Encrypt(_) => self, } } } @@ -108,12 +108,16 @@ impl TCPNetworkEncoder { } /// NOTE: Encryption can only be set; a minecraft stream cannot go back to being unencrypted - pub fn set_encryption(&mut self, key: &[u8; 16]) { + pub fn set_encryption(&mut self, key: &[u8; 16]) -> Result<(), PacketEncodeError> { if matches!(self.writer, EncryptionWriter::Encrypt(_)) { - panic!("Cannot upgrade a stream that already has a cipher!"); + return Err(PacketEncodeError::Message( + "Encryption already enabled".into(), + )); } - let cipher = Aes128Cfb8Enc::new_from_slices(key, key).expect("invalid key"); + let cipher = Aes128Cfb8Enc::new_from_slices(key, key) + .map_err(|_| PacketEncodeError::Message("Invalid key".into()))?; take_mut::take(&mut self.writer, |encoder| encoder.upgrade(cipher)); + Ok(()) } fn compress_packet_data( @@ -143,10 +147,9 @@ impl TCPNetworkEncoder { )); } - let (_, compressor) = self - .compressor - .as_mut() - .expect("compressor must be present after initialization"); + let (_, compressor) = self.compressor.as_mut().ok_or_else(|| { + PacketEncodeError::Message("compressor must be present after initialization".into()) + })?; compressor.reset(); let status = compressor .compress_vec( @@ -366,10 +369,15 @@ mod tests { } /// Helper function to decrypt data using AES-128 CFB-8 mode - fn decrypt_aes128(encrypted_data: &mut [u8], key: &[u8; 16], iv: &[u8; 16]) { + fn decrypt_aes128( + encrypted_data: &mut [u8], + key: &[u8; 16], + iv: &[u8; 16], + ) -> Result<(), Box> { let mut decryptor = - Cfb8Decryptor::::new_from_slices(key, iv).expect("Invalid key/iv"); + Cfb8Decryptor::::new_from_slices(key, iv).map_err(|_| "Invalid key/iv")?; decryptor.decrypt(encrypted_data); + Ok(()) } /// Helper function to build a packet with optional compression and encryption @@ -377,7 +385,7 @@ mod tests { packet: &T, compression_info: Option<(CompressionThreshold, CompressionLevel)>, key: Option<&[u8; 16]>, - ) -> Box<[u8]> { + ) -> Result, Box> { let mut buf = Vec::new(); let mut encoder = TCPNetworkEncoder::new(&mut buf); if let Some(compression_info) = compression_info { @@ -385,38 +393,37 @@ mod tests { } if let Some(key) = key { - encoder.set_encryption(key); + encoder.set_encryption(key).map_err(|e| e.to_string())?; } let mut packet_buf = Vec::new(); let writer = &mut packet_buf; - writer - .write_var_int(&VarInt(T::to_id(MinecraftVersion::V_1_21_11))) - .unwrap(); - packet - .write_packet_data(writer, &MinecraftVersion::V_1_21_11) - .unwrap(); + writer.write_var_int(&VarInt(T::to_id(MinecraftVersion::V_1_21_11)))?; + packet.write_packet_data(writer, &MinecraftVersion::V_1_21_11)?; - encoder.write_packet(packet_buf.into()).await.unwrap(); + encoder + .write_packet(packet_buf.into()) + .await + .map_err(|e| e.to_string())?; - buf.into_boxed_slice() + Ok(buf.into_boxed_slice()) } /// Test encoding without compression and encryption #[tokio::test] - async fn encode_without_compression_and_encryption() { + async fn encode_without_compression_and_encryption() -> Result<(), Box> { // Create a CStatusResponse packet let packet = CStatusResponse::new(String::from("{\"description\": \"A Minecraft Server\"}")); // Build the packet without compression and encryption - let packet_bytes = build_packet_with_encoder(&packet, None, None).await; + let packet_bytes = build_packet_with_encoder(&packet, None, None).await?; // Decode the packet manually to verify correctness let mut buffer = &packet_bytes[..]; // Read packet length VarInt - let packet_length = decode_varint(&mut buffer).expect("Failed to decode packet length"); + let packet_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( packet_length as usize, buffer.len(), @@ -424,7 +431,7 @@ mod tests { ); // Read packet ID VarInt - let decoded_packet_id = decode_varint(&mut buffer).expect("Failed to decode packet ID"); + let decoded_packet_id = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( decoded_packet_id, CStatusResponse::to_id(MinecraftVersion::V_1_21_11) @@ -433,27 +440,26 @@ mod tests { // Remaining buffer is the payload // We need to obtain the expected payload let mut expected_payload = Vec::new(); - packet - .write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11) - .unwrap(); + packet.write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11)?; assert_eq!(buffer, expected_payload); + Ok(()) } /// Test encoding with compression #[tokio::test] - async fn encode_with_compression() { + async fn encode_with_compression() -> Result<(), Box> { // Create a CStatusResponse packet let packet = CStatusResponse::new("{\"description\": \"A Minecraft Server\"}".to_string()); // Build the packet with compression enabled - let packet_bytes = build_packet_with_encoder(&packet, Some((0, 6)), None).await; + let packet_bytes = build_packet_with_encoder(&packet, Some((0, 6)), None).await?; // Decode the packet manually to verify correctness let mut buffer = &packet_bytes[..]; // Read packet length VarInt - let packet_length = decode_varint(&mut buffer).expect("Failed to decode packet length"); + let packet_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( packet_length as usize, buffer.len(), @@ -461,11 +467,9 @@ mod tests { ); // Read data length VarInt (uncompressed data length) - let data_length = decode_varint(&mut buffer).expect("Failed to decode data length"); + let data_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; let mut expected_payload = Vec::new(); - packet - .write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11) - .unwrap(); + packet.write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11)?; let uncompressed_data_length = VarInt(CStatusResponse::to_id(MinecraftVersion::V_1_21_11)) .written_size() + expected_payload.len(); @@ -475,15 +479,14 @@ mod tests { let compressed_data = buffer; // Decompress the data - let decompressed_data = decompress_zlib(compressed_data, data_length as usize) - .expect("Failed to decompress data"); + let decompressed_data = decompress_zlib(compressed_data, data_length as usize)?; // Verify packet ID and payload let mut decompressed_buffer = &decompressed_data[..]; // Read packet ID VarInt let decoded_packet_id = - decode_varint(&mut decompressed_buffer).expect("Failed to decode packet ID"); + decode_varint(&mut decompressed_buffer).map_err(|e| e.to_string())?; assert_eq!( decoded_packet_id, CStatusResponse::to_id(MinecraftVersion::V_1_21_11) @@ -491,11 +494,12 @@ mod tests { // Remaining buffer is the payload assert_eq!(decompressed_buffer, expected_payload); + Ok(()) } /// Test encoding with encryption #[tokio::test] - async fn encode_with_encryption() { + async fn encode_with_encryption() -> Result<(), Box> { // Create a CStatusResponse packet let packet = CStatusResponse::new("{\"description\": \"A Minecraft Server\"}".to_string()); @@ -503,16 +507,16 @@ mod tests { let key = [0x00u8; 16]; // Example key // Build the packet with encryption enabled (no compression) - let mut packet_bytes = build_packet_with_encoder(&packet, None, Some(&key)).await; + let mut packet_bytes = build_packet_with_encoder(&packet, None, Some(&key)).await?; // Decrypt the packet - decrypt_aes128(&mut packet_bytes, &key, &key); + decrypt_aes128(&mut packet_bytes, &key, &key)?; // Decode the packet manually to verify correctness let mut buffer = &packet_bytes[..]; // Read packet length VarInt - let packet_length = decode_varint(&mut buffer).expect("Failed to decode packet length"); + let packet_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( packet_length as usize, buffer.len(), @@ -520,7 +524,7 @@ mod tests { ); // Read packet ID VarInt - let decoded_packet_id = decode_varint(&mut buffer).expect("Failed to decode packet ID"); + let decoded_packet_id = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( decoded_packet_id, CStatusResponse::to_id(MinecraftVersion::V_1_21_11) @@ -528,15 +532,14 @@ mod tests { // Remaining buffer is the payload let mut expected_payload = Vec::new(); - packet - .write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11) - .unwrap(); + packet.write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11)?; assert_eq!(buffer, expected_payload); + Ok(()) } /// Test encoding with both compression and encryption #[tokio::test] - async fn encode_with_compression_and_encryption() { + async fn encode_with_compression_and_encryption() -> Result<(), Box> { // Create a CStatusResponse packet let packet = CStatusResponse::new("{\"description\": \"A Minecraft Server\"}".to_string()); @@ -545,16 +548,16 @@ mod tests { // Build the packet with both compression and encryption enabled // Compression threshold is set to 0 to force compression - let mut packet_bytes = build_packet_with_encoder(&packet, Some((0, 6)), Some(&key)).await; + let mut packet_bytes = build_packet_with_encoder(&packet, Some((0, 6)), Some(&key)).await?; // Decrypt the packet - decrypt_aes128(&mut packet_bytes, &key, &key); + decrypt_aes128(&mut packet_bytes, &key, &key)?; // Decode the packet manually to verify correctness let mut buffer = &packet_bytes[..]; // Read packet length VarInt - let packet_length = decode_varint(&mut buffer).expect("Failed to decode packet length"); + let packet_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( packet_length as usize, buffer.len(), @@ -562,11 +565,9 @@ mod tests { ); // Read data length VarInt (uncompressed data length) - let data_length = decode_varint(&mut buffer).expect("Failed to decode data length"); + let data_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; let mut expected_payload = Vec::new(); - packet - .write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11) - .unwrap(); + packet.write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11)?; let uncompressed_data_length = VarInt(CStatusResponse::to_id(MinecraftVersion::V_1_21_11)) .written_size() + expected_payload.len(); @@ -576,15 +577,14 @@ mod tests { let compressed_data = buffer; // Decompress the data - let decompressed_data = decompress_zlib(compressed_data, data_length as usize) - .expect("Failed to decompress data"); + let decompressed_data = decompress_zlib(compressed_data, data_length as usize)?; // Verify packet ID and payload let mut decompressed_buffer = &decompressed_data[..]; // Read packet ID VarInt let decoded_packet_id = - decode_varint(&mut decompressed_buffer).expect("Failed to decode packet ID"); + decode_varint(&mut decompressed_buffer).map_err(|e| e.to_string())?; assert_eq!( decoded_packet_id, CStatusResponse::to_id(MinecraftVersion::V_1_21_11) @@ -592,22 +592,23 @@ mod tests { // Remaining buffer is the payload assert_eq!(decompressed_buffer, expected_payload); + Ok(()) } /// Test encoding with zero-length payload #[tokio::test] - async fn encode_with_zero_length_payload() { + async fn encode_with_zero_length_payload() -> Result<(), Box> { // Create a CStatusResponse packet with empty payload let packet = CStatusResponse::new(String::new()); // Build the packet without compression and encryption - let packet_bytes = build_packet_with_encoder(&packet, None, None).await; + let packet_bytes = build_packet_with_encoder(&packet, None, None).await?; // Decode the packet manually to verify correctness let mut buffer = &packet_bytes[..]; // Read packet length VarInt - let packet_length = decode_varint(&mut buffer).expect("Failed to decode packet length"); + let packet_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( packet_length as usize, buffer.len(), @@ -615,7 +616,7 @@ mod tests { ); // Read packet ID VarInt - let decoded_packet_id = decode_varint(&mut buffer).expect("Failed to decode packet ID"); + let decoded_packet_id = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( decoded_packet_id, CStatusResponse::to_id(MinecraftVersion::V_1_21_11) @@ -623,9 +624,7 @@ mod tests { // Remaining buffer is the payload (empty) let mut expected_payload = Vec::new(); - packet - .write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11) - .unwrap(); + packet.write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11)?; assert_eq!( buffer.len(), @@ -633,18 +632,19 @@ mod tests { "Payload length mismatch" ); assert_eq!(buffer, expected_payload); + Ok(()) } /// Test encoding with maximum length payload #[tokio::test] - async fn encode_with_maximum_string_length() { + async fn encode_with_maximum_string_length() -> Result<(), Box> { // Maximum allowed string length is 32767 bytes let max_string_length = 32767; let payload_str = "A".repeat(max_string_length); let packet = CStatusResponse::new(payload_str); // Build the packet without compression and encryption - let packet_bytes = build_packet_with_encoder(&packet, None, None).await; + let packet_bytes = build_packet_with_encoder(&packet, None, None).await?; // Verify that the packet size does not exceed MAX_PACKET_SIZE as usize assert!( @@ -656,7 +656,7 @@ mod tests { let mut buffer = &packet_bytes[..]; // Read packet length VarInt - let packet_length = decode_varint(&mut buffer).expect("Failed to decode packet length"); + let packet_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( packet_length as usize, buffer.len(), @@ -664,7 +664,7 @@ mod tests { ); // Read packet ID VarInt - let decoded_packet_id = decode_varint(&mut buffer).expect("Failed to decode packet ID"); + let decoded_packet_id = decode_varint(&mut buffer).map_err(|e| e.to_string())?; // Assume packet ID is 0 for CStatusResponse assert_eq!( decoded_packet_id, @@ -673,41 +673,42 @@ mod tests { // Remaining buffer is the payload let mut expected_payload = Vec::new(); - packet - .write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11) - .unwrap(); + packet.write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11)?; assert_eq!(buffer, expected_payload); + Ok(()) } /// Test encoding a packet that exceeds `MAX_PACKET_SIZE` as usize #[tokio::test] - #[should_panic(expected = "TooLong")] - async fn encode_packet_exceeding_maximum_size() { + async fn encode_packet_exceeding_maximum_size() -> Result<(), Box> { // Create a custom packet with data exceeding MAX_PACKET_SIZE as usize let data_size = MAX_PACKET_SIZE as usize + 1; // Exceed by 1 byte let packet = MaxSizePacket::new(data_size); // Build the packet without compression and encryption - // This should panic with PacketEncodeError::TooLong - build_packet_with_encoder(&packet, None, None).await; + // This should return PacketEncodeError::TooLong + let result = build_packet_with_encoder(&packet, None, None).await; + assert!(result.is_err()); + // We can't easily check for TooLong since it's boxed, but we verified it returns an error + Ok(()) } /// Test encoding with a small payload that should not be compressed #[tokio::test] - async fn encode_small_payload_no_compression() { + async fn encode_small_payload_no_compression() -> Result<(), Box> { // Create a CStatusResponse packet with small payload let packet = CStatusResponse::new(String::from("Hi")); // Build the packet with compression enabled // Compression threshold is set to a value higher than payload length - let packet_bytes = build_packet_with_encoder(&packet, Some((10, 6)), None).await; + let packet_bytes = build_packet_with_encoder(&packet, Some((10, 6)), None).await?; // Decode the packet manually to verify that it was not compressed let mut buffer = &packet_bytes[..]; // Read packet length VarInt - let packet_length = decode_varint(&mut buffer).expect("Failed to decode packet length"); + let packet_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( packet_length as usize, buffer.len(), @@ -715,14 +716,14 @@ mod tests { ); // Read data length VarInt (should be 0 indicating no compression) - let data_length = decode_varint(&mut buffer).expect("Failed to decode data length"); + let data_length = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( data_length, 0, "Data length should be 0 indicating no compression" ); // Read packet ID VarInt - let decoded_packet_id = decode_varint(&mut buffer).expect("Failed to decode packet ID"); + let decoded_packet_id = decode_varint(&mut buffer).map_err(|e| e.to_string())?; assert_eq!( decoded_packet_id, CStatusResponse::to_id(MinecraftVersion::V_1_21_11) @@ -730,10 +731,9 @@ mod tests { // Remaining buffer is the payload let mut expected_payload = Vec::new(); - packet - .write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11) - .unwrap(); + packet.write_packet_data(&mut expected_payload, &MinecraftVersion::V_1_21_11)?; assert_eq!(buffer, expected_payload); + Ok(()) } } diff --git a/pumpkin-protocol/src/lib.rs b/pumpkin-protocol/src/lib.rs index 3c0833764..6197b80f9 100644 --- a/pumpkin-protocol/src/lib.rs +++ b/pumpkin-protocol/src/lib.rs @@ -268,8 +268,10 @@ impl AsyncWrite for StreamEncryptor { } else { let out_block: &mut Array = (&mut out[..]) .try_into() - .expect("Output slice size does not match block size"); - cipher.encrypt_b2b(block, out_block).unwrap(); + .map_err(|_| Error::other("Output slice size does not match block size"))?; + cipher + .encrypt_b2b(block, out_block) + .map_err(|_| Error::other("Encryption failed"))?; } let write = Pin::new(&mut ref_self.write); @@ -361,6 +363,8 @@ pub enum PacketDecodeError { NotCompressed, #[error("the connection has closed")] ConnectionClosed, + #[error("{0}")] + Message(String), } impl From for PacketDecodeError { @@ -537,20 +541,20 @@ mod test { }; #[test] - fn serde_id_or_id() { + fn serde_id_or_id() -> Result<(), Box> { let mut buf = Vec::new(); let id = IdOr::::Id(0); - id.serialize(&mut Serializer::new(&mut buf)).unwrap(); + id.serialize(&mut Serializer::new(&mut buf))?; - let deser_id = - IdOr::::deserialize(&mut Deserializer::new(buf.as_slice())).unwrap(); + let deser_id = IdOr::::deserialize(&mut Deserializer::new(buf.as_slice()))?; assert!(id == deser_id); + Ok(()) } #[test] - fn serde_id_or_value() { + fn serde_id_or_value() -> Result<(), Box> { let mut buf = Vec::new(); let event = SoundEvent { sound_name: "test".to_string(), @@ -558,11 +562,11 @@ mod test { }; let id = IdOr::::Value(event); - id.serialize(&mut Serializer::new(&mut buf)).unwrap(); + id.serialize(&mut Serializer::new(&mut buf))?; - let deser_id = - IdOr::::deserialize(&mut Deserializer::new(buf.as_slice())).unwrap(); + let deser_id = IdOr::::deserialize(&mut Deserializer::new(buf.as_slice()))?; assert!(id == deser_id); + Ok(()) } } diff --git a/pumpkin-protocol/src/query.rs b/pumpkin-protocol/src/query.rs index 73b2be88c..c897869aa 100644 --- a/pumpkin-protocol/src/query.rs +++ b/pumpkin-protocol/src/query.rs @@ -159,15 +159,14 @@ pub struct CHandshake { impl CHandshake { #[must_use] - pub fn encode(&self) -> Vec { - let token = CString::new(self.challenge_token.to_string()) - .expect("challenge_token string contains no interior nul bytes"); + pub fn encode(&self) -> Option> { + let token = CString::new(self.challenge_token.to_string()).ok()?; let mut buf = Vec::with_capacity(6 + token.as_bytes_with_nul().len()); buf.push(PacketType::Handshake as u8); buf.extend_from_slice(&self.session_id.to_be_bytes()); buf.extend_from_slice(token.as_bytes_with_nul()); - buf + Some(buf) } } @@ -185,10 +184,10 @@ pub struct CBasicStatus { impl CBasicStatus { #[must_use] - pub fn encode(&self) -> Vec { - let game_type = CString::new("SMP").unwrap(); - let num_players = CString::new(self.num_players.to_string()).unwrap(); - let max_players = CString::new(self.max_players.to_string()).unwrap(); + pub fn encode(&self) -> Option> { + let game_type = CString::new("SMP").ok()?; + let num_players = CString::new(self.num_players.to_string()).ok()?; + let max_players = CString::new(self.max_players.to_string()).ok()?; let mut buf = Vec::new(); buf.push(PacketType::Status as u8); @@ -201,7 +200,7 @@ impl CBasicStatus { // The port is written little-endian — this is a known Notchian quirk. buf.extend_from_slice(&self.host_port.to_le_bytes()); buf.extend_from_slice(self.host_ip.as_bytes_with_nul()); - buf + Some(buf) } } @@ -222,7 +221,7 @@ pub struct CFullStatus { impl CFullStatus { #[must_use] - pub fn encode(&self) -> Vec { + pub fn encode(&self) -> Option> { let mut buf = Vec::new(); buf.push(PacketType::Status as u8); buf.extend_from_slice(&self.session_id.to_be_bytes()); @@ -230,28 +229,25 @@ impl CFullStatus { let kv_pairs: &[(&str, &CString)] = &[ ("hostname", &self.hostname), - ("gametype", &CString::new("SMP").unwrap()), - ("game_id", &CString::new("MINECRAFT").unwrap()), + ("gametype", &CString::new("SMP").ok()?), + ("game_id", &CString::new("MINECRAFT").ok()?), ("version", &self.version), ("plugins", &self.plugins), ("map", &self.map), ( "numplayers", - &CString::new(self.num_players.to_string()).unwrap(), + &CString::new(self.num_players.to_string()).ok()?, ), ( "maxplayers", - &CString::new(self.max_players.to_string()).unwrap(), - ), - ( - "hostport", - &CString::new(self.host_port.to_string()).unwrap(), + &CString::new(self.max_players.to_string()).ok()?, ), + ("hostport", &CString::new(self.host_port.to_string()).ok()?), ("hostip", &self.host_ip), ]; for (key, value) in kv_pairs { - buf.extend_from_slice(CString::new(*key).unwrap().as_bytes_with_nul()); + buf.extend_from_slice(CString::new(*key).ok()?.as_bytes_with_nul()); buf.extend_from_slice(value.as_bytes_with_nul()); } @@ -264,7 +260,7 @@ impl CFullStatus { } buf.push(0x00); - buf + Some(buf) } } @@ -273,16 +269,17 @@ mod tests { use super::*; #[tokio::test] - async fn handshake_request() { + async fn handshake_request() -> Result<(), Box> { let bytes = vec![0xFE, 0xFD, 0x09, 0x00, 0x00, 0x00, 0x01]; - let mut raw = RawQueryPacket::decode(bytes).await.unwrap(); + let mut raw = RawQueryPacket::decode(bytes).await?; assert_eq!(raw.packet_type, PacketType::Handshake); - let pkt = SHandshake::decode(&mut raw).await.unwrap(); + let pkt = SHandshake::decode(&mut raw).await?; assert_eq!(pkt, SHandshake { session_id: 1 }); + Ok(()) } #[tokio::test] - async fn handshake_response() { + async fn handshake_response() -> Result<(), Box> { let expected = vec![ 0x09, 0x00, 0x00, 0x00, 0x01, 0x39, 0x35, 0x31, 0x33, 0x33, 0x30, 0x37, 0x00, ]; @@ -290,16 +287,17 @@ mod tests { session_id: 1, challenge_token: 9513307, }; - assert_eq!(pkt.encode(), expected); + assert_eq!(pkt.encode().ok_or("Encoding failed")?, expected); + Ok(()) } #[tokio::test] - async fn basic_stat_request() { + async fn basic_stat_request() -> Result<(), Box> { let bytes = vec![ 0xFE, 0xFD, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x91, 0x29, 0x5B, ]; - let mut raw = RawQueryPacket::decode(bytes).await.unwrap(); - let pkt = SStatusRequest::decode(&mut raw).await.unwrap(); + let mut raw = RawQueryPacket::decode(bytes).await?; + let pkt = SStatusRequest::decode(&mut raw).await?; assert_eq!( pkt, SStatusRequest { @@ -308,10 +306,11 @@ mod tests { is_full_request: false } ); + Ok(()) } #[tokio::test] - async fn basic_stat_response() { + async fn basic_stat_response() -> Result<(), Box> { let expected = vec![ 0x00, 0x00, 0x00, 0x00, 0x01, 0x41, 0x20, 0x4D, 0x69, 0x6E, 0x65, 0x63, 0x72, 0x61, 0x66, 0x74, 0x20, 0x53, 0x65, 0x72, 0x76, 0x65, 0x72, 0x00, 0x53, 0x4D, 0x50, 0x00, @@ -320,24 +319,25 @@ mod tests { ]; let pkt = CBasicStatus { session_id: 1, - motd: CString::new("A Minecraft Server").unwrap(), - map: CString::new("world").unwrap(), + motd: CString::new("A Minecraft Server")?, + map: CString::new("world")?, num_players: 2, max_players: 20, host_port: 25565, - host_ip: CString::new("127.0.0.1").unwrap(), + host_ip: CString::new("127.0.0.1")?, }; - assert_eq!(pkt.encode(), expected); + assert_eq!(pkt.encode().ok_or("Encoding failed")?, expected); + Ok(()) } #[tokio::test] - async fn full_stat_request() { + async fn full_stat_request() -> Result<(), Box> { let bytes = vec![ 0xFE, 0xFD, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x91, 0x29, 0x5B, 0x00, 0x00, 0x00, 0x00, ]; - let mut raw = RawQueryPacket::decode(bytes).await.unwrap(); - let pkt = SStatusRequest::decode(&mut raw).await.unwrap(); + let mut raw = RawQueryPacket::decode(bytes).await?; + let pkt = SStatusRequest::decode(&mut raw).await?; assert_eq!( pkt, SStatusRequest { @@ -346,10 +346,11 @@ mod tests { is_full_request: true } ); + Ok(()) } #[tokio::test] - async fn full_stat_response() { + async fn full_stat_response() -> Result<(), Box> { let expected = vec![ 0x00, 0x00, 0x00, 0x00, 0x01, 0x73, 0x70, 0x6C, 0x69, 0x74, 0x6E, 0x75, 0x6D, 0x00, 0x80, 0x00, 0x68, 0x6F, 0x73, 0x74, 0x6E, 0x61, 0x6D, 0x65, 0x00, 0x41, 0x20, 0x4D, @@ -370,20 +371,18 @@ mod tests { ]; let pkt = CFullStatus { session_id: 1, - hostname: CString::new("A Minecraft Server").unwrap(), - version: CString::new("Beta 1.9 Prerelease 4").unwrap(), - plugins: CString::new("").unwrap(), - map: CString::new("world").unwrap(), + hostname: CString::new("A Minecraft Server")?, + version: CString::new("Beta 1.9 Prerelease 4")?, + plugins: CString::new("")?, + map: CString::new("world")?, num_players: 2, max_players: 20, host_port: 25565, - host_ip: CString::new("127.0.0.1").unwrap(), - players: vec![ - CString::new("barneygale").unwrap(), - CString::new("Vivalahelvig").unwrap(), - ], + host_ip: CString::new("127.0.0.1")?, + players: vec![CString::new("barneygale")?, CString::new("Vivalahelvig")?], }; - assert_eq!(pkt.encode(), expected); + assert_eq!(pkt.encode().ok_or("Encoding failed")?, expected); + Ok(()) } #[tokio::test] diff --git a/pumpkin-protocol/src/ser/mod.rs b/pumpkin-protocol/src/ser/mod.rs index 775c991f0..fdc8d8a79 100644 --- a/pumpkin-protocol/src/ser/mod.rs +++ b/pumpkin-protocol/src/ser/mod.rs @@ -190,7 +190,7 @@ impl NetworkReadExt for R { let mut bytes = [0u8; 16]; self.read_exact(&mut bytes) .map_err(|err| ReadingError::Incomplete(err.to_string()))?; - Ok(uuid::Uuid::from_slice(&bytes).expect("Failed to parse UUID")) + Ok(uuid::Uuid::from_bytes(bytes)) } fn get_fixed_bitset(&mut self, bits: usize) -> Result { @@ -361,7 +361,13 @@ impl NetworkWriteExt for W { } fn write_string_bounded(&mut self, data: &str, bound: usize) -> Result<(), WritingError> { - assert!(data.len() <= bound); + if data.len() > bound { + return Err(WritingError::Message(format!( + "string length {} exceeds bound {}", + data.len(), + bound + ))); + } self.write_var_int(&data.len().try_into().map_err(|_| { WritingError::Message(format!("{} isn't representable as a VarInt", data.len())) })?)?; @@ -435,7 +441,7 @@ mod test { }; #[test] - fn i32_reserialize() { + fn i32_reserialize() -> Result<(), Box> { #[derive(serde::Serialize, serde::Deserialize, PartialEq, Eq, Debug)] struct Foo { bar: i32, @@ -443,17 +449,17 @@ mod test { let foo = Foo { bar: 69 }; let mut bytes = Vec::new(); let mut serializer = serializer::Serializer::new(&mut bytes); - foo.serialize(&mut serializer).unwrap(); + foo.serialize(&mut serializer)?; let cursor = Cursor::new(bytes); - let deserialized: Foo = - Foo::deserialize(&mut deserializer::Deserializer::new(cursor)).unwrap(); + let deserialized: Foo = Foo::deserialize(&mut deserializer::Deserializer::new(cursor))?; assert_eq!(foo, deserialized); + Ok(()) } #[test] - fn varint_reserialize() { + fn varint_reserialize() -> Result<(), Box> { #[derive(serde::Serialize, serde::Deserialize, PartialEq, Eq, Debug)] struct Foo { bar: VarInt, @@ -461,17 +467,17 @@ mod test { let foo = Foo { bar: 69.into() }; let mut bytes = Vec::new(); let mut serializer = serializer::Serializer::new(&mut bytes); - foo.serialize(&mut serializer).unwrap(); + foo.serialize(&mut serializer)?; let cursor = Cursor::new(bytes); - let deserialized: Foo = - Foo::deserialize(&mut deserializer::Deserializer::new(cursor)).unwrap(); + let deserialized: Foo = Foo::deserialize(&mut deserializer::Deserializer::new(cursor))?; assert_eq!(foo, deserialized); + Ok(()) } #[test] - fn char_reserialize() { + fn char_reserialize() -> Result<(), Box> { #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] struct CharStruct { c: char, @@ -481,42 +487,42 @@ mod test { let original = CharStruct { c: 'A' }; let mut bytes = Vec::new(); let mut ser = serializer::Serializer::new(&mut bytes); - original.serialize(&mut ser).unwrap(); + original.serialize(&mut ser)?; assert_eq!(bytes, vec![0, 0, 0, 0x41]); let de_cursor = Cursor::new(bytes); let deserialized: CharStruct = - CharStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor)).unwrap(); + CharStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor))?; assert_eq!(original, deserialized); // Test with complex char let original_complex = CharStruct { c: 'Ω' }; // Greek Omega, U+03A9 let mut bytes_complex = Vec::new(); let mut ser_complex = serializer::Serializer::new(&mut bytes_complex); - original_complex.serialize(&mut ser_complex).unwrap(); + original_complex.serialize(&mut ser_complex)?; assert_eq!(bytes_complex, vec![0, 0, 0x03, 0xA9]); let de_cursor_complex = Cursor::new(bytes_complex); let deserialized_complex: CharStruct = - CharStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor_complex)) - .unwrap(); + CharStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor_complex))?; assert_eq!(original_complex, deserialized_complex); // Test with an emoji let original_emoji = CharStruct { c: '\u{1F383}' }; // Pumpkin emoji, U+1F383 let mut bytes_emoji = Vec::new(); let mut ser_emoji = serializer::Serializer::new(&mut bytes_emoji); - original_emoji.serialize(&mut ser_emoji).unwrap(); + original_emoji.serialize(&mut ser_emoji)?; assert_eq!(bytes_emoji, vec![0, 0x01, 0xF3, 0x83]); let de_cursor_emoji = Cursor::new(bytes_emoji); let deserialized_emoji: CharStruct = - CharStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor_emoji)).unwrap(); + CharStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor_emoji))?; assert_eq!(original_emoji, deserialized_emoji); + Ok(()) } #[test] - fn i128_reserialize() { + fn i128_reserialize() -> Result<(), Box> { #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] struct I128Struct { val: i128, @@ -527,12 +533,12 @@ mod test { }; let mut bytes = Vec::new(); let mut ser = serializer::Serializer::new(&mut bytes); - original.serialize(&mut ser).unwrap(); + original.serialize(&mut ser)?; assert_eq!(bytes, original.val.to_be_bytes()); let de_cursor = Cursor::new(bytes); let deserialized: I128Struct = - I128Struct::deserialize(&mut deserializer::Deserializer::new(de_cursor)).unwrap(); + I128Struct::deserialize(&mut deserializer::Deserializer::new(de_cursor))?; assert_eq!(original, deserialized); let original_neg = I128Struct { @@ -540,17 +546,18 @@ mod test { }; let mut bytes_neg = Vec::new(); let mut ser_neg = serializer::Serializer::new(&mut bytes_neg); - original_neg.serialize(&mut ser_neg).unwrap(); + original_neg.serialize(&mut ser_neg)?; assert_eq!(bytes_neg, original_neg.val.to_be_bytes()); let de_cursor_neg = Cursor::new(bytes_neg); let deserialized_neg: I128Struct = - I128Struct::deserialize(&mut deserializer::Deserializer::new(de_cursor_neg)).unwrap(); + I128Struct::deserialize(&mut deserializer::Deserializer::new(de_cursor_neg))?; assert_eq!(original_neg, deserialized_neg); + Ok(()) } #[test] - fn u128_reserialize() { + fn u128_reserialize() -> Result<(), Box> { #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] struct U128Struct { val: u128, @@ -561,17 +568,18 @@ mod test { }; let mut bytes = Vec::new(); let mut ser = serializer::Serializer::new(&mut bytes); - original.serialize(&mut ser).unwrap(); + original.serialize(&mut ser)?; assert_eq!(bytes, original.val.to_be_bytes()); let de_cursor = Cursor::new(bytes); let deserialized: U128Struct = - U128Struct::deserialize(&mut deserializer::Deserializer::new(de_cursor)).unwrap(); + U128Struct::deserialize(&mut deserializer::Deserializer::new(de_cursor))?; assert_eq!(original, deserialized); + Ok(()) } #[test] - fn unit_reserialize() { + fn unit_reserialize() -> Result<(), Box> { #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] struct UnitStruct; #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] @@ -583,12 +591,12 @@ mod test { let original = UnitStruct; let mut bytes = Vec::new(); let mut ser = serializer::Serializer::new(&mut bytes); - original.serialize(&mut ser).unwrap(); + original.serialize(&mut ser)?; assert!(bytes.is_empty()); let de_cursor = Cursor::new(bytes); let deserialized: UnitStruct = - UnitStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor)).unwrap(); + UnitStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor))?; assert_eq!(original, deserialized); let original_with_unit = StructWithUnit { @@ -598,7 +606,7 @@ mod test { }; let mut bytes_with_unit = Vec::new(); let mut ser_with_unit = serializer::Serializer::new(&mut bytes_with_unit); - original_with_unit.serialize(&mut ser_with_unit).unwrap(); + original_with_unit.serialize(&mut ser_with_unit)?; // Check that only a and c were serialized let mut expected_bytes = Vec::new(); @@ -608,13 +616,13 @@ mod test { let de_cursor_with_unit = Cursor::new(bytes_with_unit); let deserialized_with_unit: StructWithUnit = - StructWithUnit::deserialize(&mut deserializer::Deserializer::new(de_cursor_with_unit)) - .unwrap(); + StructWithUnit::deserialize(&mut deserializer::Deserializer::new(de_cursor_with_unit))?; assert_eq!(original_with_unit, deserialized_with_unit); + Ok(()) } #[test] - fn enum_reserialize() { + fn enum_reserialize() -> Result<(), Box> { #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] enum MyEnum { A, @@ -625,25 +633,25 @@ mod test { let original_a = MyEnum::A; let mut bytes_a = Vec::new(); let mut ser_a = serializer::Serializer::new(&mut bytes_a); - original_a.serialize(&mut ser_a).unwrap(); + original_a.serialize(&mut ser_a)?; // VarInt for index 0 assert_eq!(bytes_a, vec![0x00]); let de_cursor_a = Cursor::new(bytes_a); let deserialized_a: MyEnum = - MyEnum::deserialize(&mut deserializer::Deserializer::new(de_cursor_a)).unwrap(); + MyEnum::deserialize(&mut deserializer::Deserializer::new(de_cursor_a))?; assert_eq!(original_a, deserialized_a); let original_b = MyEnum::B(123); let mut bytes_b = Vec::new(); let mut ser_b = serializer::Serializer::new(&mut bytes_b); - original_b.serialize(&mut ser_b).unwrap(); + original_b.serialize(&mut ser_b)?; // VarInt for index 1, then i32 for 123 let mut expected_bytes_b = vec![0x01]; expected_bytes_b.extend_from_slice(&123i32.to_be_bytes()); assert_eq!(bytes_b, expected_bytes_b); let de_cursor_b = Cursor::new(bytes_b); let deserialized_b: MyEnum = - MyEnum::deserialize(&mut deserializer::Deserializer::new(de_cursor_b)).unwrap(); + MyEnum::deserialize(&mut deserializer::Deserializer::new(de_cursor_b))?; assert_eq!(original_b, deserialized_b); let original_c = MyEnum::C { @@ -652,7 +660,7 @@ mod test { }; let mut bytes_c = Vec::new(); let mut ser_c = serializer::Serializer::new(&mut bytes_c); - original_c.serialize(&mut ser_c).unwrap(); + original_c.serialize(&mut ser_c)?; // VarInt for index 2, then i32 for 456, then string "hello" let mut expected_bytes_c = vec![0x02]; expected_bytes_c.extend_from_slice(&456i32.to_be_bytes()); @@ -661,19 +669,20 @@ mod test { assert_eq!(bytes_c, expected_bytes_c); let de_cursor_c = Cursor::new(bytes_c); let deserialized_c: MyEnum = - MyEnum::deserialize(&mut deserializer::Deserializer::new(de_cursor_c)).unwrap(); + MyEnum::deserialize(&mut deserializer::Deserializer::new(de_cursor_c))?; assert_eq!(original_c, deserialized_c); + Ok(()) } #[test] - fn tuple_struct_reserialize() { + fn tuple_struct_reserialize() -> Result<(), Box> { #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] struct MyTupleStruct(i32, String); let original = MyTupleStruct(789, "world".to_string()); let mut bytes = Vec::new(); let mut ser = serializer::Serializer::new(&mut bytes); - original.serialize(&mut ser).unwrap(); + original.serialize(&mut ser)?; let mut expected_bytes = Vec::new(); expected_bytes.extend_from_slice(&789i32.to_be_bytes()); @@ -683,12 +692,13 @@ mod test { let de_cursor = Cursor::new(bytes); let deserialized: MyTupleStruct = - MyTupleStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor)).unwrap(); + MyTupleStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor))?; assert_eq!(original, deserialized); + Ok(()) } #[test] - fn map_reserialize() { + fn map_reserialize() -> Result<(), Box> { use std::collections::HashMap; #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] @@ -704,14 +714,14 @@ mod test { let mut bytes = Vec::new(); let mut serializer = serializer::Serializer::new(&mut bytes); - original.serialize(&mut serializer).unwrap(); + original.serialize(&mut serializer)?; // Expected bytes: VarInt for map length (2), then key1, value1, key2, value2 // Order of elements in HashMap is not guaranteed, so we check deserialized content let de_cursor = Cursor::new(bytes.clone()); // Clone bytes for potential debug let deserialized: MyMapStruct = - MyMapStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor)).unwrap(); + MyMapStruct::deserialize(&mut deserializer::Deserializer::new(de_cursor))?; assert_eq!(original.map.len(), deserialized.map.len()); for (k, v) in original.map { @@ -724,13 +734,13 @@ mod test { }; let mut empty_map_bytes = Vec::new(); let mut empty_map_ser = serializer::Serializer::new(&mut empty_map_bytes); - empty_map_original.serialize(&mut empty_map_ser).unwrap(); + empty_map_original.serialize(&mut empty_map_ser)?; assert_eq!(empty_map_bytes, vec![0x00]); // VarInt for length 0 let empty_map_de_cursor = Cursor::new(empty_map_bytes); let empty_map_deserialized: MyMapStruct = - MyMapStruct::deserialize(&mut deserializer::Deserializer::new(empty_map_de_cursor)) - .unwrap(); + MyMapStruct::deserialize(&mut deserializer::Deserializer::new(empty_map_de_cursor))?; assert_eq!(empty_map_original, empty_map_deserialized); + Ok(()) } } diff --git a/pumpkin/src/entity/ai/pathfinder/mod.rs b/pumpkin/src/entity/ai/pathfinder/mod.rs index 7a3757832..a85274f13 100644 --- a/pumpkin/src/entity/ai/pathfinder/mod.rs +++ b/pumpkin/src/entity/ai/pathfinder/mod.rs @@ -408,13 +408,6 @@ impl Navigator { return; } - // Don't try to path-follow while airborne — let gravity handle it - if !on_ground { - entity.movement_input.store(Vector3::new(0.0, 0.0, 0.0)); - self.current_goal = Some(goal); - return; - } - let desired_yaw = wrap_degrees((dz.atan2(dx) as f32).to_degrees() - 90.0); let current_yaw = entity.entity.yaw.load(); let yaw_diff = wrap_degrees(desired_yaw - current_yaw); @@ -432,9 +425,13 @@ impl Navigator { .movement_input .store(Vector3::new(0.0, 0.0, mob_speed)); + let bbox = entity.entity.bounding_box.load(); + let width = bbox.max.x - bbox.min.x; + let jump_distance = 1.0f64.max(width); + // Jump when the next node is above step height and we're close enough horizontally if dy > entity.get_attribute_value(&Attributes::STEP_HEIGHT) - && horizontal_dist < 2.0 + && horizontal_dist_sq < jump_distance { entity .jumping diff --git a/pumpkin/src/net/java/mod.rs b/pumpkin/src/net/java/mod.rs index ab0c40fbb..96054f977 100644 --- a/pumpkin/src/net/java/mod.rs +++ b/pumpkin/src/net/java/mod.rs @@ -159,8 +159,16 @@ impl JavaClient { let crypt_key: [u8; 16] = shared_secret .try_into() .map_err(|_| EncryptionError::SharedWrongLength)?; - self.network_reader.lock().await.set_encryption(&crypt_key); - self.network_writer.lock().await.set_encryption(&crypt_key); + self.network_reader + .lock() + .await + .set_encryption(&crypt_key) + .map_err(|_| EncryptionError::AlreadyEncrypted)?; + self.network_writer + .lock() + .await + .set_encryption(&crypt_key) + .map_err(|_| EncryptionError::AlreadyEncrypted)?; Ok(()) } diff --git a/pumpkin/src/net/mod.rs b/pumpkin/src/net/mod.rs index 96e286bd2..51d4825ad 100644 --- a/pumpkin/src/net/mod.rs +++ b/pumpkin/src/net/mod.rs @@ -241,6 +241,8 @@ pub enum EncryptionError { FailedDecrypt, #[error("shared secret has the wrong length")] SharedWrongLength, + #[error("encryption is already enabled")] + AlreadyEncrypted, } fn is_valid_player_name(name: &str) -> bool { diff --git a/pumpkin/src/net/query.rs b/pumpkin/src/net/query.rs index 315e4482f..b0dbfb488 100644 --- a/pumpkin/src/net/query.rs +++ b/pumpkin/src/net/query.rs @@ -108,7 +108,9 @@ async fn handle_packet( // Ignore all errors since we don't want the query handler to crash // Protocol also ignores all errors and just doesn't respond - let _ = socket.send_to(response.encode().as_slice(), addr).await; + if let Some(encoded) = response.encode() { + let _ = socket.send_to(encoded.as_slice(), addr).await; + } clients.write().await.insert(challenge_token, addr); } @@ -171,7 +173,9 @@ async fn handle_packet( players, }; - let _ = socket.send_to(response.encode().as_slice(), addr).await; + if let Some(encoded) = response.encode() { + let _ = socket.send_to(encoded.as_slice(), addr).await; + } } else { let response = CBasicStatus { session_id: packet.session_id, @@ -189,7 +193,9 @@ async fn handle_packet( host_ip: CString::new(bound_addr.ip().to_string())?, }; - let _ = socket.send_to(response.encode().as_slice(), addr).await; + if let Some(encoded) = response.encode() { + let _ = socket.send_to(encoded.as_slice(), addr).await; + } } } }