feat: Remove original file stem from filenames in the blobstorage (#4309)

This way filenames in the blobstorage are just random hex numbers. This also allows us to get rid of
the `sanitize-filename` dependency.

This also requires `Param::Filename` to be set to "debug_logging*.xdc" for messages containing
logging webxdc-s, otherwise they are not detected properly. This is done in "fix:
Message::set_file_from_bytes(): Set Param::Filename", so don't forget to update senders as well.

Cargo.lock

@@ -1144,7 +1144,6 @@ dependencies = [
  "reqwest",
  "rusqlite",
  "rust-hsluv",
- "sanitize-filename",
  "serde",
  "serde_json",
  "sha-1",

Cargo.toml

@@ -82,7 +82,6 @@ regex = "1.9"
 reqwest = { version = "0.11.23", features = ["json"] }
 rusqlite = { version = "0.30", features = ["sqlcipher"] }
 rust-hsluv = "0.1"
-sanitize-filename = "0.5"
 serde_json = "1"
 serde = { version = "1.0", features = ["derive"] }
 sha-1 = "0.10"

deltachat-ffi/deltachat.h

@@ -4050,6 +4050,19 @@ char*           dc_msg_get_subject            (const dc_msg_t* msg);
 char*           dc_msg_get_file               (const dc_msg_t* msg);
 
 
+/**
+ * Save file copy at the user-provided path.
+ *
+ * Fails if file already exists at the provided path.
+ *
+ * @memberof dc_msg_t
+ * @param msg The message object.
+ * @param path Destination file path with filename and extension.
+ * @return 0 on failure, 1 on success.
+ */
+int             dc_msg_save_file              (const dc_msg_t* msg, const char* path);
+
+
 /**
  * Get an original attachment filename, with extension but without the path. To get the full path,
  * use dc_msg_get_file().

deltachat-ffi/src/lib.rs

@@ -3362,6 +3362,34 @@ pub unsafe extern "C" fn dc_msg_get_file(msg: *mut dc_msg_t) -> *mut libc::c_cha
         .unwrap_or_else(|| "".strdup())
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn dc_msg_save_file(
+    msg: *mut dc_msg_t,
+    path: *const libc::c_char,
+) -> libc::c_int {
+    if msg.is_null() || path.is_null() {
+        eprintln!("ignoring careless call to dc_msg_save_file()");
+        return 0;
+    }
+    let ffi_msg = &*msg;
+    let ctx = &*ffi_msg.context;
+    let path = to_string_lossy(path);
+    let r = block_on(
+        ffi_msg
+            .message
+            .save_file(ctx, &std::path::PathBuf::from(path)),
+    );
+    match r {
+        Ok(()) => 1,
+        Err(_) => {
+            r.context("Failed to save file from message")
+                .log_err(ctx)
+                .unwrap_or_default();
+            0
+        }
+    }
+}
+
 #[no_mangle]
 pub unsafe extern "C" fn dc_msg_get_filename(msg: *mut dc_msg_t) -> *mut libc::c_char {
     if msg.is_null() {

deltachat-jsonrpc/src/api.rs

@@ -1,4 +1,5 @@
 use std::collections::BTreeMap;
+use std::path::Path;
 use std::sync::Arc;
 use std::{collections::HashMap, str::FromStr};
 
@@ -1927,19 +1928,21 @@ impl CommandApi {
         );
         let destination_path = account_folder.join("stickers").join(collection);
         fs::create_dir_all(&destination_path).await?;
-        let file = message.get_file(&ctx).context("no file")?;
-        fs::copy(
-            &file,
-            destination_path.join(format!(
-                "{}.{}",
-                msg_id,
-                file.extension()
-                    .unwrap_or_default()
-                    .to_str()
-                    .unwrap_or_default()
-            )),
-        )
-        .await?;
+        let file = message.get_filename().context("no file?")?;
+        message
+            .save_file(
+                &ctx,
+                &destination_path.join(format!(
+                    "{}.{}",
+                    msg_id,
+                    Path::new(&file)
+                        .extension()
+                        .unwrap_or_default()
+                        .to_str()
+                        .unwrap_or_default()
+                )),
+            )
+            .await?;
         Ok(())
     }
 

node/test/test.mjs

@@ -441,7 +441,6 @@ 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.includes('image')).to.be.true
     expect(blobPath.endsWith('.jpeg')).to.be.true
 
     context.setChatProfileImage(chatId, null)

python/src/deltachat/message.py

@@ -107,8 +107,8 @@ class Message:
         lib.dc_msg_set_html(self._dc_msg, as_dc_charpointer(html_text))
 
     @props.with_doc
-    def filename(self):
-        """filename if there was an attachment, otherwise empty string."""
+    def file_path(self):
+        """file path if there was an attachment, otherwise empty string."""
         return from_dc_charpointer(lib.dc_msg_get_file(self._dc_msg))
 
     def set_file(self, path, mime_type=None):
@@ -119,9 +119,8 @@ class Message:
         lib.dc_msg_set_file(self._dc_msg, as_dc_charpointer(path), mtype)
 
     @props.with_doc
-    def basename(self) -> str:
-        """basename of the attachment if it exists, otherwise empty string."""
-        # FIXME, it does not return basename
+    def filename(self) -> str:
+        """filename of the attachment if any, otherwise empty string."""
         return from_dc_charpointer(lib.dc_msg_get_filename(self._dc_msg))
 
     @props.with_doc

python/tests/test_1_online.py

@@ -181,16 +181,14 @@ def test_send_file_twice_unicode_filename_mangling(tmp_path, acfactory, lp):
 
     msg = send_and_receive_message()
     assert msg.text == "withfile"
-    assert open(msg.filename).read() == "some data"
-    msg.filename.index(basename)
-    assert msg.filename.endswith(ext)
+    assert open(msg.file_path).read() == "some data"
+    assert msg.file_path.endswith(ext)
 
     msg2 = send_and_receive_message()
     assert msg2.text == "withfile"
-    assert open(msg2.filename).read() == "some data"
-    msg2.filename.index(basename)
-    assert msg2.filename.endswith(ext)
-    assert msg.filename != msg2.filename
+    assert open(msg2.file_path).read() == "some data"
+    assert msg2.file_path.endswith(ext)
+    assert msg.file_path != msg2.file_path
 
 
 def test_send_file_html_attachment(tmp_path, acfactory, lp):
@@ -214,9 +212,8 @@ def test_send_file_html_attachment(tmp_path, acfactory, lp):
     assert ev.data2 > dc.const.DC_CHAT_ID_LAST_SPECIAL
     msg = ac2.get_message_by_id(ev.data2)
 
-    assert open(msg.filename).read() == content
-    msg.filename.index(basename)
-    assert msg.filename.endswith(ext)
+    assert open(msg.file_path).read() == content
+    assert msg.file_path.endswith(ext)
 
 
 def test_html_message(acfactory, lp):
@@ -310,7 +307,7 @@ def test_webxdc_message(acfactory, data, lp):
     msg1.set_file(data.get_path("webxdc/minimal.xdc"))
     msg1 = chat.send_msg(msg1)
     assert msg1.is_webxdc()
-    assert msg1.filename
+    assert msg1.file_path
 
     assert msg1.send_status_update({"payload": "test1"}, "some test data")
     assert msg1.send_status_update({"payload": "test2"}, "more test data")
@@ -323,7 +320,7 @@ def test_webxdc_message(acfactory, data, lp):
     msg2 = ac2._evtracker.wait_next_incoming_message()
     assert msg2.text == "message1"
     assert msg2.is_webxdc()
-    assert msg2.filename
+    assert msg2.file_path
     ac2._evtracker.get_info_contains("Marked messages [0-9]+ in folder INBOX as seen.")
     ac2.direct_imap.select_folder("Inbox")
     assert len(list(ac2.direct_imap.conn.fetch(AND(seen=True)))) == 1
@@ -338,7 +335,7 @@ def test_webxdc_huge_update(acfactory, data, lp):
     msg1.set_file(data.get_path("webxdc/minimal.xdc"))
     msg1 = chat.send_msg(msg1)
     assert msg1.is_webxdc()
-    assert msg1.filename
+    assert msg1.file_path
 
     msg2 = ac2._evtracker.wait_next_incoming_message()
     assert msg2.is_webxdc()
@@ -360,7 +357,7 @@ def test_webxdc_download_on_demand(acfactory, data, lp):
     msg1.set_file(data.get_path("webxdc/minimal.xdc"))
     msg1 = chat.send_msg(msg1)
     assert msg1.is_webxdc()
-    assert msg1.filename
+    assert msg1.file_path
 
     msg2 = ac2._evtracker.wait_next_incoming_message()
     assert msg2.is_webxdc()
@@ -1350,7 +1347,7 @@ def test_quote_attachment(tmp_path, acfactory, lp):
     received_reply = ac1._evtracker.wait_next_incoming_message()
     assert received_reply.text == "message reply"
     assert received_reply.quoted_text == received_message.text
-    assert open(received_reply.filename).read() == "data to send"
+    assert open(received_reply.file_path).read() == "data to send"
 
 
 def test_saved_mime_on_received_message(acfactory, lp):
@@ -1443,8 +1440,8 @@ def test_send_and_receive_image(acfactory, lp, data):
     assert ev.data2 == msg_out.id
     msg_in = ac2.get_message_by_id(msg_out.id)
     assert msg_in.is_image()
-    assert os.path.exists(msg_in.filename)
-    assert os.stat(msg_in.filename).st_size == os.stat(path).st_size
+    assert os.path.exists(msg_in.file_path)
+    assert os.stat(msg_in.file_path).st_size == os.stat(path).st_size
     m = message_queue.get()
     assert m == msg_in
 
@@ -1577,7 +1574,7 @@ def test_import_export_online_all(acfactory, tmp_path, data, lp):
         assert len(messages) == 3
         assert messages[0].text == "msg1"
         assert messages[1].filemime == "image/png"
-        assert os.stat(messages[1].filename).st_size == os.stat(original_image_path).st_size
+        assert os.stat(messages[1].file_path).st_size == os.stat(original_image_path).st_size
         ac.set_config("displayname", "new displayname")
         assert ac.get_config("displayname") == "new displayname"
 
@@ -1650,7 +1647,7 @@ def test_ac_setup_message(acfactory, lp):
     with pytest.raises(ValueError):
         msg.continue_key_transfer(str(reversed(setup_code)))
     lp.sec("try a good setup code")
-    print("*************** Incoming ASM File at: ", msg.filename)
+    print("*************** Incoming ASM File at: ", msg.file_path)
     print("*************** Setup Code: ", setup_code)
     msg.continue_key_transfer(setup_code)
     assert ac1.get_info()["fingerprint"] == ac2.get_info()["fingerprint"]

python/tests/test_2_increation.py

@@ -50,8 +50,7 @@ class TestOnlineInCreation:
         src = tmp_path / "file.txt"
         src.write_text("hello there\n")
         msg = chat.send_file(str(src))
-        assert msg.filename.startswith(os.path.join(ac1.get_blobdir(), "file"))
-        assert msg.filename.endswith(".txt")
+        assert msg.file_path.endswith(".txt")
 
     def test_forward_increation(self, acfactory, data, lp):
         ac1, ac2 = acfactory.get_online_accounts(2)
@@ -99,9 +98,9 @@ class TestOnlineInCreation:
 
         lp.sec("wait1 for original or forwarded messages to arrive")
         received_original = ac2._evtracker.wait_next_incoming_message()
-        assert cmp(received_original.filename, orig, shallow=False)
+        assert cmp(received_original.file_path, orig, shallow=False)
 
         lp.sec("wait2 for original or forwarded messages to arrive")
         received_copy = ac2._evtracker.wait_next_incoming_message()
         assert received_copy.id != received_original.id
-        assert cmp(received_copy.filename, orig, shallow=False)
+        assert cmp(received_copy.file_path, orig, shallow=False)

python/tests/test_3_offline.py

@@ -440,31 +440,34 @@ class TestOfflineChat:
         assert msg.is_image()
         assert msg
         assert msg.id > 0
-        assert os.path.exists(msg.filename)
+        assert os.path.exists(msg.file_path)
         assert msg.filemime == "image/png"
 
     @pytest.mark.parametrize(
-        ("fn", "typein", "typeout"),
+        ("stem", "ext", "typein", "typeout"),
         [
-            ("r", None, "application/octet-stream"),
-            ("r.txt", None, "text/plain"),
-            ("r.txt", "text/plain", "text/plain"),
-            ("r.txt", "image/png", "image/png"),
+            ("r", "", None, "application/octet-stream"),
+            ("r", ".txt", None, "text/plain"),
+            ("r", ".txt", "text/plain", "text/plain"),
+            ("r", ".txt", "image/png", "image/png"),
         ],
     )
-    def test_message_file(self, chat1, data, lp, fn, typein, typeout):
+    def test_message_file(self, chat1, data, lp, stem, ext, typein, typeout):
         lp.sec("sending file")
+        fn = stem + ext
         fp = data.get_path(fn)
         msg = chat1.send_file(fp, typein)
         assert msg
         assert msg.id > 0
         assert msg.is_file()
-        assert os.path.exists(msg.filename)
-        assert msg.filename.endswith(msg.basename)
+        assert os.path.exists(msg.file_path)
+        assert msg.file_path.endswith(ext)
+        assert msg.filename == fn
         assert msg.filemime == typeout
         msg2 = chat1.send_file(fp, typein)
         assert msg2 != msg
-        assert msg2.filename != msg.filename
+        assert msg2.file_path != msg.file_path
+        assert msg2.filename == fn
 
     def test_create_contact(self, acfactory):
         ac1 = acfactory.get_pseudo_configured_account()
@@ -532,7 +535,7 @@ class TestOfflineChat:
         messages = chat2.get_messages()
         assert len(messages) == 2
         assert messages[0].text == "msg1"
-        assert os.path.exists(messages[1].filename)
+        assert os.path.exists(messages[1].file_path)
 
     def test_import_export_on_encrypted_acct(self, acfactory, tmp_path):
         passphrase1 = "passphrase1"
@@ -570,7 +573,7 @@ class TestOfflineChat:
         messages = chat2.get_messages()
         assert len(messages) == 2
         assert messages[0].text == "msg1"
-        assert os.path.exists(messages[1].filename)
+        assert os.path.exists(messages[1].file_path)
 
         ac2.shutdown()
 
@@ -587,7 +590,7 @@ class TestOfflineChat:
         messages = chat2.get_messages()
         assert len(messages) == 2
         assert messages[0].text == "msg1"
-        assert os.path.exists(messages[1].filename)
+        assert os.path.exists(messages[1].file_path)
 
     def test_import_export_with_passphrase(self, acfactory, tmp_path):
         passphrase = "test_passphrase"
@@ -626,7 +629,7 @@ class TestOfflineChat:
         messages = chat2.get_messages()
         assert len(messages) == 2
         assert messages[0].text == "msg1"
-        assert os.path.exists(messages[1].filename)
+        assert os.path.exists(messages[1].file_path)
 
     def test_import_encrypted_bak_into_encrypted_acct(self, acfactory, tmp_path):
         """
@@ -671,7 +674,7 @@ class TestOfflineChat:
         messages = chat2.get_messages()
         assert len(messages) == 2
         assert messages[0].text == "msg1"
-        assert os.path.exists(messages[1].filename)
+        assert os.path.exists(messages[1].file_path)
 
         ac2.shutdown()
 
@@ -688,7 +691,7 @@ class TestOfflineChat:
         messages = chat2.get_messages()
         assert len(messages) == 2
         assert messages[0].text == "msg1"
-        assert os.path.exists(messages[1].filename)
+        assert os.path.exists(messages[1].file_path)
 
     def test_set_get_draft(self, chat1):
         msg = Message.new_empty(chat1.account, "text")

src/blob.rs

@@ -16,7 +16,7 @@ use tokio::{fs, io};
 use tokio_stream::wrappers::ReadDirStream;
 
 use crate::config::Config;
-use crate::constants::{self, MediaQuality};
+use crate::constants::{self, MediaQuality, BLOB_CREATE_ATTEMPTS};
 use crate::context::Context;
 use crate::events::EventType;
 use crate::log::LogExt;
@@ -47,8 +47,8 @@ impl<'a> BlobObject<'a> {
         data: &[u8],
     ) -> Result<BlobObject<'a>> {
         let blobdir = context.get_blobdir();
-        let (stem, ext) = BlobObject::sanitise_name(suggested_name);
-        let (name, mut file) = BlobObject::create_new_file(context, blobdir, &stem, &ext).await?;
+        let ext = BlobObject::get_extension(suggested_name);
+        let (name, mut file) = BlobObject::create_new_file(context, blobdir, &ext).await?;
         file.write_all(data).await.context("file write failure")?;
 
         // workaround a bug in async-std
@@ -68,13 +68,11 @@ impl<'a> BlobObject<'a> {
     async fn create_new_file(
         context: &Context,
         dir: &Path,
-        stem: &str,
         ext: &str,
     ) -> Result<(String, fs::File)> {
-        const MAX_ATTEMPT: u32 = 16;
         let mut attempt = 0;
-        let mut name = format!("{stem}{ext}");
         loop {
+            let name = format!("{:016x}{}", rand::random::<u64>(), ext);
             attempt += 1;
             let path = dir.join(&name);
             match fs::OpenOptions::new()
@@ -85,12 +83,10 @@ impl<'a> BlobObject<'a> {
             {
                 Ok(file) => return Ok((name, file)),
                 Err(err) => {
-                    if attempt >= MAX_ATTEMPT {
+                    if attempt >= BLOB_CREATE_ATTEMPTS {
                         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);
                     }
                 }
             }
@@ -107,9 +103,9 @@ impl<'a> BlobObject<'a> {
         let mut src_file = fs::File::open(src)
             .await
             .with_context(|| format!("failed to open file {}", src.display()))?;
-        let (stem, ext) = BlobObject::sanitise_name(&src.to_string_lossy());
+        let ext = BlobObject::get_extension(&src.to_string_lossy());
         let (name, mut dst_file) =
-            BlobObject::create_new_file(context, context.get_blobdir(), &stem, &ext).await?;
+            BlobObject::create_new_file(context, context.get_blobdir(), &ext).await?;
         let name_for_err = name.clone();
         if let Err(err) = io::copy(&mut src_file, &mut dst_file).await {
             // Attempt to remove the failed file, swallow errors resulting from that.
@@ -133,10 +129,8 @@ impl<'a> BlobObject<'a> {
     ///
     /// If the source file is not a path to into the blob directory
     /// the file will be copied into the blob directory first.  If the
-    /// source file is already in the blobdir it will not be copied
-    /// and only be created if it is a valid blobname, that is no
-    /// subdirectory is used and [BlobObject::sanitise_name] does not
-    /// modify the filename.
+    /// source file is already in the blobdir (but not in a subdirectory)
+    /// it will not be copied and only be created if it is a valid blobname.
     ///
     /// Paths into the blob directory may be either defined by an absolute path
     /// or by the relative prefix `$BLOBDIR`.
@@ -153,8 +147,7 @@ impl<'a> BlobObject<'a> {
     /// Returns a [BlobObject] for an existing blob from a path.
     ///
     /// The path must designate a file directly in the blobdir and
-    /// must use a valid blob name.  That is after sanitisation the
-    /// name must still be the same, that means it must be valid UTF-8
+    /// must use a valid blob name. That means it must be valid UTF-8
     /// and not have any special characters in it.
     pub fn from_path(context: &'a Context, path: &Path) -> Result<BlobObject<'a>> {
         let rel_path = path
@@ -228,21 +221,8 @@ impl<'a> BlobObject<'a> {
         }
     }
 
-    /// Create a safe name based on a messy input string.
-    ///
-    /// The safe name will be a valid filename on Unix and Windows and
-    /// not contain any path separators.  The input can contain path
-    /// segments separated by either Unix or Windows path separators,
-    /// the rightmost non-empty segment will be used as name,
-    /// sanitised for special characters.
-    ///
-    /// The resulting name is returned as a tuple, the first part
-    /// being the stem or basename and the second being an extension,
-    /// including the dot.  E.g. "foo.txt" is returned as `("foo",
-    /// ".txt")` while "bar" is returned as `("bar", "")`.
-    ///
-    /// The extension part will always be lowercased.
-    fn sanitise_name(name: &str) -> (String, String) {
+    /// Get a file extension if any, including the dot, in lower case, otherwise an empty string.
+    fn get_extension(name: &str) -> String {
         let mut name = name.to_string();
         for part in name.rsplit('/') {
             if !part.is_empty() {
@@ -256,20 +236,12 @@ impl<'a> BlobObject<'a> {
                 break;
             }
         }
-        let opts = sanitize_filename::Options {
-            truncate: true,
-            windows: true,
-            replacement: "",
-        };
 
-        let clean = sanitize_filename::sanitize_with_options(name, opts);
         // Let's take the tricky filename
         // "file.with_lots_of_characters_behind_point_and_double_ending.tar.gz" as an example.
         // Split it into "file" and "with_lots_of_characters_behind_point_and_double_ending.tar.gz":
-        let mut iter = clean.splitn(2, '.');
-
-        let stem: String = iter.next().unwrap_or_default().chars().take(64).collect();
-        // stem == "file"
+        let mut iter = name.splitn(2, '.');
+        iter.next();
 
         let ext_chars = iter.next().unwrap_or_default().chars();
         let ext: String = ext_chars
@@ -282,11 +254,10 @@ impl<'a> BlobObject<'a> {
         // ext == "d_point_and_double_ending.tar.gz"
 
         if ext.is_empty() {
-            (stem, "".to_string())
+            ext
         } else {
-            (stem, format!(".{ext}").to_lowercase())
-            // Return ("file", ".d_point_and_double_ending.tar.gz")
-            // which is not perfect but acceptable.
+            format!(".{ext}").to_lowercase()
+            // Return ".d_point_and_double_ending.tar.gz", which is not perfect but acceptable.
         }
     }
 
@@ -674,6 +645,7 @@ mod tests {
     use anyhow::Result;
     use fs::File;
     use image::{GenericImageView, Pixel};
+    use regex::Regex;
 
     use super::*;
     use crate::chat::{self, create_group_chat, ProtectionStatus};
@@ -693,32 +665,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("^[[: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/[[: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("^[[: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("^[[:xdigit:]]{16}.txt$").unwrap();
+        assert!(re.is_match(blob.as_rel_path().to_str().unwrap()));
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -733,30 +716,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("^[[: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("^[[: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
@@ -769,7 +752,6 @@ mod tests {
             } else {
                 let name = fname.to_str().unwrap();
                 println!("{name}");
-                assert!(name.starts_with("foo"));
                 assert!(name.ends_with(".tar.gz"));
             }
         }
@@ -790,7 +772,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/[[: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");
 
@@ -811,7 +794,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/[[: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");
 
@@ -822,19 +806,6 @@ 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;
-        let src_ext = t.dir.path().join("autocrypt-setup-message-4137848473.html");
-        fs::write(&src_ext, b"boo").await.unwrap();
-        let blob = BlobObject::new_from_path(&t, src_ext.as_ref())
-            .await
-            .unwrap();
-        assert_eq!(
-            blob.as_name(),
-            "$BLOBDIR/autocrypt-setup-message-4137848473.html"
-        );
-    }
 
     #[test]
     fn test_is_blob_name() {
@@ -847,42 +818,24 @@ mod tests {
     }
 
     #[test]
-    fn test_sanitise_name() {
-        let (stem, ext) =
-            BlobObject::sanitise_name("Я ЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯ.txt");
+    fn test_get_extension() {
+        let ext = BlobObject::get_extension("Я ЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯ.txt");
         assert_eq!(ext, ".txt");
-        assert!(!stem.is_empty());
 
-        // the extensions are kept together as between stem and extension a number may be added -
-        // and `foo.tar.gz` should become `foo-1234.tar.gz` and not `foo.tar-1234.gz`
-        let (stem, ext) = BlobObject::sanitise_name("wot.tar.gz");
-        assert_eq!(stem, "wot");
+        let ext = BlobObject::get_extension("wot.tar.gz");
         assert_eq!(ext, ".tar.gz");
 
-        let (stem, ext) = BlobObject::sanitise_name(".foo.bar");
-        assert_eq!(stem, "");
+        let ext = BlobObject::get_extension(".foo.bar");
         assert_eq!(ext, ".foo.bar");
 
-        let (stem, ext) = BlobObject::sanitise_name("foo?.bar");
-        assert!(stem.contains("foo"));
-        assert!(!stem.contains('?'));
+        let ext = BlobObject::get_extension("foo?.bar");
         assert_eq!(ext, ".bar");
 
-        let (stem, ext) = BlobObject::sanitise_name("no-extension");
-        assert_eq!(stem, "no-extension");
+        let ext = BlobObject::get_extension("no-extension");
         assert_eq!(ext, "");
 
-        let (stem, ext) = BlobObject::sanitise_name("path/ignored\\this: is* forbidden?.c");
+        let ext = BlobObject::get_extension("path/ignored\\this: is* forbidden?.c");
         assert_eq!(ext, ".c");
-        assert!(!stem.contains("path"));
-        assert!(!stem.contains("ignored"));
-        assert!(stem.contains("this"));
-        assert!(stem.contains("forbidden"));
-        assert!(!stem.contains('/'));
-        assert!(!stem.contains('\\'));
-        assert!(!stem.contains(':'));
-        assert!(!stem.contains('*'));
-        assert!(!stem.contains('?'));
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -891,19 +844,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("[[: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,
         );
@@ -913,7 +868,7 @@ mod tests {
             file.metadata().await.unwrap().len()
         }
 
-        let mut blob = BlobObject::new_from_path(&t, &avatar_blob).await.unwrap();
+        let mut blob = BlobObject::new_from_path(&t, avatar_blob).await.unwrap();
         let maybe_sticker = &mut false;
         let strict_limits = true;
         blob.recode_to_size(
@@ -925,8 +880,8 @@ mod tests {
             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);
@@ -966,18 +921,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("[[: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)]
@@ -1182,23 +1138,26 @@ mod tests {
         let alice_msg = alice.get_last_msg().await;
         assert_eq!(alice_msg.get_width() as u32, compressed_width);
         assert_eq!(alice_msg.get_height() as u32, compressed_height);
-        check_image_size(
-            alice_msg.get_file(&alice).unwrap(),
-            compressed_width,
-            compressed_height,
-        );
+        let file_saved = alice
+            .get_blobdir()
+            .join("saved-".to_string() + &alice_msg.get_filename().unwrap());
+        alice_msg.save_file(&alice, &file_saved).await?;
+        check_image_size(file_saved, compressed_width, compressed_height);
 
         let bob_msg = bob.recv_msg(&sent).await;
         assert_eq!(bob_msg.get_viewtype(), Viewtype::Image);
         assert_eq!(bob_msg.get_width() as u32, compressed_width);
         assert_eq!(bob_msg.get_height() as u32, compressed_height);
-        let file = bob_msg.get_file(&bob).unwrap();
+        let file_saved = bob
+            .get_blobdir()
+            .join("saved-".to_string() + &bob_msg.get_filename().unwrap());
+        bob_msg.save_file(&bob, &file_saved).await?;
 
-        let blob = BlobObject::new_from_path(&bob, &file).await?;
+        let blob = BlobObject::new_from_path(&bob, &file_saved).await?;
         let (_, exif) = blob.metadata()?;
         assert!(exif.is_none());
 
-        let img = check_image_size(file, compressed_width, compressed_height);
+        let img = check_image_size(file_saved, compressed_width, compressed_height);
         Ok(img)
     }
 

src/chat.rs

@@ -4473,6 +4473,7 @@ mod tests {
     use crate::message::delete_msgs;
     use crate::receive_imf::receive_imf;
     use crate::test_utils::{sync, TestContext, TestContextManager};
+    use regex::Regex;
     use strum::IntoEnumIterator;
     use tokio::fs;
 
@@ -7019,9 +7020,11 @@ mod tests {
 
         // the file bob receives should not contain BIDI-control characters
         assert_eq!(
-            Some("$BLOBDIR/harmless_file.txt.exe"),
-            msg.param.get(Param::File),
+            msg.param.get(Param::Filename).unwrap(),
+            "harmless_file.txt.exe"
         );
+        let re = Regex::new("^\\$BLOBDIR/[[:xdigit:]]{16}.txt.exe$").unwrap();
+        assert!(re.is_match(msg.param.get(Param::File).unwrap()));
         Ok(())
     }
 

src/constants.rs

@@ -209,6 +209,9 @@ pub const WORSE_IMAGE_SIZE: u32 = 640;
 // this value can be increased if the folder configuration is changed and must be redone on next program start
 pub(crate) const DC_FOLDERS_CONFIGURED_VERSION: i32 = 4;
 
+// Maximum attemps to create a blob file.
+pub(crate) const BLOB_CREATE_ATTEMPTS: u32 = 2;
+
 // If more recipients are needed in SMTP's `RCPT TO:` header, the recipient list is split into
 // chunks. This does not affect MIME's `To:` header. Can be overwritten by setting
 // `max_smtp_rcpt_to` in the provider db.

src/debug_logging.rs

@@ -9,7 +9,6 @@ use crate::tools::time;
 use crate::webxdc::StatusUpdateItem;
 use async_channel::{self as channel, Receiver, Sender};
 use serde_json::json;
-use std::path::PathBuf;
 use tokio::task;
 
 #[derive(Debug)]
@@ -97,7 +96,7 @@ pub async fn maybe_set_logging_xdc(
         context,
         msg.get_viewtype(),
         chat_id,
-        msg.param.get_path(Param::File, context).unwrap_or_default(),
+        msg.param.get(Param::Filename),
         msg.get_id(),
     )
     .await?;
@@ -110,18 +109,16 @@ pub async fn maybe_set_logging_xdc_inner(
     context: &Context,
     viewtype: Viewtype,
     chat_id: ChatId,
-    file: Option<PathBuf>,
+    file_name: Option<&str>,
     msg_id: MsgId,
 ) -> anyhow::Result<()> {
     if viewtype == Viewtype::Webxdc {
-        if let Some(file) = file {
-            if let Some(file_name) = file.file_name().and_then(|name| name.to_str()) {
-                if file_name.starts_with("debug_logging")
-                    && file_name.ends_with(".xdc")
-                    && chat_id.is_self_talk(context).await?
-                {
-                    set_debug_logging_xdc(context, Some(msg_id)).await?;
-                }
+        if let Some(file_name) = file_name {
+            if file_name.starts_with("debug_logging")
+                && file_name.ends_with(".xdc")
+                && chat_id.is_self_talk(context).await?
+            {
+                set_debug_logging_xdc(context, Some(msg_id)).await?;
             }
         }
     }

src/imex/transfer.rs

@@ -656,6 +656,12 @@ mod tests {
         let text = fs::read_to_string(&path).await.unwrap();
         assert_eq!(text, "i am attachment");
 
+        let path = path.with_file_name("saved.txt");
+        msg.save_file(&ctx1, &path).await.unwrap();
+        let text = fs::read_to_string(&path).await.unwrap();
+        assert_eq!(text, "i am attachment");
+        assert!(msg.save_file(&ctx1, &path).await.is_err());
+
         // Check that both received the ImexProgress events.
         ctx0.evtracker
             .get_matching(|ev| matches!(ev, EventType::ImexProgress(1000)))

src/message.rs

@@ -6,6 +6,7 @@ use std::path::{Path, PathBuf};
 use anyhow::{ensure, format_err, Context as _, Result};
 use deltachat_derive::{FromSql, ToSql};
 use serde::{Deserialize, Serialize};
+use tokio::{fs, io};
 
 use crate::blob::BlobObject;
 use crate::chat::{Chat, ChatId};
@@ -579,6 +580,19 @@ impl Message {
         self.param.get_path(Param::File, context).unwrap_or(None)
     }
 
+    /// Save file copy at the user-provided path.
+    pub async fn save_file(&self, context: &Context, path: &Path) -> Result<()> {
+        let path_src = self.get_file(context).context("No file")?;
+        let mut src = fs::OpenOptions::new().read(true).open(path_src).await?;
+        let mut dst = fs::OpenOptions::new()
+            .write(true)
+            .create_new(true)
+            .open(path)
+            .await?;
+        io::copy(&mut src, &mut dst).await?;
+        Ok(())
+    }
+
     /// If message is an image or gif, set Param::Width and Param::Height
     pub(crate) async fn try_calc_and_set_dimensions(&mut self, context: &Context) -> Result<()> {
         if self.viewtype.has_file() {
@@ -1017,6 +1031,7 @@ impl Message {
         filemime: Option<&str>,
     ) -> Result<()> {
         let blob = BlobObject::create(context, suggested_name, data).await?;
+        self.param.set(Param::Filename, suggested_name);
         self.param.set(Param::File, blob.as_name());
         self.param.set_optional(Param::MimeType, filemime);
         Ok(())

src/mimeparser.rs

@@ -2221,6 +2221,7 @@ mod tests {
     #![allow(clippy::indexing_slicing)]
 
     use mailparse::ParsedMail;
+    use regex::Regex;
 
     use super::*;
     use crate::{
@@ -3740,10 +3741,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/[[: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()));
 

src/param.rs

@@ -543,8 +543,6 @@ mod tests {
         assert!(p.get_blob(Param::File, &t, false).await.is_err());
 
         fs::write(fname, b"boo").await.unwrap();
-        let blob = p.get_blob(Param::File, &t, true).await.unwrap().unwrap();
-        assert!(blob.as_file_name().starts_with("foo"));
 
         // Blob in blobdir, expect blob.
         let bar_path = t.get_blobdir().join("bar");

src/receive_imf.rs

@@ -1462,9 +1462,7 @@ RETURNING id
             context,
             part.typ,
             chat_id,
-            part.param
-                .get_path(Param::File, context)
-                .unwrap_or_default(),
+            part.param.get(Param::Filename),
             *msg_id,
         )
         .await?;

src/receive_imf/tests.rs

@@ -1550,7 +1550,6 @@ async fn test_pdf_filename_simple() {
     assert_eq!(msg.viewtype, Viewtype::File);
     assert_eq!(msg.text, "mail body");
     let file_path = msg.param.get(Param::File).unwrap();
-    assert!(file_path.starts_with("$BLOBDIR/simple"));
     assert!(file_path.ends_with(".pdf"));
 }
 
@@ -1566,7 +1565,6 @@ async fn test_pdf_filename_continuation() {
     assert_eq!(msg.viewtype, Viewtype::File);
     assert_eq!(msg.text, "mail body");
     let file_path = msg.param.get(Param::File).unwrap();
-    assert!(file_path.starts_with("$BLOBDIR/test pdf äöüß"));
     assert!(file_path.ends_with(".pdf"));
 }
 
@@ -2921,11 +2919,15 @@ async fn test_long_and_duplicated_filenames() -> Result<()> {
             let resulting_filename = msg.get_filename().unwrap();
             assert_eq!(resulting_filename, filename);
             let path = msg.get_file(t).unwrap();
+            let path2 = path.with_file_name("saved.txt");
+            msg.save_file(t, &path2).await.unwrap();
             assert!(
                 path.to_str().unwrap().ends_with(".tar.gz"),
                 "path {path:?} doesn't end with .tar.gz"
             );
-            assert_eq!(fs::read_to_string(path).await.unwrap(), content);
+            assert_eq!(fs::read_to_string(&path).await.unwrap(), content);
+            assert_eq!(fs::read_to_string(&path2).await.unwrap(), content);
+            fs::remove_file(path2).await.unwrap();
         }
         check_message(&msg_alice, &alice, filename_sent, &content).await;
         check_message(&msg_bob, &bob, filename_sent, &content).await;