Consolidate unsafe byte conversions in valence_nbt (#465)

# Objective

When using valence_nbt, it's often necessary to convert to and from
slices and vecs of `i8` and `u8`. But this requires unsafe code if you
want to avoid copying things.

# Solution

Expose the following functions in valence_nbt:
- `u8_vec_into_i8_vec(vec: Vec<u8>) -> Vec<i8>`
- `i8_vec_into_u8_vec(vec: Vec<i8>) -> Vec<u8>`
- `u8_slice_as_i8_slice(slice: &[u8]) -> &[i8]`
- `i8_slice_as_u8_slice(slice: &[i8]) -> &[u8]`

We've also made use of these functions ourselves to reduce the total
amount of unsafe code. Should also help in #263

Author: rj00a

crates/valence_nbt/src/binary/decode.rs

@@ -1,11 +1,10 @@
 use std::io::Write;
-use std::slice;
 
 use byteorder::{BigEndian, WriteBytesExt};
 
 use super::{modified_utf8, Error, Result};
 use crate::tag::Tag;
-use crate::{Compound, List, Value};
+use crate::{i8_slice_as_u8_slice, Compound, List, Value};
 
 impl Compound {
     /// Encodes uncompressed NBT binary data to the provided writer.
@@ -146,10 +145,7 @@ impl<W: Write> EncodeState<W> {
             }
         }
 
-        // SAFETY: i8 has the same layout as u8.
-        let bytes = unsafe { slice::from_raw_parts(bytes.as_ptr() as *const u8, bytes.len()) };
-
-        Ok(self.writer.write_all(bytes)?)
+        Ok(self.writer.write_all(i8_slice_as_u8_slice(bytes))?)
     }
 
     fn write_string(&mut self, s: &str) -> Result<()> {
@@ -197,10 +193,7 @@ impl<W: Write> EncodeState<W> {
                     }
                 }
 
-                // SAFETY: i8 has the same layout as u8.
-                let bytes = unsafe { slice::from_raw_parts(bl.as_ptr() as *const u8, bl.len()) };
-
-                Ok(self.writer.write_all(bytes)?)
+                Ok(self.writer.write_all(i8_slice_as_u8_slice(bl))?)
             }
             List::Short(sl) => self.write_list(sl, Tag::Short, |st, s| st.write_short(*s)),
             List::Int(il) => self.write_list(il, Tag::Int, |st, i| st.write_int(*i)),

crates/valence_nbt/src/lib.rs

@@ -17,6 +17,8 @@
     clippy::dbg_macro
 )]
 
+use std::mem::ManuallyDrop;
+
 pub use compound::Compound;
 pub use tag::Tag;
 pub use value::{List, Value};
@@ -75,3 +77,39 @@ macro_rules! compound {
         ])
     }
 }
+
+/// Converts a `Vec<u8>` into a `Vec<i8>` without cloning.
+#[inline]
+pub fn u8_vec_into_i8_vec(vec: Vec<u8>) -> Vec<i8> {
+    // 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<i8>` into a `Vec<u8>` without cloning.
+#[inline]
+pub fn i8_vec_into_u8_vec(vec: Vec<i8>) -> Vec<u8> {
+    // 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()) }
+}

crates/valence_nbt/src/serde.rs

@@ -1,5 +1,4 @@
 use std::fmt;
-use std::mem::ManuallyDrop;
 
 pub use ser::*;
 use thiserror::Error;
@@ -38,23 +37,3 @@ impl serde::ser::Error for Error {
         Self::new(format!("{msg}"))
     }
 }
-
-#[inline]
-fn u8_vec_to_i8_vec(vec: Vec<u8>) -> Vec<i8> {
-    // 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())
-    }
-}
-
-#[inline]
-fn i8_vec_to_u8_vec(vec: Vec<i8>) -> Vec<u8> {
-    // 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())
-    }
-}

crates/valence_nbt/src/serde/de.rs

@@ -1,12 +1,11 @@
-use std::{fmt, slice};
+use std::fmt;
 
 use serde::de::value::{MapAccessDeserializer, MapDeserializer, SeqAccessDeserializer};
 use serde::de::{self, IntoDeserializer, SeqAccess, Visitor};
 use serde::{forward_to_deserialize_any, Deserialize, Deserializer};
 
 use super::Error;
-use crate::serde::{i8_vec_to_u8_vec, u8_vec_to_i8_vec};
-use crate::{Compound, List, Value};
+use crate::{i8_vec_into_u8_vec, u8_slice_as_i8_slice, u8_vec_into_i8_vec, Compound, List, Value};
 
 impl<'de> Deserialize<'de> for Value {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
@@ -117,17 +116,14 @@ impl<'de> Deserialize<'de> for Value {
             where
                 E: de::Error,
             {
-                let slice: &[i8] =
-                    unsafe { slice::from_raw_parts(v.as_ptr() as *const i8, v.len()) };
-
-                Ok(Value::ByteArray(slice.into()))
+                Ok(Value::ByteArray(u8_slice_as_i8_slice(v).into()))
             }
 
             fn visit_byte_buf<E>(self, v: Vec<u8>) -> Result<Self::Value, E>
             where
                 E: de::Error,
             {
-                Ok(Value::ByteArray(u8_vec_to_i8_vec(v)))
+                Ok(Value::ByteArray(u8_vec_into_i8_vec(v)))
             }
 
             fn visit_seq<A>(self, seq: A) -> Result<Self::Value, A::Error>
@@ -190,17 +186,14 @@ impl<'de> Deserialize<'de> for List {
             where
                 E: de::Error,
             {
-                Ok(List::Byte(u8_vec_to_i8_vec(v)))
+                Ok(List::Byte(u8_vec_into_i8_vec(v)))
             }
 
             fn visit_bytes<E>(self, v: &[u8]) -> Result<Self::Value, E>
             where
                 E: de::Error,
             {
-                let bytes: &[i8] =
-                    unsafe { slice::from_raw_parts(v.as_ptr() as *const i8, v.len()) };
-
-                Ok(List::Byte(bytes.into()))
+                Ok(List::Byte(u8_slice_as_i8_slice(v).into()))
             }
         }
 
@@ -269,7 +262,7 @@ impl<'de> Deserializer<'de> for Value {
             Value::Long(v) => visitor.visit_i64(v),
             Value::Float(v) => visitor.visit_f32(v),
             Value::Double(v) => visitor.visit_f64(v),
-            Value::ByteArray(v) => visitor.visit_byte_buf(i8_vec_to_u8_vec(v)),
+            Value::ByteArray(v) => visitor.visit_byte_buf(i8_vec_into_u8_vec(v)),
             Value::String(v) => visitor.visit_string(v),
             Value::List(v) => v.deserialize_any(visitor),
             Value::Compound(v) => v.into_deserializer().deserialize_any(visitor),
@@ -347,7 +340,7 @@ impl<'de> Deserializer<'de> for List {
 
         match self {
             List::End => visitor.visit_seq(EndSeqAccess),
-            List::Byte(v) => visitor.visit_byte_buf(i8_vec_to_u8_vec(v)),
+            List::Byte(v) => visitor.visit_byte_buf(i8_vec_into_u8_vec(v)),
             List::Short(v) => v.into_deserializer().deserialize_any(visitor),
             List::Int(v) => v.into_deserializer().deserialize_any(visitor),
             List::Long(v) => v.into_deserializer().deserialize_any(visitor),

crates/valence_nbt/src/serde/ser.rs

@@ -1,11 +1,10 @@
-use core::slice;
 use std::marker::PhantomData;
 
 use serde::ser::{Impossible, SerializeMap, SerializeSeq, SerializeStruct};
 use serde::{Serialize, Serializer};
 
-use super::{u8_vec_to_i8_vec, Error};
-use crate::{Compound, List, Value};
+use super::Error;
+use crate::{i8_slice_as_u8_slice, u8_vec_into_i8_vec, Compound, List, Value};
 
 impl Serialize for Value {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
@@ -19,12 +18,7 @@ impl Serialize for Value {
             Value::Long(v) => serializer.serialize_i64(*v),
             Value::Float(v) => serializer.serialize_f32(*v),
             Value::Double(v) => serializer.serialize_f64(*v),
-            Value::ByteArray(v) => {
-                // SAFETY: i8 has the same layout as u8.
-                let bytes = unsafe { slice::from_raw_parts(v.as_ptr() as *const u8, v.len()) };
-
-                serializer.serialize_bytes(bytes)
-            }
+            Value::ByteArray(v) => serializer.serialize_bytes(i8_slice_as_u8_slice(v)),
             Value::String(v) => serializer.serialize_str(v),
             Value::List(v) => v.serialize(serializer),
             Value::Compound(v) => v.serialize(serializer),
@@ -317,7 +311,7 @@ impl Serializer for ValueSerializer {
     }
 
     fn serialize_bytes(self, v: &[u8]) -> Result<Self::Ok, Self::Error> {
-        Ok(Value::ByteArray(u8_vec_to_i8_vec(v.into())))
+        Ok(Value::ByteArray(u8_vec_into_i8_vec(v.into())))
     }
 
     fn serialize_none(self) -> Result<Self::Ok, Self::Error> {

crates/valence_protocol/src/impls.rs

@@ -18,7 +18,7 @@ use valence_generated::block::{BlockEntityKind, BlockKind, BlockState};
 use valence_generated::item::ItemKind;
 use valence_ident::{Ident, IdentError};
 use valence_math::*;
-use valence_nbt::Compound;
+use valence_nbt::{i8_slice_as_u8_slice, u8_slice_as_i8_slice, Compound};
 use valence_text::Text;
 
 use super::var_int::VarInt;
@@ -69,9 +69,7 @@ impl Encode for i8 {
     }
 
     fn encode_slice(slice: &[i8], mut w: impl Write) -> Result<()> {
-        // SAFETY: i8 has the same layout as u8.
-        let bytes = unsafe { slice::from_raw_parts(slice.as_ptr() as *const u8, slice.len()) };
-        Ok(w.write_all(bytes)?)
+        Ok(w.write_all(i8_slice_as_u8_slice(slice))?)
     }
 }
 
@@ -546,10 +544,7 @@ impl<'a> Decode<'a> for &'a [i8] {
     fn decode(r: &mut &'a [u8]) -> Result<Self> {
         let bytes = <&[u8]>::decode(r)?;
 
-        // SAFETY: i8 and u8 have the same layout.
-        let bytes = unsafe { slice::from_raw_parts(bytes.as_ptr() as *const i8, bytes.len()) };
-
-        Ok(bytes)
+        Ok(u8_slice_as_i8_slice(bytes))
     }
 }