From ff3ccdefa9a00332e38511cb4690b3ff37a1db49 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 21 Jul 2023 11:01:07 +0200 Subject: [PATCH] CBOR API fixes --- libraries/cbor/src/macros.rs | 67 ++++++++++++++-------- libraries/cbor/src/values.rs | 105 +++++++++++++++++++++++++++++------ libraries/cbor/src/writer.rs | 46 +-------------- 3 files changed, 133 insertions(+), 85 deletions(-) diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs index e437a05a..090cbd71 100644 --- a/libraries/cbor/src/macros.rs +++ b/libraries/cbor/src/macros.rs @@ -139,9 +139,12 @@ macro_rules! assert_sorted_keys { /// /// Keys and values are expressions and converted into CBOR Keys and Values. /// The syntax for these pairs is `key_expression => value_expression,`. -/// Duplicate keys will lead to invalid CBOR, i.e. writing these values fails. /// Keys do not have to be sorted. /// +/// # Panics +/// +/// You may not call this function with identical keys in its argument. +/// /// Example usage: /// /// ```rust @@ -308,7 +311,7 @@ macro_rules! cbor_bool { #[macro_export] macro_rules! cbor_unsigned { ( $x:expr ) => { - $crate::values::Value::from($x) + $crate::values::Value::unsigned($x as u64) }; } @@ -316,7 +319,7 @@ macro_rules! cbor_unsigned { #[macro_export] macro_rules! cbor_int { ( $x:expr ) => { - $crate::values::Value::integer($x) + $crate::values::Value::integer($x as i64) }; } @@ -324,7 +327,7 @@ macro_rules! cbor_int { #[macro_export] macro_rules! cbor_text { ( $x:expr ) => { - $crate::values::Value::from($x) + $crate::values::Value::text_string($x.into()) }; } @@ -332,7 +335,7 @@ macro_rules! cbor_text { #[macro_export] macro_rules! cbor_bytes { ( $x:expr ) => { - $crate::values::Value::from($x) + $crate::values::Value::byte_string($x) }; } @@ -501,23 +504,20 @@ mod test { #[test] fn test_cbor_map() { let a = cbor_map! { - -1 => -23, 4 => 56, - "foo" => true, - b"bar" => cbor_null!(), 5 => "foo", 6 => b"bar", 7 => cbor_array![], 8 => cbor_array![0, 1], 9 => cbor_map!{}, 10 => cbor_map!{2 => 3}, + -1 => -23, + b"bar" => cbor_null!(), + "foo" => true, }; let b = Value::map( [ - (Value::from(-1), Value::from(-23)), (Value::from(4), Value::from(56)), - (Value::from(String::from("foo")), Value::bool_value(true)), - (Value::from(b"bar".to_vec()), Value::null_value()), (Value::from(5), Value::from(String::from("foo"))), (Value::from(6), Value::from(b"bar".to_vec())), (Value::from(7), Value::array(Vec::new())), @@ -530,6 +530,9 @@ mod test { Value::from(10), Value::map([(Value::from(2), Value::from(3))].iter().cloned().collect()), ), + (Value::from(-1), Value::from(-23)), + (Value::from(b"bar".to_vec()), Value::null_value()), + (Value::from(String::from("foo")), Value::bool_value(true)), ] .iter() .cloned() @@ -541,31 +544,28 @@ mod test { #[test] fn test_cbor_map_options() { let a = cbor_map_options! { - -1 => -23, 4 => Some(56), + 5 => "foo", + 6 => Some(b"bar" as &[u8]), + 7 => cbor_array![], + 8 => Some(cbor_array![0, 1]), + 9 => cbor_map!{}, + 10 => Some(cbor_map!{2 => 3}), 11 => None::, - "foo" => true, 12 => None::<&str>, - b"bar" => Some(cbor_null!()), 13 => None::>, - 5 => "foo", 14 => None::<&[u8]>, - 6 => Some(b"bar" as &[u8]), 15 => None::, - 7 => cbor_array![], 16 => None::, - 8 => Some(cbor_array![0, 1]), 17 => None::, - 9 => cbor_map!{}, 18 => None::, - 10 => Some(cbor_map!{2 => 3}), + -1 => -23, + b"bar" => Some(cbor_null!()), + "foo" => true, }; let b = Value::map( [ - (Value::from(-1), Value::from(-23)), (Value::from(4), Value::from(56)), - (Value::from(String::from("foo")), Value::bool_value(true)), - (Value::from(b"bar".to_vec()), Value::null_value()), (Value::from(5), Value::from(String::from("foo"))), (Value::from(6), Value::from(b"bar".to_vec())), (Value::from(7), Value::array(Vec::new())), @@ -578,6 +578,9 @@ mod test { Value::from(10), Value::map([(Value::from(2), Value::from(3))].iter().cloned().collect()), ), + (Value::from(-1), Value::from(-23)), + (Value::from(b"bar".to_vec()), Value::null_value()), + (Value::from(String::from("foo")), Value::bool_value(true)), ] .iter() .cloned() @@ -691,4 +694,22 @@ mod test { assert_eq!(x4, Some(cbor_unsigned!(40))); assert_eq!(x5, None); } + + #[test] + fn test_destructure_unsorted_cbor_map() { + let map = cbor_map! { + 2 => 20, + 1 => 10, + }; + + destructure_cbor_map! { + let { + 1 => x1, + 2 => x2, + } = extract_map(map); + } + + assert_eq!(x1, Some(cbor_unsigned!(10))); + assert_eq!(x2, Some(cbor_unsigned!(20))); + } } diff --git a/libraries/cbor/src/values.rs b/libraries/cbor/src/values.rs index 1bd705ab..7a08d85b 100644 --- a/libraries/cbor/src/values.rs +++ b/libraries/cbor/src/values.rs @@ -75,6 +75,11 @@ impl Constants { } impl Value { + /// Creates a CBOR unsigned value. + pub fn unsigned(int: u64) -> Value { + Value(ValueImpl::Unsigned(int)) + } + /// Create an appropriate CBOR integer value (uint/nint). /// For simplicity, this only takes i64. Construct directly for the last bit. pub fn integer(int: i64) -> Value { @@ -85,13 +90,35 @@ impl Value { } } + /// Creates a CBOR byte string value. + pub fn byte_string(bytes: Vec) -> Value { + Value(ValueImpl::ByteString(bytes)) + } + + /// Creates a CBOR text string value. + pub fn text_string(text: String) -> Value { + Value(ValueImpl::TextString(text)) + } + /// Create a CBOR array value. pub fn array(a: Vec) -> Value { Value(ValueImpl::Array(a)) } /// Create a CBOR map value. - pub fn map(m: Vec<(Value, Value)>) -> Value { + /// + /// Keys do not have to be sorted. + /// + /// # Panics + /// + /// You may not call this function with identical keys in its argument. + pub fn map(mut m: Vec<(Value, Value)>) -> Value { + m.sort_by(|a, b| a.0.cmp(&b.0)); + let map_len = m.len(); + m.dedup_by(|a, b| a.0.eq(&b.0)); + if map_len != m.len() { + panic!(); + } Value(ValueImpl::Map(m)) } @@ -293,8 +320,26 @@ impl SimpleValue { } impl From for Value { - fn from(unsigned: u64) -> Self { - Value(ValueImpl::Unsigned(unsigned)) + fn from(u: u64) -> Self { + Value::unsigned(u) + } +} + +impl From for Value { + fn from(u: u32) -> Self { + Value::unsigned(u as u64) + } +} + +impl From for Value { + fn from(u: u16) -> Self { + Value::unsigned(u as u64) + } +} + +impl From for Value { + fn from(u: u8) -> Self { + Value::unsigned(u as u64) } } @@ -310,6 +355,18 @@ impl From for Value { } } +impl From for Value { + fn from(i: i16) -> Self { + Value::integer(i as i64) + } +} + +impl From for Value { + fn from(i: i8) -> Self { + Value::integer(i as i64) + } +} + impl From> for Value { fn from(bytes: Vec) -> Self { Value(ValueImpl::ByteString(bytes)) @@ -348,7 +405,7 @@ impl From> for Value { impl From> for Value { fn from(map: Vec<(Value, Value)>) -> Self { - Value(ValueImpl::Map(map)) + Value::map(map) } } @@ -406,11 +463,23 @@ mod test { }; use alloc::vec; + #[test] + #[should_panic] + fn test_duplicate_map_key() { + let _map = cbor_map! { + 0 => "a", + -1 => "c", + b"a" => "e", + "c" => "g", + 0 => "b", + }; + } + #[test] fn test_extract_unsigned() { assert_eq!(cbor_int!(1).extract_unsigned(), Some(1)); assert_eq!(cbor_int!(-1).extract_unsigned(), None); - assert_eq!(cbor_bytes!(b"").extract_unsigned(), None); + assert_eq!(cbor_bytes!(vec![]).extract_unsigned(), None); assert_eq!(cbor_text!("").extract_unsigned(), None); assert_eq!(cbor_array![].extract_unsigned(), None); assert_eq!(cbor_map! {}.extract_unsigned(), None); @@ -442,7 +511,7 @@ mod test { fn test_extract_integer() { assert_eq!(cbor_int!(1).extract_integer(), Some(1)); assert_eq!(cbor_int!(-1).extract_integer(), Some(-1)); - assert_eq!(cbor_bytes!(b"").extract_integer(), None); + assert_eq!(cbor_bytes!(vec![]).extract_integer(), None); assert_eq!(cbor_text!("").extract_integer(), None); assert_eq!(cbor_array![].extract_integer(), None); assert_eq!(cbor_map! {}.extract_integer(), None); @@ -474,7 +543,7 @@ mod test { fn test_extract_byte_string() { assert_eq!(cbor_int!(1).extract_byte_string(), None); assert_eq!(cbor_int!(-1).extract_byte_string(), None); - assert_eq!(cbor_bytes!(b"").extract_byte_string(), Some(Vec::new())); + assert_eq!(cbor_bytes!(vec![]).extract_byte_string(), Some(Vec::new())); assert_eq!( cbor_bytes_lit!(b"bar").extract_byte_string(), Some(b"bar".to_vec()) @@ -490,7 +559,7 @@ mod test { fn test_extract_text_string() { assert_eq!(cbor_int!(1).extract_text_string(), None); assert_eq!(cbor_int!(-1).extract_text_string(), None); - assert_eq!(cbor_bytes!(b"").extract_text_string(), None); + assert_eq!(cbor_bytes!(vec![]).extract_text_string(), None); assert_eq!(cbor_text!("").extract_text_string(), Some(String::new())); assert_eq!(cbor_text!("s").extract_text_string(), Some("s".to_string())); assert_eq!(cbor_array![].extract_text_string(), None); @@ -503,7 +572,7 @@ mod test { fn test_extract_array() { assert_eq!(cbor_int!(1).extract_array(), None); assert_eq!(cbor_int!(-1).extract_array(), None); - assert_eq!(cbor_bytes!(b"").extract_array(), None); + assert_eq!(cbor_bytes!(vec![]).extract_array(), None); assert_eq!(cbor_text!("").extract_array(), None); assert_eq!(cbor_array![].extract_array(), Some(Vec::new())); assert_eq!( @@ -519,7 +588,7 @@ mod test { fn test_extract_map() { assert_eq!(cbor_int!(1).extract_map(), None); assert_eq!(cbor_int!(-1).extract_map(), None); - assert_eq!(cbor_bytes!(b"").extract_map(), None); + assert_eq!(cbor_bytes!(vec![]).extract_map(), None); assert_eq!(cbor_text!("").extract_map(), None); assert_eq!(cbor_array![].extract_map(), None); assert_eq!(cbor_map! {}.extract_map(), Some(Vec::new())); @@ -535,7 +604,7 @@ mod test { fn test_extract_tag() { assert_eq!(cbor_int!(1).extract_tag(), None); assert_eq!(cbor_int!(-1).extract_tag(), None); - assert_eq!(cbor_bytes!(b"").extract_tag(), None); + assert_eq!(cbor_bytes!(vec![]).extract_tag(), None); assert_eq!(cbor_text!("").extract_tag(), None); assert_eq!(cbor_array![].extract_tag(), None); assert_eq!(cbor_map! {}.extract_tag(), None); @@ -550,7 +619,7 @@ mod test { fn test_extract_bool() { assert_eq!(cbor_int!(1).extract_bool(), None); assert_eq!(cbor_int!(-1).extract_bool(), None); - assert_eq!(cbor_bytes!(b"").extract_bool(), None); + assert_eq!(cbor_bytes!(vec![]).extract_bool(), None); assert_eq!(cbor_text!("").extract_bool(), None); assert_eq!(cbor_array![].extract_bool(), None); assert_eq!(cbor_map! {}.extract_bool(), None); @@ -565,7 +634,7 @@ mod test { fn test_extract_null() { assert_eq!(cbor_int!(1).extract_null(), None); assert_eq!(cbor_int!(-1).extract_null(), None); - assert_eq!(cbor_bytes!(b"").extract_null(), None); + assert_eq!(cbor_bytes!(vec![]).extract_null(), None); assert_eq!(cbor_text!("").extract_null(), None); assert_eq!(cbor_array![].extract_null(), None); assert_eq!(cbor_map! {}.extract_null(), None); @@ -580,7 +649,7 @@ mod test { fn test_extract_undefined() { assert_eq!(cbor_int!(1).extract_undefined(), None); assert_eq!(cbor_int!(-1).extract_undefined(), None); - assert_eq!(cbor_bytes!(b"").extract_undefined(), None); + assert_eq!(cbor_bytes!(vec![]).extract_undefined(), None); assert_eq!(cbor_text!("").extract_undefined(), None); assert_eq!(cbor_array![].extract_undefined(), None); assert_eq!(cbor_map! {}.extract_undefined(), None); @@ -604,8 +673,8 @@ mod test { assert!(cbor_int!(-24) < cbor_int!(-1000)); assert!(cbor_int!(-1000) < cbor_int!(-1000000)); assert!(cbor_int!(-1000000) < cbor_int!(core::i64::MIN)); - assert!(cbor_int!(core::i64::MIN) < cbor_bytes!(b"")); - assert!(cbor_bytes!(b"") < cbor_bytes!(vec![0x00])); + assert!(cbor_int!(core::i64::MIN) < cbor_bytes!(vec![])); + assert!(cbor_bytes!(vec![]) < cbor_bytes!(vec![0x00])); assert!(cbor_bytes!(vec![0x00]) < cbor_bytes!(vec![0x01])); assert!(cbor_bytes!(vec![0x01]) < cbor_bytes!(vec![0xFF])); assert!(cbor_bytes!(vec![0xFF]) < cbor_bytes!(vec![0x00, 0x00])); @@ -632,9 +701,9 @@ mod test { assert!(cbor_map! {"" => 0} < cbor_map! {cbor_array![] => 0}); assert!(cbor_map! {cbor_array![] => 0} < cbor_map! {cbor_map!{} => 0}); assert!(cbor_map! {cbor_map!{} => 0} < cbor_map! {false => 0}); - assert!(cbor_map! {false => 0} < cbor_map! {0 => 0, 0 => 0}); + assert!(cbor_map! {false => 0} < cbor_map! {0 => 0, 1 => 0}); assert!(cbor_map! {0 => 0} < cbor_tagged!(2, cbor_int!(0))); - assert!(cbor_map! {0 => 0, 0 => 0} < cbor_bool!(false)); + assert!(cbor_map! {0 => 0, 1 => 0} < cbor_bool!(false)); assert!(cbor_bool!(false) < cbor_bool!(true)); assert!(cbor_bool!(true) < cbor_null!()); assert!(cbor_null!() < cbor_undefined!()); diff --git a/libraries/cbor/src/writer.rs b/libraries/cbor/src/writer.rs index e3b37547..ac291351 100644 --- a/libraries/cbor/src/writer.rs +++ b/libraries/cbor/src/writer.rs @@ -78,14 +78,8 @@ impl<'a> Writer<'a> { self.encode_cbor(el, remaining_depth.map(|d| d - 1))?; } } - ValueImpl::Map(mut map) => { - map.sort_by(|a, b| a.0.cmp(&b.0)); - let map_len = map.len(); - map.dedup_by(|a, b| a.0.eq(&b.0)); - if map_len != map.len() { - return Err(EncoderError::DuplicateMapKey); - } - self.start_item(type_label, map_len as u64); + ValueImpl::Map(map) => { + self.start_item(type_label, map.len() as u64); for (k, v) in map { self.encode_cbor(k, remaining_depth.map(|d| d - 1))?; self.encode_cbor(v, remaining_depth.map(|d| d - 1))?; @@ -342,42 +336,6 @@ mod test { assert_eq!(write_return(sorted_map), write_return(unsorted_map)); } - #[test] - fn test_write_map_duplicates() { - let duplicate0 = cbor_map! { - 0 => "a", - -1 => "c", - b"a" => "e", - "c" => "g", - 0 => "b", - }; - assert_eq!(write_return(duplicate0), None); - let duplicate1 = cbor_map! { - 0 => "a", - -1 => "c", - b"a" => "e", - "c" => "g", - -1 => "d", - }; - assert_eq!(write_return(duplicate1), None); - let duplicate2 = cbor_map! { - 0 => "a", - -1 => "c", - b"a" => "e", - "c" => "g", - b"a" => "f", - }; - assert_eq!(write_return(duplicate2), None); - let duplicate3 = cbor_map! { - 0 => "a", - -1 => "c", - b"a" => "e", - "c" => "g", - "c" => "h", - }; - assert_eq!(write_return(duplicate3), None); - } - #[test] fn test_write_map_with_array() { let value_map = cbor_map! {