Unverified Commit f386e412 authored by thiolliere's avatar thiolliere Committed by GitHub
Browse files

Backport fix decode vec (#235)



* Fix Vec::decode as reported by Brian and endianness (#231)

Co-authored-by: Cheme
Co-authored-by: default avatarBrian Anderson <andersrb@gmail.com>
Co-authored-by: default avatarBastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: David's avatarDavid <dvdplm@gmail.com>

* bump version
Co-authored-by: default avatarBrian Anderson <andersrb@gmail.com>
Co-authored-by: default avatarBastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: David's avatarDavid <dvdplm@gmail.com>
parent 27429b31
Pipeline #119725 failed with stages
in 8 seconds
[package]
name = "parity-scale-codec"
description = "SCALE - Simple Concatenating Aggregated Little Endians"
version = "1.3.5"
version = "1.3.6"
authors = ["Parity Technologies <admin@parity.io>"]
license = "Apache-2.0"
repository = "https://github.com/paritytech/parity-scale-codec"
......
......@@ -61,6 +61,18 @@ fn vec_extend_from_slice(b: &mut Bencher) {
});
}
struct NoLimitInput<'a>(&'a [u8]);
impl<'a> Input for NoLimitInput<'a> {
fn remaining_len(&mut self) -> Result<Option<usize>, Error> {
Ok(None)
}
fn read(&mut self, into: &mut [u8]) -> Result<(), Error> {
self.0.read(into)
}
}
#[derive(Encode, Decode)]
enum Event {
ComplexEvent(Vec<u8>, u32, i32, u128, i8),
......@@ -126,6 +138,21 @@ fn encode_decode_vec<T: TryFrom<u8> + Codec>(c: &mut Criterion) where T::Error:
let _: Vec<T> = Decode::decode(&mut &vec[..]).unwrap();
})
}, vec![1, 2, 5, 32, 1024, 2048, 16384]);
c.bench_function_over_inputs(&format!("vec_decode_no_limit_{}", type_name::<T>()), |b, &vec_size| {
let vec: Vec<T> = (0..=127u8)
.cycle()
.take(vec_size)
.map(|v| v.try_into().unwrap())
.collect();
let vec = vec.encode();
let vec = black_box(vec);
b.iter(|| {
let _: Vec<T> = Decode::decode(&mut NoLimitInput(&vec[..])).unwrap();
})
}, vec![16384, 131072]);
}
fn encode_decode_complex_type(c: &mut Criterion) {
......
......@@ -15,11 +15,12 @@
//! `BitVec` specific serialization.
use core::mem;
use crate::alloc::vec::Vec;
use bitvec::{vec::BitVec, store::BitStore, order::BitOrder, slice::BitSlice, boxed::BitBox};
use byte_slice_cast::{AsByteSlice, ToByteSlice, FromByteSlice, Error as FromByteSliceError};
use crate::codec::{Encode, Decode, Input, Output, Error, read_vec_u8};
use crate::codec::{Encode, Decode, Input, Output, Error, read_vec_from_u8s};
use crate::compact::Compact;
use crate::EncodeLike;
......@@ -42,6 +43,22 @@ impl<O: BitOrder, T: BitStore + ToByteSlice> Encode for BitSlice<O, T> {
}
}
/// Reverse bytes of element for element of size `size_of_t`.
///
/// E.g. if size is 2 `[1, 2, 3, 4]` is changed to `[2, 1, 4, 3]`.
fn reverse_endian(vec_u8: &mut [u8], size_of_t: usize) {
for i in 0..vec_u8.len() / size_of_t {
for j in 0..size_of_t / 2 {
vec_u8.swap(i * size_of_t + j, i * size_of_t + (size_of_t - 1) - j);
}
}
}
/// # WARNING
///
/// In bitvec v0.17.4 the only implementations of BitStore are u8, u16, u32, u64, and usize.
/// This implementation actually only support u8, u16, u32 and u64, as encoding of uszie
/// is inconsistent between platforms.
impl<O: BitOrder, T: BitStore + ToByteSlice> Encode for BitVec<O, T> {
fn encode_to<W: Output>(&self, dest: &mut W) {
let len = self.len();
......@@ -50,21 +67,48 @@ impl<O: BitOrder, T: BitStore + ToByteSlice> Encode for BitVec<O, T> {
"Attempted to serialize a collection with too many elements.",
);
Compact(len as u32).encode_to(dest);
dest.write(self.as_slice().as_byte_slice());
let byte_slice: &[u8] = self.as_slice().as_byte_slice();
if cfg!(target_endian = "big") && mem::size_of::<T>() > 1 {
let mut vec_u8: Vec<u8> = byte_slice.into();
reverse_endian(&mut vec_u8[..], mem::size_of::<T>());
dest.write(&vec_u8);
} else {
dest.write(byte_slice);
}
}
}
impl<O: BitOrder, T: BitStore + ToByteSlice> EncodeLike for BitVec<O, T> {}
/// # WARNING
///
/// In bitvec v0.17.4 the only implementations of BitStore are u8, u16, u32, u64, and usize.
/// This implementation actually only support u8, u16, u32 and u64, as encoding of usize
/// is inconsistent between platforms.
impl<O: BitOrder, T: BitStore + FromByteSlice> Decode for BitVec<O, T> {
fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
<Compact<u32>>::decode(input).and_then(move |Compact(bits)| {
let bits = bits as usize;
let required_bytes = required_bytes::<T>(bits);
let vec = read_vec_u8(input, required_bytes)?;
let mut vec_u8 = read_vec_from_u8s::<I, u8>(input, required_bytes)?;
if cfg!(target_endian = "big") && mem::size_of::<T>() > 1 {
reverse_endian(&mut vec_u8[..], mem::size_of::<T>());
}
let mut aligned_vec: Vec<T> = vec![0u8.into(); required_bytes / mem::size_of::<T>()];
let mut result = Self::from_slice(T::from_byte_slice(&vec)?);
unsafe {
let aligned_u8_ptr = aligned_vec.as_mut_ptr() as *mut u8;
for (i, v) in vec_u8.iter().enumerate() {
*aligned_u8_ptr.add(i) = *v;
}
}
let mut result = Self::from_vec(aligned_vec);
assert!(bits <= result.len());
unsafe { result.set_len(bits); }
Ok(result)
......@@ -160,6 +204,7 @@ mod tests {
}
#[test]
#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
fn bitvec_u8() {
for v in &test_data!(u8) {
let encoded = v.encode();
......@@ -168,6 +213,7 @@ mod tests {
}
#[test]
#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
fn bitvec_u16() {
for v in &test_data!(u16) {
let encoded = v.encode();
......@@ -176,6 +222,7 @@ mod tests {
}
#[test]
#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
fn bitvec_u32() {
for v in &test_data!(u32) {
let encoded = v.encode();
......@@ -184,6 +231,7 @@ mod tests {
}
#[test]
#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
fn bitvec_u64() {
for v in &test_data!(u64) {
let encoded = dbg!(v.encode());
......@@ -192,6 +240,7 @@ mod tests {
}
#[test]
#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
fn bitslice() {
let data: &[u8] = &[0x69];
let slice = BitSlice::<Msb0, u8>::from_slice(data);
......@@ -208,4 +257,25 @@ mod tests {
let decoded = BitBox::<Msb0, u8>::decode(&mut &encoded[..]).unwrap();
assert_eq!(bb, decoded);
}
#[test]
fn reverse_endian_works() {
let data = vec![1, 2, 3, 4, 5, 6, 7, 8];
let mut data_to_u8 = data.clone();
reverse_endian(&mut data_to_u8[..], mem::size_of::<u8>());
assert_eq!(data_to_u8, data);
let mut data_to_u16 = data.clone();
reverse_endian(&mut data_to_u16[..], mem::size_of::<u16>());
assert_eq!(data_to_u16, vec![2, 1, 4, 3, 6, 5, 8, 7]);
let mut data_to_u32 = data.clone();
reverse_endian(&mut data_to_u32[..], mem::size_of::<u32>());
assert_eq!(data_to_u32, vec![4, 3, 2, 1, 8, 7, 6, 5]);
let mut data_to_u64 = data.clone();
reverse_endian(&mut data_to_u64[..], mem::size_of::<u64>());
assert_eq!(data_to_u64, vec![8, 7, 6, 5, 4, 3, 2, 1]);
}
}
......@@ -31,7 +31,7 @@ use core::num::{
};
use arrayvec::ArrayVec;
use byte_slice_cast::{AsByteSlice, IntoVecOf};
use byte_slice_cast::{AsByteSlice, AsMutByteSlice, ToMutByteSlice};
#[cfg(any(feature = "std", feature = "full"))]
use crate::alloc::{
......@@ -629,8 +629,14 @@ macro_rules! impl_array {
$dest.write(&typed)
}};
( $ty:ty, $self:ident, $dest:ident ) => {{
let typed = unsafe { mem::transmute::<&[T], &[$ty]>(&$self[..]) };
$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed))
if cfg!(target_endian = "little") {
let typed = unsafe { mem::transmute::<&[T], &[$ty]>(&$self[..]) };
$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed))
} else {
for item in $self.iter() {
item.encode_to($dest);
}
}
}};
}
......@@ -759,8 +765,14 @@ impl<T: Encode> Encode for [T] {
$dest.write(&typed)
}};
( $ty:ty, $self:ident, $dest:ident ) => {{
let typed = unsafe { mem::transmute::<&[T], &[$ty]>($self) };
$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed))
if cfg!(target_endian = "little") {
let typed = unsafe { mem::transmute::<&[T], &[$ty]>($self) };
$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed))
} else {
for item in $self {
item.encode_to($dest);
}
}
}};
}
......@@ -776,36 +788,67 @@ impl<T: Encode> Encode for [T] {
}
}
/// Read an `u8` vector from the given input.
pub(crate) fn read_vec_u8<I: Input>(input: &mut I, len: usize) -> Result<Vec<u8>, Error> {
/// Create a `Vec<T>` by casting directly from a buffer of read `u8`s
///
/// The encoding of `T` must be equal to its binary representation, and size of `T` must be less or
/// equal to [`MAX_PREALLOCATION`].
pub(crate) fn read_vec_from_u8s<I, T>(input: &mut I, items_len: usize) -> Result<Vec<T>, Error>
where
I: Input,
T: ToMutByteSlice + Default + Clone,
{
debug_assert!(MAX_PREALLOCATION >= mem::size_of::<T>(), "Invalid precondition");
let byte_len = items_len.checked_mul(mem::size_of::<T>())
.ok_or_else(|| "Item is too big and cannot be allocated")?;
let input_len = input.remaining_len()?;
// If there is input len and it cannot be pre-allocated then return directly.
if input_len.map(|l| l < len).unwrap_or(false) {
if input_len.map(|l| l < byte_len).unwrap_or(false) {
return Err("Not enough data to decode vector".into())
}
// In both these branches we're going to be creating and resizing a Vec<T>,
// but casting it to a &mut [u8] for reading.
// Note: we checked that if input_len is some then it can preallocated.
let r = if input_len.is_some() || len < MAX_PREALLOCATION {
let r = if input_len.is_some() || byte_len < MAX_PREALLOCATION {
// Here we pre-allocate the whole buffer.
let mut r = vec![0; len];
input.read(&mut r)?;
let mut items: Vec<T> = vec![Default::default(); items_len];
let mut bytes_slice = items.as_mut_byte_slice();
input.read(&mut bytes_slice)?;
r
items
} else {
// An allowed number of preallocated item.
// Note: `MAX_PREALLOCATION` is expected to be more or equal to size of `T`, precondition.
let max_preallocated_items = MAX_PREALLOCATION / mem::size_of::<T>();
// Here we pre-allocate only the maximum pre-allocation
let mut r = vec![];
let mut remains = len;
while remains != 0 {
let len_read = MAX_PREALLOCATION.min(remains);
let len_filled = r.len();
r.resize(len_filled + len_read, 0);
input.read(&mut r[len_filled..])?;
remains -= len_read;
let mut items: Vec<T> = vec![];
let mut items_remains = items_len;
while items_remains > 0 {
let items_len_read = max_preallocated_items.min(items_remains);
let items_len_filled = items.len();
let items_new_size = items_len_filled + items_len_read;
items.reserve_exact(items_len_read);
unsafe {
items.set_len(items_new_size);
}
let bytes_slice = items.as_mut_byte_slice();
let bytes_len_filled = items_len_filled * mem::size_of::<T>();
input.read(&mut bytes_slice[bytes_len_filled..])?;
items_remains = items_remains.saturating_sub(items_len_read);
}
r
items
};
Ok(r)
......@@ -821,21 +864,31 @@ impl<T: Decode> Decode for Vec<T> {
<Compact<u32>>::decode(input).and_then(move |Compact(len)| {
let len = len as usize;
fn decode_unoptimized<I: Input, T: Decode>(
input: &mut I,
items_len: usize,
) -> Result<Vec<T>, Error> {
let input_capacity = input.remaining_len()?
.unwrap_or(MAX_PREALLOCATION)
.checked_div(mem::size_of::<T>())
.unwrap_or(0);
let mut r = Vec::with_capacity(input_capacity.min(items_len));
input.descend_ref()?;
for _ in 0..items_len {
r.push(T::decode(input)?);
}
input.ascend_ref();
Ok(r)
}
macro_rules! decode {
( u8, $input:ident, $len:ident ) => {{
let vec = read_vec_u8($input, $len)?;
Ok(unsafe { mem::transmute::<Vec<u8>, Vec<T>>(vec) })
}};
( i8, $input:ident, $len:ident ) => {{
let vec = read_vec_u8($input, $len)?;
Ok(unsafe { mem::transmute::<Vec<u8>, Vec<T>>(vec) })
}};
( $ty:ty, $input:ident, $len:ident ) => {{
let vec = read_vec_u8($input, $len * mem::size_of::<$ty>())?;
let typed = vec.into_vec_of::<$ty>()
.map_err(|_| "Failed to convert from `Vec<u8>` to typed vec")?;
Ok(unsafe { mem::transmute::<Vec<$ty>, Vec<T>>(typed) })
if cfg!(target_endian = "little") || mem::size_of::<T>() == 1 {
let vec = read_vec_from_u8s::<_, $ty>($input, $len)?;
Ok(unsafe { mem::transmute::<Vec<$ty>, Vec<T>>(vec) })
} else {
decode_unoptimized($input, $len)
}
}};
}
......@@ -843,17 +896,7 @@ impl<T: Decode> Decode for Vec<T> {
<T as Decode>::TYPE_INFO,
decode(input, len),
{
let input_capacity = input.remaining_len()?
.unwrap_or(MAX_PREALLOCATION)
.checked_div(mem::size_of::<T>())
.unwrap_or(0);
let mut r = Vec::with_capacity(input_capacity.min(len));
input.descend_ref()?;
for _ in 0..len {
r.push(T::decode(input)?);
}
input.ascend_ref();
Ok(r)
decode_unoptimized(input, len)
},
}
})
......@@ -930,13 +973,19 @@ impl<T: Encode> Encode for VecDeque<T> {
macro_rules! encode_to {
( $ty:ty, $self:ident, $dest:ident ) => {{
let slices = $self.as_slices();
let typed = unsafe {
core::mem::transmute::<(&[T], &[T]), (&[$ty], &[$ty])>(slices)
};
$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed.0));
$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed.1));
if cfg!(target_endian = "little") || mem::size_of::<T>() == 1 {
let slices = $self.as_slices();
let typed = unsafe {
core::mem::transmute::<(&[T], &[T]), (&[$ty], &[$ty])>(slices)
};
$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed.0));
$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed.1));
} else {
for item in $self {
item.encode_to($dest);
}
}
}};
}
......
......@@ -110,7 +110,7 @@ fn extract_length_data(data: &[u8], input_len: usize) -> Result<(u32, usize, usi
let len = u32::from(Compact::<u32>::decode(&mut &data[..])?);
let new_len = len
.checked_add(input_len as u32)
.ok_or_else(|| "New vec length greater than `u32::max_value()`.")?;
.ok_or_else(|| "New vec length greater than `u32::TEST_VALUE()`.")?;
let encoded_len = Compact::<u32>::compact_len(&len);
let encoded_new_len = Compact::<u32>::compact_len(&new_len);
......@@ -177,28 +177,31 @@ mod tests {
use crate::{Input, Encode, EncodeLike};
use std::collections::VecDeque;
const TEST_VALUE: u32 = {
#[cfg(not(miri))]
{ 1_000_000 }
#[cfg(miri)]
{ 1_000 }
};
#[test]
fn vec_encode_append_works() {
let max_value = 1_000_000;
let encoded = (0..max_value).fold(Vec::new(), |encoded, v| {
let encoded = (0..TEST_VALUE).fold(Vec::new(), |encoded, v| {
<Vec<u32> as EncodeAppend>::append_or_new(encoded, std::iter::once(&v)).unwrap()
});
let decoded = Vec::<u32>::decode(&mut &encoded[..]).unwrap();
assert_eq!(decoded, (0..max_value).collect::<Vec<_>>());
assert_eq!(decoded, (0..TEST_VALUE).collect::<Vec<_>>());
}
#[test]
fn vec_encode_append_multiple_items_works() {
let max_value = 1_000_000u32;
let encoded = (0..max_value).fold(Vec::new(), |encoded, v| {
let encoded = (0..TEST_VALUE).fold(Vec::new(), |encoded, v| {
<Vec<u32> as EncodeAppend>::append_or_new(encoded, &[v, v, v, v]).unwrap()
});
let decoded = Vec::<u32>::decode(&mut &encoded[..]).unwrap();
let expected = (0..max_value).fold(Vec::new(), |mut vec, i| {
let expected = (0..TEST_VALUE).fold(Vec::new(), |mut vec, i| {
vec.append(&mut vec![i, i, i, i]);
vec
});
......@@ -207,26 +210,22 @@ mod tests {
#[test]
fn vecdeque_encode_append_works() {
let max_value = 1_000_000;
let encoded = (0..max_value).fold(Vec::new(), |encoded, v| {
let encoded = (0..TEST_VALUE).fold(Vec::new(), |encoded, v| {
<VecDeque<u32> as EncodeAppend>::append_or_new(encoded, std::iter::once(&v)).unwrap()
});
let decoded = VecDeque::<u32>::decode(&mut &encoded[..]).unwrap();
assert_eq!(decoded, (0..max_value).collect::<Vec<_>>());
assert_eq!(decoded, (0..TEST_VALUE).collect::<Vec<_>>());
}
#[test]
fn vecdeque_encode_append_multiple_items_works() {
let max_value = 1_000_000u32;
let encoded = (0..max_value).fold(Vec::new(), |encoded, v| {
let encoded = (0..TEST_VALUE).fold(Vec::new(), |encoded, v| {
<VecDeque<u32> as EncodeAppend>::append_or_new(encoded, &[v, v, v, v]).unwrap()
});
let decoded = VecDeque::<u32>::decode(&mut &encoded[..]).unwrap();
let expected = (0..max_value).fold(Vec::new(), |mut vec, i| {
let expected = (0..TEST_VALUE).fold(Vec::new(), |mut vec, i| {
vec.append(&mut vec![i, i, i, i]);
vec
});
......@@ -262,13 +261,11 @@ mod tests {
#[test]
fn vec_encode_like_append_works() {
let max_value = 1_000_000;
let encoded = (0..max_value).fold(Vec::new(), |encoded, v| {
let encoded = (0..TEST_VALUE).fold(Vec::new(), |encoded, v| {
<Vec<u32> as EncodeAppend>::append_or_new(encoded, std::iter::once(Box::new(v as u32))).unwrap()
});
let decoded = Vec::<u32>::decode(&mut &encoded[..]).unwrap();
assert_eq!(decoded, (0..max_value).collect::<Vec<_>>());
assert_eq!(decoded, (0..TEST_VALUE).collect::<Vec<_>>());
}
}
......@@ -44,7 +44,7 @@ impl<T: Decode, L: generic_array::ArrayLength<T>> Decode for generic_array::Gene
#[cfg(test)]
mod tests {
use super::*;
use generic_array::{typenum, GenericArray, arr, arr_impl};
use generic_array::{typenum, GenericArray, arr};
#[test]
fn generic_array() {
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment