Have Media db return optional content_type, conversion fixes

src/client_server/media.rs

@@ -66,7 +66,7 @@ pub async fn get_content_route(
     {
         Ok(get_content::Response {
             file,
-            content_type: Some(content_type),
+            content_type,
             content_disposition: filename,
         }
         .into())
@@ -116,11 +116,7 @@ pub async fn get_content_thumbnail_route(
             .try_into()
             .map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Width is invalid."))?,
     )? {
-        Ok(get_content_thumbnail::Response {
-            file,
-            content_type: Some(content_type),
-        }
-        .into())
+        Ok(get_content_thumbnail::Response { file, content_type }.into())
     } else if &*body.server_name != db.globals.server_name() && body.allow_remote {
         let get_thumbnail_response = server_server::send_request(
             &db.globals,

src/client_server/membership.rs

@@ -510,8 +510,7 @@ async fn join_room_by_id_helper(
             .expect("event is valid, we just created it"),
         );
 
-        // TODO fixup CanonicalJsonValue
-        // use that instead of serde_json::Map... maybe?
+        // Convert `serde_json;:Value` to `CanonicalJsonObj` for hashing/signing
         let mut canon_json_stub: BTreeMap<_, ruma::signatures::CanonicalJsonValue> =
             serde_json::from_value(join_event_stub_value).expect("json Value is canonical JSON");
 

src/database/media.rs

@@ -5,7 +5,7 @@ use std::mem;
 
 pub struct FileMeta {
     pub filename: Option<String>,
-    pub content_type: String,
+    pub content_type: Option<String>,
     pub file: Vec<u8>,
 }
 
@@ -83,12 +83,14 @@ impl Media {
             let (key, file) = r?;
             let mut parts = key.rsplit(|&b| b == 0xff);
 
-            let content_type = utils::string_from_bytes(
-                parts
-                    .next()
-                    .ok_or_else(|| Error::bad_database("Media ID in db is invalid."))?,
-            )
-            .map_err(|_| Error::bad_database("Content type in mediaid_file is invalid unicode."))?;
+            let content_type = parts
+                .next()
+                .map(|bytes| {
+                    Ok::<_, Error>(utils::string_from_bytes(bytes).map_err(|_| {
+                        Error::bad_database("Content type in mediaid_file is invalid unicode.")
+                    })?)
+                })
+                .transpose()?;
 
             let filename_bytes = parts
                 .next()
@@ -158,12 +160,14 @@ impl Media {
             let (key, file) = r?;
             let mut parts = key.rsplit(|&b| b == 0xff);
 
-            let content_type = utils::string_from_bytes(
-                parts
-                    .next()
-                    .ok_or_else(|| Error::bad_database("Invalid Media ID in db"))?,
-            )
-            .map_err(|_| Error::bad_database("Content type in mediaid_file is invalid unicode."))?;
+            let content_type = parts
+                .next()
+                .map(|bytes| {
+                    Ok::<_, Error>(utils::string_from_bytes(bytes).map_err(|_| {
+                        Error::bad_database("Content type in mediaid_file is invalid unicode.")
+                    })?)
+                })
+                .transpose()?;
 
             let filename_bytes = parts
                 .next()
@@ -189,12 +193,14 @@ impl Media {
             let (key, file) = r?;
             let mut parts = key.rsplit(|&b| b == 0xff);
 
-            let content_type = utils::string_from_bytes(
-                parts
-                    .next()
-                    .ok_or_else(|| Error::bad_database("Media ID in db is invalid."))?,
-            )
-            .map_err(|_| Error::bad_database("Content type in mediaid_file is invalid unicode."))?;
+            let content_type = parts
+                .next()
+                .map(|bytes| {
+                    Ok::<_, Error>(utils::string_from_bytes(bytes).map_err(|_| {
+                        Error::bad_database("Content type in mediaid_file is invalid unicode.")
+                    })?)
+                })
+                .transpose()?;
 
             let filename_bytes = parts
                 .next()

src/database/rooms.rs

@@ -499,7 +499,6 @@ impl Rooms {
         Ok(())
     }
 
-    #[allow(clippy::too_many_arguments)]
     /// Creates a new persisted data unit and adds it to a room.
     ///
     /// By this point the incoming event should be fully authenticated, no auth happens
@@ -856,9 +855,8 @@ impl Rooms {
         };
 
         // Hash and sign
-        let mut pdu_json: BTreeMap<String, ruma::serde::CanonicalJsonValue> =
-            serde_json::from_value(serde_json::json!(&pdu))
-                .expect("event is valid, we just created it");
+        let mut pdu_json =
+            utils::to_canonical_object(&pdu).expect("event is valid, we just created it");
 
         pdu_json.remove("event_id");
 

src/utils.rs

@@ -1,6 +1,7 @@
 use argon2::{Config, Variant};
 use cmp::Ordering;
 use rand::prelude::*;
+use ruma::serde::{try_from_json_map, CanonicalJsonError, CanonicalJsonObject};
 use sled::IVec;
 use std::{
     cmp,
@@ -94,3 +95,19 @@ pub fn common_elements(
             .all(|b| b)
     }))
 }
+
+/// Fallible conversion from any value that implements `Serialize` to a `CanonicalJsonObject`.
+///
+/// `value` must serialize to an `serde_json::Value::Object`.
+pub fn to_canonical_object<T: serde::Serialize>(
+    value: T,
+) -> Result<CanonicalJsonObject, CanonicalJsonError> {
+    use serde::ser::Error;
+
+    match serde_json::to_value(value).map_err(CanonicalJsonError::SerDe)? {
+        serde_json::Value::Object(map) => try_from_json_map(map),
+        _ => Err(CanonicalJsonError::SerDe(serde_json::Error::custom(
+            "Value must be an object",
+        ))),
+    }
+}