feat: protect the Date header

python/tests/test_1_online.py

@@ -333,7 +333,7 @@ def test_move_works(acfactory):
 
 
 def test_move_avoids_loop(acfactory):
-    """Test that the message is only moved once.
+    """Test that the message is only moved from INBOX to DeltaChat.
 
     This is to avoid busy loop if moved message reappears in the Inbox
     or some scanned folder later.
@@ -344,27 +344,43 @@ def test_move_avoids_loop(acfactory):
     ac1 = acfactory.new_online_configuring_account()
     ac2 = acfactory.new_online_configuring_account(mvbox_move=True)
     acfactory.bring_accounts_online()
+
+    # Create INBOX.DeltaChat folder and make sure
+    # it is detected by full folder scan.
+    ac2.direct_imap.create_folder("INBOX.DeltaChat")
+    ac2.stop_io()
+    ac2.start_io()
+    ac2._evtracker.get_info_contains("Found folders:")  #  Wait until the end of folder scan.
+
     ac1_chat = acfactory.get_accepted_chat(ac1, ac2)
     ac1_chat.send_text("Message 1")
 
     # Message is moved to the DeltaChat folder and downloaded.
     ac2_msg1 = ac2._evtracker.wait_next_incoming_message()
     assert ac2_msg1.text == "Message 1"
 
-    # Move the message to the INBOX again.
+    # Move the message to the INBOX.DeltaChat again.
+    # We assume that test server uses "." as the delimiter.
     ac2.direct_imap.select_folder("DeltaChat")
-    ac2.direct_imap.conn.move(["*"], "INBOX")
+    ac2.direct_imap.conn.move(["*"], "INBOX.DeltaChat")
 
     ac1_chat.send_text("Message 2")
     ac2_msg2 = ac2._evtracker.wait_next_incoming_message()
     assert ac2_msg2.text == "Message 2"
 
-    # Check that Message 1 is still in the INBOX folder
+    # Stop and start I/O to trigger folder scan.
+    ac2.stop_io()
+    ac2.start_io()
+    ac2._evtracker.get_info_contains("Found folders:")  #  Wait until the end of folder scan.
+
+    # Check that Message 1 is still in the INBOX.DeltaChat folder
     # and Message 2 is in the DeltaChat folder.
     ac2.direct_imap.select_folder("INBOX")
-    assert len(ac2.direct_imap.get_all_messages()) == 1
+    assert len(ac2.direct_imap.get_all_messages()) == 0
     ac2.direct_imap.select_folder("DeltaChat")
     assert len(ac2.direct_imap.get_all_messages()) == 1
+    ac2.direct_imap.select_folder("INBOX.DeltaChat")
+    assert len(ac2.direct_imap.get_all_messages()) == 1
 
 
 def test_move_works_on_self_sent(acfactory):
@@ -471,14 +487,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
 
 
@@ -1770,8 +1791,8 @@ def test_group_quote(acfactory, lp):
         (
             "xyz",
             True,
-            "DeltaChat",
-        ),  # ...emails are found in a random folder and moved to DeltaChat
+            "xyz",
+        ),  # ...emails are found in a random folder and downloaded without moving
         (
             "Spam",
             False,

src/config.rs

@@ -735,9 +735,6 @@ impl Context {
             _ => Default::default(),
         };
         self.set_config_internal(key, value).await?;
-        if key == Config::SentboxWatch {
-            self.last_full_folder_scan.lock().await.take();
-        }
         Ok(())
     }
 

src/context.rs

@@ -261,8 +261,6 @@ pub struct InnerContext {
     /// IMAP METADATA.
     pub(crate) metadata: RwLock<Option<ServerMetadata>>,
 
-    pub(crate) last_full_folder_scan: Mutex<Option<tools::Time>>,
-
     /// ID for this `Context` in the current process.
     ///
     /// This allows for multiple `Context`s open in a single process where each context can
@@ -455,7 +453,6 @@ impl Context {
             server_id: RwLock::new(None),
             metadata: RwLock::new(None),
             creation_time: tools::Time::now(),
-            last_full_folder_scan: Mutex::new(None),
             last_error: parking_lot::RwLock::new("".to_string()),
             migration_error: parking_lot::RwLock::new(None),
             debug_logging: std::sync::RwLock::new(None),

src/download.rs

@@ -162,25 +162,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(())
 }
@@ -194,7 +187,6 @@ impl Session {
         &mut self,
         context: &Context,
         folder: &str,
-        uidvalidity: u32,
         uid: u32,
         rfc724_mid: String,
     ) -> Result<()> {
@@ -214,14 +206,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

@@ -581,52 +581,26 @@ impl Imap {
 
             // Determine the target folder where the message should be moved to.
             //
-            // If we have seen the message on the IMAP server before, do not move it.
+            // We only move the messages from the INBOX folder.
             // This is required to avoid infinite MOVE loop on IMAP servers
             // that alias `DeltaChat` folder to other names.
             // For example, some Dovecot servers alias `DeltaChat` folder to `INBOX.DeltaChat`.
-            // In this case Delta Chat configured with `DeltaChat` as the destination folder
-            // would detect messages in the `INBOX.DeltaChat` folder
-            // and try to move them to the `DeltaChat` folder.
-            // Such move to the same folder results in the messages
-            // getting a new UID, so the messages will be detected as new
+            // In this case moving from `INBOX.DeltaChat` to `DeltaChat`
+            // results in the messages getting a new UID,
+            // so the messages will be detected as new
             // in the `INBOX.DeltaChat` folder again.
             let _target;
             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 +681,6 @@ impl Imap {
                     .fetch_many_msgs(
                         context,
                         folder,
-                        uid_validity,
                         uids_fetch_in_batch.split_off(0),
                         &uid_message_ids,
                         fetch_partially,
@@ -1305,7 +1278,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 +1405,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);
@@ -1952,7 +1913,9 @@ pub async fn target_folder_cfg(
 
     if folder_meaning == FolderMeaning::Spam {
         spam_target_folder_cfg(context, headers).await
-    } else if needs_move_to_mvbox(context, headers).await? {
+    } else if folder_meaning == FolderMeaning::Inbox
+        && needs_move_to_mvbox(context, headers).await?
+    {
         Ok(Some(Config::ConfiguredMvboxFolder))
     } else {
         Ok(None)
@@ -2121,7 +2084,9 @@ fn get_folder_meaning_by_name(folder_name: &str) -> FolderMeaning {
     ];
     let lower = folder_name.to_lowercase();
 
-    if SENT_NAMES.iter().any(|s| s.to_lowercase() == lower) {
+    if lower == "inbox" {
+        FolderMeaning::Inbox
+    } else if SENT_NAMES.iter().any(|s| s.to_lowercase() == lower) {
         FolderMeaning::Sent
     } else if SPAM_NAMES.iter().any(|s| s.to_lowercase() == lower) {
         FolderMeaning::Spam
@@ -2280,15 +2245,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/imap/imap_tests.rs

@@ -180,7 +180,7 @@ const COMBINATIONS_ACCEPTED_CHAT: &[(&str, bool, bool, &str)] = &[
     ("Sent", false, false, "Sent"),
     ("Sent", false, true, "Sent"),
     ("Sent", true, false, "Sent"),
-    ("Sent", true, true, "DeltaChat"),
+    ("Sent", true, true, "Sent"),
     ("Spam", false, false, "INBOX"), // Move classical emails in accepted chats from Spam to Inbox, not 100% sure on this, we could also just never move non-chat-msgs
     ("Spam", false, true, "INBOX"),
     ("Spam", true, false, "INBOX"), // Move classical emails in accepted chats from Spam to Inbox, not 100% sure on this, we could also just never move non-chat-msgs
@@ -196,7 +196,7 @@ const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[
     ("Sent", false, false, "Sent"),
     ("Sent", false, true, "Sent"),
     ("Sent", true, false, "Sent"),
-    ("Sent", true, true, "DeltaChat"),
+    ("Sent", true, true, "Sent"),
     ("Spam", false, false, "Spam"),
     ("Spam", false, true, "INBOX"),
     ("Spam", true, false, "Spam"),

src/imap/scan_folders.rs

@@ -18,7 +18,7 @@ impl Imap {
     ) -> Result<bool> {
         // First of all, debounce to once per minute:
         {
-            let mut last_scan = context.last_full_folder_scan.lock().await;
+            let mut last_scan = session.last_full_folder_scan.lock().await;
             if let Some(last_scan) = *last_scan {
                 let elapsed_secs = time_elapsed(&last_scan).as_secs();
                 let debounce_secs = context

src/imap/session.rs

@@ -5,9 +5,11 @@ use anyhow::{Context as _, Result};
 use async_imap::types::Mailbox;
 use async_imap::Session as ImapSession;
 use futures::TryStreamExt;
+use tokio::sync::Mutex;
 
 use crate::imap::capabilities::Capabilities;
 use crate::net::session::SessionStream;
+use crate::tools;
 
 /// Prefetch:
 /// - Message-ID to check if we already have the message.
@@ -40,6 +42,8 @@ pub(crate) struct Session {
 
     pub selected_folder_needs_expunge: bool,
 
+    pub(crate) last_full_folder_scan: Mutex<Option<tools::Time>>,
+
     /// True if currently selected folder has new messages.
     ///
     /// Should be false if no folder is currently selected.
@@ -71,6 +75,7 @@ impl Session {
             selected_folder: None,
             selected_mailbox: None,
             selected_folder_needs_expunge: false,
+            last_full_folder_scan: Mutex::new(None),
             new_mail: false,
         }
     }