fix(imap): avoid IMAP move loops when DeltaChat folder is aliased

python/tests/test_1_online.py

@@ -353,6 +353,41 @@ def test_move_works(acfactory):
     assert ev.data2 > dc.const.DC_CHAT_ID_LAST_SPECIAL
 
 
+def test_move_avoids_loop(acfactory):
+    """Test that the message is only moved once.
+
+    This is to avoid busy loop if moved message reappears in the Inbox
+    or some scanned folder later.
+    For example, this happens on servers that alias `INBOX.DeltaChat` to `DeltaChat` folder,
+    so the message moved to `DeltaChat` appears as a new message in the `INBOX.DeltaChat` folder.
+    We do not want to move this message from `INBOX.DeltaChat` to `DeltaChat` again.
+    """
+    ac1 = acfactory.new_online_configuring_account()
+    ac2 = acfactory.new_online_configuring_account(mvbox_move=True)
+    acfactory.bring_accounts_online()
+    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.
+    ac2.direct_imap.select_folder("DeltaChat")
+    ac2.direct_imap.conn.move(["*"], "INBOX")
+
+    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
+    # and Message 2 is in the DeltaChat folder.
+    ac2.direct_imap.select_folder("INBOX")
+    assert len(ac2.direct_imap.get_all_messages()) == 1
+    ac2.direct_imap.select_folder("DeltaChat")
+    assert len(ac2.direct_imap.get_all_messages()) == 1
+
+
 def test_move_works_on_self_sent(acfactory):
     ac1 = acfactory.new_online_configuring_account(mvbox_move=True)
     ac2 = acfactory.new_online_configuring_account()

src/imap.rs

@@ -747,9 +747,51 @@ impl Imap {
                 }
             };
 
-            // Get the Message-ID or generate a fake one to identify the message in the database.
-            let message_id = prefetch_get_or_create_message_id(&headers);
-            let target = target_folder(context, folder, folder_meaning, &headers).await?;
+            let message_id = prefetch_get_message_id(&headers);
+
+            // 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.
+            // 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 the `INBOX.DeltaChat` folder again.
+            let target = if let Some(message_id) = &message_id {
+                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.to_string()
+                } else {
+                    target_folder(context, folder, folder_meaning, &headers).await?
+                }
+            } else {
+                // Do not move the messages without Message-ID.
+                // We cannot reliably determine if we have seen them before,
+                // so it is safer not to move them.
+                warn!(
+                    context,
+                    "Not moving the message that does not have a Message-ID."
+                );
+                folder.to_string()
+            };
+
+            // Generate a fake Message-ID to identify the message in the database
+            // if the message has no real Message-ID.
+            let message_id = message_id.unwrap_or_else(create_message_id);
 
             context
                 .sql
@@ -2060,16 +2102,15 @@ fn get_fetch_headers(prefetch_msg: &Fetch) -> Result<Vec<mailparse::MailHeader>>
     }
 }
 
-fn prefetch_get_message_id(headers: &[mailparse::MailHeader]) -> Option<String> {
+pub(crate) fn prefetch_get_message_id(headers: &[mailparse::MailHeader]) -> Option<String> {
     headers
         .get_header_value(HeaderDef::XMicrosoftOriginalMessageId)
         .or_else(|| headers.get_header_value(HeaderDef::MessageId))
         .and_then(|msgid| mimeparser::parse_message_id(&msgid).ok())
 }
 
-pub(crate) fn prefetch_get_or_create_message_id(headers: &[mailparse::MailHeader]) -> String {
-    prefetch_get_message_id(headers)
-        .unwrap_or_else(|| format!("{}{}", GENERATED_PREFIX, create_id()))
+pub(crate) fn create_message_id() -> String {
+    format!("{}{}", GENERATED_PREFIX, create_id())
 }
 
 /// Returns chat by prefetched headers.

src/receive_imf.rs

@@ -74,7 +74,8 @@ pub async fn receive_imf(
     seen: bool,
 ) -> Result<Option<ReceivedMsg>> {
     let mail = parse_mail(imf_raw).context("can't parse mail")?;
-    let rfc724_mid = imap::prefetch_get_or_create_message_id(&mail.headers);
+    let rfc724_mid =
+        imap::prefetch_get_message_id(&mail.headers).unwrap_or_else(imap::create_message_id);
     receive_imf_inner(context, &rfc724_mid, imf_raw, seen, None, false).await
 }