feat: Use random filename suffixes for blobstorage (#4309)

deltachat · Jul 18, 2023 · 20bb6d7 · 20bb6d7
1 parent 6d37e86
commit 20bb6d7
Show file tree

Hide file tree

Showing 9 changed files with 111 additions and 76 deletions.
diff --git a/node/test/test.js b/node/test/test.js
@@ -446,7 +446,8 @@ describe('Offline Tests with unconfigured account', function () {
     context.setChatProfileImage(chatId, imagePath)
     const blobPath = context.getChat(chatId).getProfileImage()
     expect(blobPath.startsWith(blobs)).to.be.true
-    expect(blobPath.endsWith(image)).to.be.true
+    expect(blobPath.includes('image')).to.be.true
+    expect(blobPath.endsWith('.jpeg')).to.be.true
 
     context.setChatProfileImage(chatId, null)
     expect(context.getChat(chatId).getProfileImage()).to.be.equal(

diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py
@@ -161,8 +161,9 @@ def test_send_file_twice_unicode_filename_mangling(tmp_path, acfactory, lp):
     ac1, ac2 = acfactory.get_online_accounts(2)
     chat = acfactory.get_accepted_chat(ac1, ac2)
 
-    basename = "somedäüta.html.zip"
-    p = tmp_path / basename
+    basename = "somedäüta"
+    ext = ".html.zip"
+    p = tmp_path / (basename + ext)
     p.write_text("some data")
 
     def send_and_receive_message():
@@ -180,23 +181,26 @@ def send_and_receive_message():
     msg = send_and_receive_message()
     assert msg.text == "withfile"
     assert open(msg.filename).read() == "some data"
-    assert msg.filename.endswith(basename)
+    msg.filename.index(basename)
+    assert msg.filename.endswith(ext)
 
     msg2 = send_and_receive_message()
     assert msg2.text == "withfile"
     assert open(msg2.filename).read() == "some data"
-    assert msg2.filename.endswith("html.zip")
+    msg2.filename.index(basename)
+    assert msg2.filename.endswith(ext)
     assert msg.filename != msg2.filename
 
 
 def test_send_file_html_attachment(tmp_path, acfactory, lp):
     ac1, ac2 = acfactory.get_online_accounts(2)
     chat = acfactory.get_accepted_chat(ac1, ac2)
 
-    basename = "test.html"
+    basename = "test"
+    ext = ".html"
     content = "<html><body>text</body>data"
 
-    p = tmp_path / basename
+    p = tmp_path / (basename + ext)
     # write wrong html to see if core tries to parse it
     # (it shouldn't as it's a file attachment)
     p.write_text(content)
@@ -210,7 +214,8 @@ def test_send_file_html_attachment(tmp_path, acfactory, lp):
     msg = ac2.get_message_by_id(ev.data2)
 
     assert open(msg.filename).read() == content
-    assert msg.filename.endswith(basename)
+    msg.filename.index(basename)
+    assert msg.filename.endswith(ext)
 
 
 def test_html_message(acfactory, lp):

diff --git a/python/tests/test_2_increation.py b/python/tests/test_2_increation.py
@@ -49,10 +49,9 @@ def test_no_increation_copies_to_blobdir(self, tmp_path, acfactory, lp):
         assert str(tmp_path) != ac1.get_blobdir()
         src = tmp_path / "file.txt"
         src.write_text("hello there\n")
-        chat.send_file(str(src))
-
-        blob_src = os.path.join(ac1.get_blobdir(), "file.txt")
-        assert os.path.exists(blob_src), "file.txt not copied to blobdir"
+        msg = chat.send_file(str(src))
+        assert msg.filename.startswith(os.path.join(ac1.get_blobdir(), "file"))
+        assert msg.filename.endswith(".txt")
 
     def test_forward_increation(self, acfactory, data, lp):
         ac1, ac2 = acfactory.get_online_accounts(2)

diff --git a/src/blob.rs b/src/blob.rs
@@ -11,6 +11,7 @@ use anyhow::{format_err, Context as _, Result};
 use futures::StreamExt;
 use image::{DynamicImage, ImageFormat, ImageOutputFormat};
 use num_traits::FromPrimitive;
+use regex::Regex;
 use tokio::io::AsyncWriteExt;
 use tokio::{fs, io};
 use tokio_stream::wrappers::ReadDirStream;
@@ -71,10 +72,16 @@ impl<'a> BlobObject<'a> {
         stem: &str,
         ext: &str,
     ) -> Result<(String, fs::File)> {
-        const MAX_ATTEMPT: u32 = 16;
+        const MAX_ATTEMPT: u32 = 2;
         let mut attempt = 0;
-        let mut name = format!("{stem}{ext}");
+        let re = Regex::new("-0x[[:xdigit:]]{16}$")?;
+        let stem = if re.is_match(stem) {
+            stem.rsplit_once('-').context("No '-'??")?.0
+        } else {
+            stem
+        };
         loop {
+            let name = format!("{}-0x{:016x}{}", stem, rand::random::<u64>(), ext);
             attempt += 1;
             let path = dir.join(&name);
             match fs::OpenOptions::new()
@@ -89,8 +96,6 @@ impl<'a> BlobObject<'a> {
                         return Err(err).context("failed to create file");
                     } else if attempt == 1 && !dir.exists() {
                         fs::create_dir_all(dir).await.log_err(context).ok();
-                    } else {
-                        name = format!("{}-{}{}", stem, rand::random::<u32>(), ext);
                     }
                 }
             }
@@ -640,32 +645,43 @@ mod tests {
     async fn test_create() {
         let t = TestContext::new().await;
         let blob = BlobObject::create(&t, "foo", b"hello").await.unwrap();
-        let fname = t.get_blobdir().join("foo");
+        let re = Regex::new("^foo-0x[[:xdigit:]]{16}$").unwrap();
+        assert!(re.is_match(blob.as_file_name()));
+        let fname = t.get_blobdir().join(blob.as_file_name());
         let data = fs::read(fname).await.unwrap();
         assert_eq!(data, b"hello");
-        assert_eq!(blob.as_name(), "$BLOBDIR/foo");
-        assert_eq!(blob.to_abs_path(), t.get_blobdir().join("foo"));
+        assert_eq!(
+            blob.as_name(),
+            "$BLOBDIR/".to_string() + blob.as_file_name()
+        );
+        assert_eq!(
+            blob.to_abs_path(),
+            t.get_blobdir().join(blob.as_file_name())
+        );
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_lowercase_ext() {
         let t = TestContext::new().await;
         let blob = BlobObject::create(&t, "foo.TXT", b"hello").await.unwrap();
-        assert_eq!(blob.as_name(), "$BLOBDIR/foo.txt");
+        let re = Regex::new("^\\$BLOBDIR/foo-0x[[:xdigit:]]{16}.txt$").unwrap();
+        assert!(re.is_match(blob.as_name()));
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_as_file_name() {
         let t = TestContext::new().await;
         let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap();
-        assert_eq!(blob.as_file_name(), "foo.txt");
+        let re = Regex::new("^foo-0x[[:xdigit:]]{16}.txt$").unwrap();
+        assert!(re.is_match(blob.as_file_name()));
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_as_rel_path() {
         let t = TestContext::new().await;
         let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap();
-        assert_eq!(blob.as_rel_path(), Path::new("foo.txt"));
+        let re = Regex::new("^foo-0x[[:xdigit:]]{16}.txt$").unwrap();
+        assert!(re.is_match(blob.as_rel_path().to_str().unwrap()));
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -680,30 +696,30 @@ mod tests {
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_create_dup() {
         let t = TestContext::new().await;
-        BlobObject::create(&t, "foo.txt", b"hello").await.unwrap();
-        let foo_path = t.get_blobdir().join("foo.txt");
+        let re = Regex::new("^foo-0x[[:xdigit:]]{16}.txt$").unwrap();
+
+        let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap();
+        assert!(re.is_match(blob.as_rel_path().to_str().unwrap()));
+        let foo_path = t.get_blobdir().join(blob.as_file_name());
         assert!(foo_path.exists());
-        BlobObject::create(&t, "foo.txt", b"world").await.unwrap();
-        let mut dir = fs::read_dir(t.get_blobdir()).await.unwrap();
-        while let Ok(Some(dirent)) = dir.next_entry().await {
-            let fname = dirent.file_name();
-            if fname == foo_path.file_name().unwrap() {
-                assert_eq!(fs::read(&foo_path).await.unwrap(), b"hello");
-            } else {
-                let name = fname.to_str().unwrap();
-                assert!(name.starts_with("foo"));
-                assert!(name.ends_with(".txt"));
-            }
-        }
+
+        let blob = BlobObject::create(&t, "foo.txt", b"world").await.unwrap();
+        assert!(re.is_match(blob.as_rel_path().to_str().unwrap()));
+        let foo_path2 = t.get_blobdir().join(blob.as_file_name());
+        assert!(foo_path2.exists());
+
+        assert!(foo_path != foo_path2);
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_double_ext_preserved() {
         let t = TestContext::new().await;
-        BlobObject::create(&t, "foo.tar.gz", b"hello")
+        let blob = BlobObject::create(&t, "foo.tar.gz", b"hello")
             .await
             .unwrap();
-        let foo_path = t.get_blobdir().join("foo.tar.gz");
+        let re = Regex::new("^foo-0x[[:xdigit:]]{16}.tar.gz$").unwrap();
+        assert!(re.is_match(blob.as_file_name()));
+        let foo_path = t.get_blobdir().join(blob.as_file_name());
         assert!(foo_path.exists());
         BlobObject::create(&t, "foo.tar.gz", b"world")
             .await
@@ -737,7 +753,8 @@ mod tests {
         let src = t.dir.path().join("src");
         fs::write(&src, b"boo").await.unwrap();
         let blob = BlobObject::create_and_copy(&t, src.as_ref()).await.unwrap();
-        assert_eq!(blob.as_name(), "$BLOBDIR/src");
+        let re = Regex::new("^\\$BLOBDIR/src-0x[[:xdigit:]]{16}$").unwrap();
+        assert!(re.is_match(blob.as_name()));
         let data = fs::read(blob.to_abs_path()).await.unwrap();
         assert_eq!(data, b"boo");
 
@@ -758,7 +775,8 @@ mod tests {
         let blob = BlobObject::new_from_path(&t, src_ext.as_ref())
             .await
             .unwrap();
-        assert_eq!(blob.as_name(), "$BLOBDIR/external");
+        let re = Regex::new("^\\$BLOBDIR/external-0x[[:xdigit:]]{16}$").unwrap();
+        assert!(re.is_match(blob.as_name()));
         let data = fs::read(blob.to_abs_path()).await.unwrap();
         assert_eq!(data, b"boo");
 
@@ -769,6 +787,7 @@ mod tests {
         let data = fs::read(blob.to_abs_path()).await.unwrap();
         assert_eq!(data, b"boo");
     }
+
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_create_from_name_long() {
         let t = TestContext::new().await;
@@ -777,10 +796,10 @@ mod tests {
         let blob = BlobObject::new_from_path(&t, src_ext.as_ref())
             .await
             .unwrap();
-        assert_eq!(
-            blob.as_name(),
-            "$BLOBDIR/autocrypt-setup-message-4137848473.html"
-        );
+        let re =
+            Regex::new("^\\$BLOBDIR/autocrypt-setup-message-4137848473-0x[[:xdigit:]]{16}.html$")
+                .unwrap();
+        assert!(re.is_match(blob.as_name()));
     }
 
     #[test]
@@ -838,19 +857,21 @@ mod tests {
         let avatar_src = t.dir.path().join("avatar.jpg");
         let avatar_bytes = include_bytes!("../test-data/image/avatar1000x1000.jpg");
         fs::write(&avatar_src, avatar_bytes).await.unwrap();
-        let avatar_blob = t.get_blobdir().join("avatar.jpg");
-        assert!(!avatar_blob.exists());
         t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap()))
             .await
             .unwrap();
+        let avatar_blob = t.get_config(Config::Selfavatar).await.unwrap().unwrap();
+        let blobdir = t.get_blobdir().to_str().unwrap();
+        assert!(avatar_blob.starts_with(blobdir));
+        let re = Regex::new("avatar-0x[[:xdigit:]]{16}.jpg$").unwrap();
+        assert!(re.is_match(&avatar_blob));
+        let avatar_blob = Path::new(&avatar_blob);
         assert!(avatar_blob.exists());
         assert!(fs::metadata(&avatar_blob).await.unwrap().len() < avatar_bytes.len() as u64);
-        let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap();
-        assert_eq!(avatar_cfg, avatar_blob.to_str().map(|s| s.to_string()));
 
         check_image_size(avatar_src, 1000, 1000);
         check_image_size(
-            &avatar_blob,
+            avatar_blob,
             constants::BALANCED_AVATAR_SIZE,
             constants::BALANCED_AVATAR_SIZE,
         );
@@ -860,12 +881,12 @@ mod tests {
             file.metadata().await.unwrap().len()
         }
 
-        let blob = BlobObject::new_from_path(&t, &avatar_blob).await.unwrap();
+        let blob = BlobObject::new_from_path(&t, avatar_blob).await.unwrap();
         let strict_limits = true;
         blob.recode_to_size(&t, blob.to_abs_path(), 1000, 3000, strict_limits)
             .unwrap();
-        assert!(file_size(&avatar_blob).await <= 3000);
-        assert!(file_size(&avatar_blob).await > 2000);
+        assert!(file_size(avatar_blob).await <= 3000);
+        assert!(file_size(avatar_blob).await > 2000);
         tokio::task::block_in_place(move || {
             let img = image::open(avatar_blob).unwrap();
             assert!(img.width() > 130);
@@ -905,18 +926,19 @@ mod tests {
         let avatar_src = t.dir.path().join("avatar.png");
         let avatar_bytes = include_bytes!("../test-data/image/avatar64x64.png");
         fs::write(&avatar_src, avatar_bytes).await.unwrap();
-        let avatar_blob = t.get_blobdir().join("avatar.png");
-        assert!(!avatar_blob.exists());
         t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap()))
             .await
             .unwrap();
-        assert!(avatar_blob.exists());
+        let avatar_blob = t.get_config(Config::Selfavatar).await.unwrap().unwrap();
+        let blobdir = t.get_blobdir().to_str().unwrap();
+        assert!(avatar_blob.starts_with(blobdir));
+        let re = Regex::new("avatar-0x[[:xdigit:]]{16}.png$").unwrap();
+        assert!(re.is_match(&avatar_blob));
+        assert!(Path::new(&avatar_blob).exists());
         assert_eq!(
             fs::metadata(&avatar_blob).await.unwrap().len(),
             avatar_bytes.len() as u64
         );
-        let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap();
-        assert_eq!(avatar_cfg, avatar_blob.to_str().map(|s| s.to_string()));
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]

diff --git a/src/chat.rs b/src/chat.rs
@@ -3840,6 +3840,7 @@ mod tests {
     use crate::message::delete_msgs;
     use crate::receive_imf::receive_imf;
     use crate::test_utils::{TestContext, TestContextManager};
+    use regex::Regex;
     use tokio::fs;
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -5505,7 +5506,9 @@ mod tests {
         let msg = bob.recv_msg(&sent_msg).await;
         assert_eq!(msg.chat_id, bob_chat.id);
         assert_eq!(msg.get_viewtype(), Viewtype::Sticker);
-        assert_eq!(msg.get_filename(), Some(filename.to_string()));
+        let (filename, ext) = filename.split_once('.').unwrap();
+        let re = Regex::new(&format!("^{filename}-0x[[:xdigit:]]{{16}}.{ext}$")).unwrap();
+        assert!(re.is_match(&msg.get_filename().unwrap()));
         assert_eq!(msg.get_width(), w);
         assert_eq!(msg.get_height(), h);
         assert!(msg.get_filebytes(&bob).await?.unwrap() > 250);
@@ -6235,10 +6238,8 @@ mod tests {
         let msg = bob.recv_msg(&alice.send_msg(chat_id, &mut msg).await).await;
 
         // the file bob receives should not contain BIDI-control characters
-        assert_eq!(
-            Some("$BLOBDIR/harmless_file.txt.exe"),
-            msg.param.get(Param::File),
-        );
+        let re = Regex::new("^\\$BLOBDIR/harmless_file-0x[[:xdigit:]]{16}.txt.exe$").unwrap();
+        assert!(re.is_match(msg.param.get(Param::File).unwrap()));
         Ok(())
     }
 }
diff --git a/src/mimeparser.rs b/src/mimeparser.rs
@@ -2191,6 +2191,7 @@ mod tests {
     #![allow(clippy::indexing_slicing)]
 
     use mailparse::ParsedMail;
+    use regex::Regex;
 
     use super::*;
     use crate::{
@@ -3388,7 +3389,8 @@ On 2020-10-25, Bob wrote:
         assert_eq!(msg.state, MessageState::InFresh);
         assert_eq!(msg.get_filebytes(&t).await.unwrap().unwrap(), 2115);
         assert!(msg.get_file(&t).is_some());
-        assert_eq!(msg.get_filename().unwrap(), "avatar64x64.png");
+        assert!(msg.get_filename().unwrap().starts_with("avatar64x64"));
+        assert!(msg.get_filename().unwrap().ends_with(".png"));
         assert_eq!(msg.get_width(), 64);
         assert_eq!(msg.get_height(), 64);
         assert_eq!(msg.get_filemime().unwrap(), "image/png");
@@ -3710,10 +3712,8 @@ Message.
             mime_message.parts[0].msg,
             "this is a classic email – I attached the .EML file".to_string()
         );
-        assert_eq!(
-            mime_message.parts[0].param.get(Param::File),
-            Some("$BLOBDIR/.eml")
-        );
+        let re = Regex::new("^\\$BLOBDIR/-0x[[:xdigit:]]{16}.eml$").unwrap();
+        assert!(re.is_match(mime_message.parts[0].param.get(Param::File).unwrap()));
 
         assert_eq!(mime_message.parts[0].org_filename, Some(".eml".to_string()));
 

diff --git a/src/param.rs b/src/param.rs
@@ -528,7 +528,7 @@ mod tests {
 
         fs::write(fname, b"boo").await.unwrap();
         let blob = p.get_blob(Param::File, &t, true).await.unwrap().unwrap();
-        assert_eq!(blob, BlobObject::from_name(&t, "foo".to_string()).unwrap());
+        assert!(blob.as_file_name().starts_with("foo"));
 
         // Blob in blobdir, expect blob.
         let bar_path = t.get_blobdir().join("bar");