Add Packable::unpack variants (iotaledger#814)

* add `CHECK` parameter to `Packable::unpack` * update `bee-packable-derive` * update `bee-message` * update the `bee-storage` crates * add `metadata` feature to `bee-storage-rocksdb` * move helper methods to `PackableExt` * rename `CHECK` to `VERIFY` * rename `unpacked_(un)?checked` to `unpacked_(un)?verified` * address review comments
thibault-martinez · Nov 16, 2021 · acd179c · acd179c
1 parent 0d56a06
commit acd179c
Show file tree

Hide file tree

Showing 72 changed files with 360 additions and 274 deletions.
diff --git a/bee-common/bee-packable-derive/src/fragments.rs b/bee-common/bee-packable-derive/src/fragments.rs
@@ -35,7 +35,7 @@ impl Fragments {
                 Ok(())
             },
             unpack: quote! {Ok(#path {
-                #(#fields_pattern_ident: <#fields_type>::unpack(unpacker).map_packable_err(#fields_unpack_error_with).coerce()?,)*
+                #(#fields_pattern_ident: <#fields_type>::unpack::<_, VERIFY>(unpacker).map_packable_err(#fields_unpack_error_with).coerce()?,)*
             })},
         }
     }

diff --git a/bee-common/bee-packable-derive/src/trait_impl.rs b/bee-common/bee-packable-derive/src/trait_impl.rs
@@ -117,7 +117,7 @@ impl TraitImpl {
                         #(#tag_decls)*
 
                         #[deny(unreachable_patterns)]
-                        match <#tag_type>::unpack(unpacker).infallible()? {
+                        match <#tag_type>::unpack::<_, VERIFY>(unpacker).infallible()? {
                             #(#unpack_arms)*
                             tag => Err(bee_packable::error::UnpackError::from_packable(#tag_with_error(tag)))
                         }
@@ -158,7 +158,7 @@ impl ToTokens for TraitImpl {
                     #pack
                 }
 
-                fn unpack<U: bee_packable::unpacker::Unpacker>(unpacker: &mut U) -> Result<Self, bee_packable::error::UnpackError<Self::UnpackError, U::Error>> {
+                fn unpack<U: bee_packable::unpacker::Unpacker, const VERIFY: bool>(unpacker: &mut U) -> Result<Self, bee_packable::error::UnpackError<Self::UnpackError, U::Error>> {
                     use bee_packable::error::UnpackErrorExt;
                     #unpack
                 }

diff --git a/bee-common/bee-packable-derive/tests/pass/enum_discriminant.rs b/bee-common/bee-packable-derive/tests/pass/enum_discriminant.rs
@@ -1,7 +1,7 @@
 // Copyright 2021 IOTA Stiftung
 // SPDX-License-Identifier: Apache-2.0
 
-use bee_packable::Packable;
+use bee_packable::{Packable, PackableExt};
 
 // repr type is used as tag type and discriminant values are used as tags.
 #[derive(Packable)]

diff --git a/bee-common/bee-packable-derive/tests/pass/error_coercion.rs b/bee-common/bee-packable-derive/tests/pass/error_coercion.rs
@@ -29,8 +29,10 @@ impl Packable for Picky {
         self.0.pack(packer)
     }
 
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
-        let value = u8::unpack(unpacker).infallible()?;
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+        let value = u8::unpack::<_, VERIFY>(unpacker).infallible()?;
 
         if value == 42 {
             Ok(Self(value))

diff --git a/bee-common/bee-packable/src/packable/array.rs b/bee-common/bee-packable/src/packable/array.rs
@@ -20,13 +20,15 @@ impl<T: Packable, const N: usize> Packable for [T; N] {
         Ok(())
     }
 
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
         // Safety: an uninitialized array of [`MaybeUninit`]s is safe to be considered initialized.
         // FIXME: replace with [`MaybeUninit::uninit_array`] when stabilized.
         let mut array = unsafe { MaybeUninit::<[MaybeUninit<T>; N]>::uninit().assume_init() };
 
         for item in array.iter_mut() {
-            let unpacked = T::unpack(unpacker)?;
+            let unpacked = T::unpack::<_, VERIFY>(unpacker)?;
             // Safety: each `item` is only visited once so we are never overwriting nor dropping
             // values that are already initialized.
             unsafe {

diff --git a/bee-common/bee-packable/src/packable/bool.rs b/bee-common/bee-packable/src/packable/bool.rs
@@ -23,7 +23,9 @@ impl Packable for bool {
     }
 
     /// Booleans are unpacked if the byte used to represent them is non-zero.
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
-        Ok(u8::unpack(unpacker).infallible()? != 0)
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+        Ok(u8::unpack::<_, VERIFY>(unpacker).infallible()? != 0)
     }
 }
diff --git a/bee-common/bee-packable/src/packable/bounded.rs b/bee-common/bee-packable/src/packable/bounded.rs
@@ -113,11 +113,17 @@ macro_rules! bounded {
                 self.0.pack(packer)
             }
 
-            fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
-                $ty::unpack(unpacker)
-                    .infallible()?
-                    .try_into()
-                    .map_err(UnpackError::Packable)
+
+            fn unpack<U: Unpacker, const VERIFY: bool>(
+                unpacker: &mut U,
+            ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+                let value = $ty::unpack::<_, VERIFY>(unpacker).infallible()?;
+
+                if VERIFY && !(MIN..=MAX).contains(&value) {
+                    return Err(UnpackError::Packable($error(value)));
+                }
+
+                Ok(Self(value))
             }
         }
     };

diff --git a/bee-common/bee-packable/src/packable/box.rs b/bee-common/bee-packable/src/packable/box.rs
@@ -21,8 +21,10 @@ impl<T: Packable> Packable for Box<T> {
         Ok(())
     }
 
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
-        Ok(Box::new(T::unpack(unpacker)?))
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+        Ok(Box::new(T::unpack::<_, VERIFY>(unpacker)?))
     }
 }
 
@@ -44,7 +46,9 @@ impl<T: Packable> Packable for Box<[T]> {
         Ok(())
     }
 
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
-        Ok(Vec::<T>::unpack(unpacker)?.into_boxed_slice())
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+        Ok(Vec::<T>::unpack::<_, VERIFY>(unpacker)?.into_boxed_slice())
     }
 }
diff --git a/bee-common/bee-packable/src/packable/integer.rs b/bee-common/bee-packable/src/packable/integer.rs
@@ -23,7 +23,9 @@ macro_rules! impl_packable_for_integer {
                 packer.pack_bytes(&self.to_le_bytes())
             }
 
-            fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+            fn unpack<U: Unpacker, const VERIFY: bool>(
+                unpacker: &mut U,
+            ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
                 let mut bytes = [0u8; core::mem::size_of::<Self>()];
                 unpacker.unpack_bytes(&mut bytes)?;
                 Ok(Self::from_le_bytes(bytes))
@@ -51,8 +53,10 @@ impl Packable for usize {
         (*self as u64).pack(packer)
     }
 
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
-        Ok(u64::unpack(unpacker).infallible()? as usize)
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+        Ok(u64::unpack::<_, VERIFY>(unpacker).infallible()? as usize)
     }
 }
 
@@ -75,7 +79,9 @@ impl Packable for isize {
         (*self as i64).pack(packer)
     }
 
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
-        Ok(i64::unpack(unpacker).infallible()? as isize)
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+        Ok(i64::unpack::<_, VERIFY>(unpacker).infallible()? as isize)
     }
 }
diff --git a/bee-common/bee-packable/src/packable/mod.rs b/bee-common/bee-packable/src/packable/mod.rs
@@ -44,7 +44,29 @@ pub trait Packable: Sized {
     /// Packs this value into the given [`Packer`].
     fn pack<P: Packer>(&self, packer: &mut P) -> Result<(), P::Error>;
 
+    /// Unpacks this value from the given [`Unpacker`].
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>>;
+}
+
+/// Extension trait for types that implement [`Packable`].
+pub trait PackableExt: Packable {
     /// Convenience method that packs this value into a [`Vec<u8>`].
+    fn pack_to_vec(&self) -> Vec<u8>;
+
+    /// Unpacks this value from a sequence of bytes doing syntactical checks.
+    fn unpack_verified<T: AsRef<[u8]>>(
+        bytes: T,
+    ) -> Result<Self, UnpackError<<Self as Packable>::UnpackError, UnexpectedEOF>>;
+
+    /// Unpacks this value from a sequence of bytes without doing syntactical checks.
+    fn unpack_unverified<T: AsRef<[u8]>>(
+        bytes: T,
+    ) -> Result<Self, UnpackError<<Self as Packable>::UnpackError, UnexpectedEOF>>;
+}
+
+impl<P: Packable> PackableExt for P {
     fn pack_to_vec(&self) -> Vec<u8> {
         let mut packer = VecPacker::with_capacity(self.packed_len());
 
@@ -54,12 +76,15 @@ pub trait Packable: Sized {
         packer.into_vec()
     }
 
-    /// Unpacks this value from the given [`Unpacker`].
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>>;
+    fn unpack_verified<T: AsRef<[u8]>>(
+        bytes: T,
+    ) -> Result<Self, UnpackError<<Self as Packable>::UnpackError, UnexpectedEOF>> {
+        Self::unpack::<_, true>(&mut SliceUnpacker::new(bytes.as_ref()))
+    }
 
-    /// Unpacks this value from a type that implements [`AsRef<[u8]>`].
-    fn unpack_from_slice<T: AsRef<[u8]>>(bytes: T) -> Result<Self, UnpackError<Self::UnpackError, UnexpectedEOF>> {
-        let mut unpacker = SliceUnpacker::new(bytes.as_ref());
-        Packable::unpack(&mut unpacker)
+    fn unpack_unverified<T: AsRef<[u8]>>(
+        bytes: T,
+    ) -> Result<Self, UnpackError<<Self as Packable>::UnpackError, UnexpectedEOF>> {
+        Self::unpack::<_, false>(&mut SliceUnpacker::new(bytes.as_ref()))
     }
 }
diff --git a/bee-common/bee-packable/src/packable/option.rs b/bee-common/bee-packable/src/packable/option.rs
@@ -47,10 +47,12 @@ impl<T: Packable> Packable for Option<T> {
         }
     }
 
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
-        match u8::unpack(unpacker).infallible()? {
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+        match u8::unpack::<_, VERIFY>(unpacker).infallible()? {
             0 => Ok(None),
-            1 => Ok(Some(T::unpack(unpacker).coerce()?)),
+            1 => Ok(Some(T::unpack::<_, VERIFY>(unpacker).coerce()?)),
             n => Err(UnpackError::Packable(Self::UnpackError::UnknownTag(n))),
         }
     }

diff --git a/bee-common/bee-packable/src/packable/prefix.rs b/bee-common/bee-packable/src/packable/prefix.rs
@@ -172,16 +172,18 @@ macro_rules! impl_vec_prefix {
                 Ok(())
             }
 
-            fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+            fn unpack<U: Unpacker, const VERIFY: bool>(
+                unpacker: &mut U,
+            ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
                 // The length of any dynamically-sized sequence must be prefixed.
-                let len = <$bounded<MIN, MAX>>::unpack(unpacker)
+                let len = <$bounded<MIN, MAX>>::unpack::<_, VERIFY>(unpacker)
                     .map_packable_err(UnpackPrefixError::Prefix)?
                     .into();
 
                 let mut inner = Vec::with_capacity(len as usize);
 
                 for _ in 0..len {
-                    let item = T::unpack(unpacker).coerce()?;
+                    let item = T::unpack::<_, VERIFY>(unpacker).coerce()?;
                     inner.push(item);
                 }
 
@@ -288,16 +290,18 @@ macro_rules! impl_boxed_slice_prefix {
                 Ok(())
             }
 
-            fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+            fn unpack<U: Unpacker, const VERIFY: bool>(
+                unpacker: &mut U,
+            ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
                 // The length of any dynamically-sized sequence must be prefixed.
-                let len = <$bounded<MIN, MAX>>::unpack(unpacker)
+                let len = <$bounded<MIN, MAX>>::unpack::<_, VERIFY>(unpacker)
                     .map_packable_err(UnpackPrefixError::Prefix)?
                     .into();
 
                 let mut inner = Vec::with_capacity(len as usize);
 
                 for _ in 0..len {
-                    let item = T::unpack(unpacker).coerce()?;
+                    let item = T::unpack::<_, VERIFY>(unpacker).coerce()?;
                     inner.push(item);
                 }
 

diff --git a/bee-common/bee-packable/src/packable/vec.rs b/bee-common/bee-packable/src/packable/vec.rs
@@ -30,14 +30,16 @@ impl<T: Packable> Packable for Vec<T> {
         Ok(())
     }
 
-    fn unpack<U: Unpacker>(unpacker: &mut U) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
+    fn unpack<U: Unpacker, const VERIFY: bool>(
+        unpacker: &mut U,
+    ) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
         // The length of any dynamically-sized sequence must be prefixed.
-        let len = u64::unpack(unpacker).infallible()?;
+        let len = u64::unpack::<_, VERIFY>(unpacker).infallible()?;
 
         let mut vec = Self::with_capacity(len as usize);
 
         for _ in 0..len {
-            let item = T::unpack(unpacker)?;
+            let item = T::unpack::<_, VERIFY>(unpacker)?;
             vec.push(item);
         }
 

diff --git a/bee-common/bee-packable/src/packer/vec.rs b/bee-common/bee-packable/src/packer/vec.rs
@@ -3,10 +3,10 @@
 
 extern crate alloc;
 
-use crate::{packer::Packer, unpacker::SliceUnpacker};
+use crate::packer::Packer;
 
 use alloc::vec::Vec;
-use core::convert::Infallible;
+use core::{convert::Infallible, ops::Deref};
 
 /// A [`Packer`] backed by a [`Vec<u8>`].
 #[derive(Default)]
@@ -23,11 +23,6 @@ impl VecPacker {
         Self(Vec::with_capacity(capacity))
     }
 
-    /// Uses the backing [`Vec<u8>`] to create an [`Unpacker`](crate::unpacker::Unpacker).
-    pub fn as_slice(&self) -> SliceUnpacker<'_> {
-        SliceUnpacker::new(self.0.as_slice())
-    }
-
     /// Consumes the [`VecPacker`] and returns the inner [`Vec<u8>`].
     pub fn into_vec(self) -> Vec<u8> {
         self.0
@@ -52,3 +47,11 @@ impl Packer for VecPacker {
         Ok(())
     }
 }
+
+impl Deref for VecPacker {
+    type Target = Vec<u8>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
diff --git a/bee-common/bee-packable/tests/bool.rs b/bee-common/bee-packable/tests/bool.rs
@@ -3,7 +3,7 @@
 
 mod common;
 
-use bee_packable::{packer::VecPacker, Packable};
+use bee_packable::{packer::VecPacker, Packable, PackableExt};
 
 #[test]
 fn packable_bool() {
@@ -16,7 +16,7 @@ fn packable_bool_packed_non_zero_bytes_are_truthy() {
     let mut packer = VecPacker::default();
     42u8.pack(&mut packer).unwrap();
 
-    let is_true = bool::unpack(&mut packer.as_slice()).unwrap();
+    let is_true = bool::unpack_verified(&mut packer.as_slice()).unwrap();
 
     assert!(is_true);
 }
diff --git a/bee-common/bee-packable/tests/bounded.rs b/bee-common/bee-packable/tests/bounded.rs
@@ -9,7 +9,7 @@ use bee_packable::{
         InvalidBoundedU64, InvalidBoundedU8,
     },
     error::UnpackError,
-    Packable,
+    PackableExt,
 };
 
 macro_rules! impl_bounds_test_for_bounded_integer {
@@ -51,7 +51,7 @@ macro_rules! impl_packable_test_for_bounded_integer {
         #[test]
         fn $packable_invalid_name() {
             let bytes = vec![0u8; core::mem::size_of::<$wrapped>()];
-            let unpacked = <$wrapper>::unpack_from_slice(&bytes);
+            let unpacked = <$wrapper>::unpack_verified(&bytes);
 
             assert!(matches!(unpacked, Err(UnpackError::Packable($error(0)))))
         }