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

@@ -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?
+    if context.get_config_bool(Config::BccSelf).await?
-        || msg.param.get_cmd() == SystemMessage::AutocryptSetupMessage)
+        || msg.param.get_cmd() == SystemMessage::AutocryptSetupMessage
-        && (context.get_config_delete_server_after().await? != Some(0) || !recipients.is_empty())
     {
-        // Avoid sending unencrypted messages to all transports, chatmail relays won't accept
+        smtp::add_self_recipients(context, &mut recipients, needs_encryption).await?;
-        // 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);
     }
 
     // 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)
-                    Ok((msg_id, rfc724_mid))
                 },
             )
             .await?;
-        for (msg_id, rfc724_mid) in &hidden_messages {
+        message::markseen_msgs(context, hidden_messages).await?;
-            message::update_msg_state(context, *msg_id, MessageState::InSeen).await?;
-            imap::markseen_on_imap_table(context, rfc724_mid).await?;
-        }
-
         if noticed_msgs_count == 0 {
             return Ok(());
         }

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<String>,
@@ -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>>("blocked")?
                             .unwrap_or_default(),
@@ -1837,6 +1840,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> 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<MsgId>) -> 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<Blocked> = 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<MsgId>) -> 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<MsgId>) -> 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<MsgId>) -> Result<()>
 
             markseen_on_imap_table(context, &curr_rfc724_mid).await?;
 
-            // Read receipts for system messages are never sent. These messages have no place to
+            // Read receipts for system messages are never sent to contacts.
-            // display received read receipt anyway.  And since their text is locally generated,
+            // 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 <name>", 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<MsgId>) -> 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;
             }
+            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 {

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<Vec<(String, SignedPublicKey)>>,
 
@@ -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 {

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<String>,
+    pub original_message_id: Option<String>,
     /// Additional-Message-IDs
-    additional_message_ids: Vec<String>,
+    pub additional_message_ids: Vec<String>,
 }
 
 /// 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!(
+        // MDNs to self are handled in receive_imf_inner().
-            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`.
         return Ok(());
     }
 

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.

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<?",
+                        (
+                            MessageState::InNoticed,
+                            MessageState::InFresh,
+                            chat_id,
+                            timestamp_sort,
+                        ),
+                    )
+                    .await
+                    .context("UPDATE msgs.state")?;
+                // Removes all notifications for the chat in UIs. Ideally should be under
+                // `if chat_id.get_fresh_msg_cnt(context).await? == 0`, but this causes a not
+                // updated profile badge counter in the UIs as of v2.35.0. Removed notifications for
+                // new messages is an already existing and known problem, so let's emit the event
+                // unconditionally for now.
+                context.emit_event(EventType::MsgsNoticed(chat_id));
+                chatlist_events::emit_chatlist_item_changed(context, chat_id);
+            }
+            if archived_chats_maybe_noticed {
+                context.on_archived_chats_maybe_noticed();
+            }
+        }
     }
 
     if mime_parser.is_call() {
@@ -1696,8 +1765,6 @@ async fn add_parts(
     }
 
     let is_location_kml = mime_parser.location_kml.is_some();
-    let is_mdn = !mime_parser.mdn_reports.is_empty();
-
     let mut group_changes = match chat.typ {
         _ if chat.id.is_special() => 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,
     })
 }
 

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<MsgId>,
 ) -> 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 mut recipients = Vec::new();
-    let recipient = async_smtp::EmailAddress::new(addr.to_string())
+    if contact_id != ContactId::SELF {
-        .map_err(|err| format_err!("invalid recipient: {addr} {err:?}"))?;
+        recipients.push(contact.get_addr().to_string());
-    let recipients = vec![recipient];
+    }
+    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}.");
+            }
             context
                 .sql
                 .transaction(|transaction| {
@@ -667,3 +685,34 @@ async fn send_mdn(context: &Context, smtp: &mut Smtp) -> Result<bool> {
         }
     }
 }
+
+/// 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<String>,
+    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(())
+}