diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 4b6b4120fb..221b8ef4c6 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -3978,31 +3978,31 @@ 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 - * message. + * Find out full path of the file associated with a 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(). + * The returned value must be released using dc_str_unref(). */ 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 - * is not returned. To get the full path, use dc_msg_get_file(). + * Get an original attachment filename, with extension but without the path. To get the full path, + * 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 - * associated with the message, an empty string is returned. The returned - * value must be released using dc_str_unref(). + * @return The attachment filename. If there is no file associated with the message, an empty string + * is returned. The returned value must be released using dc_str_unref(). */ char* dc_msg_get_filename (const dc_msg_t* msg); diff --git a/node/test/test.js b/node/test/test.js index 3c1baed01c..2c79125840 100644 --- 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 index 0f9674808a..aeba9b0356 100644 --- a/python/tests/test_1_online.py +++ b/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" - 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(): @@ -181,12 +182,14 @@ 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 @@ -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 = "textdata" - 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): diff --git a/python/tests/test_2_increation.py b/python/tests/test_2_increation.py index 761b3eb864..2f053e0028 100644 --- 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/chat.rs b/src/chat.rs index db8a955d7c..ec5dac52b0 100644 --- a/src/chat.rs +++ b/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); diff --git a/src/imex/transfer.rs b/src/imex/transfer.rs index d9a7e8b91e..545bd94bd5 100644 --- a/src/imex/transfer.rs +++ b/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"); diff --git a/src/message.rs b/src/message.rs index a90c7c7192..97c101cda2 100644 --- a/src/message.rs +++ b/src/message.rs @@ -688,11 +688,13 @@ impl Message { &self.subject } - /// Returns base file name without the path. - /// The base file name includes the extension. + /// Returns original filename (as shown in chat). /// /// To get the full path, use [`Self::get_file()`]. pub fn get_filename(&self) -> Option { + if let Some(name) = self.param.get(Param::Filename).map(|name| name.to_string()) { + return Some(name); + } 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); diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 9314ea3ef4..724340348c 100644 --- a/src/mimefactory.rs +++ b/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 */ diff --git a/src/mimeparser.rs b/src/mimeparser.rs index c6d9e340b3..febc20bda0 100644 --- a/src/mimeparser.rs +++ b/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; diff --git a/src/param.rs b/src/param.rs index ae01d3853f..8768f996d2 100644 --- a/src/param.rs +++ b/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"); diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index 03823b36ab..5036b1a24a 100644 --- a/src/receive_imf/tests.rs +++ b/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!( - msg.param.get(Param::File).unwrap(), - "$BLOBDIR/test pdf äöüß.pdf" - ); + let file_path = msg.param.get(Param::File).unwrap(); + assert!(file_path.starts_with("$BLOBDIR/test 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_bob, &bob, &content).await; + check_message(&msg_alice, &alice, filename_sent, &content).await; + check_message(&msg_bob, &bob, filename_sent, &content).await; } Ok(()) diff --git a/src/summary.rs b/src/summary.rs index f5c1f92450..731f4edf0e 100644 --- a/src/summary.rs +++ b/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 - .param - .get_path(Param::File, context) - .unwrap_or(None) - .and_then(|path| { - path.file_name() - .map(|fname| fname.to_string_lossy().into_owned()) - }) + let file_name = self + .get_filename() .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)] diff --git a/src/webxdc.rs b/src/webxdc.rs index 935837349e..26c912e408 100644 --- a/src/webxdc.rs +++ b/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!(