From 1ddde71963ca3b15e8d97d1e4383b61431a30bbc Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Tue, 5 Sep 2023 18:06:07 -0700 Subject: [PATCH] NBT Tweaks (#522) # Objective `valence_nbt` could use some adjustments. # Solution - Move top-level conversion functions to `conv` module. - Remove public `Display` impl from `Tag`. - Remove implicit "null" byte check when decoding compounds. This check can be done in advance if desired. - Remove potentially dangerous memory preallocation when decoding binary. - Make `Compound::to_binary` and `Compound::from_binary` free functions for discoverability and familiarity with serde_json. --- crates/valence_anvil/src/lib.rs | 10 +- crates/valence_nbt/src/binary.rs | 33 ++++- crates/valence_nbt/src/binary/decode.rs | 70 +++++----- crates/valence_nbt/src/binary/encode.rs | 146 ++++++++++----------- crates/valence_nbt/src/binary/tests.rs | 17 +-- crates/valence_nbt/src/conv.rs | 52 ++++++++ crates/valence_nbt/src/lib.rs | 41 +----- crates/valence_nbt/src/serde/de.rs | 3 +- crates/valence_nbt/src/serde/ser.rs | 3 +- crates/valence_nbt/src/tag.rs | 32 +---- crates/valence_protocol/src/impls/other.rs | 14 +- crates/valence_registry/src/codec.rs | 2 +- 12 files changed, 222 insertions(+), 201 deletions(-) create mode 100644 crates/valence_nbt/src/conv.rs diff --git a/crates/valence_anvil/src/lib.rs b/crates/valence_anvil/src/lib.rs index 0f1ffff1c..3770f1d94 100644 --- a/crates/valence_anvil/src/lib.rs +++ b/crates/valence_anvil/src/lib.rs @@ -493,7 +493,7 @@ impl Region { None => return Err(RegionError::InvalidCompressionScheme(compression)), }; - let (data, _) = Compound::from_binary(&mut nbt_slice)?; + let (data, _) = valence_nbt::from_binary(&mut nbt_slice)?; if !nbt_slice.is_empty() { return Err(RegionError::TrailingNbtData); @@ -553,15 +553,17 @@ impl Region { compress_buf.clear(); let mut compress_cursor = Cursor::new(compress_buf); match options.compression { - Compression::Gzip => chunk.to_binary( + Compression::Gzip => valence_nbt::to_binary( + chunk, GzEncoder::new(&mut compress_cursor, flate2::Compression::default()), "", )?, - Compression::Zlib => chunk.to_binary( + Compression::Zlib => valence_nbt::to_binary( + chunk, ZlibEncoder::new(&mut compress_cursor, flate2::Compression::default()), "", )?, - Compression::None => chunk.to_binary(&mut compress_cursor, "")?, + Compression::None => valence_nbt::to_binary(chunk, &mut compress_cursor, "")?, } let compress_buf = compress_cursor.into_inner(); diff --git a/crates/valence_nbt/src/binary.rs b/crates/valence_nbt/src/binary.rs index 7e074653c..e71741c2a 100644 --- a/crates/valence_nbt/src/binary.rs +++ b/crates/valence_nbt/src/binary.rs @@ -4,7 +4,7 @@ //! # Examples //! //! ``` -//! use valence_nbt::{compound, Compound, List}; +//! use valence_nbt::{compound, to_binary, Compound, List}; //! //! let c = compound! { //! "byte" => 5_i8, @@ -18,13 +18,13 @@ //! //! let mut buf = vec![]; //! -//! c.to_binary(&mut buf, "").unwrap(); +//! to_binary(&c, &mut buf, "").unwrap(); //! ``` //! //! Decode NBT data from its binary form. //! //! ``` -//! use valence_nbt::{compound, Compound}; +//! use valence_nbt::{compound, from_binary, Compound}; //! //! let some_bytes = [10, 0, 0, 3, 0, 3, 105, 110, 116, 0, 0, 222, 173, 0]; //! @@ -32,7 +32,7 @@ //! "int" => 0xdead //! }; //! -//! let (nbt, root_name) = Compound::from_binary(&mut some_bytes.as_slice()).unwrap(); +//! let (nbt, root_name) = from_binary(&mut some_bytes.as_slice()).unwrap(); //! //! assert_eq!(nbt, expected_value); //! assert_eq!(root_name, ""); @@ -45,4 +45,29 @@ mod modified_utf8; #[cfg(test)] mod tests; +pub use decode::from_binary; +pub use encode::{to_binary, written_size}; pub use error::*; + +use crate::Tag; + +impl Tag { + /// Returns the name of this tag for error reporting purposes. + const fn name(self) -> &'static str { + match self { + Tag::End => "end", + Tag::Byte => "byte", + Tag::Short => "short", + Tag::Int => "int", + Tag::Long => "long", + Tag::Float => "float", + Tag::Double => "double", + Tag::ByteArray => "byte array", + Tag::String => "string", + Tag::List => "list", + Tag::Compound => "compound", + Tag::IntArray => "int array", + Tag::LongArray => "long array", + } + } +} diff --git a/crates/valence_nbt/src/binary/decode.rs b/crates/valence_nbt/src/binary/decode.rs index 547809726..b735ffd75 100644 --- a/crates/valence_nbt/src/binary/decode.rs +++ b/crates/valence_nbt/src/binary/decode.rs @@ -7,35 +7,28 @@ use super::{Error, Result}; use crate::tag::Tag; use crate::{Compound, List, Value}; -impl Compound { - /// Decodes uncompressed NBT binary data from the provided slice. - /// - /// The string returned in the tuple is the name of the root compound - /// (typically the empty string). - pub fn from_binary(slice: &mut &[u8]) -> Result<(Self, String)> { - let mut state = DecodeState { slice, depth: 0 }; - - let root_tag = state.read_tag()?; - - // For cases such as Block Entity Data in the chunk packet. - // https://wiki.vg/Protocol#Chunk_Data_and_Update_Light - if root_tag == Tag::End { - return Ok((Compound::new(), String::new())); - } - - if root_tag != Tag::Compound { - return Err(Error::new_owned(format!( - "expected root tag for compound (got {root_tag})", - ))); - } +/// Decodes uncompressed NBT binary data from the provided slice. +/// +/// The string returned in the tuple is the name of the root compound +/// (typically the empty string). +pub fn from_binary(slice: &mut &[u8]) -> Result<(Compound, String)> { + let mut state = DecodeState { slice, depth: 0 }; + + let root_tag = state.read_tag()?; + + if root_tag != Tag::Compound { + return Err(Error::new_owned(format!( + "expected root tag for compound (got {})", + root_tag.name(), + ))); + } - let root_name = state.read_string()?; - let root = state.read_compound()?; + let root_name = state.read_string()?; + let root = state.read_compound()?; - debug_assert_eq!(state.depth, 0); + debug_assert_eq!(state.depth, 0); - Ok((root, root_name)) - } + Ok((root, root_name)) } /// Maximum recursion depth to prevent overflowing the call stack. @@ -183,23 +176,23 @@ impl DecodeState<'_, '_> { .read_list(Tag::Double, 8, |st| st.read_double())? .into()), Tag::ByteArray => Ok(self - .read_list(Tag::ByteArray, 4, |st| st.read_byte_array())? + .read_list(Tag::ByteArray, 0, |st| st.read_byte_array())? .into()), Tag::String => Ok(self - .read_list(Tag::String, 2, |st| st.read_string())? + .read_list(Tag::String, 0, |st| st.read_string())? .into()), Tag::List => self - .check_depth(|st| Ok(st.read_list(Tag::List, 5, |st| st.read_any_list())?.into())), + .check_depth(|st| Ok(st.read_list(Tag::List, 0, |st| st.read_any_list())?.into())), Tag::Compound => self.check_depth(|st| { Ok(st - .read_list(Tag::Compound, 1, |st| st.read_compound())? + .read_list(Tag::Compound, 0, |st| st.read_compound())? .into()) }), Tag::IntArray => Ok(self - .read_list(Tag::IntArray, 4, |st| st.read_int_array())? + .read_list(Tag::IntArray, 0, |st| st.read_int_array())? .into()), Tag::LongArray => Ok(self - .read_list(Tag::LongArray, 4, |st| st.read_long_array())? + .read_list(Tag::LongArray, 0, |st| st.read_long_array())? .into()), } } @@ -211,7 +204,7 @@ impl DecodeState<'_, '_> { fn read_list( &mut self, elem_type: Tag, - min_elem_size: usize, + elem_size: usize, mut read_elem: F, ) -> Result> where @@ -221,19 +214,22 @@ impl DecodeState<'_, '_> { if len.is_negative() { return Err(Error::new_owned(format!( - "negative {elem_type} list length of {len}", + "negative {} list length of {len}", + elem_type.name() ))); } // Ensure we don't reserve more than the maximum amount of memory required given // the size of the remaining input. - if len as u64 * min_elem_size as u64 > self.slice.len() as u64 { + if len as u64 * elem_size as u64 > self.slice.len() as u64 { return Err(Error::new_owned(format!( - "{elem_type} list of length {len} exceeds remainder of input" + "{} list of length {len} exceeds remainder of input", + elem_type.name() ))); } - let mut list = Vec::with_capacity(len as usize); + let mut list = Vec::with_capacity(if elem_size == 0 { 0 } else { len as usize }); + for _ in 0..len { list.push(read_elem(self)?); } diff --git a/crates/valence_nbt/src/binary/encode.rs b/crates/valence_nbt/src/binary/encode.rs index a663a43fc..231885d43 100644 --- a/crates/valence_nbt/src/binary/encode.rs +++ b/crates/valence_nbt/src/binary/encode.rs @@ -3,85 +3,84 @@ use std::io::Write; use byteorder::{BigEndian, WriteBytesExt}; use super::{modified_utf8, Error, Result}; +use crate::conv::i8_slice_as_u8_slice; use crate::tag::Tag; -use crate::{i8_slice_as_u8_slice, Compound, List, Value}; - -impl Compound { - /// Encodes uncompressed NBT binary data to the provided writer. - /// - /// Only compounds are permitted at the top level. This is why the function - /// accepts a [`Compound`] reference rather than a [`Value`]. - /// - /// Additionally, the root compound can be given a name. Typically the empty - /// string `""` is used. - pub fn to_binary(&self, writer: W, root_name: &str) -> Result<()> { - let mut state = EncodeState { writer }; - - state.write_tag(Tag::Compound)?; - state.write_string(root_name)?; - state.write_compound(self)?; - - Ok(()) - } - - /// Returns the number of bytes that will be written when - /// [`Compound::to_binary`] is called with this compound and root name. - /// - /// If `to_binary` results in `Ok`, the exact number of bytes - /// reported by this function will have been written. If the result is - /// `Err`, then the reported count will be greater than or equal to the - /// number of bytes that have actually been written. - pub fn written_size(&self, root_name: &str) -> usize { - fn value_size(val: &Value) -> usize { - match val { - Value::Byte(_) => 1, - Value::Short(_) => 2, - Value::Int(_) => 4, - Value::Long(_) => 8, - Value::Float(_) => 4, - Value::Double(_) => 8, - Value::ByteArray(v) => 4 + v.len(), - Value::String(v) => string_size(v), - Value::List(v) => list_size(v), - Value::Compound(v) => compound_size(v), - Value::IntArray(v) => 4 + v.len() * 4, - Value::LongArray(v) => 4 + v.len() * 8, - } - } +use crate::{Compound, List, Value}; + +/// Encodes uncompressed NBT binary data to the provided writer. +/// +/// Only compounds are permitted at the top level. This is why the function +/// accepts a [`Compound`] reference rather than a [`Value`]. +/// +/// Additionally, the root compound can be given a name. Typically the empty +/// string `""` is used. +pub fn to_binary(comp: &Compound, writer: W, root_name: &str) -> Result<()> { + let mut state = EncodeState { writer }; + + state.write_tag(Tag::Compound)?; + state.write_string(root_name)?; + state.write_compound(comp)?; + + Ok(()) +} - fn list_size(l: &List) -> usize { - let elems_size = match l { - List::End => 0, - List::Byte(v) => v.len(), - List::Short(v) => v.len() * 2, - List::Int(v) => v.len() * 4, - List::Long(v) => v.len() * 8, - List::Float(v) => v.len() * 4, - List::Double(v) => v.len() * 8, - List::ByteArray(v) => v.iter().map(|b| 4 + b.len()).sum(), - List::String(v) => v.iter().map(|s| string_size(s)).sum(), - List::List(v) => v.iter().map(list_size).sum(), - List::Compound(v) => v.iter().map(compound_size).sum(), - List::IntArray(v) => v.iter().map(|i| 4 + i.len() * 4).sum(), - List::LongArray(v) => v.iter().map(|l| 4 + l.len() * 8).sum(), - }; - - 1 + 4 + elems_size +/// Returns the number of bytes that will be written when +/// [`to_binary`] is called with this compound and root name. +/// +/// If `to_binary` results in `Ok`, the exact number of bytes +/// reported by this function will have been written. If the result is +/// `Err`, then the reported count will be greater than or equal to the +/// number of bytes that have actually been written. +pub fn written_size(comp: &Compound, root_name: &str) -> usize { + fn value_size(val: &Value) -> usize { + match val { + Value::Byte(_) => 1, + Value::Short(_) => 2, + Value::Int(_) => 4, + Value::Long(_) => 8, + Value::Float(_) => 4, + Value::Double(_) => 8, + Value::ByteArray(v) => 4 + v.len(), + Value::String(v) => string_size(v), + Value::List(v) => list_size(v), + Value::Compound(v) => compound_size(v), + Value::IntArray(v) => 4 + v.len() * 4, + Value::LongArray(v) => 4 + v.len() * 8, } + } - fn string_size(s: &str) -> usize { - 2 + modified_utf8::encoded_len(s) - } + fn list_size(l: &List) -> usize { + let elems_size = match l { + List::End => 0, + List::Byte(v) => v.len(), + List::Short(v) => v.len() * 2, + List::Int(v) => v.len() * 4, + List::Long(v) => v.len() * 8, + List::Float(v) => v.len() * 4, + List::Double(v) => v.len() * 8, + List::ByteArray(v) => v.iter().map(|b| 4 + b.len()).sum(), + List::String(v) => v.iter().map(|s| string_size(s)).sum(), + List::List(v) => v.iter().map(list_size).sum(), + List::Compound(v) => v.iter().map(compound_size).sum(), + List::IntArray(v) => v.iter().map(|i| 4 + i.len() * 4).sum(), + List::LongArray(v) => v.iter().map(|l| 4 + l.len() * 8).sum(), + }; + + 1 + 4 + elems_size + } - fn compound_size(c: &Compound) -> usize { - c.iter() - .map(|(k, v)| 1 + string_size(k) + value_size(v)) - .sum::() - + 1 - } + fn string_size(s: &str) -> usize { + 2 + modified_utf8::encoded_len(s) + } - 1 + string_size(root_name) + compound_size(self) + fn compound_size(c: &Compound) -> usize { + c.iter() + .map(|(k, v)| 1 + string_size(k) + value_size(v)) + .sum::() + + 1 } + + 1 + string_size(root_name) + compound_size(comp) } struct EncodeState { @@ -223,8 +222,9 @@ impl EncodeState { Ok(len) => self.writer.write_i32::(len)?, Err(_) => { return Err(Error::new_owned(format!( - "{elem_type} list of length {} exceeds maximum of i32::MAX", + "{} list of length {} exceeds maximum of i32::MAX", list.len(), + elem_type.name() ))) } } diff --git a/crates/valence_nbt/src/binary/tests.rs b/crates/valence_nbt/src/binary/tests.rs index 897cabbf1..4ac0bca11 100644 --- a/crates/valence_nbt/src/binary/tests.rs +++ b/crates/valence_nbt/src/binary/tests.rs @@ -1,5 +1,6 @@ +use crate::binary::written_size; use crate::tag::Tag; -use crate::{compound, Compound, List, Value}; +use crate::{compound, from_binary, to_binary, Compound, List, Value}; const ROOT_NAME: &str = "The root name‽"; @@ -9,11 +10,11 @@ fn round_trip() { let compound = example_compound(); - compound.to_binary(&mut buf, ROOT_NAME).unwrap(); + to_binary(&compound, &mut buf, ROOT_NAME).unwrap(); println!("{buf:?}"); - let (decoded, root_name) = Compound::from_binary(&mut buf.as_slice()).unwrap(); + let (decoded, root_name) = crate::from_binary(&mut buf.as_slice()).unwrap(); assert_eq!(root_name, ROOT_NAME); assert_eq!(compound, decoded); @@ -28,7 +29,7 @@ fn check_min_sizes() { let dbg = format!("{min_val:?}"); let mut buf = vec![]; - compound!("" => min_val).to_binary(&mut buf, "").unwrap(); + to_binary(&compound!("" => min_val), &mut buf, "").unwrap(); assert_eq!( expected_size, @@ -65,7 +66,7 @@ fn deeply_nested_compound_decode() { buf.push(Tag::End as u8); // End root compound // Should not overflow the stack - let _ = Compound::from_binary(&mut buf.as_slice()); + let _ = from_binary(&mut buf.as_slice()); } #[test] @@ -84,7 +85,7 @@ fn deeply_nested_list_decode() { buf.push(Tag::End as u8); // End root compound // Should not overflow the stack - let _ = Compound::from_binary(&mut buf.as_slice()); + let _ = from_binary(&mut buf.as_slice()); } #[test] @@ -92,9 +93,9 @@ fn correct_length() { let c = example_compound(); let mut buf = vec![]; - c.to_binary(&mut buf, "abc").unwrap(); + to_binary(&c, &mut buf, "abc").unwrap(); - assert_eq!(c.written_size("abc"), buf.len()); + assert_eq!(written_size(&c, "abc"), buf.len()); } fn example_compound() -> Compound { diff --git a/crates/valence_nbt/src/conv.rs b/crates/valence_nbt/src/conv.rs new file mode 100644 index 000000000..9ff0bb4ac --- /dev/null +++ b/crates/valence_nbt/src/conv.rs @@ -0,0 +1,52 @@ +//! Zero-cost conversion functions for `valence_nbt`. +//! +//! While working with [`Value`], it is often necessary to convert between +//! collections of signed and unsigned integer types due to API +//! differences. For instance, you may be given a `&[i8]` from +//! [`Value::ByteArray`], but functions like [`Write::write_all`] expect to +//! receive a `&[u8]`. +//! +//! This module provides functions to perform conversions between these types +//! with zero-cost and no `unsafe` code on your part. +//! +//! [`Value`]: crate::Value +//! [`Value::ByteArray`]: crate::Value::ByteArray +//! [`Write::write_all`]: std::io::Write::write_all + +use std::mem::ManuallyDrop; + +/// Converts a `Vec` into a `Vec` without cloning. +#[inline] +pub fn u8_vec_into_i8_vec(vec: Vec) -> Vec { + // SAFETY: Layouts of u8 and i8 are the same and we're being careful not to drop + // the original vec after calling Vec::from_raw_parts. + unsafe { + let mut vec = ManuallyDrop::new(vec); + Vec::from_raw_parts(vec.as_mut_ptr() as *mut i8, vec.len(), vec.capacity()) + } +} + +/// Converts a `Vec` into a `Vec` without cloning. +#[inline] +pub fn i8_vec_into_u8_vec(vec: Vec) -> Vec { + // SAFETY: Layouts of u8 and i8 are the same and we're being careful not to drop + // the original vec after calling Vec::from_raw_parts. + unsafe { + let mut vec = ManuallyDrop::new(vec); + Vec::from_raw_parts(vec.as_mut_ptr() as *mut u8, vec.len(), vec.capacity()) + } +} + +/// Converts a `&[u8]` into a `&[i8]`. +#[inline] +pub fn u8_slice_as_i8_slice(slice: &[u8]) -> &[i8] { + // SAFETY: i8 has the same layout as u8. + unsafe { std::slice::from_raw_parts(slice.as_ptr() as *const i8, slice.len()) } +} + +/// Converts a `&[i8]` into a `&[u8]`. +#[inline] +pub fn i8_slice_as_u8_slice(slice: &[i8]) -> &[u8] { + // SAFETY: i8 has the same layout as u8. + unsafe { std::slice::from_raw_parts(slice.as_ptr() as *const u8, slice.len()) } +} diff --git a/crates/valence_nbt/src/lib.rs b/crates/valence_nbt/src/lib.rs index 15746da02..440623f00 100644 --- a/crates/valence_nbt/src/lib.rs +++ b/crates/valence_nbt/src/lib.rs @@ -17,8 +17,8 @@ clippy::dbg_macro )] -use std::mem::ManuallyDrop; - +#[cfg(feature = "binary")] +pub use binary::{from_binary, to_binary}; pub use compound::Compound; pub use list::List; pub use tag::Tag; @@ -27,6 +27,7 @@ pub use value::Value; #[cfg(feature = "binary")] pub mod binary; pub mod compound; +pub mod conv; pub mod list; #[cfg(feature = "serde")] pub mod serde; @@ -79,39 +80,3 @@ macro_rules! compound { ]) } } - -/// Converts a `Vec` into a `Vec` without cloning. -#[inline] -pub fn u8_vec_into_i8_vec(vec: Vec) -> Vec { - // SAFETY: Layouts of u8 and i8 are the same and we're being careful not to drop - // the original vec after calling Vec::from_raw_parts. - unsafe { - let mut vec = ManuallyDrop::new(vec); - Vec::from_raw_parts(vec.as_mut_ptr() as *mut i8, vec.len(), vec.capacity()) - } -} - -/// Converts a `Vec` into a `Vec` without cloning. -#[inline] -pub fn i8_vec_into_u8_vec(vec: Vec) -> Vec { - // SAFETY: Layouts of u8 and i8 are the same and we're being careful not to drop - // the original vec after calling Vec::from_raw_parts. - unsafe { - let mut vec = ManuallyDrop::new(vec); - Vec::from_raw_parts(vec.as_mut_ptr() as *mut u8, vec.len(), vec.capacity()) - } -} - -/// Converts a `&[u8]` into a `&[i8]`. -#[inline] -pub fn u8_slice_as_i8_slice(slice: &[u8]) -> &[i8] { - // SAFETY: i8 has the same layout as u8. - unsafe { std::slice::from_raw_parts(slice.as_ptr() as *const i8, slice.len()) } -} - -/// Converts a `&[i8]` into a `&[u8]`. -#[inline] -pub fn i8_slice_as_u8_slice(slice: &[i8]) -> &[u8] { - // SAFETY: i8 has the same layout as u8. - unsafe { std::slice::from_raw_parts(slice.as_ptr() as *const u8, slice.len()) } -} diff --git a/crates/valence_nbt/src/serde/de.rs b/crates/valence_nbt/src/serde/de.rs index 9bc5f7327..751b18004 100644 --- a/crates/valence_nbt/src/serde/de.rs +++ b/crates/valence_nbt/src/serde/de.rs @@ -5,7 +5,8 @@ use serde::de::{self, IntoDeserializer, SeqAccess, Visitor}; use serde::{forward_to_deserialize_any, Deserialize, Deserializer}; use super::Error; -use crate::{i8_vec_into_u8_vec, u8_slice_as_i8_slice, u8_vec_into_i8_vec, Compound, List, Value}; +use crate::conv::{i8_vec_into_u8_vec, u8_slice_as_i8_slice, u8_vec_into_i8_vec}; +use crate::{Compound, List, Value}; impl<'de> Deserialize<'de> for Value { fn deserialize(deserializer: D) -> Result diff --git a/crates/valence_nbt/src/serde/ser.rs b/crates/valence_nbt/src/serde/ser.rs index 4b76e0131..5fac4b04b 100644 --- a/crates/valence_nbt/src/serde/ser.rs +++ b/crates/valence_nbt/src/serde/ser.rs @@ -4,7 +4,8 @@ use serde::ser::{Impossible, SerializeMap, SerializeSeq, SerializeStruct}; use serde::{Serialize, Serializer}; use super::Error; -use crate::{i8_slice_as_u8_slice, u8_vec_into_i8_vec, Compound, List, Value}; +use crate::conv::{i8_slice_as_u8_slice, u8_vec_into_i8_vec}; +use crate::{Compound, List, Value}; impl Serialize for Value { fn serialize(&self, serializer: S) -> Result diff --git a/crates/valence_nbt/src/tag.rs b/crates/valence_nbt/src/tag.rs index 258053ac5..4bc9e2639 100644 --- a/crates/valence_nbt/src/tag.rs +++ b/crates/valence_nbt/src/tag.rs @@ -1,7 +1,5 @@ -use std::fmt; -use std::fmt::Formatter; - -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +/// One of the possible NBT data types. +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)] pub enum Tag { // Variant order is significant! End, @@ -18,29 +16,3 @@ pub enum Tag { IntArray, LongArray, } - -impl Tag { - pub const fn name(self) -> &'static str { - match self { - Tag::End => "end", - Tag::Byte => "byte", - Tag::Short => "short", - Tag::Int => "int", - Tag::Long => "long", - Tag::Float => "float", - Tag::Double => "double", - Tag::ByteArray => "byte array", - Tag::String => "string", - Tag::List => "list", - Tag::Compound => "compound", - Tag::IntArray => "int array", - Tag::LongArray => "long array", - } - } -} - -impl fmt::Display for Tag { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.name()) - } -} diff --git a/crates/valence_protocol/src/impls/other.rs b/crates/valence_protocol/src/impls/other.rs index 4387481f1..256db9548 100644 --- a/crates/valence_protocol/src/impls/other.rs +++ b/crates/valence_protocol/src/impls/other.rs @@ -44,15 +44,21 @@ impl<'a> Decode<'a> for Uuid { impl Encode for Compound { fn encode(&self, w: impl Write) -> anyhow::Result<()> { - Ok(self.to_binary(w, "")?) + Ok(valence_nbt::to_binary(self, w, "")?) } } impl Decode<'_> for Compound { fn decode(r: &mut &[u8]) -> anyhow::Result { - // TODO: Bound the input slice or do something else to prevent this from eating - // too much memory. - Ok(Self::from_binary(r)?.0) + // Check for null compound. + if r.first() == Some(&0) { + *r = &r[1..]; + return Ok(Compound::new()); + } + + // TODO: consider if we need to bound the input slice or add some other + // mitigation to prevent excessive memory usage on hostile input. + Ok(valence_nbt::from_binary(r)?.0) } } diff --git a/crates/valence_registry/src/codec.rs b/crates/valence_registry/src/codec.rs index 35a7447ce..ee8d9f0ab 100644 --- a/crates/valence_registry/src/codec.rs +++ b/crates/valence_registry/src/codec.rs @@ -52,7 +52,7 @@ impl RegistryCodec { impl Default for RegistryCodec { fn default() -> Self { let codec = include_bytes!("../extracted/registry_codec.dat"); - let compound = Compound::from_binary(&mut codec.as_slice()) + let compound = valence_nbt::from_binary(&mut codec.as_slice()) .expect("failed to decode vanilla registry codec") .0;