Merge #493

493: Strict ordering for DAG-CBOR-encoded map keys r=koivunej a=cdata This change proposes a fix for #492 . `Ipld::Map` keys are sorted according to [the strictness guidelines in the DAG-CBOR spec](https://github.com/ipld/ipld/blob/master/specs/codecs/dag-cbor/spec.md#:~:text=The%20keys%20in,order%20sorts%20earlier.) when encoding them as DAG-CBOR. Co-authored-by: Chris Joel <[email protected]> Co-authored-by: Christopher Joel <[email protected]>
rs-ipfs · Feb 1, 2022 · 3eff4e1 · 3eff4e1
2 parents a78a623 + 80d2b79
commit 3eff4e1
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 * perf: use hash_hasher where the key is Cid [#467]
 * chore: upgrade to libp2p 0.39.1, update most of the other deps with the notable exception of cid and multihash [#472]
 * refactor(swarm): swarm cleanup following libp2p upgrade to v0.39.1 [#473]
+* fix: strict ordering for DAG-CBOR-encoded map keys [#493]
 
 [#429]: https://github.com/rs-ipfs/rust-ipfs/pull/429
 [#428]: https://github.com/rs-ipfs/rust-ipfs/pull/428
@@ -27,6 +28,7 @@
 [#467]: https://github.com/rs-ipfs/rust-ipfs/pull/467
 [#472]: https://github.com/rs-ipfs/rust-ipfs/pull/472
 [#473]: https://github.com/rs-ipfs/rust-ipfs/pull/473
+[#493]: https://github.com/rs-ipfs/rust-ipfs/pull/493
 
 # 0.2.1
 

diff --git a/src/dag.rs b/src/dag.rs
@@ -598,7 +598,7 @@ fn resolve_local_ipld<'a>(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{make_ipld, Node};
+    use crate::{ipld::dag_cbor::DagCborCodec, make_ipld, Node};
 
     #[tokio::test]
     async fn test_resolve_root_cid() {
@@ -996,4 +996,24 @@ mod tests {
             format!("no link named \"second-best-file\" under {}", cids[1])
         );
     }
+
+    #[test]
+    fn observes_strict_order_of_map_keys() {
+        let map = make_ipld!({
+            "omega": Ipld::Null,
+            "bar": Ipld::Null,
+            "alpha": Ipld::Null,
+            "foo": Ipld::Null,
+        });
+
+        let bytes = DagCborCodec::encode(&map).unwrap();
+
+        assert_eq!(
+            bytes.as_ref(),
+            &[
+                164, 99, 98, 97, 114, 246, 99, 102, 111, 111, 246, 101, 97, 108, 112, 104, 97, 246,
+                101, 111, 109, 101, 103, 97, 246
+            ]
+        );
+    }
 }
diff --git a/src/ipld/dag_cbor.rs b/src/ipld/dag_cbor.rs
@@ -321,10 +321,17 @@ impl<T: WriteCbor + 'static> WriteCbor for BTreeMap<String, T> {
     #[inline]
     fn write_cbor<W: Write>(&self, w: &mut W) -> CborResult<()> {
         write_u64(w, 5, self.len() as u64)?;
-        for (k, v) in self {
+        let mut keys: Vec<&String> = self.keys().collect();
+
+        // See: https://github.com/ipld/ipld/blob/master/specs/codecs/dag-cbor/spec.md#strictness
+        keys.sort_by(|l, r| l.len().cmp(&r.len()).then_with(|| l.cmp(r)));
+
+        for k in keys {
+            let v = self.get(k).unwrap();
             k.write_cbor(w)?;
             v.write_cbor(w)?;
         }
+
         Ok(())
     }
 }