From 846c8e7f1b7c07d5ec0817a1a82bc55bea9a72e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Kl=C3=A4hn?= <39526136+Septias@users.noreply.github.com> Date: Mon, 5 May 2025 17:06:05 +0200 Subject: [PATCH] Generate rfc724_mid when creating Message (#6704) Set `rfc724_mid` in `Message::new()`, `Message::new_text()`, and `Message::default()` instead of when sending the message. This way the rfc724 mid can be read in the draft stage which makes it more consistent for bots. Tests had to be adjusted to create multiple messages to get unique mid, otherwise core would not send the messages out. --- deltachat-ffi/src/lib.rs | 3 ++- src/chat.rs | 17 ++++++++++------- src/chat/chat_tests.rs | 14 +++++++++++--- src/imex/key_transfer.rs | 5 +---- src/message.rs | 3 +++ src/message/message_tests.rs | 12 ++++++------ src/mimefactory/mimefactory_tests.rs | 1 + src/receive_imf/receive_imf_tests.rs | 2 +- src/webxdc/webxdc_tests.rs | 22 ++++++++++++++++++++++ 9 files changed, 57 insertions(+), 22 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 8f55d00ce..5ad6f9286 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -37,6 +37,7 @@ use deltachat::*; use deltachat::{accounts::Accounts, log::LogExt}; use deltachat_jsonrpc::api::CommandApi; use deltachat_jsonrpc::yerpc::{OutReceiver, RpcClient, RpcSession}; +use message::Viewtype; use num_traits::{FromPrimitive, ToPrimitive}; use rand::Rng; use tokio::runtime::Runtime; @@ -2076,7 +2077,7 @@ pub unsafe extern "C" fn dc_get_msg(context: *mut dc_context_t, msg_id: u32) -> ctx, "dc_get_msg called with special msg_id={msg_id}, returning empty msg" ); - message::Message::default() + message::Message::new(Viewtype::default()) } else { warn!(ctx, "dc_get_msg could not retrieve msg_id {msg_id}: {e:#}"); return ptr::null_mut(); diff --git a/src/chat.rs b/src/chat.rs index b5d8d5d61..2e68bc4cd 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2005,7 +2005,9 @@ impl Chat { let mut to_id = 0; let mut location_id = 0; - let new_rfc724_mid = create_outgoing_rfc724_mid(); + if msg.rfc724_mid.is_empty() { + msg.rfc724_mid = create_outgoing_rfc724_mid(); + } if self.typ == Chattype::Single { if let Some(id) = context @@ -2103,7 +2105,7 @@ impl Chat { if references_vec.is_empty() { // As a fallback, use our Message-ID, // same as in the case of top-level message. - new_references = new_rfc724_mid.clone(); + new_references = msg.rfc724_mid.clone(); } else { new_references = references_vec.join(" "); } @@ -2113,7 +2115,7 @@ impl Chat { // This allows us to identify replies to our message even if // email server such as Outlook changes `Message-ID:` header. // MUAs usually keep the first Message-ID in `References:` header unchanged. - new_references = new_rfc724_mid.clone(); + new_references = msg.rfc724_mid.clone(); } // add independent location to database @@ -2181,7 +2183,6 @@ impl Chat { msg.chat_id = self.id; msg.from_id = ContactId::SELF; - msg.rfc724_mid = new_rfc724_mid; msg.timestamp_sort = timestamp; // add message to the database @@ -3846,7 +3847,7 @@ pub(crate) async fn add_contact_to_chat_ex( ) -> Result { ensure!(!chat_id.is_special(), "can not add member to special chats"); let contact = Contact::get_by_id(context, contact_id).await?; - let mut msg = Message::default(); + let mut msg = Message::new(Viewtype::default()); chat_id.reset_gossiped_timestamp(context).await?; @@ -4077,7 +4078,7 @@ pub async fn remove_contact_from_chat( "Cannot remove special contact" ); - let mut msg = Message::default(); + let mut msg = Message::new(Viewtype::default()); let chat = Chat::load_from_db(context, chat_id).await?; if chat.typ == Chattype::Group || chat.typ == Chattype::Broadcast { @@ -4184,7 +4185,7 @@ async fn rename_ex( ensure!(!chat_id.is_special(), "Invalid chat ID"); let chat = Chat::load_from_db(context, chat_id).await?; - let mut msg = Message::default(); + let mut msg = Message::new(Viewtype::default()); if chat.typ == Chattype::Group || chat.typ == Chattype::Mailinglist @@ -4347,9 +4348,11 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) msg.subject = "".to_string(); msg.state = MessageState::OutPending; + msg.rfc724_mid = create_outgoing_rfc724_mid(); let new_msg_id = chat .prepare_msg_raw(context, &mut msg, None, curr_timestamp) .await?; + curr_timestamp += 1; if !create_send_msg_jobs(context, &mut msg).await?.is_empty() { context.scheduler.interrupt_smtp().await; diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 225eb36f2..37c983e73 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2040,20 +2040,28 @@ async fn test_sticker_forward() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_forward() -> Result<()> { +async fn test_forward_basic() -> Result<()> { let alice = TestContext::new_alice().await; let bob = TestContext::new_bob().await; let alice_chat = alice.create_chat(&bob).await; let bob_chat = bob.create_chat(&alice).await; - let mut msg = Message::new_text("Hi Bob".to_owned()); - let sent_msg = alice.send_msg(alice_chat.get_id(), &mut msg).await; + let mut alice_msg = Message::new_text("Hi Bob".to_owned()); + let sent_msg = alice.send_msg(alice_chat.get_id(), &mut alice_msg).await; let msg = bob.recv_msg(&sent_msg).await; + assert_eq!(alice_msg.rfc724_mid, msg.rfc724_mid); forward_msgs(&bob, &[msg.id], bob_chat.get_id()).await?; let forwarded_msg = bob.pop_sent_msg().await; + assert_eq!(bob_chat.id.get_msg_cnt(&bob).await?, 2); + assert_ne!( + forwarded_msg.load_from_db().await.rfc724_mid, + msg.rfc724_mid, + ); + let msg_bob = Message::load_from_db(&bob, forwarded_msg.sender_msg_id).await?; let msg = alice.recv_msg(&forwarded_msg).await; + assert_eq!(msg.rfc724_mid(), msg_bob.rfc724_mid()); assert_eq!(msg.get_text(), "Hi Bob"); assert!(msg.is_forwarded()); Ok(()) diff --git a/src/imex/key_transfer.rs b/src/imex/key_transfer.rs index 75f369c0d..88f61742e 100644 --- a/src/imex/key_transfer.rs +++ b/src/imex/key_transfer.rs @@ -32,10 +32,7 @@ pub async fn initiate_key_transfer(context: &Context) -> Result { )?; let chat_id = ChatId::create_for_contact(context, ContactId::SELF).await?; - let mut msg = Message { - viewtype: Viewtype::File, - ..Default::default() - }; + let mut msg = Message::new(Viewtype::File); msg.param.set(Param::File, setup_file_blob.as_name()); msg.param .set(Param::Filename, "autocrypt-setup-message.html"); diff --git a/src/message.rs b/src/message.rs index 67601d618..5e5192446 100644 --- a/src/message.rs +++ b/src/message.rs @@ -33,6 +33,7 @@ use crate::reaction::get_msg_reactions; use crate::sql; use crate::summary::Summary; use crate::sync::SyncData; +use crate::tools::create_outgoing_rfc724_mid; use crate::tools::{ buf_compress, buf_decompress, get_filebytes, get_filemeta, gm2local_offset, read_file, sanitize_filename, time, timestamp_to_str, @@ -470,6 +471,7 @@ impl Message { pub fn new(viewtype: Viewtype) -> Self { Message { viewtype, + rfc724_mid: create_outgoing_rfc724_mid(), ..Default::default() } } @@ -479,6 +481,7 @@ impl Message { Message { viewtype: Viewtype::Text, text, + rfc724_mid: create_outgoing_rfc724_mid(), ..Default::default() } } diff --git a/src/message/message_tests.rs b/src/message/message_tests.rs index 708cac527..7c4bb0734 100644 --- a/src/message/message_tests.rs +++ b/src/message/message_tests.rs @@ -135,7 +135,7 @@ async fn test_get_width_height() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_quote() { +async fn test_quote_basic() { let d = TestContext::new().await; let ctx = &d.ctx; @@ -144,13 +144,10 @@ async fn test_quote() { .unwrap(); let chat = d.create_chat_with_contact("", "dest@example.com").await; - let mut msg = Message::new_text("Quoted message".to_string()); - // Send message, so it gets a Message-Id. - assert!(msg.rfc724_mid.is_empty()); - let msg_id = chat::send_msg(ctx, chat.id, &mut msg).await.unwrap(); - let msg = Message::load_from_db(ctx, msg_id).await.unwrap(); + // Message has to be sent such that it gets saved to db. + chat::send_msg(ctx, chat.id, &mut msg).await.unwrap(); assert!(!msg.rfc724_mid.is_empty()); let mut msg2 = Message::new(Viewtype::Text); @@ -358,6 +355,7 @@ async fn test_markseen_msgs() -> Result<()> { let sent1 = alice.send_msg(alice_chat.id, &mut msg).await; let msg1 = bob.recv_msg(&sent1).await; let bob_chat_id = msg1.chat_id; + let mut msg = Message::new_text("this is the text!".to_string()); let sent2 = alice.send_msg(alice_chat.id, &mut msg).await; let msg2 = bob.recv_msg(&sent2).await; assert_eq!(msg1.chat_id, msg2.chat_id); @@ -380,9 +378,11 @@ async fn test_markseen_msgs() -> Result<()> { // bob sends to alice, // alice knows bob and messages appear in normal chat + let mut msg = Message::new_text("this is the text!".to_string()); let msg1 = alice .recv_msg(&bob.send_msg(bob_chat_id, &mut msg).await) .await; + let mut msg = Message::new_text("this is the text!".to_string()); let msg2 = alice .recv_msg(&bob.send_msg(bob_chat_id, &mut msg).await) .await; diff --git a/src/mimefactory/mimefactory_tests.rs b/src/mimefactory/mimefactory_tests.rs index fb909840e..22306ddeb 100644 --- a/src/mimefactory/mimefactory_tests.rs +++ b/src/mimefactory/mimefactory_tests.rs @@ -728,6 +728,7 @@ async fn test_selfavatar_unencrypted_signed() { .is_some()); // if another message is sent, that one must not contain the avatar + let mut msg = Message::new_text("this is the text!".to_string()); let sent_msg = t.send_msg(chat.id, &mut msg).await; let mut payload = sent_msg.payload().splitn(4, "\r\n\r\n"); diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 85e15e703..c9093eb85 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -5090,7 +5090,7 @@ async fn test_references() -> Result<()> { alice.set_config_bool(Config::BccSelf, true).await?; let alice_chat_id = create_group_chat(alice, ProtectionStatus::Unprotected, "Group").await?; - let _sent = alice + alice .send_text(alice_chat_id, "Hi! I created a group.") .await; diff --git a/src/webxdc/webxdc_tests.rs b/src/webxdc/webxdc_tests.rs index 04a12bf2f..c0a9d347c 100644 --- a/src/webxdc/webxdc_tests.rs +++ b/src/webxdc/webxdc_tests.rs @@ -2195,3 +2195,25 @@ async fn test_webxdc_href() -> Result<()> { Ok(()) } + +/// Test that the draft `selfAddr` is equal to the `selfAddr` of the sent message. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_self_addr_consistency() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let alice_chat = alice + .create_group_with_members(ProtectionStatus::Unprotected, "No friends :(", &[]) + .await; + let mut instance = create_webxdc_instance( + alice, + "minimal.xdc", + include_bytes!("../../test-data/webxdc/minimal.xdc"), + )?; + alice_chat.set_draft(alice, Some(&mut instance)).await?; + let self_addr = instance.get_webxdc_self_addr(alice).await?; + let sent = alice.send_msg(alice_chat, &mut instance).await; + let db_msg = Message::load_from_db(alice, sent.sender_msg_id).await?; + assert_eq!(db_msg.get_webxdc_self_addr(alice).await?, self_addr); + assert_eq!(alice_chat.get_msg_cnt(alice).await?, 1); + Ok(()) +}