From d3889946576c92d9e4241325c610f11e582728fe Mon Sep 17 00:00:00 2001
From: Charles Hall <charles@computer.surgery>
Date: Thu, 19 Sep 2024 13:38:41 -0700
Subject: [PATCH] rewrite media key parser

Fixes a regression in e2cba15ed2ec1681cc455d66a7bb2053cc204f23 where the
Content-Type and Content-Disposition parts are extracted in the wrong
order.

Fixes a long-standing issue in b6d721374f970cca912477a3972bb857758d983d
where the Content-Type part was allowed to be completely missing rather
than present and 0 bytes long.

Improves the error messages for various parsing failures to be unique
and more obvious.
---
 src/database/key_value/media.rs | 78 +++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 34 deletions(-)

diff --git a/src/database/key_value/media.rs b/src/database/key_value/media.rs
index e3f771d3..9fc15e54 100644
--- a/src/database/key_value/media.rs
+++ b/src/database/key_value/media.rs
@@ -26,57 +26,67 @@ impl TryFrom<&MediaFileKey> for MediaFileKeyParts {
     fn try_from(key: &MediaFileKey) -> Result<MediaFileKeyParts> {
         let mut parts = key.as_bytes().split(|&b| b == 0xFF);
 
+        // Extract parts
+
         let mxc_bytes = parts
             .next()
-            .ok_or_else(|| Error::BadDatabase("Media ID in db is invalid."))?;
+            .ok_or_else(|| Error::BadDatabase("Missing MXC URI bytes"))?;
+
+        let thumbnail_size_bytes = parts.next().ok_or_else(|| {
+            Error::BadDatabase("Missing thumbnail size bytes")
+        })?;
+
+        let content_disposition_bytes = parts.next().ok_or_else(|| {
+            Error::BadDatabase("Missing Content-Disposition bytes")
+        })?;
+
+        let content_type_bytes = parts
+            .next()
+            .ok_or_else(|| Error::BadDatabase("Missing Content-Type bytes"))?;
+
+        if parts.next().is_some() {
+            return Err(Error::BadDatabase("Too many parts"));
+        }
+
+        // Parse parts
+
         let mxc = utils::string_from_bytes(mxc_bytes)
-            .map_err(|_| {
-                Error::BadDatabase("Media MXC URI in db is invalid unicode.")
-            })?
+            .map_err(|_| Error::BadDatabase("Invalid unicode in MXC URI"))?
             .into();
 
-        let thumbnail_size_bytes = parts
-            .next()
-            .ok_or_else(|| Error::BadDatabase("Media ID in db is invalid."))?;
-        let thumbnail_size_bytes: &[u8; 8] =
-            thumbnail_size_bytes.try_into().map_err(|_| {
-                Error::BadDatabase("Media ID thumbnail size in db is invalid")
-            })?;
-        let width = u32::from_be_bytes(
-            thumbnail_size_bytes[..4].try_into().expect("should be 4 bytes"),
-        );
-        let height = u32::from_be_bytes(
-            thumbnail_size_bytes[4..].try_into().expect("should be 4 bytes"),
-        );
+        let (width, height) = <&[u8; 8]>::try_from(thumbnail_size_bytes)
+            .map(|eight_bytes| {
+                let width = u32::from_be_bytes(
+                    eight_bytes[..4].try_into().expect("should be 4 bytes"),
+                );
+                let height = u32::from_be_bytes(
+                    eight_bytes[4..].try_into().expect("should be 4 bytes"),
+                );
 
-        let content_type = parts
-            .next()
-            .map(|bytes| {
-                utils::string_from_bytes(bytes).map_err(|_| {
-                    Error::BadDatabase(
-                        "Content type in mediaid_file is invalid unicode.",
-                    )
-                })
+                (width, height)
             })
-            .transpose()?;
-
-        let content_disposition_bytes = parts
-            .next()
-            .ok_or_else(|| Error::BadDatabase("Media ID in db is invalid."))?;
+            .map_err(|_| {
+                Error::BadDatabase("Wrong number of thumbnail size bytes")
+            })?;
 
         let content_disposition = if content_disposition_bytes.is_empty() {
             None
         } else {
             Some(utils::string_from_bytes(content_disposition_bytes).map_err(
                 |_| {
-                    Error::BadDatabase(
-                        "Content Disposition in mediaid_file is invalid \
-                         unicode.",
-                    )
+                    Error::BadDatabase("Invalid unicode in Content-Disposition")
                 },
             )?)
         };
 
+        let content_type = if content_type_bytes.is_empty() {
+            None
+        } else {
+            Some(utils::string_from_bytes(content_type_bytes).map_err(
+                |_| Error::BadDatabase("Invalid unicode in Content-Type"),
+            )?)
+        };
+
         Ok(MediaFileKeyParts {
             mxc,
             width,