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..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() @@ -2492,6 +2496,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/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/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 e92f6d21d6..83f8779d53 100644 --- a/src/tests/pre_messages/receiving.rs +++ b/src/tests/pre_messages/receiving.rs @@ -1,23 +1,26 @@ //! Tests about receiving Pre-Messages and Post-Message +use std::time::Duration; + use anyhow::{Context as _, Result}; 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}; -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 @@ -261,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(); @@ -546,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; @@ -587,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; @@ -625,7 +686,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"); @@ -640,10 +701,23 @@ 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 ); + 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;