feat: protect the Date header

We do not try to delete resent messages
anymore. Previously resent messages
were distinguised by having duplicate Message-ID,
but future Date, but now we need to download
the message before we even see the Date.
We now move the message to the destination folder
but do not fetch it.

It may not be a good idea to delete
the duplicate in multi-device setups anyway,
because the device which has a message
may delete the duplicate of a message
the other device missed.

Author: link2xt

python/tests/test_1_online.py

@@ -471,14 +471,19 @@ def test_resend_message(acfactory, lp):
     lp.sec("ac2: receive message")
     msg_in = ac2._evtracker.wait_next_incoming_message()
     assert msg_in.text == "message"
-    chat2 = msg_in.chat
-    chat2_msg_cnt = len(chat2.get_messages())
 
     lp.sec("ac1: resend message")
     ac1.resend_messages([msg_in])
 
-    lp.sec("ac2: check that message is deleted")
-    ac2._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_DELETED")
+    lp.sec("ac1: send another message")
+    chat1.send_text("another message")
+
+    lp.sec("ac2: receive another message")
+    msg_in = ac2._evtracker.wait_next_incoming_message()
+    assert msg_in.text == "another message"
+    chat2 = msg_in.chat
+    chat2_msg_cnt = len(chat2.get_messages())
+
     assert len(chat2.get_messages()) == chat2_msg_cnt
 
 

src/download.rs

@@ -161,25 +161,18 @@ pub(crate) async fn download_msg(
             |row| {
                 let server_uid: u32 = row.get(0)?;
                 let server_folder: String = row.get(1)?;
-                let uidvalidity: u32 = row.get(2)?;
-                Ok((server_uid, server_folder, uidvalidity))
+                Ok((server_uid, server_folder))
             },
         )
         .await?;
 
-    let Some((server_uid, server_folder, uidvalidity)) = row else {
+    let Some((server_uid, server_folder)) = row else {
         // No IMAP record found, we don't know the UID and folder.
         return Err(anyhow!("Call download_full() again to try over."));
     };
 
     session
-        .fetch_single_msg(
-            context,
-            &server_folder,
-            uidvalidity,
-            server_uid,
-            msg.rfc724_mid.clone(),
-        )
+        .fetch_single_msg(context, &server_folder, server_uid, msg.rfc724_mid.clone())
         .await?;
     Ok(())
 }
@@ -193,7 +186,6 @@ impl Session {
         &mut self,
         context: &Context,
         folder: &str,
-        uidvalidity: u32,
         uid: u32,
         rfc724_mid: String,
     ) -> Result<()> {
@@ -213,14 +205,7 @@ impl Session {
         let mut uid_message_ids: BTreeMap<u32, String> = BTreeMap::new();
         uid_message_ids.insert(uid, rfc724_mid);
         let (last_uid, _received) = self
-            .fetch_many_msgs(
-                context,
-                folder,
-                uidvalidity,
-                vec![uid],
-                &uid_message_ids,
-                false,
-            )
+            .fetch_many_msgs(context, folder, vec![uid], &uid_message_ids, false)
             .await?;
         if last_uid.is_none() {
             bail!("Failed to fetch UID {uid}");

src/imap.rs

@@ -595,38 +595,14 @@ impl Imap {
             let target = if let Some(message_id) = &message_id {
                 let msg_info =
                     message::rfc724_mid_exists_ex(context, message_id, "deleted=1").await?;
-                let delete = if let Some((_, _, true)) = msg_info {
+                let delete = if let Some((_, true)) = msg_info {
                     info!(context, "Deleting locally deleted message {message_id}.");
                     true
-                } else if let Some((_, ts_sent_old, _)) = msg_info {
-                    let is_chat_msg = headers.get_header_value(HeaderDef::ChatVersion).is_some();
-                    let ts_sent = headers
-                        .get_header_value(HeaderDef::Date)
-                        .and_then(|v| mailparse::dateparse(&v).ok())
-                        .unwrap_or_default();
-                    let is_dup = is_dup_msg(is_chat_msg, ts_sent, ts_sent_old);
-                    if is_dup {
-                        info!(context, "Deleting duplicate message {message_id}.");
-                    }
-                    is_dup
                 } else {
                     false
                 };
                 if delete {
                     &delete_target
-                } else if context
-                    .sql
-                    .exists(
-                        "SELECT COUNT (*) FROM imap WHERE rfc724_mid=?",
-                        (message_id,),
-                    )
-                    .await?
-                {
-                    info!(
-                        context,
-                        "Not moving the message {} that we have seen before.", &message_id
-                    );
-                    folder
                 } else {
                     _target = target_folder(context, folder, folder_meaning, &headers).await?;
                     &_target
@@ -707,7 +683,6 @@ impl Imap {
                     .fetch_many_msgs(
                         context,
                         folder,
-                        uid_validity,
                         uids_fetch_in_batch.split_off(0),
                         &uid_message_ids,
                         fetch_partially,
@@ -1305,7 +1280,6 @@ impl Session {
         &mut self,
         context: &Context,
         folder: &str,
-        uidvalidity: u32,
         request_uids: Vec<u32>,
         uid_message_ids: &BTreeMap<u32, String>,
         fetch_partially: bool,
@@ -1433,18 +1407,7 @@ impl Session {
                     context,
                     "Passing message UID {} to receive_imf().", request_uid
                 );
-                match receive_imf_inner(
-                    context,
-                    folder,
-                    uidvalidity,
-                    request_uid,
-                    rfc724_mid,
-                    body,
-                    is_seen,
-                    partial,
-                )
-                .await
-                {
+                match receive_imf_inner(context, rfc724_mid, body, is_seen, partial).await {
                     Ok(received_msg) => {
                         if let Some(m) = received_msg {
                             received_msgs.push(m);
@@ -2280,15 +2243,6 @@ pub(crate) async fn prefetch_should_download(
     Ok(should_download)
 }
 
-/// Returns whether a message is a duplicate (resent message).
-pub(crate) fn is_dup_msg(is_chat_msg: bool, ts_sent: i64, ts_sent_old: i64) -> bool {
-    // If the existing message has timestamp_sent == 0, that means we don't know its actual sent
-    // timestamp, so don't delete the new message. E.g. outgoing messages have zero timestamp_sent
-    // because they are stored to the db before sending. Also consider as duplicates only messages
-    // with greater timestamp to avoid deleting both messages in a multi-device setting.
-    is_chat_msg && ts_sent_old != 0 && ts_sent > ts_sent_old
-}
-
 /// Marks messages in `msgs` table as seen, searching for them by UID.
 ///
 /// Returns updated chat ID if any message was marked as seen.

src/message.rs

@@ -1319,7 +1319,7 @@ impl Message {
     /// `References` header is not taken into account.
     pub async fn parent(&self, context: &Context) -> Result<Option<Message>> {
         if let Some(in_reply_to) = &self.in_reply_to {
-            if let Some((msg_id, _ts_sent)) = rfc724_mid_exists(context, in_reply_to).await? {
+            if let Some(msg_id) = rfc724_mid_exists(context, in_reply_to).await? {
                 let msg = Message::load_from_db_optional(context, msg_id).await?;
                 return Ok(msg);
             }
@@ -2139,13 +2139,13 @@ pub async fn estimate_deletion_cnt(
 pub(crate) async fn rfc724_mid_exists(
     context: &Context,
     rfc724_mid: &str,
-) -> Result<Option<(MsgId, i64)>> {
+) -> Result<Option<MsgId>> {
     Ok(rfc724_mid_exists_ex(context, rfc724_mid, "1")
         .await?
-        .map(|(id, ts_sent, _)| (id, ts_sent)))
+        .map(|(id, _)| id))
 }
 
-/// Returns [MsgId] and "sent" timestamp of the most recent message with given `rfc724_mid`
+/// Returns [MsgId] of the most recent message with given `rfc724_mid`
 /// (Message-ID header) and bool `expr` result if such messages exists in the db.
 ///
 /// * `expr`: SQL expression additionally passed into `SELECT`. Evaluated to `true` iff it is true
@@ -2154,7 +2154,7 @@ pub(crate) async fn rfc724_mid_exists_ex(
     context: &Context,
     rfc724_mid: &str,
     expr: &str,
-) -> Result<Option<(MsgId, i64, bool)>> {
+) -> Result<Option<(MsgId, bool)>> {
     let rfc724_mid = rfc724_mid.trim_start_matches('<').trim_end_matches('>');
     if rfc724_mid.is_empty() {
         warn!(context, "Empty rfc724_mid passed to rfc724_mid_exists");
@@ -2172,9 +2172,8 @@ pub(crate) async fn rfc724_mid_exists_ex(
             (rfc724_mid,),
             |row| {
                 let msg_id: MsgId = row.get(0)?;
-                let timestamp_sent: i64 = row.get(1)?;
                 let expr_res: bool = row.get(2)?;
-                Ok((msg_id, timestamp_sent, expr_res))
+                Ok((msg_id, expr_res))
             },
         )
         .await?;
@@ -2194,7 +2193,7 @@ pub(crate) async fn get_by_rfc724_mids(
 ) -> Result<Option<Message>> {
     let mut latest = None;
     for id in mids.iter().rev() {
-        let Some((msg_id, _)) = rfc724_mid_exists(context, id).await? else {
+        let Some(msg_id) = rfc724_mid_exists(context, id).await? else {
             continue;
         };
         let Some(msg) = Message::load_from_db_optional(context, msg_id).await? else {

src/mimefactory.rs

@@ -871,6 +871,14 @@ impl MimeFactory {
                 } else {
                     unprotected_headers.push(header.clone());
                 }
+            } else if is_encrypted && header_name == "date" {
+                protected_headers.push(header.clone());
+                unprotected_headers.push((
+                    "Date",
+                    mail_builder::headers::HeaderType::Raw(
+                        "Thu, 01 Jan 1970 00:00:00 +0000".into(),
+                    ),
+                ));
             } else if is_encrypted {
                 protected_headers.push(header.clone());
 
@@ -881,8 +889,7 @@ impl MimeFactory {
                             mail_builder::headers::raw::Raw::new("[...]").into(),
                         ));
                     }
-                    "date"
-                    | "in-reply-to"
+                    "in-reply-to"
                     | "references"
                     | "auto-submitted"
                     | "chat-version"

src/reaction.rs

@@ -277,7 +277,7 @@ pub(crate) async fn set_msg_reaction(
     reaction: Reaction,
     is_incoming_fresh: bool,
 ) -> Result<()> {
-    if let Some((msg_id, _)) = rfc724_mid_exists(context, in_reply_to).await? {
+    if let Some(msg_id) = rfc724_mid_exists(context, in_reply_to).await? {
         set_msg_id_reaction(context, msg_id, chat_id, contact_id, timestamp, &reaction).await?;
 
         if is_incoming_fresh

src/receive_imf.rs

@@ -16,6 +16,7 @@ use crate::aheader::EncryptPreference;
 use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ProtectionStatus};
 use crate::config::Config;
 use crate::constants::{Blocked, Chattype, ShowEmails, DC_CHAT_ID_TRASH, EDITED_PREFIX};
+use crate::contact;
 use crate::contact::{Contact, ContactId, Origin};
 use crate::context::Context;
 use crate::debug_logging::maybe_set_logging_xdc_inner;
@@ -41,7 +42,6 @@ use crate::stock_str;
 use crate::sync::Sync::*;
 use crate::tools::{self, buf_compress, remove_subject_prefix};
 use crate::{chatlist_events, location};
-use crate::{contact, imap};
 
 /// This is the struct that is returned after receiving one email (aka MIME message).
 ///
@@ -85,7 +85,7 @@ pub async fn receive_imf(
 ) -> Result<Option<ReceivedMsg>> {
     let mail = mailparse::parse_mail(imf_raw).context("can't parse mail")?;
     let rfc724_mid =
-        imap::prefetch_get_message_id(&mail.headers).unwrap_or_else(imap::create_message_id);
+        crate::imap::prefetch_get_message_id(&mail.headers).unwrap_or_else(imap::create_message_id);
     if let Some(download_limit) = context.download_limit().await? {
         let download_limit: usize = download_limit.try_into()?;
         if imf_raw.len() > download_limit {
@@ -117,17 +117,7 @@ pub(crate) async fn receive_imf_from_inbox(
     seen: bool,
     is_partial_download: Option<u32>,
 ) -> Result<Option<ReceivedMsg>> {
-    receive_imf_inner(
-        context,
-        "INBOX",
-        0,
-        0,
-        rfc724_mid,
-        imf_raw,
-        seen,
-        is_partial_download,
-    )
-    .await
+    receive_imf_inner(context, rfc724_mid, imf_raw, seen, is_partial_download).await
 }
 
 /// Inserts a tombstone into `msgs` table
@@ -159,12 +149,8 @@ async fn insert_tombstone(context: &Context, rfc724_mid: &str) -> Result<MsgId>
 /// If `is_partial_download` is set, it contains the full message size in bytes.
 /// Do not confuse that with `replace_msg_id` that will be set when the full message is loaded
 /// later.
-#[expect(clippy::too_many_arguments)]
 pub(crate) async fn receive_imf_inner(
     context: &Context,
-    folder: &str,
-    uidvalidity: u32,
-    uid: u32,
     rfc724_mid: &str,
     imf_raw: &[u8],
     seen: bool,
@@ -228,7 +214,7 @@ pub(crate) async fn receive_imf_inner(
     // check, if the mail is already in our database.
     // make sure, this check is done eg. before securejoin-processing.
     let (replace_msg_id, replace_chat_id);
-    if let Some((old_msg_id, _)) = message::rfc724_mid_exists(context, rfc724_mid).await? {
+    if let Some(old_msg_id) = message::rfc724_mid_exists(context, rfc724_mid).await? {
         if is_partial_download.is_some() {
             // Should never happen, see imap::prefetch_should_download(), but still.
             info!(
@@ -252,24 +238,9 @@ pub(crate) async fn receive_imf_inner(
     } else {
         replace_msg_id = if rfc724_mid_orig == rfc724_mid {
             None
-        } else if let Some((old_msg_id, old_ts_sent)) =
+        } else if let Some(old_msg_id) =
             message::rfc724_mid_exists(context, rfc724_mid_orig).await?
         {
-            if imap::is_dup_msg(
-                mime_parser.has_chat_version(),
-                mime_parser.timestamp_sent,
-                old_ts_sent,
-            ) {
-                info!(context, "Deleting duplicate message {rfc724_mid_orig}.");
-                let target = context.get_delete_msgs_target().await?;
-                context
-                    .sql
-                    .execute(
-                        "UPDATE imap SET target=? WHERE folder=? AND uidvalidity=? AND uid=?",
-                        (target, folder, uidvalidity, uid),
-                    )
-                    .await?;
-            }
             Some(old_msg_id)
         } else {
             None
@@ -1466,7 +1437,7 @@ async fn add_parts(
         chat_id = DC_CHAT_ID_TRASH;
         match mime_parser.get_header(HeaderDef::InReplyTo) {
             Some(in_reply_to) => match rfc724_mid_exists(context, in_reply_to).await? {
-                Some((instance_id, _ts_sent)) => {
+                Some(instance_id) => {
                     if let Err(err) =
                         add_gossip_peer_from_header(context, instance_id, node_addr).await
                     {
@@ -1766,7 +1737,7 @@ async fn handle_edit_delete(
     from_id: ContactId,
 ) -> Result<bool> {
     if let Some(rfc724_mid) = mime_parser.get_header(HeaderDef::ChatEdit) {
-        if let Some((original_msg_id, _)) = rfc724_mid_exists(context, rfc724_mid).await? {
+        if let Some(original_msg_id) = rfc724_mid_exists(context, rfc724_mid).await? {
             if let Some(mut original_msg) =
                 Message::load_from_db_optional(context, original_msg_id).await?
             {
@@ -1809,9 +1780,7 @@ async fn handle_edit_delete(
 
                 let rfc724_mid_vec: Vec<&str> = rfc724_mid_list.split_whitespace().collect();
                 for rfc724_mid in rfc724_mid_vec {
-                    if let Some((msg_id, _)) =
-                        message::rfc724_mid_exists(context, rfc724_mid).await?
-                    {
+                    if let Some(msg_id) = message::rfc724_mid_exists(context, rfc724_mid).await? {
                         if let Some(msg) = Message::load_from_db_optional(context, msg_id).await? {
                             if msg.from_id == from_id {
                                 message::delete_msg_locally(context, &msg).await?;
@@ -3184,7 +3153,7 @@ async fn get_previous_message(
 ) -> Result<Option<Message>> {
     if let Some(field) = mime_parser.get_header(HeaderDef::References) {
         if let Some(rfc724mid) = parse_message_ids(field).last() {
-            if let Some((msg_id, _)) = rfc724_mid_exists(context, rfc724mid).await? {
+            if let Some(msg_id) = rfc724_mid_exists(context, rfc724mid).await? {
                 return Message::load_from_db_optional(context, msg_id).await;
             }
         }

src/receive_imf/receive_imf_tests.rs

@@ -1973,7 +1973,7 @@ async fn check_alias_reply(from_dc: bool, chat_request: bool, group_request: boo
         .await
         .unwrap();
 
-    let (msg_id, _) = rfc724_mid_exists(&claire, "non-dc-1@example.org")
+    let msg_id = rfc724_mid_exists(&claire, "non-dc-1@example.org")
         .await
         .unwrap()
         .unwrap();