Skip to content

Commit

Permalink
Made arrays never encode their length (#625)
Browse files Browse the repository at this point in the history 
* Made arrays with 32 elements or less never encode their length

* Removed `write_fixed_array_length` and `skip_fixed_array_length` as this was based on incorrect assumptions on how serde and bincode 1 works

---------

Co-authored-by: Victor Koenders <[email protected]>
  • Loading branch information
@VictorKoenders
VictorKoenders and VictorKoenders authored Mar 30, 2023
1 parent ebc3b6b commit b34ee95
Show file tree
Hide file tree
Showing 14 changed files with 146 additions and 206 deletions.
12 changes: 8 additions & 4 deletions compatibility/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,22 @@ where
// This is what bincode 1 serializes to. This will be our comparison value.
let encoded = bincode_1_options.serialize(t).unwrap();

println!("Encoded {:?} as {:?}", t, encoded);
println!("Encoded {t:?} as {encoded:?}");

// Test bincode 2 encode
let bincode_2_output = bincode_2::encode_to_vec(t, bincode_2_config).unwrap();
assert_eq!(encoded, bincode_2_output, "{:?} serializes differently", t);
assert_eq!(
encoded,
bincode_2_output,
"{t:?} serializes differently\nbincode 2 config {:?}",
core::any::type_name::<C>(),
);

// Test bincode 2 serde serialize
let bincode_2_serde_output = bincode_2::serde::encode_to_vec(t, bincode_2_config).unwrap();
assert_eq!(
encoded, bincode_2_serde_output,
"{:?} serializes differently",
t
"{t:?} serializes differently"
);

// Test bincode 1 deserialize
Expand Down
77 changes: 77 additions & 0 deletions compatibility/src/misc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,83 @@ fn test() {
super::test_same(std::net::Ipv6Addr::LOCALHOST);
}

#[test]
fn test_arrays() {
// serde is known to be weird with arrays
// Arrays of length 32 and less are encoded as tuples, but arrays 33 and up are encoded as slices
// we need to make sure we're compatible with this
super::test_same([0u8; 0]);
super::test_same([1u8; 1]);
super::test_same([1u8, 2]);
super::test_same([1u8, 2, 3]);
super::test_same([1u8, 2, 3, 4]);
super::test_same([1u8, 2, 3, 4, 5]);
super::test_same([1u8, 2, 3, 4, 5, 6]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]);
super::test_same([1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26, 27,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26, 27, 28,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26, 27, 28, 29,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26, 27, 28, 29, 30,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26, 27, 28, 29, 30, 31,
]);
super::test_same([
1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26, 27, 28, 29, 30, 31, 32,
]);
}

#[derive(
bincode_2::Encode, bincode_2::Decode, serde::Serialize, serde::Deserialize, Debug, PartialEq,
)]
Expand Down
16 changes: 1 addition & 15 deletions docs/spec.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ assert_eq!(encoded.as_slice(), &[

# Arrays

Array length is encoded based on the `.write_fixed_array_length` and `.skip_fixed_array_length()` config. When an array length is written, it will be encoded as a `u64`.
Array length is never encoded.

Note that `&[T]` is encoded as a [Collection](#collections).

Expand All @@ -135,15 +135,9 @@ Note that `&[T]` is encoded as a [Collection](#collections).
let arr: [u8; 5] = [10, 20, 30, 40, 50];
let encoded = bincode::encode_to_vec(arr, bincode::config::legacy()).unwrap();
assert_eq!(encoded.as_slice(), &[
5, 0, 0, 0, 0, 0, 0, 0, // The length, as a u64
10, 20, 30, 40, 50, // the bytes
]);

let encoded = bincode::encode_to_vec(arr, bincode::config::legacy().skip_fixed_array_length()).unwrap();
assert_eq!(encoded.as_slice(), &[
// no length
10, 20, 30, 40, 50, // the bytes
]);
```

This applies to any type `T` that implements `Encode`/`Decode`
Expand All @@ -168,14 +162,6 @@ let arr: [Foo; 2] = [

let encoded = bincode::encode_to_vec(&arr, bincode::config::legacy()).unwrap();
assert_eq!(encoded.as_slice(), &[
2, 0, 0, 0, 0, 0, 0, 0, // Length of the array
10, 20, // First Foo
30, 40, // Second Foo
]);

let encoded = bincode::encode_to_vec(&arr, bincode::config::legacy().skip_fixed_array_length()).unwrap();
assert_eq!(encoded.as_slice(), &[
// no length
10, 20, // First Foo
30, 40, // Second Foo
]);
Expand Down
94 changes: 17 additions & 77 deletions src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@
//! .with_little_endian()
//! // pick one of:
//! .with_variable_int_encoding()
//! .with_fixed_int_encoding()
//! // pick one of:
//! .skip_fixed_array_length()
//! .write_fixed_array_length();
//! .with_fixed_int_encoding();
//! ```
//!
//! See [Configuration] for more information on the configuration options.
Expand All @@ -29,20 +26,16 @@ use core::marker::PhantomData;
///
/// - [with_little_endian] and [with_big_endian]
/// - [with_fixed_int_encoding] and [with_variable_int_encoding]
/// - [skip_fixed_array_length] and [write_fixed_array_length]
///
///
/// [with_little_endian]: #method.with_little_endian
/// [with_big_endian]: #method.with_big_endian
/// [with_fixed_int_encoding]: #method.with_fixed_int_encoding
/// [with_variable_int_encoding]: #method.with_variable_int_encoding
/// [skip_fixed_array_length]: #method.skip_fixed_array_length
/// [write_fixed_array_length]: #method.write_fixed_array_length
#[derive(Copy, Clone)]
pub struct Configuration<E = LittleEndian, I = Varint, A = WriteFixedArrayLength, L = NoLimit> {
pub struct Configuration<E = LittleEndian, I = Varint, L = NoLimit> {
_e: PhantomData<E>,
_i: PhantomData<I>,
_a: PhantomData<A>,
_l: PhantomData<L>,
}

Expand All @@ -59,42 +52,39 @@ pub struct Configuration<E = LittleEndian, I = Varint, A = WriteFixedArrayLength
/// The default config for bincode 2.0. By default this will be:
/// - Little endian
/// - Variable int encoding
/// - Write fixed array length
pub const fn standard() -> Configuration {
generate()
}

/// Creates the "legacy" default config. This is the default config that was present in bincode 1.0
/// - Little endian
/// - Fixed int length encoding
/// - Write fixed array length
pub const fn legacy() -> Configuration<LittleEndian, Fixint, WriteFixedArrayLength, NoLimit> {
pub const fn legacy() -> Configuration<LittleEndian, Fixint, NoLimit> {
generate()
}

impl<E, I, A, L> Default for Configuration<E, I, A, L> {
impl<E, I, L> Default for Configuration<E, I, L> {
fn default() -> Self {
generate()
}
}

const fn generate<E, I, A, L>() -> Configuration<E, I, A, L> {
const fn generate<E, I, L>() -> Configuration<E, I, L> {
Configuration {
_e: PhantomData,
_i: PhantomData,
_a: PhantomData,
_l: PhantomData,
}
}

impl<E, I, A, L> Configuration<E, I, A, L> {
impl<E, I, L> Configuration<E, I, L> {
/// Makes bincode encode all integer types in big endian.
pub const fn with_big_endian(self) -> Configuration<BigEndian, I, A, L> {
pub const fn with_big_endian(self) -> Configuration<BigEndian, I, L> {
generate()
}

/// Makes bincode encode all integer types in little endian.
pub const fn with_little_endian(self) -> Configuration<LittleEndian, I, A, L> {
pub const fn with_little_endian(self) -> Configuration<LittleEndian, I, L> {
generate()
}

Expand Down Expand Up @@ -155,7 +145,7 @@ impl<E, I, A, L> Configuration<E, I, A, L> {
///
/// Note that u256 and the like are unsupported by this format; if and when they are added to the
/// language, they may be supported via the extension point given by the 255 byte.
pub const fn with_variable_int_encoding(self) -> Configuration<E, Varint, A, L> {
pub const fn with_variable_int_encoding(self) -> Configuration<E, Varint, L> {
generate()
}

Expand All @@ -164,51 +154,29 @@ impl<E, I, A, L> Configuration<E, I, A, L> {
/// * Fixed size integers are encoded directly
/// * Enum discriminants are encoded as u32
/// * Lengths and usize are encoded as u64
pub const fn with_fixed_int_encoding(self) -> Configuration<E, Fixint, A, L> {
generate()
}

/// Skip writing the length of fixed size arrays (`[u8; N]`) before writing the array
///
/// **NOTE:** This is not supported if you're using the `bincode::serde::*` functions, the `#[bincode(with_serde)]` attribute, or the `Compat` struct.
pub const fn skip_fixed_array_length(self) -> Configuration<E, I, SkipFixedArrayLength, L> {
generate()
}

/// Write the length of fixed size arrays (`[u8; N]`) before writing the array
pub const fn write_fixed_array_length(self) -> Configuration<E, I, WriteFixedArrayLength, L> {
pub const fn with_fixed_int_encoding(self) -> Configuration<E, Fixint, L> {
generate()
}

/// Sets the byte limit to `limit`.
pub const fn with_limit<const N: usize>(self) -> Configuration<E, I, A, Limit<N>> {
pub const fn with_limit<const N: usize>(self) -> Configuration<E, I, Limit<N>> {
generate()
}

/// Clear the byte limit.
pub const fn with_no_limit(self) -> Configuration<E, I, A, NoLimit> {
pub const fn with_no_limit(self) -> Configuration<E, I, NoLimit> {
generate()
}
}

/// Indicates a type is valid for controlling the bincode configuration
pub trait Config:
InternalEndianConfig
+ InternalArrayLengthConfig
+ InternalIntEncodingConfig
+ InternalLimitConfig
+ Copy
+ Clone
InternalEndianConfig + InternalIntEncodingConfig + InternalLimitConfig + Copy + Clone
{
}

impl<T> Config for T where
T: InternalEndianConfig
+ InternalArrayLengthConfig
+ InternalIntEncodingConfig
+ InternalLimitConfig
+ Copy
+ Clone
T: InternalEndianConfig + InternalIntEncodingConfig + InternalLimitConfig + Copy + Clone
{
}

Expand Down Expand Up @@ -244,22 +212,6 @@ impl InternalIntEncodingConfig for Varint {
const INT_ENCODING: IntEncoding = IntEncoding::Variable;
}

/// Skip writing the length of fixed size arrays (`[u8; N]`) before writing the array.
#[derive(Copy, Clone)]
pub struct SkipFixedArrayLength {}

impl InternalArrayLengthConfig for SkipFixedArrayLength {
const SKIP_FIXED_ARRAY_LENGTH: bool = true;
}

/// Write the length of fixed size arrays (`[u8; N]`) before writing the array.
#[derive(Copy, Clone)]
pub struct WriteFixedArrayLength {}

impl InternalArrayLengthConfig for WriteFixedArrayLength {
const SKIP_FIXED_ARRAY_LENGTH: bool = false;
}

/// Sets an unlimited byte limit.
#[derive(Copy, Clone)]
pub struct NoLimit {}
Expand All @@ -281,7 +233,7 @@ mod internal {
const ENDIAN: Endian;
}

impl<E: InternalEndianConfig, I, A, L> InternalEndianConfig for Configuration<E, I, A, L> {
impl<E: InternalEndianConfig, I, L> InternalEndianConfig for Configuration<E, I, L> {
const ENDIAN: Endian = E::ENDIAN;
}

Expand All @@ -295,9 +247,7 @@ mod internal {
const INT_ENCODING: IntEncoding;
}

impl<E, I: InternalIntEncodingConfig, A, L> InternalIntEncodingConfig
for Configuration<E, I, A, L>
{
impl<E, I: InternalIntEncodingConfig, L> InternalIntEncodingConfig for Configuration<E, I, L> {
const INT_ENCODING: IntEncoding = I::INT_ENCODING;
}

Expand All @@ -307,21 +257,11 @@ mod internal {
Variable,
}

pub trait InternalArrayLengthConfig {
const SKIP_FIXED_ARRAY_LENGTH: bool;
}

impl<E, I, A: InternalArrayLengthConfig, L> InternalArrayLengthConfig
for Configuration<E, I, A, L>
{
const SKIP_FIXED_ARRAY_LENGTH: bool = A::SKIP_FIXED_ARRAY_LENGTH;
}

pub trait InternalLimitConfig {
const LIMIT: Option<usize>;
}

impl<E, I, A, L: InternalLimitConfig> InternalLimitConfig for Configuration<E, I, A, L> {
impl<E, I, L: InternalLimitConfig> InternalLimitConfig for Configuration<E, I, L> {
const LIMIT: Option<usize> = L::LIMIT;
}
}
Loading

0 comments on commit b34ee95

Please sign in to comment.