CBOR maps use Vec instead of BTreeMap

libraries/cbor/src/macros.rs

@@ -13,14 +13,14 @@
 // limitations under the License.
 
 use crate::values::{KeyType, Value};
-use alloc::collections::btree_map;
+use alloc::vec;
 use core::cmp::Ordering;
 use core::iter::Peekable;
 
-/// This macro generates code to extract multiple values from a `BTreeMap<KeyType, Value>` at once
-/// in an optimized manner, consuming the input map.
+/// This macro generates code to extract multiple values from a `Vec<(KeyType, Value)>` at once
+/// in an optimized manner, consuming the input vector.
 ///
-/// It takes as input a `BTreeMap` as well as a list of identifiers and keys, and generates code
+/// It takes as input a `Vec` as well as a list of identifiers and keys, and generates code
 /// that assigns the corresponding values to new variables using the given identifiers. Each of
 /// these variables has type `Option<Value>`, to account for the case where keys aren't found.
 ///
@@ -32,16 +32,14 @@ use core::iter::Peekable;
 /// the keys are indeed sorted. This macro is therefore **not suitable for dynamic keys** that can
 /// change at runtime.
 ///
-/// Semantically, provided that the keys are sorted as specified above, the following two snippets
-/// of code are equivalent, but the `destructure_cbor_map!` version is more optimized, as it doesn't
-/// re-balance the `BTreeMap` for each key, contrary to the `BTreeMap::remove` operations.
+/// Example usage:
 ///
 /// ```rust
 /// # extern crate alloc;
 /// # use cbor::destructure_cbor_map;
 /// #
 /// # fn main() {
-/// #     let map = alloc::collections::BTreeMap::new();
+/// #     let map = alloc::vec::Vec::new();
 /// destructure_cbor_map! {
 ///     let {
 ///         1 => x,
@@ -50,17 +48,6 @@ use core::iter::Peekable;
 /// }
 /// # }
 /// ```
-///
-/// ```rust
-/// # extern crate alloc;
-/// #
-/// # fn main() {
-/// #     let mut map = alloc::collections::BTreeMap::<cbor::KeyType, _>::new();
-/// use cbor::values::IntoCborKey;
-/// let x: Option<cbor::Value> = map.remove(&1.into_cbor_key());
-/// let y: Option<cbor::Value> = map.remove(&"key".into_cbor_key());
-/// # }
-/// ```
 #[macro_export]
 macro_rules! destructure_cbor_map {
     ( let { $( $key:expr => $variable:ident, )+ } = $map:expr; ) => {
@@ -100,7 +87,7 @@ macro_rules! destructure_cbor_map {
 /// would be inlined for every use case. As of June 2020, this saves ~40KB of binary size for the
 /// CTAP2 application of OpenSK.
 pub fn destructure_cbor_map_peek_value(
-    it: &mut Peekable<btree_map::IntoIter<KeyType, Value>>,
+    it: &mut Peekable<vec::IntoIter<(KeyType, Value)>>,
     needle: KeyType,
 ) -> Option<Value> {
     loop {
@@ -157,9 +144,9 @@ macro_rules! cbor_map {
             // The import is unused if the list is empty.
             #[allow(unused_imports)]
             use $crate::values::{IntoCborKey, IntoCborValue};
-            let mut _map = ::alloc::collections::BTreeMap::new();
+            let mut _map = ::alloc::vec::Vec::new();
             $(
-                _map.insert($key.into_cbor_key(), $value.into_cbor_value());
+                _map.push(($key.into_cbor_key(), $value.into_cbor_value()));
             )*
             $crate::values::Value::Map(_map)
         }
@@ -178,12 +165,12 @@ macro_rules! cbor_map_options {
             // The import is unused if the list is empty.
             #[allow(unused_imports)]
             use $crate::values::{IntoCborKey, IntoCborValueOption};
-            let mut _map = ::alloc::collections::BTreeMap::<_, $crate::values::Value>::new();
+            let mut _map = ::alloc::vec::Vec::<(_, $crate::values::Value)>::new();
             $(
             {
                 let opt: Option<$crate::values::Value> = $value.into_cbor_value_option();
                 if let Some(val) = opt {
-                    _map.insert($key.into_cbor_key(), val);
+                    _map.push(($key.into_cbor_key(), val));
                 }
             }
             )*
@@ -193,10 +180,10 @@ macro_rules! cbor_map_options {
 }
 
 #[macro_export]
-macro_rules! cbor_map_btree {
-    ( $tree:expr ) => {
-        $crate::values::Value::Map($tree)
-    };
+macro_rules! cbor_map_collection {
+    ( $tree:expr ) => {{
+        $crate::values::Value::from($tree)
+    }};
 }
 
 #[macro_export]
@@ -421,7 +408,7 @@ mod test {
                 Value::KeyValue(KeyType::Unsigned(0)),
                 Value::KeyValue(KeyType::Unsigned(1)),
             ]),
-            Value::Map(BTreeMap::new()),
+            Value::Map(Vec::new()),
             Value::Map(
                 [(KeyType::Unsigned(2), Value::KeyValue(KeyType::Unsigned(3)))]
                     .iter()
@@ -518,7 +505,7 @@ mod test {
                         Value::KeyValue(KeyType::Unsigned(1)),
                     ]),
                 ),
-                (KeyType::Unsigned(9), Value::Map(BTreeMap::new())),
+                (KeyType::Unsigned(9), Value::Map(Vec::new())),
                 (
                     KeyType::Unsigned(10),
                     Value::Map(
@@ -589,7 +576,7 @@ mod test {
                         Value::KeyValue(KeyType::Unsigned(1)),
                     ]),
                 ),
-                (KeyType::Unsigned(9), Value::Map(BTreeMap::new())),
+                (KeyType::Unsigned(9), Value::Map(Vec::new())),
                 (
                     KeyType::Unsigned(10),
                     Value::Map(
@@ -608,30 +595,26 @@ mod test {
     }
 
     #[test]
-    fn test_cbor_map_btree_empty() {
-        let a = cbor_map_btree!(BTreeMap::new());
-        let b = Value::Map(BTreeMap::new());
+    fn test_cbor_map_collection_empty() {
+        let a = cbor_map_collection!(BTreeMap::new());
+        let b = Value::Map(Vec::new());
         assert_eq!(a, b);
     }
 
     #[test]
-    fn test_cbor_map_btree_foo() {
-        let a = cbor_map_btree!(
-            [(KeyType::Unsigned(2), Value::KeyValue(KeyType::Unsigned(3)))]
-                .iter()
-                .cloned()
-                .collect()
-        );
-        let b = Value::Map(
-            [(KeyType::Unsigned(2), Value::KeyValue(KeyType::Unsigned(3)))]
-                .iter()
-                .cloned()
-                .collect(),
-        );
+    fn test_cbor_map_collection_foo() {
+        let a = cbor_map_collection!(vec![(
+            KeyType::Unsigned(2),
+            Value::KeyValue(KeyType::Unsigned(3))
+        )]);
+        let b = Value::Map(vec![(
+            KeyType::Unsigned(2),
+            Value::KeyValue(KeyType::Unsigned(3)),
+        )]);
         assert_eq!(a, b);
     }
 
-    fn extract_map(cbor_value: Value) -> BTreeMap<KeyType, Value> {
+    fn extract_map(cbor_value: Value) -> Vec<(KeyType, Value)> {
         match cbor_value {
             Value::Map(map) => map,
             _ => panic!("Expected CBOR map."),

libraries/cbor/src/reader.rs

@@ -13,8 +13,7 @@
 // limitations under the License.
 
 use super::values::{Constants, KeyType, SimpleValue, Value};
-use crate::{cbor_array_vec, cbor_bytes_lit, cbor_map_btree, cbor_text, cbor_unsigned};
-use alloc::collections::BTreeMap;
+use crate::{cbor_array_vec, cbor_bytes_lit, cbor_map_collection, cbor_text, cbor_unsigned};
 use alloc::str;
 use alloc::vec::Vec;
 
@@ -174,7 +173,7 @@ impl<'a> Reader<'a> {
         size_value: u64,
         remaining_depth: i8,
     ) -> Result<Value, DecoderError> {
-        let mut value_map = BTreeMap::new();
+        let mut value_map = Vec::new();
         let mut last_key_option = None;
         for _ in 0..size_value {
             let key_value = self.decode_complete_data_item(remaining_depth - 1)?;
@@ -185,12 +184,12 @@ impl<'a> Reader<'a> {
                     }
                 }
                 last_key_option = Some(key.clone());
-                value_map.insert(key, self.decode_complete_data_item(remaining_depth - 1)?);
+                value_map.push((key, self.decode_complete_data_item(remaining_depth - 1)?));
             } else {
                 return Err(DecoderError::IncorrectMapKeyType);
             }
         }
-        Ok(cbor_map_btree!(value_map))
+        Ok(cbor_map_collection!(value_map))
     }
 
     fn decode_to_simple_value(

libraries/cbor/src/values.rs

@@ -21,7 +21,7 @@ use core::cmp::Ordering;
 pub enum Value {
     KeyValue(KeyType),
     Array(Vec<Value>),
-    Map(BTreeMap<KeyType, Value>),
+    Map(Vec<(KeyType, Value)>),
     // TAG is omitted
     Simple(SimpleValue),
 }
@@ -183,6 +183,22 @@ where
     }
 }
 
+impl From<Vec<(KeyType, Value)>> for Value {
+    fn from(map: Vec<(KeyType, Value)>) -> Self {
+        Value::Map(map)
+    }
+}
+
+impl From<BTreeMap<KeyType, Value>> for Value {
+    fn from(map: BTreeMap<KeyType, Value>) -> Self {
+        let mut map_array = Vec::new();
+        for (k, v) in map {
+            map_array.push((k, v));
+        }
+        Value::Map(map_array)
+    }
+}
+
 impl From<bool> for Value {
     fn from(b: bool) -> Self {
         Value::bool_value(b)

libraries/cbor/src/writer.rs

@@ -56,8 +56,9 @@ impl<'a> Writer<'a> {
                     }
                 }
             }
-            Value::Map(map) => {
+            Value::Map(mut map) => {
                 self.start_item(5, map.len() as u64);
+                map.sort_by(|a, b| a.0.cmp(&b.0));
                 for (k, v) in map {
                     if !self.encode_cbor(Value::KeyValue(k), remaining_depth - 1) {
                         return false;
@@ -209,9 +210,16 @@ mod test {
     #[test]
     fn test_write_map() {
         let value_map = cbor_map! {
-            "aa" => "AA",
-            "e" => "E",
-            "" => ".",
+            0 => "a",
+            23 => "b",
+            24 => "c",
+            std::u8::MAX as i64 => "d",
+            256 => "e",
+            std::u16::MAX as i64 => "f",
+            65536 => "g",
+            std::u32::MAX as i64 => "h",
+            4294967296_i64 => "i",
+            std::i64::MAX => "j",
             -1 => "k",
             -24 => "l",
             -25 => "m",
@@ -224,16 +232,9 @@ mod test {
             b"a" => 2,
             b"bar" => 3,
             b"foo" => 4,
-            0 => "a",
-            23 => "b",
-            24 => "c",
-            std::u8::MAX as i64 => "d",
-            256 => "e",
-            std::u16::MAX as i64 => "f",
-            65536 => "g",
-            std::u32::MAX as i64 => "h",
-            4294967296_i64 => "i",
-            std::i64::MAX => "j",
+            "" => ".",
+            "e" => "E",
+            "aa" => "AA",
         };
         let expected_cbor = vec![
             0xb8, 0x19, // map of 25 pairs:

src/ctap/command.rs

@@ -594,14 +594,14 @@ mod test {
             0x01 => vec![0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F],
             0x02 => cbor_map! {
                 "id" => "example.com",
-                "name" => "Example",
                 "icon" => "example.com/icon.png",
+                "name" => "Example",
             },
             0x03 => cbor_map! {
                 "id" => vec![0x1D, 0x1D, 0x1D, 0x1D],
+                "icon" => "example.com/foo/icon.png",
                 "name" => "foo",
                 "displayName" => "bar",
-                "icon" => "example.com/foo/icon.png",
             },
             0x04 => cbor_array![ES256_CRED_PARAM],
             0x05 => cbor_array![],
@@ -656,8 +656,8 @@ mod test {
             0x01 => "example.com",
             0x02 => vec![0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F],
             0x03 => cbor_array![ cbor_map! {
-                "type" => "public-key",
                 "id" => vec![0x2D, 0x2D, 0x2D, 0x2D],
+                "type" => "public-key",
                 "transports" => cbor_array!["usb"],
             } ],
             0x06 => vec![0x12, 0x34],