Rework hash_and_sign_event to mutate JSON.

src/functions.rs

@@ -2,7 +2,7 @@
 
 use base64::{encode_config, STANDARD_NO_PAD};
 use ring::digest::{digest, SHA256};
-use serde_json::{from_str, json, to_string, to_value, Value};
+use serde_json::{from_str, map::Map, to_string, to_value, Value};
 
 use crate::{
     keys::KeyPair,
@@ -186,7 +186,11 @@ pub fn reference_hash(value: &Value) -> Result<String, Error> {
     Ok(encode_config(&hash, STANDARD_NO_PAD))
 }
 
-/// Hashes and signs the JSON representation of an event.
+/// Hashes and signs the JSON representation of an event and adds the hash and signature to objects
+/// under the keys `hashes` and `signatures`, respectively.
+///
+/// If `hashes` and/or `signatures` are already present, the new data will be appended to the
+/// existing data.
 ///
 /// # Parameters
 ///
@@ -196,41 +200,40 @@ pub fn reference_hash(value: &Value) -> Result<String, Error> {
 pub fn hash_and_sign_event<K>(
     server_name: &str,
     key_pair: &K,
-    value: &Value,
-) -> Result<Value, Error>
+    value: &mut Value,
+) -> Result<(), Error>
 where
     K: KeyPair,
 {
     let hash = content_hash(value)?;
 
-    if !value.is_object() {
-        return Err(Error::new("JSON value must be a JSON object"));
-    }
-
-    let mut owned_value = value.clone();
-
-    // Limit the scope of the mutable borrow so `owned_value` can be passed immutably to `redact`
-    // below.
+    // Limit the scope of the mutable borrow so `value` can be passed immutably to `redact` below.
     {
-        let object = owned_value
-            .as_object_mut()
-            .expect("safe since we checked above");
+        let map = match value {
+            Value::Object(ref mut map) => map,
+            _ => return Err(Error::new("JSON value must be a JSON object")),
+        };
 
-        let hashes = json!({ "sha256": hash });
+        let hashes_value = map
+            .entry("hashes")
+            .or_insert_with(|| Value::Object(Map::with_capacity(1)));
 
-        object.insert("hashes".to_string(), hashes);
+        match hashes_value.as_object_mut() {
+            Some(hashes) => hashes.insert("sha256".to_string(), Value::String(hash)),
+            None => return Err(Error::new("Field `hashes` must be a JSON object")),
+        };
     }
 
-    let mut redacted = redact(&owned_value)?;
+    let mut redacted = redact(value)?;
 
     sign_json(server_name, key_pair, &mut redacted)?;
 
-    let object = owned_value
-        .as_object_mut()
-        .expect("safe since we checked above");
-    object.insert("signatures".to_string(), redacted["signatures"].take());
+    // Safe to unwrap because we did this exact check at the beginning of the function.
+    let map = value.as_object_mut().unwrap();
 
-    Ok(owned_value)
+    map.insert("signatures".to_string(), redacted["signatures"].take());
+
+    Ok(())
 }
 
 /// Internal implementation detail of the canonical JSON algorithm. Allows customization of the

src/lib.rs

@@ -224,8 +224,8 @@ mod test {
     use serde_json::{from_str, to_string, to_value, Value};
 
     use super::{
-        sign_json, to_canonical_json, verify_json, Ed25519KeyPair, Ed25519Verifier, KeyPair,
-        Signature,
+        hash_and_sign_event, sign_json, to_canonical_json, verify_json, Ed25519KeyPair,
+        Ed25519Verifier, KeyPair, Signature,
     };
     const EMPTY_JSON_SIGNATURE: &str =
         "K8280/U9SSy9IVtjBuVeLr+HpOB4BQFWbg+UZaADMtTdGYI7Geitb76LTrr5QV/7Xg4ahLwYGYZzuHGZKM5ZAQ";
@@ -498,4 +498,43 @@ mod test {
         )
         .is_err());
     }
+
+    #[test]
+    fn sign_minimal_event() {
+        let key_pair = Ed25519KeyPair::new(
+            decode_config(&PUBLIC_KEY, STANDARD_NO_PAD)
+                .unwrap()
+                .as_slice(),
+            decode_config(&PRIVATE_KEY, STANDARD_NO_PAD)
+                .unwrap()
+                .as_slice(),
+            "1".to_string(),
+        )
+        .unwrap();
+
+        let json = r#"{
+            "room_id": "!x:domain",
+            "sender": "@a:domain",
+            "origin": "domain",
+            "origin_server_ts": 1000000,
+            "signatures": {},
+            "hashes": {},
+            "type": "X",
+            "content": {},
+            "prev_events": [],
+            "auth_events": [],
+            "depth": 3,
+            "unsigned": {
+                "age_ts": 1000000
+            }
+        }"#;
+
+        let mut value = from_str::<Value>(json).unwrap();
+        hash_and_sign_event("domain", &key_pair, &mut value).unwrap();
+
+        assert_eq!(
+            to_string(&value).unwrap(),
+            r#"{"auth_events":[],"content":{},"depth":3,"hashes":{"sha256":"5jM4wQpv6lnBo7CLIghJuHdW+s2CMBJPUOGOC89ncos"},"origin":"domain","origin_server_ts":1000000,"prev_events":[],"room_id":"!x:domain","sender":"@a:domain","signatures":{"domain":{"ed25519:1":"KxwGjPSDEtvnFgU00fwFz+l6d2pJM6XBIaMEn81SXPTRl16AqLAYqfIReFGZlHi5KLjAWbOoMszkwsQma+lYAg"}},"type":"X","unsigned":{"age_ts":1000000}}"#
+        );
+    }
 }