feat: Make dc_msg_get_filename() return the original attachment filename (#4309)

It can be used e.g. as a default in the file saving dialog. Also display the original filename in
the message info. For these purposes add Param::Filename in addition to Param::File and use it as an
attachment filename in sent emails.

deltachat-ffi/deltachat.h

@@ -3978,16 +3978,17 @@ char*           dc_msg_get_text               (const dc_msg_t* msg);
  */
 char*           dc_msg_get_subject            (const dc_msg_t* msg);
 
+
 /**
- * Find out full path, file name and extension of the file associated with a
+ * Find out full path of the file associated with a message.
- * message.
  *
  * Typically files are associated with images, videos, audios, documents.
  * Plain text messages do not have a file.
+ * File name may be mangled. To obtain the original attachment filename use dc_msg_get_filename().
  *
  * @memberof dc_msg_t
  * @param msg The message object.
- * @return The full path, the file name, and the extension of the file associated with the message.
+ * @return The full path (with file name and extension) of the file associated with the message.
  *     If there is no file associated with the message, an empty string is returned.
  *     NULL is never returned and the returned value must be released using dc_str_unref().
  */
@@ -3995,14 +3996,13 @@ char*           dc_msg_get_file               (const dc_msg_t* msg);
 
 
 /**
- * Get a base file name without the path. The base file name includes the extension; the path
+ * Get an original attachment filename, with extension but without the path. To get the full path,
- * is not returned. To get the full path, use dc_msg_get_file().
+ * use dc_msg_get_file().
  *
  * @memberof dc_msg_t
  * @param msg The message object.
- * @return The base file name plus the extension without part. If there is no file
+ * @return The attachment filename. If there is no file associated with the message, an empty string
- *     associated with the message, an empty string is returned. The returned
+ *     is returned. The returned value must be released using dc_str_unref().
- *     value must be released using dc_str_unref().
  */
 char*           dc_msg_get_filename           (const dc_msg_t* msg);
 

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(

python/tests/test_1_online.py

@@ -162,8 +162,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"
+    basename = "somedäüta"
-    p = tmp_path / basename
+    ext = ".html.zip"
+    p = tmp_path / (basename + ext)
     p.write_text("some data")
 
     def send_and_receive_message():
@@ -181,12 +182,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"
-    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
 
 
@@ -194,10 +197,11 @@ 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)
@@ -211,7 +215,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):

python/tests/test_2_increation.py

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

src/chat.rs

@@ -2042,6 +2042,14 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
             }
         }
         msg.param.set(Param::File, blob.as_name());
+        if let (Some(filename), Some(blob_ext)) = (msg.param.get(Param::Filename), blob.suffix()) {
+            let stem = match filename.rsplit_once('.') {
+                Some((stem, _)) => stem,
+                None => filename,
+            };
+            msg.param
+                .set(Param::Filename, stem.to_string() + "." + blob_ext);
+        }
 
         if msg.viewtype == Viewtype::File || msg.viewtype == Viewtype::Image {
             // Correct the type, take care not to correct already very special
@@ -5521,7 +5529,7 @@ 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()));
+        assert_eq!(msg.get_filename().unwrap(), filename);
         assert_eq!(msg.get_width(), w);
         assert_eq!(msg.get_height(), h);
         assert!(msg.get_filebytes(&bob).await?.unwrap() > 250);

src/imex/transfer.rs

@@ -650,7 +650,9 @@ mod tests {
             _ => panic!("wrong chat item"),
         };
         let msg = Message::load_from_db(&ctx1, *msgid).await.unwrap();
+
         let path = msg.get_file(&ctx1).unwrap();
+        assert_eq!(path.with_file_name("hello.txt"), path);
         let text = fs::read_to_string(&path).await.unwrap();
         assert_eq!(text, "i am attachment");
 

src/message.rs

@@ -688,11 +688,13 @@ impl Message {
         &self.subject
     }
 
-    /// Returns base file name without the path.
+    /// Returns original filename (as shown in chat).
-    /// The base file name includes the extension.
     ///
     /// To get the full path, use [`Self::get_file()`].
     pub fn get_filename(&self) -> Option<String> {
+        if let Some(name) = self.param.get(Param::Filename) {
+            return Some(name.to_string());
+        }
         self.param
             .get(Param::File)
             .and_then(|file| Path::new(file).file_name())
@@ -972,6 +974,11 @@ impl Message {
     /// the file will only be used when the message is prepared
     /// for sending.
     pub fn set_file(&mut self, file: impl ToString, filemime: Option<&str>) {
+        if let Some(name) = Path::new(&file.to_string()).file_name() {
+            if let Some(name) = name.to_str() {
+                self.param.set(Param::Filename, name);
+            }
+        }
         self.param.set(Param::File, file);
         if let Some(filemime) = filemime {
             self.param.set(Param::MimeType, filemime);

src/mimefactory.rs

@@ -1386,7 +1386,7 @@ async fn build_body_file(
         .param
         .get_blob(Param::File, context, true)
         .await?
-        .context("msg has no filename")?;
+        .context("msg has no file")?;
     let suffix = blob.suffix().unwrap_or("dat");
 
     // Get file name to use for sending.  For privacy purposes, we do
@@ -1431,7 +1431,11 @@ async fn build_body_file(
                 ),
             &suffix
         ),
-        _ => blob.as_file_name().to_string(),
+        _ => msg
+            .param
+            .get(Param::Filename)
+            .unwrap_or_else(|| blob.as_file_name())
+            .to_string(),
     };
 
     /* check mimetype */

src/mimeparser.rs

@@ -1268,6 +1268,7 @@ impl MimeMessage {
         part.mimetype = Some(mime_type);
         part.bytes = decoded_data.len();
         part.param.set(Param::File, blob.as_name());
+        part.param.set(Param::Filename, filename);
         part.param.set(Param::MimeType, raw_mime);
         part.is_related = is_related;
 

src/param.rs

@@ -21,6 +21,9 @@ pub enum Param {
     /// For messages and jobs
     File = b'f',
 
+    /// For messages: original filename (as shown in chat)
+    Filename = b'v',
+
     /// For messages: This name should be shown instead of contact.get_display_name()
     /// (used if this is a mailinglist
     /// or explicitly set using set_override_sender_name(), eg. by bots)
@@ -528,7 +531,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");

src/receive_imf/tests.rs

@@ -1456,7 +1456,9 @@ async fn test_pdf_filename_simple() {
     .await;
     assert_eq!(msg.viewtype, Viewtype::File);
     assert_eq!(msg.text, "mail body");
-    assert_eq!(msg.param.get(Param::File).unwrap(), "$BLOBDIR/simple.pdf");
+    let file_path = msg.param.get(Param::File).unwrap();
+    assert!(file_path.starts_with("$BLOBDIR/simple"));
+    assert!(file_path.ends_with(".pdf"));
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -1470,10 +1472,9 @@ async fn test_pdf_filename_continuation() {
     .await;
     assert_eq!(msg.viewtype, Viewtype::File);
     assert_eq!(msg.text, "mail body");
-    assert_eq!(
+    let file_path = msg.param.get(Param::File).unwrap();
-        msg.param.get(Param::File).unwrap(),
+    assert!(file_path.starts_with("$BLOBDIR/test pdf äöüß"));
-        "$BLOBDIR/test pdf äöüß.pdf"
+    assert!(file_path.ends_with(".pdf"));
-    );
 }
 
 /// HTML-images may come with many embedded images, eg. tiny icons, corners for formatting,
@@ -2797,7 +2798,7 @@ Reply from different address
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_long_filenames() -> Result<()> {
+async fn test_long_and_duplicated_filenames() -> Result<()> {
     let mut tcm = TestContextManager::new();
     let alice = tcm.alice().await;
     let bob = tcm.bob().await;
@@ -2809,6 +2810,7 @@ async fn test_long_filenames() -> Result<()> {
         "foo. .tar.gz",
         "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.tar.gz",
         "a.tar.gz",
+        "a.tar.gz",
         "a.a..a.a.a.a.tar.gz",
     ] {
         let attachment = alice.blobdir.join(filename_sent);
@@ -2823,22 +2825,19 @@ async fn test_long_filenames() -> Result<()> {
 
         let msg_bob = bob.recv_msg(&sent).await;
 
-        async fn check_message(msg: &Message, t: &TestContext, content: &str) {
+        async fn check_message(msg: &Message, t: &TestContext, filename: &str, content: &str) {
             assert_eq!(msg.get_viewtype(), Viewtype::File);
             let resulting_filename = msg.get_filename().unwrap();
+            assert_eq!(resulting_filename, filename);
             let path = msg.get_file(t).unwrap();
-            assert!(
-                resulting_filename.ends_with(".tar.gz"),
-                "{resulting_filename:?} doesn't end with .tar.gz, path: {path:?}"
-            );
             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);
         }
-        check_message(&msg_alice, &alice, &content).await;
+        check_message(&msg_alice, &alice, filename_sent, &content).await;
-        check_message(&msg_bob, &bob, &content).await;
+        check_message(&msg_bob, &bob, filename_sent, &content).await;
     }
 
     Ok(())

src/summary.rs

@@ -9,7 +9,6 @@ use crate::contact::{Contact, ContactId};
 use crate::context::Context;
 use crate::message::{Message, MessageState, Viewtype};
 use crate::mimeparser::SystemMessage;
-use crate::param::Param;
 use crate::stock_str;
 use crate::tools::truncate;
 
@@ -133,14 +132,8 @@ impl Message {
                     append_text = false;
                     stock_str::ac_setup_msg_subject(context).await
                 } else {
-                    let file_name: String = self
+                    let file_name = self
-                        .param
+                        .get_filename()
-                        .get_path(Param::File, context)
-                        .unwrap_or(None)
-                        .and_then(|path| {
-                            path.file_name()
-                                .map(|fname| fname.to_string_lossy().into_owned())
-                        })
                         .unwrap_or_else(|| String::from("ErrFileName"));
                     let label = if self.viewtype == Viewtype::Audio {
                         stock_str::audio(context).await
@@ -200,6 +193,7 @@ impl Message {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::param::Param;
     use crate::test_utils as test;
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]

src/webxdc.rs

@@ -1088,7 +1088,7 @@ mod tests {
         .await?;
         let instance = t.get_last_msg().await;
         assert_eq!(instance.viewtype, Viewtype::Webxdc);
-        assert_eq!(instance.get_filename(), Some("minimal.xdc".to_string()));
+        assert_eq!(instance.get_filename().unwrap(), "minimal.xdc");
 
         receive_imf(
             &t,
@@ -1098,7 +1098,7 @@ mod tests {
         .await?;
         let instance = t.get_last_msg().await;
         assert_eq!(instance.viewtype, Viewtype::File); // we require the correct extension, only a mime type is not sufficient
-        assert_eq!(instance.get_filename(), Some("index.html".to_string()));
+        assert_eq!(instance.get_filename().unwrap(), "index.html");
 
         Ok(())
     }
@@ -1786,7 +1786,7 @@ mod tests {
         // bob receives the instance together with the initial updates in a single message
         let bob_instance = bob.recv_msg(&sent1).await;
         assert_eq!(bob_instance.viewtype, Viewtype::Webxdc);
-        assert_eq!(bob_instance.get_filename(), Some("minimal.xdc".to_string()));
+        assert_eq!(bob_instance.get_filename().unwrap(), "minimal.xdc");
         assert!(sent1.payload().contains("Content-Type: application/json"));
         assert!(sent1.payload().contains("status-update.json"));
         assert_eq!(