Merge branch 'stable'

deltachat-ffi/deltachat.h

@@ -420,11 +420,11 @@ char*           dc_get_blobdir               (const dc_context_t* context);
  *                    0=watch all folders normally (default)
  *                    changes require restarting IO by calling dc_stop_io() and then dc_start_io().
  * - `show_emails`  = DC_SHOW_EMAILS_OFF (0)=
- *                    show direct replies to chats only (default),
+ *                    show direct replies to chats only,
  *                    DC_SHOW_EMAILS_ACCEPTED_CONTACTS (1)=
  *                    also show all mails of confirmed contacts,
  *                    DC_SHOW_EMAILS_ALL (2)=
- *                    also show mails of unconfirmed contacts.
+ *                    also show mails of unconfirmed contacts (default).
  * - `key_gen_type` = DC_KEY_GEN_DEFAULT (0)=
  *                    generate recommended key type (default),
  *                    DC_KEY_GEN_RSA2048 (1)=

python/examples/group_tracking.py

@@ -32,25 +32,13 @@ class GroupTrackingPlugin:
 
     @account_hookimpl
     def ac_member_added(self, chat, contact, actor, message):
-        print(
+        print(f"ac_member_added {contact.addr} to chat {chat.id} from {actor or message.get_sender_contact().addr}")
-            "ac_member_added {} to chat {} from {}".format(
-                contact.addr,
-                chat.id,
-                actor or message.get_sender_contact().addr,
-            ),
-        )
         for member in chat.get_contacts():
             print(f"chat member: {member.addr}")
 
     @account_hookimpl
     def ac_member_removed(self, chat, contact, actor, message):
-        print(
+        print(f"ac_member_removed {contact.addr} from chat {chat.id} by {actor or message.get_sender_contact().addr}")
-            "ac_member_removed {} from chat {} by {}".format(
-                contact.addr,
-                chat.id,
-                actor or message.get_sender_contact().addr,
-            ),
-        )
 
 
 def main(argv=None):

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_message_id(&headers);
-            let message_id = prefetch_get_or_create_message_id(&headers);
+
-            let target = target_folder(context, folder, folder_meaning, &headers).await?;
+            // 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 {
+pub(crate) fn create_message_id() -> String {
-    prefetch_get_message_id(headers)
+    format!("{}{}", GENERATED_PREFIX, create_id())
-        .unwrap_or_else(|| 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
 }
 
@@ -1193,8 +1194,9 @@ SET rfc724_mid=excluded.rfc724_mid, chat_id=excluded.chat_id,
     mime_compressed=excluded.mime_compressed, mime_in_reply_to=excluded.mime_in_reply_to,
     mime_references=excluded.mime_references, mime_modified=excluded.mime_modified, error=excluded.error, ephemeral_timer=excluded.ephemeral_timer,
     ephemeral_timestamp=excluded.ephemeral_timestamp, download_state=excluded.download_state, hop_info=excluded.hop_info
+RETURNING id
 "#)?;
-                stmt.execute(params![
+                let row_id: MsgId = stmt.query_row(params![
                     replace_msg_id,
                     rfc724_mid,
                     if trash { DC_CHAT_ID_TRASH } else { chat_id },
@@ -1233,8 +1235,12 @@ SET rfc724_mid=excluded.rfc724_mid, chat_id=excluded.chat_id,
                         DownloadState::Done
                     },
                     mime_parser.hop_info
-                ])?;
+                ],
-                let row_id = conn.last_insert_rowid();
+                |row| {
+                    let msg_id: MsgId = row.get(0)?;
+                    Ok(msg_id)
+                }
+                )?;
                 Ok(row_id)
             })
             .await?;
@@ -1243,7 +1249,8 @@ SET rfc724_mid=excluded.rfc724_mid, chat_id=excluded.chat_id,
         // afterwards insert additional parts.
         replace_msg_id = None;
 
-        created_db_entries.push(MsgId::new(u32::try_from(row_id)?));
+        debug_assert!(!row_id.is_special());
+        created_db_entries.push(row_id);
     }
 
     // check all parts whether they contain a new logging webxdc

src/webxdc.rs

@@ -1177,7 +1177,7 @@ mod tests {
         assert_eq!(bob_instance.download_state, DownloadState::Available);
 
         // Bob downloads instance, updates should be assigned correctly
-        receive_imf_inner(
+        let received_msg = receive_imf_inner(
             &bob,
             &alice_instance.rfc724_mid,
             sent1.payload().as_bytes(),
@@ -1185,7 +1185,9 @@ mod tests {
             None,
             false,
         )
-        .await?;
+        .await?
+        .unwrap();
+        assert_eq!(*received_msg.msg_ids.get(0).unwrap(), bob_instance.id);
         let bob_instance = bob.get_last_msg().await;
         assert_eq!(bob_instance.viewtype, Viewtype::Webxdc);
         assert_eq!(bob_instance.download_state, DownloadState::Done);