From e4ae501112b007b4decd67d8c0ed8bd68ae2cd4a Mon Sep 17 00:00:00 2001 From: iequidoo Date: Tue, 17 Mar 2026 13:51:42 -0300 Subject: [PATCH 1/3] test: test_markseen_pre_msg: Check that MDN on pre-message changes message state to OutMdnRcvd (#8004) --- src/smtp.rs | 105 ++++++++++++++++++---------- src/tests/pre_messages/receiving.rs | 18 ++++- 2 files changed, 85 insertions(+), 38 deletions(-) diff --git a/src/smtp.rs b/src/smtp.rs index 020c575fd3..76deb73f87 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -13,7 +13,7 @@ use crate::config::Config; use crate::contact::{Contact, ContactId}; use crate::context::Context; use crate::events::EventType; -use crate::log::{LogExt, warn}; +use crate::log::warn; use crate::message::Message; use crate::message::{self, MsgId}; use crate::mimefactory::MimeFactory; @@ -590,44 +590,77 @@ async fn send_mdn_rfc724_mid( if context.get_config_bool(Config::BccSelf).await? { add_self_recipients(context, &mut recipients, encrypted).await?; } - let recipients: Vec<_> = recipients - .into_iter() - .filter_map(|addr| { - async_smtp::EmailAddress::new(addr.clone()) - .with_context(|| format!("Invalid recipient: {addr}")) - .log_err(context) - .ok() - }) - .collect(); - - match smtp_send(context, &recipients, &body, smtp, None).await { - SendResult::Success => { - if !recipients.is_empty() { - info!(context, "Successfully sent MDN for {rfc724_mid}."); + #[cfg(not(test))] + { + use crate::log::LogExt; + + let recipients: Vec<_> = recipients + .into_iter() + .filter_map(|addr| { + async_smtp::EmailAddress::new(addr.clone()) + .with_context(|| format!("Invalid recipient: {addr}")) + .log_err(context) + .ok() + }) + .collect(); + + match smtp_send(context, &recipients, &body, smtp, None).await { + SendResult::Success => { + if !recipients.is_empty() { + info!(context, "Successfully sent MDN for {rfc724_mid}."); + } + context + .sql + .transaction(|transaction| { + let mut stmt = + transaction.prepare("DELETE FROM smtp_mdns WHERE rfc724_mid = ?")?; + stmt.execute((rfc724_mid,))?; + for additional_rfc724_mid in additional_rfc724_mids { + stmt.execute((additional_rfc724_mid,))?; + } + Ok(()) + }) + .await?; + Ok(true) } - context - .sql - .transaction(|transaction| { - let mut stmt = - transaction.prepare("DELETE FROM smtp_mdns WHERE rfc724_mid = ?")?; - stmt.execute((rfc724_mid,))?; - for additional_rfc724_mid in additional_rfc724_mids { - stmt.execute((additional_rfc724_mid,))?; - } - Ok(()) - }) - .await?; - Ok(true) - } - SendResult::Retry => { - info!( - context, - "Temporary SMTP failure while sending an MDN for {rfc724_mid}." - ); - Ok(false) + SendResult::Retry => { + info!( + context, + "Temporary SMTP failure while sending an MDN for {rfc724_mid}." + ); + Ok(false) + } + SendResult::Failure(err) => Err(err), } - SendResult::Failure(err) => Err(err), } + #[cfg(test)] + { + let _ = smtp; + context + .sql + .transaction(|t| { + t.execute( + "INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id) + VALUES (?, ?, ?, ?)", + (rfc724_mid, recipients.join(" "), body, u32::MAX), + )?; + let mut stmt = t.prepare("DELETE FROM smtp_mdns WHERE rfc724_mid = ?")?; + stmt.execute((rfc724_mid,))?; + for additional_rfc724_mid in additional_rfc724_mids { + stmt.execute((additional_rfc724_mid,))?; + } + Ok(()) + }) + .await?; + Ok(true) + } +} + +#[cfg(test)] +pub(crate) async fn queue_mdn(context: &Context) -> Result<()> { + let queued = send_mdn(context, &mut Smtp::new()).await?; + assert!(queued); + Ok(()) } /// Tries to send a single MDN. Returns true if more MDNs should be sent. diff --git a/src/tests/pre_messages/receiving.rs b/src/tests/pre_messages/receiving.rs index e92f6d21d6..170c190523 100644 --- a/src/tests/pre_messages/receiving.rs +++ b/src/tests/pre_messages/receiving.rs @@ -1,4 +1,6 @@ //! Tests about receiving Pre-Messages and Post-Message +use std::time::Duration; + use anyhow::{Context as _, Result}; use pretty_assertions::assert_eq; @@ -8,16 +10,18 @@ use crate::chat::send_msg; use crate::config::Config; use crate::contact; use crate::download::{DownloadState, PRE_MSG_ATTACHMENT_SIZE_THRESHOLD, PostMsgMetadata}; -use crate::message::{Message, MessageState, Viewtype, delete_msgs, markseen_msgs}; +use crate::message::{self, Message, MessageState, Viewtype, delete_msgs, markseen_msgs}; use crate::mimeparser::MimeMessage; use crate::param::Param; use crate::reaction::{get_msg_reactions, send_reaction}; use crate::receive_imf::receive_imf; +use crate::smtp; use crate::summary::assert_summary_texts; use crate::test_utils::TestContextManager; use crate::tests::pre_messages::util::{ big_webxdc_app, send_large_file_message, send_large_image_message, send_large_webxdc_message, }; +use crate::tools::{SystemTime, time}; use crate::webxdc::StatusUpdateSerial; /// Test that mimeparser can correctly detect and parse pre-messages and Post-Messages @@ -625,7 +629,7 @@ async fn test_markseen_pre_msg() -> Result<()> { alice.create_chat(bob).await; // Make sure the chat is accepted. tcm.section("Bob sends a large message to Alice"); - let (pre_message, post_message, _bob_msg_id) = + let (pre_message, post_message, bob_msg_id) = send_large_file_message(bob, bob_chat_id, Viewtype::File, &vec![0u8; 1_000_000]).await?; tcm.section("Alice receives a pre-message message from Bob"); @@ -644,6 +648,16 @@ async fn test_markseen_pre_msg() -> Result<()> { .await?, 1 ); + smtp::queue_mdn(alice).await?; + SystemTime::shift(Duration::from_secs(3600)); + bob.recv_msg_trash(&alice.pop_sent_msg().await).await; + let bob_msg = Message::load_from_db(bob, bob_msg_id).await?; + assert_eq!(bob_msg.get_state(), MessageState::OutMdnRcvd); + let read_receipts = message::get_msg_read_receipts(bob, bob_msg_id).await?; + assert_eq!(read_receipts.len(), 1); + let (contact_id, ts) = read_receipts[0]; + assert_eq!(contact_id, bob.add_or_lookup_contact_id(alice).await); + assert!(ts + 3600 - 60 < time()); tcm.section("Alice downloads message"); alice.recv_msg_trash(&post_message).await; From 1c17ade2aa8d5e519c9c9fb693340f420f535534 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Wed, 18 Mar 2026 10:46:46 -0300 Subject: [PATCH 2/3] test: MDN on pre-message has effect if received before sending full message (#8004) Actually, the problem in #8004 is that a pre-message doesn't "want MDN" if it has no text. Anyway, the added test reproduces the bug. --- src/config.rs | 4 ++ src/context.rs | 7 +++ src/message.rs | 8 +++- src/mimeparser.rs | 2 + src/test_utils.rs | 5 ++- src/tests/pre_messages/receiving.rs | 68 +++++++++++++++++++++++++++-- 6 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/config.rs b/src/config.rs index 305e8765c9..3875e83cc1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -466,6 +466,10 @@ pub enum Config { /// storing the same token multiple times on the server. EncryptedDeviceToken, + /// Make `TestContext::pop_sent_msg_opt()` and related functions pop messages from the `smtp` + /// head, i.e. make it a queue. For historical reasons the default is a stack. + PopSentMsgFromHead, + /// Enables running test hooks, e.g. see `InnerContext::pre_encrypt_mime_hook`. /// This way is better than conditional compilation, i.e. `#[cfg(test)]`, because tests not /// using this still run unmodified code. diff --git a/src/context.rs b/src/context.rs index 801dfbce8d..91453de058 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1044,6 +1044,13 @@ impl Context { .await? .to_string(), ); + res.insert( + "pop_sent_msg_from_head", + self.sql + .get_raw_config("pop_sent_msg_from_head") + .await? + .unwrap_or_default(), + ); res.insert( "test_hooks", self.sql diff --git a/src/message.rs b/src/message.rs index 3d664b81f8..2cb92d3406 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1901,9 +1901,10 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> // // We also don't send read receipts for contact requests. // Read receipts will not be sent even after accepting the chat. + let wants_mdn = curr_param.get_bool(Param::WantsMdn).unwrap_or_default(); let to_id = if curr_blocked == Blocked::Not && !curr_hidden - && curr_param.get_bool(Param::WantsMdn).unwrap_or_default() + && wants_mdn && curr_param.get_cmd() == SystemMessage::Unknown && context.should_send_mdns().await? { @@ -1925,6 +1926,11 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> None }; if let Some(to_id) = to_id { + info!( + context, + "Queuing MDN to {to_id} for {id} from {curr_from_id}, wants_mdn={wants_mdn}, cmd={}.", + curr_param.get_cmd() + ); context .sql .execute( diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 04e618a855..62b7cb7d4e 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -2492,6 +2492,8 @@ async fn handle_mdn( let Some((msg_id, chat_id, has_mdns, is_dup)) = context .sql .query_row_optional( + // MDN on a pre-message references the post-message, see `receive_imf`. So we can't tell + // which one was seen, but this is on purpose. "SELECT m.id AS msg_id, c.id AS chat_id, diff --git a/src/test_utils.rs b/src/test_utils.rs index 5f5ad18c39..8ecf017123 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -624,7 +624,10 @@ impl TestContext { } pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option> { - let rev_order = true; + let rev_order = !self + .get_config_bool(Config::PopSentMsgFromHead) + .await + .unwrap(); self.pop_sent_msg_ex(rev_order, timeout).await } diff --git a/src/tests/pre_messages/receiving.rs b/src/tests/pre_messages/receiving.rs index 170c190523..83f8779d53 100644 --- a/src/tests/pre_messages/receiving.rs +++ b/src/tests/pre_messages/receiving.rs @@ -6,7 +6,6 @@ use pretty_assertions::assert_eq; use crate::EventType; use crate::chat; -use crate::chat::send_msg; use crate::config::Config; use crate::contact; use crate::download::{DownloadState, PRE_MSG_ATTACHMENT_SIZE_THRESHOLD, PostMsgMetadata}; @@ -265,6 +264,64 @@ async fn test_lost_pre_msg() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_pre_msg_mdn_before_sending_full() -> Result<()> { + pre_msg_mdn_before_sending_full("").await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_pre_msg_mdn_before_sending_full_with_text() -> Result<()> { + pre_msg_mdn_before_sending_full("text").await +} + +async fn pre_msg_mdn_before_sending_full(text: &str) -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + alice + .set_config_bool(Config::PopSentMsgFromHead, true) + .await?; + let bob = &tcm.bob().await; + let alice_chat_id = alice.create_group_with_members("", &[bob]).await; + + let file_bytes = include_bytes!("../../../test-data/image/screenshot.gif"); + let mut msg = Message::new(Viewtype::Image); + msg.set_file_from_bytes(alice, "a.jpg", file_bytes, None)?; + msg.set_text(text.to_string()); + let pre_msg = alice.send_msg(alice_chat_id, &mut msg).await; + let alice_msg_id = msg.id; + + let msg = bob.recv_msg(&pre_msg).await; + assert_eq!(msg.download_state, DownloadState::Available); + assert_eq!(msg.id.get_state(bob).await?, MessageState::InFresh); + assert_eq!(msg.text, text); + assert!(msg.param.get_bool(Param::WantsMdn).unwrap_or_default()); + msg.chat_id.accept(bob).await?; + markseen_msgs(bob, vec![msg.id]).await?; + assert_eq!(msg.id.get_state(bob).await?, MessageState::InSeen); + assert_eq!( + bob.sql + .count( + "SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?", + (msg.from_id,) + ) + .await?, + 1 + ); + smtp::queue_mdn(bob).await?; + alice.recv_msg_trash(&bob.pop_sent_msg().await).await; + assert_eq!( + alice_msg_id.get_state(alice).await?, + MessageState::OutPending + ); + + let _full_msg = alice.pop_sent_msg().await; + assert_eq!( + alice_msg_id.get_state(alice).await?, + MessageState::OutMdnRcvd + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_post_msg_bad_sender() -> Result<()> { let mut tcm = TestContextManager::new(); @@ -550,7 +607,7 @@ async fn test_webxdc_updates_in_post_message_after_pre_message() -> Result<()> { .send_webxdc_status_update(alice_instance.id, r#"{"payload":42, "info":"i"}"#) .await?; - send_msg(alice, alice_chat_id, &mut alice_instance).await?; + chat::send_msg(alice, alice_chat_id, &mut alice_instance).await?; let post_message = alice.pop_sent_msg().await; let pre_message = alice.pop_sent_msg().await; @@ -591,7 +648,7 @@ async fn test_webxdc_updates_in_post_message_without_pre_message() -> Result<()> .send_webxdc_status_update(alice_instance.id, r#"{"payload":42, "info":"i"}"#) .await?; - send_msg(alice, alice_chat_id, &mut alice_instance).await?; + chat::send_msg(alice, alice_chat_id, &mut alice_instance).await?; let post_message = alice.pop_sent_msg().await; let pre_message = alice.pop_sent_msg().await; @@ -644,7 +701,10 @@ async fn test_markseen_pre_msg() -> Result<()> { assert_eq!( alice .sql - .count("SELECT COUNT(*) FROM smtp_mdns", ()) + .count( + "SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?", + (msg.from_id,) + ) .await?, 1 ); From 77edcea3fc41552200f84642721c0c9dc26a12e4 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sat, 21 Mar 2026 09:27:47 -0300 Subject: [PATCH 3/3] fix: Make pre-messages w/o text want MDNs (#8004) This fixes failing `test_pre_msg_mdn_before_sending_full`. Maybe it's even good that MDNs will be sent for pre-messages only having placeholder text with the viewtype and size, at least this way we notify the contact that we've noticed the message. Anyway, with adding message previews the problem will be solved for the corresponding viewtypes. --- src/mimeparser.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 62b7cb7d4e..4468162d62 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -951,14 +951,16 @@ impl MimeMessage { self.parse_attachments(); // See if an MDN is requested from the other side + let mut wants_mdn = false; if self.decryption_error.is_none() - && !self.parts.is_empty() + && (!self.parts.is_empty() || matches!(&self.pre_message, PreMessageMode::Pre { .. })) && let Some(ref dn_to) = self.chat_disposition_notification_to { // Check that the message is not outgoing. let from = &self.from.addr; if !context.is_self_addr(from).await? { if from.to_lowercase() == dn_to.addr.to_lowercase() { + wants_mdn = true; if let Some(part) = self.parts.last_mut() { part.param.set_int(Param::WantsMdn, 1); } @@ -980,7 +982,9 @@ impl MimeMessage { typ: Viewtype::Text, ..Default::default() }; - + if wants_mdn { + part.param.set_int(Param::WantsMdn, 1); + } if let Some(ref subject) = self.get_subject() && !self.has_chat_version() && self.webxdc_status_update.is_none()