Ordering invariant does not hold between `cbor_map!` and `destructure_cbor_map!` on the develop branch
Expected Behavior
In the develop branch, the cbor_map! macro's documentation indicates that keys do not need to be sorted: https://github.com/google/OpenSK/blob/develop/libraries/cbor/src/macros.rs#L143.
It is therefore expected that a map built like:
let map = cbor_map! {
1 => 10,
2 => 20,
};
can be correctly used with destructure_cbor_map! as follows:
#[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)));
}
Actual Behavior
This test fails with:
failures:
---- macros::test::test_destructure_unsorted_cbor_map stdout ----
thread 'macros::test::test_destructure_unsorted_cbor_map' panicked at 'assertion failed: `(left == right)`
left: `None`,
right: `Some(Unsigned(10))`', src/macros.rs:667:9
This is because destructure_cbor_map! expects that both the destructured keys (left-hand-side of the arrow) and the keys in the map itself are sorted. The former constraint is well documented by destructure_cbor_map! (and checked via unit tests for lack of static assertions), but the latter does not hold if inputs are not sorted in cbor_map! (which just copies the key-value pairs into a vectors without doing any sorting).
Affected code
- #303 did change a bunch of
cbor_map!expressions to actually have sorted keys in their inputs, so I don't think the OpenSK usage ofsk-cboris affected. - The
stablebranch shouldn't be affected, because it's still using theBTreeMaprepresentation of map values (https://github.com/google/OpenSK/pull/303 wasn't applied). - A quick look at the dependents of the
sk-cborcrate (https://crates.io/crates/sk-cbor/reverse_dependencies), as well as a global GitHub search (https://github.com/search?q=destructure_cbor_map&type=Code) shows thatdestructure_cbor_map!does not seem to be used outside of OpenSK.
So there doesn't seem to be code affected by this bug at the moment.
Steps to Reproduce the Problem
- Apply the following patch to the
developbranch:
$ git diff
diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs
index d7e1185..ca9ae02 100644
--- a/libraries/cbor/src/macros.rs
+++ b/libraries/cbor/src/macros.rs
@@ -650,6 +650,24 @@ mod test {
assert_eq!(x2, Some(cbor_unsigned!(20)));
}
+ #[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)));
+ }
+
#[test]
#[should_panic]
fn test_destructure_cbor_map_unsorted() {
cd libraries/cbor && cargo test.
Specifications
- Version:
developbranch - Platform: N/A