From ee66e066de90b8f61858b1ef468a96f9279d6a78 Mon Sep 17 00:00:00 2001 From: matix2267 Date: Mon, 14 Jun 2021 13:43:37 +0200 Subject: [PATCH] Fix encoding of packed variants in new enum format Fixes #187 Legacy enum format supported packed encoding pretty well but with the switch to new enum format there was a regression (#173). This commit fixes the new enum format in packed encoding. For example consider the following structure: ```rust enum Enum { Unit, NewType(i32), Tuple(i32, i32), Struct { x: i32, y: i32 }, } ``` Legacy enum packed encodings are: ``` Empty: NewType: [, value] Tuple: [, values..] Struct: [, {}] ``` Non-legacy enum packed encodings before this commit: ``` Empty: NewType: {"": value} Tuple: {"": [values..]} Struct: {: {}} ``` Notice how NewType and Tuple store the name instead of variant number. Fixed after this commit: ``` Empty: NewType: {: value} Tuple: {: [values..]} Struct: {: {}} ``` After this commit the packed encoding can be briefly described as: All struct fields and enum variants are encoded as their field number rather than name. This applies to all types of members (unit, newtype, tuple and struct). --- src/lib.rs | 29 ----------------------------- src/ser.rs | 12 ++++++++++-- tests/enum.rs | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 55668541..29353afe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -158,35 +158,6 @@ //! assert_eq!(cbor.len(), 1); //! //! let cbor = to_vec_packed(&SecondVariant(0)).unwrap(); -//! assert_eq!(cbor.len(), 16); // Includes 13 bytes of "SecondVariant" -//! ``` -//! -//! Serialize using minimal encoding -//! -//! ```rust -//! use serde_derive::{Deserialize, Serialize}; -//! use serde_cbor::{Result, Serializer, ser::{self, IoWrite}}; -//! use WithTwoVariants::*; -//! -//! fn to_vec_minimal(value: &T) -> Result> -//! where -//! T: serde::Serialize, -//! { -//! let mut vec = Vec::new(); -//! value.serialize(&mut Serializer::new(&mut IoWrite::new(&mut vec)).packed_format().legacy_enums())?; -//! Ok(vec) -//! } -//! -//! #[derive(Debug, Serialize, Deserialize)] -//! enum WithTwoVariants { -//! FirstVariant, -//! SecondVariant(u8), -//! } -//! -//! let cbor = to_vec_minimal(&FirstVariant).unwrap(); -//! assert_eq!(cbor.len(), 1); -//! -//! let cbor = to_vec_minimal(&SecondVariant(0)).unwrap(); //! assert_eq!(cbor.len(), 3); //! ``` //! diff --git a/src/ser.rs b/src/ser.rs index 7016dc34..4fdcfeab 100644 --- a/src/ser.rs +++ b/src/ser.rs @@ -426,7 +426,11 @@ where { if self.enum_as_map { self.write_u64(5, 1u64)?; - variant.serialize(&mut *self)?; + if self.packed { + variant_index.serialize(&mut *self)?; + } else { + variant.serialize(&mut *self)?; + } } else { self.writer.write_all(&[4 << 5 | 2]).map_err(|e| e.into())?; self.serialize_unit_variant(name, variant_index, variant)?; @@ -464,7 +468,11 @@ where ) -> Result<&'a mut Serializer> { if self.enum_as_map { self.write_u64(5, 1u64)?; - variant.serialize(&mut *self)?; + if self.packed { + variant_index.serialize(&mut *self)?; + } else { + variant.serialize(&mut *self)?; + } self.serialize_tuple(len) } else { self.write_u64(4, (len + 1) as u64)?; diff --git a/tests/enum.rs b/tests/enum.rs index 630500d6..8f086db2 100644 --- a/tests/enum.rs +++ b/tests/enum.rs @@ -31,7 +31,7 @@ fn test_simple_data_enum_roundtrip() { mod std_tests { use std::collections::BTreeMap; - use serde_cbor::ser::{IoWrite, Serializer}; + use serde_cbor::ser::{to_vec_packed, IoWrite, Serializer}; use serde_cbor::value::Value; use serde_cbor::{from_slice, to_vec}; @@ -233,4 +233,49 @@ mod std_tests { let point_map_ds = from_slice(&point_map_s).unwrap(); assert_eq!(Bar::Point { x: 5, y: -5 }, point_map_ds); } + + #[test] + fn test_packed_variants() { + // unit variants serialize as just the + let empty_s = to_vec_packed(&Bar::Empty).unwrap(); + let empty_num_s = to_vec_packed(&0).unwrap(); + assert_eq!(empty_s, empty_num_s); + + // newtype variants serialize like {: value} + let number_s = to_vec_packed(&Bar::Number(42)).unwrap(); + let mut number_map = BTreeMap::new(); + number_map.insert(1, 42); + let number_map_s = to_vec_packed(&number_map).unwrap(); + assert_eq!(number_s, number_map_s); + + // multi-element tuple variants serialize like {: [values..]} + let flag_s = to_vec_packed(&Bar::Flag("foo".to_string(), true)).unwrap(); + let mut flag_map = BTreeMap::new(); + flag_map.insert(2, vec![Value::Text("foo".to_string()), Value::Bool(true)]); + let flag_map_s = to_vec_packed(&flag_map).unwrap(); + assert_eq!(flag_s, flag_map_s); + + // struct-variants serialize like {, {struct..}} + let point_s = to_vec_packed(&Bar::Point { x: 5, y: -5 }).unwrap(); + let mut struct_map = BTreeMap::new(); + struct_map.insert(Value::Integer(0), Value::Integer(5)); + struct_map.insert(Value::Integer(1), Value::Integer(-5)); + let mut point_map = BTreeMap::new(); + point_map.insert(3, Value::Map(struct_map)); + let point_map_s = to_vec_packed(&point_map).unwrap(); + assert_eq!(point_s, point_map_s); + + // deserialization of all encodings should just work + let empty_ds = from_slice(&empty_s).unwrap(); + assert_eq!(Bar::Empty, empty_ds); + + let number_ds = from_slice(&number_s).unwrap(); + assert_eq!(Bar::Number(42), number_ds); + + let flag_ds = from_slice(&flag_s).unwrap(); + assert_eq!(Bar::Flag("foo".to_string(), true), flag_ds); + + let point_ds = from_slice(&point_s).unwrap(); + assert_eq!(Bar::Point { x: 5, y: -5 }, point_ds); + } }