diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index 10fbb77dd..798deda23 100644 --- a/python/tests/test_1_online.py +++ b/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() diff --git a/src/imap.rs b/src/imap.rs index 0e63c291f..bb65c5e12 100644 --- a/src/imap.rs +++ b/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> } } -fn prefetch_get_message_id(headers: &[mailparse::MailHeader]) -> Option { +pub(crate) fn prefetch_get_message_id(headers: &[mailparse::MailHeader]) -> Option { 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. diff --git a/src/receive_imf.rs b/src/receive_imf.rs index e9b885338..423a73149 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -74,7 +74,8 @@ pub async fn receive_imf( seen: bool, ) -> Result> { 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 }