From a67a5299bf62a59e8b5bf783c222c22d5e517085 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sat, 17 Jan 2026 01:46:15 -0300 Subject: [PATCH] Send and apply MDNs to self (#7005) We currently synchronize "seen" status of messages by setting `\Seen` flag on IMAP and then looking for new `\Seen` flags using `CONDSTORE` IMAP extension. This approach has multiple disadvantages: - It requires that the server supports `CONDSTORE` extension. For example Maddy does not support CONDSTORE yet: https://github.com/foxcpp/maddy/issues/727 - It leaks the seen status to the server without any encryption. - It requires more than just store-and-forward queues and prevents replacing IMAP with simpler protocols like POP3 or UUCP or some HTTP-based API for queue polling. A simpler approach is to send MDNs to self when `Config::BccSelf` (aka multidevice) is enabled, regardless of whether the message requested and MDN. If MDN was requested and we have MDNs enabled, then also send to the message sender, but MDN to self is sent regardless of whether read receipts are actually enabled. `sync_seen_flags()` and `CONDSTORE` check is better completely removed, maybe after one release. `store_seen_flags_on_imap()` can be kept for unencrypted non-chat messages. One potential problem with sending MDNs is that it may trigger ratelimits on some providers and count as another recipient. --- src/chat.rs | 45 ++++++------------------ src/message.rs | 32 +++++++++++++---- src/mimefactory.rs | 6 ++-- src/mimeparser.rs | 12 ++----- src/reaction.rs | 14 ++++++-- src/receive_imf.rs | 86 ++++++++++++++++++++++++++++++++++++++++------ src/smtp.rs | 61 ++++++++++++++++++++++++++++---- 7 files changed, 184 insertions(+), 72 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index a5bbe9588..5d7b3a307 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -19,6 +19,7 @@ use strum_macros::EnumIter; use crate::blob::BlobObject; use crate::chatlist::Chatlist; +use crate::chatlist_events; use crate::color::str_to_color; use crate::config::Config; use crate::constants::{ @@ -42,7 +43,7 @@ use crate::mimefactory::{MimeFactory, RenderedEmail}; use crate::mimeparser::SystemMessage; use crate::param::{Param, Params}; use crate::receive_imf::ReceivedMsg; -use crate::smtp::send_msg_to_smtp; +use crate::smtp::{self, send_msg_to_smtp}; use crate::stock_str; use crate::sync::{self, Sync::*, SyncData}; use crate::tools::{ @@ -51,7 +52,6 @@ use crate::tools::{ gm2local_offset, normalize_text, smeared_time, time, truncate_msg_text, }; use crate::webxdc::StatusUpdateSerial; -use crate::{chatlist_events, imap}; pub(crate) const PARAM_BROADCAST_SECRET: Param = Param::Arg3; @@ -2135,10 +2135,11 @@ pub(crate) async fn sync(context: &Context, id: SyncId, action: SyncAction) -> R } /// Whether the chat is pinned or archived. -#[derive(Debug, Copy, Eq, PartialEq, Clone, Serialize, Deserialize, EnumIter)] +#[derive(Debug, Copy, Eq, PartialEq, Clone, Serialize, Deserialize, EnumIter, Default)] #[repr(i8)] pub enum ChatVisibility { /// Chat is neither archived nor pinned. + #[default] Normal = 0, /// Chat is archived. @@ -2838,32 +2839,11 @@ pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) - let from = context.get_primary_self_addr().await?; let lowercase_from = from.to_lowercase(); - // Send BCC to self if it is enabled. - // - // Previous versions of Delta Chat did not send BCC self - // if DeleteServerAfter was set to immediately delete messages - // from the server. This is not the case anymore - // because BCC-self messages are also used to detect - // that message was sent if SMTP server is slow to respond - // and connection is frequently lost - // before receiving status line. NB: This is not a problem for chatmail servers, so `BccSelf` - // disabled by default is fine. - // - // `from` must be the last addr, see `receive_imf_inner()` why. recipients.retain(|x| x.to_lowercase() != lowercase_from); - if (context.get_config_bool(Config::BccSelf).await? - || msg.param.get_cmd() == SystemMessage::AutocryptSetupMessage) - && (context.get_config_delete_server_after().await? != Some(0) || !recipients.is_empty()) + if context.get_config_bool(Config::BccSelf).await? + || msg.param.get_cmd() == SystemMessage::AutocryptSetupMessage { - // Avoid sending unencrypted messages to all transports, chatmail relays won't accept - // them. Normally the user should have a non-chatmail primary transport to send unencrypted - // messages. - if needs_encryption { - for addr in context.get_secondary_self_addrs().await? { - recipients.push(addr); - } - } - recipients.push(from); + smtp::add_self_recipients(context, &mut recipients, needs_encryption).await?; } // Default Webxdc integrations are hidden messages and must not be sent out @@ -3291,7 +3271,7 @@ pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> let hidden_messages = context .sql .query_map_vec( - "SELECT id, rfc724_mid FROM msgs + "SELECT id FROM msgs WHERE state=? AND hidden=1 AND chat_id=? @@ -3299,16 +3279,11 @@ pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> (MessageState::InFresh, chat_id), // No need to check for InNoticed messages, because reactions are never InNoticed |row| { let msg_id: MsgId = row.get(0)?; - let rfc724_mid: String = row.get(1)?; - Ok((msg_id, rfc724_mid)) + Ok(msg_id) }, ) .await?; - for (msg_id, rfc724_mid) in &hidden_messages { - message::update_msg_state(context, *msg_id, MessageState::InSeen).await?; - imap::markseen_on_imap_table(context, rfc724_mid).await?; - } - + message::markseen_msgs(context, hidden_messages).await?; if noticed_msgs_count == 0 { return Ok(()); } diff --git a/src/message.rs b/src/message.rs index d8dd4846c..afb33f152 100644 --- a/src/message.rs +++ b/src/message.rs @@ -452,6 +452,7 @@ pub struct Message { pub(crate) is_dc_message: MessengerMessage, pub(crate) original_msg_id: MsgId, pub(crate) mime_modified: bool, + pub(crate) chat_visibility: ChatVisibility, pub(crate) chat_blocked: Blocked, pub(crate) location_id: u32, pub(crate) error: Option, @@ -525,6 +526,7 @@ impl Message { m.param AS param, m.hidden AS hidden, m.location_id AS location, + c.archived AS visibility, c.blocked AS blocked FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id @@ -583,6 +585,7 @@ impl Message { param: row.get::<_, String>("param")?.parse().unwrap_or_default(), hidden: row.get("hidden")?, location_id: row.get("location")?, + chat_visibility: row.get::<_, Option<_>>("visibility")?.unwrap_or_default(), chat_blocked: row .get::<_, Option>("blocked")? .unwrap_or_default(), @@ -1837,6 +1840,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> m.param AS param, m.from_id AS from_id, m.rfc724_mid AS rfc724_mid, + m.hidden AS hidden, c.archived AS archived, c.blocked AS blocked FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id @@ -1848,6 +1852,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> let param: Params = row.get::<_, String>("param")?.parse().unwrap_or_default(); let from_id: ContactId = row.get("from_id")?; let rfc724_mid: String = row.get("rfc724_mid")?; + let hidden: bool = row.get("hidden")?; let visibility: ChatVisibility = row.get("archived")?; let blocked: Option = row.get("blocked")?; let ephemeral_timer: EphemeralTimer = row.get("ephemeral_timer")?; @@ -1859,6 +1864,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> param, from_id, rfc724_mid, + hidden, visibility, blocked.unwrap_or_default(), ), @@ -1891,6 +1897,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> curr_param, curr_from_id, curr_rfc724_mid, + curr_hidden, curr_visibility, curr_blocked, ), @@ -1903,8 +1910,9 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> markseen_on_imap_table(context, &curr_rfc724_mid).await?; - // Read receipts for system messages are never sent. These messages have no place to - // display received read receipt anyway. And since their text is locally generated, + // Read receipts for system messages are never sent to contacts. + // These messages have no place to display received read receipt + // anyway. And since their text is locally generated, // quoting them is dangerous as it may contain contact names. E.g., for original message // "Group left by me", a read receipt will quote "Group left by ", and the name can // be a display name stored in address book rather than the name sent in the From field by @@ -1912,25 +1920,35 @@ 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. - if curr_blocked == Blocked::Not + let to_id = if curr_blocked == Blocked::Not && curr_param.get_bool(Param::WantsMdn).unwrap_or_default() && curr_param.get_cmd() == SystemMessage::Unknown && context.should_send_mdns().await? { + Some(curr_from_id) + } else if context.get_config_bool(Config::BccSelf).await? { + Some(ContactId::SELF) + } else { + None + }; + if let Some(to_id) = to_id { context .sql .execute( "INSERT INTO smtp_mdns (msg_id, from_id, rfc724_mid) VALUES(?, ?, ?)", - (id, curr_from_id, curr_rfc724_mid), + (id, to_id, curr_rfc724_mid), ) .await .context("failed to insert into smtp_mdns")?; context.scheduler.interrupt_smtp().await; } - updated_chat_ids.insert(curr_chat_id); + if !curr_hidden { + updated_chat_ids.insert(curr_chat_id); + } } - archived_chats_maybe_noticed |= - curr_state == MessageState::InFresh && curr_visibility == ChatVisibility::Archived; + archived_chats_maybe_noticed |= curr_state == MessageState::InFresh + && !curr_hidden + && curr_visibility == ChatVisibility::Archived; } for updated_chat_id in updated_chat_ids { diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 890c4d8d3..4d30dc033 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -106,6 +106,7 @@ pub struct MimeFactory { /// addresses and OpenPGP keys /// to use for encryption. /// + /// If `Some`, encrypt to self also. /// `None` if the message is not encrypted. encryption_pubkeys: Option>, @@ -234,7 +235,6 @@ impl MimeFactory { encryption_pubkeys = if msg.param.get_bool(Param::ForcePlaintext).unwrap_or(false) { None } else { - // Encrypt, but only to self. Some(Vec::new()) }; } else if chat.is_mailing_list() { @@ -539,7 +539,9 @@ impl MimeFactory { let timestamp = create_smeared_timestamp(context); let addr = contact.get_addr().to_string(); - let encryption_pubkeys = if contact.is_key_contact() { + let encryption_pubkeys = if from_id == ContactId::SELF { + Some(Vec::new()) + } else if contact.is_key_contact() { if let Some(key) = contact.public_key(context).await? { Some(vec![(addr.clone(), key)]) } else { diff --git a/src/mimeparser.rs b/src/mimeparser.rs index c7f47dac1..8a6279623 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -2159,9 +2159,9 @@ pub(crate) struct Report { /// /// It MUST be present if the original message has a Message-ID according to RFC 8098. /// In case we can't find it (shouldn't happen), this is None. - original_message_id: Option, + pub original_message_id: Option, /// Additional-Message-IDs - additional_message_ids: Vec, + pub additional_message_ids: Vec, } /// Delivery Status Notification (RFC 3464, RFC 6533) @@ -2468,13 +2468,7 @@ async fn handle_mdn( timestamp_sent: i64, ) -> Result<()> { if from_id == ContactId::SELF { - warn!( - context, - "Ignoring MDN sent to self, this is a bug on the sender device." - ); - - // This is not an error on our side, - // we successfully ignored an invalid MDN and return `Ok`. + // MDNs to self are handled in receive_imf_inner(). return Ok(()); } diff --git a/src/reaction.rs b/src/reaction.rs index 82a75ed54..4736974eb 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -747,13 +747,23 @@ Content-Disposition: reaction\n\ alice_reaction_msg.id.get_state(&alice).await?, MessageState::InSeen ); - // Reactions don't request MDNs. + // Reactions don't request MDNs, but an MDN to self is sent. assert_eq!( alice .sql .count("SELECT COUNT(*) FROM smtp_mdns", ()) .await?, - 0 + 1 + ); + assert_eq!( + alice + .sql + .count( + "SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?", + (ContactId::SELF,) + ) + .await?, + 1 ); // Alice reacts to own message. diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 0883f067e..9910859f6 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1,5 +1,6 @@ //! Internet Message Format reception pipeline. +use std::cmp; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::iter; use std::sync::LazyLock; @@ -14,7 +15,7 @@ use mailparse::SingleInfo; use num_traits::FromPrimitive; use regex::Regex; -use crate::chat::{self, Chat, ChatId, ChatIdBlocked, save_broadcast_secret}; +use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ChatVisibility, save_broadcast_secret}; use crate::config::Config; use crate::constants::{self, Blocked, Chattype, DC_CHAT_ID_TRASH, EDITED_PREFIX, ShowEmails}; use crate::contact::{self, Contact, ContactId, Origin, mark_contact_id_as_verified}; @@ -960,6 +961,74 @@ UPDATE config SET value=? WHERE keyname='configured_addr' AND value!=?1 // This is a Delta Chat MDN. Mark as read. markseen_on_imap_table(context, rfc724_mid_orig).await?; } + if !mime_parser.incoming && !context.get_config_bool(Config::TeamProfile).await? { + let mut updated_chats = BTreeMap::new(); + let mut archived_chats_maybe_noticed = false; + for report in &mime_parser.mdn_reports { + for msg_rfc724_mid in report + .original_message_id + .iter() + .chain(&report.additional_message_ids) + { + let Some(msg_id) = rfc724_mid_exists(context, msg_rfc724_mid).await? else { + continue; + }; + let Some(msg) = Message::load_from_db_optional(context, msg_id).await? else { + continue; + }; + if msg.state < MessageState::InFresh || msg.state >= MessageState::InSeen { + continue; + } + if !mime_parser.was_encrypted() && msg.get_showpadlock() { + warn!(context, "MDN: Not encrypted. Ignoring."); + continue; + } + message::update_msg_state(context, msg_id, MessageState::InSeen).await?; + if let Err(e) = msg_id.start_ephemeral_timer(context).await { + error!(context, "start_ephemeral_timer for {msg_id}: {e:#}."); + } + if !mime_parser.has_chat_version() { + continue; + } + archived_chats_maybe_noticed |= msg.state < MessageState::InNoticed + && msg.chat_visibility == ChatVisibility::Archived; + updated_chats + .entry(msg.chat_id) + .and_modify(|ts| *ts = cmp::max(*ts, msg.timestamp_sort)) + .or_insert(msg.timestamp_sort); + } + } + for (chat_id, timestamp_sort) in updated_chats { + context + .sql + .execute( + " +UPDATE msgs SET state=? WHERE + state=? AND + hidden=0 AND + chat_id=? AND + timestamp GroupChangesInfo::default(), Chattype::Single => GroupChangesInfo::default(), @@ -1733,7 +1800,10 @@ async fn add_parts( let state = if !mime_parser.incoming { MessageState::OutDelivered - } else if seen || is_mdn || chat_id_blocked == Blocked::Yes || group_changes.silent + } else if seen + || !mime_parser.mdn_reports.is_empty() + || chat_id_blocked == Blocked::Yes + || group_changes.silent // No check for `hidden` because only reactions are such and they should be `InFresh`. { MessageState::InSeen @@ -2251,19 +2321,13 @@ RETURNING id } } - // Normally outgoing MDNs sent by us never appear in mailboxes, but Gmail saves all - // outgoing messages, including MDNs, to the Sent folder. If we detect such saved MDN, - // delete it. - let needs_delete_job = - !mime_parser.incoming && is_mdn && is_dc_message == MessengerMessage::Yes; - Ok(ReceivedMsg { chat_id, state, hidden, sort_timestamp, msg_ids: created_db_entries, - needs_delete_job, + needs_delete_job: false, }) } diff --git a/src/smtp.rs b/src/smtp.rs index 0e8b2358a..45ba6466f 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::warn; +use crate::log::{LogExt, warn}; use crate::message::Message; use crate::message::{self, MsgId}; use crate::mimefactory::MimeFactory; @@ -184,6 +184,9 @@ pub(crate) async fn smtp_send( smtp: &mut Smtp, msg_id: Option, ) -> SendResult { + if recipients.is_empty() { + return SendResult::Success; + } if std::env::var(crate::DCC_MIME_DEBUG).is_ok() { info!(context, "SMTP-sending out mime message:\n{message}"); } @@ -570,17 +573,32 @@ async fn send_mdn_rfc724_mid( additional_rfc724_mids.clone(), ) .await?; + let encrypted = mimefactory.will_be_encrypted(); let rendered_msg = mimefactory.render(context).await?; let body = rendered_msg.message; - let addr = contact.get_addr(); - let recipient = async_smtp::EmailAddress::new(addr.to_string()) - .map_err(|err| format_err!("invalid recipient: {addr} {err:?}"))?; - let recipients = vec![recipient]; + let mut recipients = Vec::new(); + if contact_id != ContactId::SELF { + recipients.push(contact.get_addr().to_string()); + } + 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 => { - info!(context, "Successfully sent MDN for {rfc724_mid}."); + if !recipients.is_empty() { + info!(context, "Successfully sent MDN for {rfc724_mid}."); + } context .sql .transaction(|transaction| { @@ -667,3 +685,34 @@ async fn send_mdn(context: &Context, smtp: &mut Smtp) -> Result { } } } + +/// Adds self-addresses to `recipients` as necessary. +/// This doesn't check `Config::BccSelf`, it should be checked by the caller if needed. +pub(crate) async fn add_self_recipients( + context: &Context, + recipients: &mut Vec, + encrypted: bool, +) -> Result<()> { + // Previous versions of Delta Chat did not send BCC self + // if DeleteServerAfter was set to immediately delete messages + // from the server. This is not the case anymore + // because BCC-self messages are also used to detect + // that message was sent if SMTP server is slow to respond + // and connection is frequently lost + // before receiving status line. NB: This is not a problem for chatmail servers, so `BccSelf` + // disabled by default is fine. + if context.get_config_delete_server_after().await? != Some(0) || !recipients.is_empty() { + // Avoid sending unencrypted messages to all transports, chatmail relays won't accept + // them. Normally the user should have a non-chatmail primary transport to send unencrypted + // messages. + if encrypted { + for addr in context.get_secondary_self_addrs().await? { + recipients.push(addr); + } + } + // `from` must be the last addr, see `receive_imf_inner()` why. + let from = context.get_primary_self_addr().await?; + recipients.push(from); + } + Ok(()) +}