From dd0afd640ad97b5ebcf887107162009a23ffdca0 Mon Sep 17 00:00:00 2001
From: Jack O'Connor <oconnor663@gmail.com>
Date: Tue, 9 Jul 2024 19:13:32 -0700
Subject: [PATCH] serialize Hash with serde_bytes

Closes #412.
---
 Cargo.toml  |  6 +++++-
 src/lib.rs  |  7 ++++++-
 src/test.rs | 28 ++++++++++++++++++++++++++--
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 036a39b2d..3ce2454cb 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -42,6 +42,8 @@ mmap = ["std", "dep:memmap2"]
 # Implement the zeroize::Zeroize trait for types in this crate.
 zeroize = ["dep:zeroize", "arrayvec/zeroize"]
 
+serde = ["dep:serde", "dep:serde_bytes"]
+
 # This crate implements traits from the RustCrypto project, exposed here as the
 # "traits-preview" feature. However, these traits aren't stable, and they're
 # expected to change in incompatible ways before they reach 1.0. For that
@@ -49,7 +51,7 @@ zeroize = ["dep:zeroize", "arrayvec/zeroize"]
 # who use it should expect breaking changes between patch versions of this
 # crate. (The "*-preview" feature name follows the conventions of the RustCrypto
 # "signature" crate.)
-traits-preview = ["digest"]
+traits-preview = ["dep:digest"]
 
 # ---------- Features below this line are undocumented and unstable. ----------
 # The following features are mainly intended for testing and benchmarking, and
@@ -103,6 +105,7 @@ digest = { version = "0.10.1", features = [ "mac" ], optional = true }
 memmap2 = { version = "0.9", optional = true }
 rayon-core = { version = "1.12.1", optional = true }
 serde = { version = "1.0", default-features = false, features = ["derive"], optional = true }
+serde_bytes = { version = "0.11.15", optional = true }
 zeroize = { version = "1", default-features = false, features = ["zeroize_derive"], optional = true }
 
 [dev-dependencies]
@@ -114,6 +117,7 @@ rand_chacha = "0.3.0"
 reference_impl = { path = "./reference_impl" }
 tempfile = "3.8.0"
 serde_json = "1.0.107"
+ciborium = "0.2.2"
 
 [build-dependencies]
 cc = "1.0.4"
diff --git a/src/lib.rs b/src/lib.rs
index d64e18fce..e6a9d6771 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -219,7 +219,12 @@ fn counter_high(counter: u64) -> u32 {
 #[cfg_attr(feature = "zeroize", derive(zeroize::Zeroize))]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 #[derive(Clone, Copy, Hash)]
-pub struct Hash([u8; OUT_LEN]);
+pub struct Hash(
+    #[rustfmt::skip]
+    // In formats like CBOR, bytestrings are more compact than lists of ints.
+    #[cfg_attr(feature = "serde", serde(with = "serde_bytes"))]
+    [u8; OUT_LEN],
+);
 
 impl Hash {
     /// The raw bytes of the `Hash`. Note that byte arrays don't provide
diff --git a/src/test.rs b/src/test.rs
index c76cbbc03..b5bd4dc55 100644
--- a/src/test.rs
+++ b/src/test.rs
@@ -809,14 +809,38 @@ fn test_mmap_rayon() -> Result<(), std::io::Error> {
 #[cfg(feature = "std")]
 #[cfg(feature = "serde")]
 fn test_serde() {
-    let hash: crate::Hash = [7; 32].into();
+    let hash: crate::Hash = [255; 32].into();
+
     let json = serde_json::to_string(&hash).unwrap();
     assert_eq!(
         json,
-        "[7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7]",
+        "[255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255]",
     );
     let hash2: crate::Hash = serde_json::from_str(&json).unwrap();
     assert_eq!(hash, hash2);
+
+    let mut cbor = Vec::<u8>::new();
+    ciborium::into_writer(&hash, &mut cbor).unwrap();
+    assert_eq!(
+        cbor,
+        [
+            88, 32, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255,
+            255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255
+        ]
+    );
+    let hash_from_cbor: crate::Hash = ciborium::from_reader(&cbor[..]).unwrap();
+    assert_eq!(hash_from_cbor, hash);
+
+    // Before we used serde_bytes, the hash [255; 32] would serialize as an array instead of a
+    // byte string, like this. Make sure we can still deserialize this representation.
+    let old_cbor: &[u8] = &[
+        152, 32, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255,
+        24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255,
+        24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255, 24, 255,
+        24, 255, 24, 255, 24, 255,
+    ];
+    let hash_from_old_cbor: crate::Hash = ciborium::from_reader(old_cbor).unwrap();
+    assert_eq!(hash_from_old_cbor, hash);
 }
 
 // `cargo +nightly miri test` currently works, but it takes forever, because some of our test