Merge branch 'shekohex/fix-media-file-name' into 'next'

fix: Media file name too long

Closes #146

See merge request famedly/conduit!467

Cargo.lock

@@ -529,6 +529,7 @@ dependencies = [
  "serde_json",
  "serde_yaml",
  "sha-1",
+ "sha2",
  "thiserror",
  "thread_local",
  "threadpool",

Cargo.toml

@@ -124,6 +124,7 @@ thread_local = "1.1.7"
 # used for TURN server authentication
 hmac = "0.12.1"
 sha-1 = "0.10.1"
+sha2 = "0.9"
 # used for conduit's CLI and admin room command parsing
 clap = { version = "4.3.0", default-features = false, features = [
   "derive",

src/database/mod.rs

@@ -937,6 +937,21 @@ impl KeyValueDatabase {
                     )?;
                 }
 
+                // Move old media files to new names
+                for (key, _) in db.mediaid_file.iter() {
+                    // we know that this method is deprecated, but we need to use it to migrate the old files
+                    // to the new location
+                    //
+                    // TODO: remove this once we're sure that all users have migrated
+                    #[allow(deprecated)]
+                    let old_path = services().globals.get_media_file_old(&key);
+                    let path = services().globals.get_media_file(&key);
+                    // move the file to the new location
+                    if old_path.exists() {
+                        tokio::fs::rename(&old_path, &path).await?;
+                    }
+                }
+
                 services().globals.bump_database_version(13)?;
 
                 warn!("Migration: 12 -> 13 finished");

src/service/globals/mod.rs

@@ -4,6 +4,7 @@ use ruma::{
     serde::Base64, MilliSecondsSinceUnixEpoch, OwnedDeviceId, OwnedEventId, OwnedRoomAliasId,
     OwnedRoomId, OwnedServerName, OwnedUserId, RoomAliasId,
 };
+use sha2::Digest;
 
 use crate::api::server_server::DestinationResponse;
 
@@ -27,7 +28,7 @@ use std::{
     str::FromStr,
     sync::{
         atomic::{self, AtomicBool},
-        Arc, RwLock as StdRwLock,
+        Arc, Mutex, RwLock as StdRwLock,
     },
     time::{Duration, Instant},
 };
@@ -474,6 +475,21 @@ impl Service {
     }
 
     pub fn get_media_file(&self, key: &[u8]) -> PathBuf {
+        let mut r = PathBuf::new();
+        r.push(self.config.database_path.clone());
+        r.push("media");
+        // Using the hash of the key as the filename
+        // This is to prevent the total length of the path from exceeding the maximum length
+        r.push(general_purpose::URL_SAFE_NO_PAD.encode(sha2::Sha256::digest(key));
+        r
+    }
+
+    /// This is the old version of `get_media_file` that uses the key as the filename.
+    ///
+    /// This is deprecated and will be removed in a future release.
+    /// Please use `get_media_file` instead.
+    #[deprecated(note = "Use get_media_file instead")]
+    pub fn get_media_file_old(&self, key: &[u8]) -> PathBuf {
         let mut r = PathBuf::new();
         r.push(self.config.database_path.clone());
         r.push("media");

src/service/media/mod.rs

@@ -233,3 +233,95 @@ impl Service {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::path::PathBuf;
+
+    use sha2::Digest;
+
+    use super::*;
+
+    struct MockedKVDatabase;
+
+    impl Data for MockedKVDatabase {
+        fn create_file_metadata(
+            &self,
+            mxc: String,
+            width: u32,
+            height: u32,
+            content_disposition: Option<&str>,
+            content_type: Option<&str>,
+        ) -> Result<Vec<u8>> {
+            // copied from src/database/key_value/media.rs
+            let mut key = mxc.as_bytes().to_vec();
+            key.push(0xff);
+            key.extend_from_slice(&width.to_be_bytes());
+            key.extend_from_slice(&height.to_be_bytes());
+            key.push(0xff);
+            key.extend_from_slice(
+                content_disposition
+                    .as_ref()
+                    .map(|f| f.as_bytes())
+                    .unwrap_or_default(),
+            );
+            key.push(0xff);
+            key.extend_from_slice(
+                content_type
+                    .as_ref()
+                    .map(|c| c.as_bytes())
+                    .unwrap_or_default(),
+            );
+
+            Ok(key)
+        }
+
+        fn search_file_metadata(
+            &self,
+            _mxc: String,
+            _width: u32,
+            _height: u32,
+        ) -> Result<(Option<String>, Option<String>, Vec<u8>)> {
+            todo!()
+        }
+    }
+
+    #[tokio::test]
+    async fn long_file_names_works() {
+        static DB: MockedKVDatabase = MockedKVDatabase;
+        let media = Service { db: &DB };
+
+        let mxc = "mxc://example.com/ascERGshawAWawugaAcauga".to_owned();
+        let width = 100;
+        let height = 100;
+        let content_disposition = "attachment; filename=\"this is a very long file name with spaces and special characters like äöüß and even emoji like 🦀.png\"";
+        let content_type = "image/png";
+        let key = media
+            .db
+            .create_file_metadata(
+                mxc,
+                width,
+                height,
+                Some(content_disposition),
+                Some(content_type),
+            )
+            .unwrap();
+        let mut r = PathBuf::new();
+        r.push("/tmp");
+        r.push("media");
+        // r.push(base64::encode_config(key, base64::URL_SAFE_NO_PAD));
+        // use the sha256 hash of the key as the file name instead of the key itself
+        // this is because the base64 encoded key can be longer than 255 characters.
+        r.push(base64::encode_config(
+            sha2::Sha256::digest(&key),
+            base64::URL_SAFE_NO_PAD,
+        ));
+        // Check that the file path is not longer than 255 characters
+        // (255 is the maximum length of a file path on most file systems)
+        assert!(
+            r.to_str().unwrap().len() <= 255,
+            "File path is too long: {}",
+            r.to_str().unwrap().len()
+        );
+    }
+}