feat: Don't copy References and In-Reply-To to outer headers

deltachat-rpc-client/tests/test_something.py

@@ -660,14 +660,12 @@ def test_download_limit_chat_assignment(acfactory, tmp_path, n_accounts):
         contact = alice.create_contact(account)
         alice_group.add_contact(contact)
 
-    if n_accounts == 2:
-        bob_chat_alice = bob.create_chat(alice)
+    bob_chat_alice = bob.create_chat(alice)
     bob.set_config("download_limit", str(download_limit))
 
     alice_group.send_text("hi")
     snapshot = bob.wait_for_incoming_msg().get_snapshot()
     assert snapshot.text == "hi"
-    bob_group = snapshot.chat
 
     path = tmp_path / "large"
     path.write_bytes(os.urandom(download_limit + 1))
@@ -677,15 +675,7 @@ def test_download_limit_chat_assignment(acfactory, tmp_path, n_accounts):
         alice_group.send_file(str(path))
         snapshot = bob.wait_for_incoming_msg().get_snapshot()
         assert snapshot.download_state == DownloadState.AVAILABLE
-        if n_accounts > 2:
-            assert snapshot.chat == bob_group
-        else:
-            # Group contains only Alice and Bob,
-            # so partially downloaded messages are
-            # hard to distinguish from private replies to group messages.
-            #
-            # Message may be a private reply, so we assign it to 1:1 chat with Alice.
-            assert snapshot.chat == bob_chat_alice
+        assert snapshot.chat == bob_chat_alice
 
 
 def test_markseen_contact_request(acfactory):

src/imap.rs

@@ -27,7 +27,7 @@ use crate::calls::{create_fallback_ice_servers, create_ice_servers_from_metadata
 use crate::chat::{self, ChatId, ChatIdBlocked, add_device_msg};
 use crate::chatlist_events;
 use crate::config::Config;
-use crate::constants::{self, Blocked, Chattype, ShowEmails};
+use crate::constants::{self, Blocked, ShowEmails};
 use crate::contact::{Contact, ContactId, Modifier, Origin};
 use crate::context::Context;
 use crate::events::EventType;
@@ -1964,21 +1964,24 @@ impl Session {
     }
 }
 
+fn is_encrypted(headers: &[mailparse::MailHeader<'_>]) -> bool {
+    let content_type = headers.get_header_value(HeaderDef::ContentType);
+    let content_type = content_type.as_ref();
+    let res = content_type.is_some_and(|v| v.contains("multipart/encrypted"));
+    // Some MUAs use "multipart/mixed", look also at Subject in this case. We can't only look at
+    // Subject as it's not mandatory (<https://datatracker.ietf.org/doc/html/rfc5322#section-3.6>)
+    // and may be user-formed.
+    res || content_type.is_some_and(|v| v.contains("multipart/mixed"))
+        && headers
+            .get_header_value(HeaderDef::Subject)
+            .is_some_and(|v| v == "..." || v == "[...]")
+}
+
 async fn should_move_out_of_spam(
     context: &Context,
     headers: &[mailparse::MailHeader<'_>],
 ) -> Result<bool> {
-    if headers.get_header_value(HeaderDef::ChatVersion).is_some() {
-        // If this is a chat message (i.e. has a ChatVersion header), then this might be
-        // a securejoin message. We can't find out at this point as we didn't prefetch
-        // the SecureJoin header. So, we always move chat messages out of Spam.
-        // Two possibilities to change this would be:
-        // 1. Remove the `&& !context.is_spam_folder(folder).await?` check from
-        // `fetch_new_messages()`, and then let `receive_imf()` check
-        // if it's a spam message and should be hidden.
-        // 2. Or add a flag to the ChatVersion header that this is a securejoin
-        // request, and return `true` here only if the message has this flag.
-        // `receive_imf()` can then check if the securejoin request is valid.
+    if headers.get_header_value(HeaderDef::SecureJoin).is_some() || is_encrypted(headers) {
         return Ok(true);
     }
 
@@ -2037,7 +2040,8 @@ async fn spam_target_folder_cfg(
         return Ok(None);
     }
 
-    if needs_move_to_mvbox(context, headers).await?
+    if is_encrypted(headers) && context.get_config_bool(Config::MvboxMove).await?
+        || needs_move_to_mvbox(context, headers).await?
         // If OnlyFetchMvbox is set, we don't want to move the message to
         // the inbox where we wouldn't fetch it again:
         || context.get_config_bool(Config::OnlyFetchMvbox).await?
@@ -2090,20 +2094,6 @@ async fn needs_move_to_mvbox(
     context: &Context,
     headers: &[mailparse::MailHeader<'_>],
 ) -> Result<bool> {
-    let has_chat_version = headers.get_header_value(HeaderDef::ChatVersion).is_some();
-    if !context.get_config_bool(Config::IsChatmail).await?
-        && has_chat_version
-        && headers
-            .get_header_value(HeaderDef::AutoSubmitted)
-            .filter(|val| val.eq_ignore_ascii_case("auto-generated"))
-            .is_some()
-    {
-        if let Some(from) = mimeparser::get_from(headers) {
-            if context.is_self_addr(&from.addr).await? {
-                return Ok(true);
-            }
-        }
-    }
     if !context.get_config_bool(Config::MvboxMove).await? {
         return Ok(false);
     }
@@ -2116,17 +2106,7 @@ async fn needs_move_to_mvbox(
         // there may be a non-delta device that wants to handle it
         return Ok(false);
     }
-
-    if has_chat_version {
-        Ok(true)
-    } else if let Some(parent) = get_prefetch_parent_message(context, headers).await? {
-        match parent.is_dc_message {
-            MessengerMessage::No => Ok(false),
-            MessengerMessage::Yes | MessengerMessage::Reply => Ok(true),
-        }
-    } else {
-        Ok(false)
-    }
+    Ok(headers.get_header_value(HeaderDef::SecureJoin).is_some() || is_encrypted(headers))
 }
 
 /// Try to get the folder meaning by the name of the folder only used if the server does not support XLIST.
@@ -2239,21 +2219,6 @@ pub(crate) fn create_message_id() -> String {
     format!("{}{}", GENERATED_PREFIX, create_id())
 }
 
-/// Returns chat by prefetched headers.
-async fn prefetch_get_chat(
-    context: &Context,
-    headers: &[mailparse::MailHeader<'_>],
-) -> Result<Option<chat::Chat>> {
-    let parent = get_prefetch_parent_message(context, headers).await?;
-    if let Some(parent) = &parent {
-        return Ok(Some(
-            chat::Chat::load_from_db(context, parent.get_chat_id()).await?,
-        ));
-    }
-
-    Ok(None)
-}
-
 /// Determines whether the message should be downloaded based on prefetched headers.
 pub(crate) async fn prefetch_should_download(
     context: &Context,
@@ -2272,14 +2237,6 @@ pub(crate) async fn prefetch_should_download(
     // We do not know the Message-ID or the Message-ID is missing (in this case, we create one in
     // the further process).
 
-    if let Some(chat) = prefetch_get_chat(context, headers).await? {
-        if chat.typ == Chattype::Group && !chat.id.is_special() {
-            // This might be a group command, like removing a group member.
-            // We really need to fetch this to avoid inconsistent group state.
-            return Ok(true);
-        }
-    }
-
     let maybe_ndn = if let Some(from) = headers.get_header_value(HeaderDef::From_) {
         let from = from.to_ascii_lowercase();
         from.contains("mailer-daemon") || from.contains("mail-daemon")
@@ -2309,27 +2266,24 @@ pub(crate) async fn prefetch_should_download(
         return Ok(false);
     }
 
-    let is_chat_message = headers.get_header_value(HeaderDef::ChatVersion).is_some();
     let accepted_contact = origin.is_known();
-    let is_reply_to_chat_message = get_prefetch_parent_message(context, headers)
-        .await?
-        .map(|parent| match parent.is_dc_message {
-            MessengerMessage::No => false,
-            MessengerMessage::Yes | MessengerMessage::Reply => true,
-        })
-        .unwrap_or_default();
-
-    let show_emails =
-        ShowEmails::from_i32(context.get_config_int(Config::ShowEmails).await?).unwrap_or_default();
-
     let show = is_autocrypt_setup_message
-        || match show_emails {
-            ShowEmails::Off => is_chat_message || is_reply_to_chat_message,
-            ShowEmails::AcceptedContacts => {
-                is_chat_message || is_reply_to_chat_message || accepted_contact
-            }
+        || headers.get_header_value(HeaderDef::SecureJoin).is_some()
+        || is_encrypted(headers)
+        || match ShowEmails::from_i32(context.get_config_int(Config::ShowEmails).await?)
+            .unwrap_or_default()
+        {
+            ShowEmails::Off => false,
+            ShowEmails::AcceptedContacts => accepted_contact,
             ShowEmails::All => true,
-        };
+        }
+        || get_prefetch_parent_message(context, headers)
+            .await?
+            .map(|parent| match parent.is_dc_message {
+                MessengerMessage::No => false,
+                MessengerMessage::Yes | MessengerMessage::Reply => true,
+            })
+            .unwrap_or_default();
 
     let should_download = (show && !blocked_contact) || maybe_ndn;
     Ok(should_download)

src/imap/imap_tests.rs

@@ -94,14 +94,14 @@ fn test_build_sequence_sets() {
 async fn check_target_folder_combination(
     folder: &str,
     mvbox_move: bool,
-    chat_msg: bool,
+    is_encrypted: bool,
     expected_destination: &str,
     accepted_chat: bool,
     outgoing: bool,
     setupmessage: bool,
 ) -> Result<()> {
     println!(
-        "Testing: For folder {folder}, mvbox_move {mvbox_move}, chat_msg {chat_msg}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}"
+        "Testing: For folder {folder}, mvbox_move {mvbox_move}, is_encrypted {is_encrypted}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}"
     );
 
     let t = TestContext::new_alice().await;
@@ -124,7 +124,6 @@ async fn check_target_folder_combination(
         temp = format!(
             "Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\
                     {}\
-                    Subject: foo\n\
                     Message-ID: <[email protected]>\n\
                     {}\
                     Date: Sun, 22 Mar 2020 22:37:57 +0000\n\
@@ -135,7 +134,12 @@ async fn check_target_folder_combination(
             } else {
                 "From: [email protected]\nTo: [email protected]\n"
             },
-            if chat_msg { "Chat-Version: 1.0\n" } else { "" },
+            if is_encrypted {
+                "Subject: [...]\n\
+                Content-Type: multipart/mixed; boundary=\"someboundary\"\n"
+            } else {
+                "Subject: foo\n"
+            },
         );
         temp.as_bytes()
     };
@@ -157,25 +161,26 @@ async fn check_target_folder_combination(
     assert_eq!(
         expected,
         actual.as_deref(),
-        "For folder {folder}, mvbox_move {mvbox_move}, chat_msg {chat_msg}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}: expected {expected:?}, got {actual:?}"
+        "For folder {folder}, mvbox_move {mvbox_move}, is_encrypted {is_encrypted}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}: expected {expected:?}, got {actual:?}"
     );
     Ok(())
 }
 
-// chat_msg means that the message was sent by Delta Chat
-// The tuples are (folder, mvbox_move, chat_msg, expected_destination)
+// The tuples are (folder, mvbox_move, is_encrypted, expected_destination)
 const COMBINATIONS_ACCEPTED_CHAT: &[(&str, bool, bool, &str)] = &[
     ("INBOX", false, false, "INBOX"),
     ("INBOX", false, true, "INBOX"),
     ("INBOX", true, false, "INBOX"),
     ("INBOX", true, true, "DeltaChat"),
-    ("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, false, "INBOX"),
     ("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
+    // Move unencrypted emails in accepted chats from Spam to INBOX, not 100% sure on this, we could
+    // also not move unencrypted emails or, if mvbox_move=1, move them to DeltaChat.
+    ("Spam", true, false, "INBOX"),
     ("Spam", true, true, "DeltaChat"),
 ];
 
-// These are the same as above, but non-chat messages in Spam stay in Spam
+// These are the same as above, but unencrypted messages in Spam stay in Spam.
 const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[
     ("INBOX", false, false, "INBOX"),
     ("INBOX", false, true, "INBOX"),
@@ -189,11 +194,11 @@ const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_target_folder_incoming_accepted() -> Result<()> {
-    for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
+    for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
         check_target_folder_combination(
             folder,
             *mvbox_move,
-            *chat_msg,
+            *is_encrypted,
             expected_destination,
             true,
             false,
@@ -206,11 +211,11 @@ async fn test_target_folder_incoming_accepted() -> Result<()> {
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_target_folder_incoming_request() -> Result<()> {
-    for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_REQUEST {
+    for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_REQUEST {
         check_target_folder_combination(
             folder,
             *mvbox_move,
-            *chat_msg,
+            *is_encrypted,
             expected_destination,
             false,
             false,
@@ -224,11 +229,11 @@ async fn test_target_folder_incoming_request() -> Result<()> {
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_target_folder_outgoing() -> Result<()> {
     // Test outgoing emails
-    for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
+    for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
         check_target_folder_combination(
             folder,
             *mvbox_move,
-            *chat_msg,
+            *is_encrypted,
             expected_destination,
             true,
             true,
@@ -242,11 +247,11 @@ async fn test_target_folder_outgoing() -> Result<()> {
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_target_folder_setupmsg() -> Result<()> {
     // Test setupmessages
-    for (folder, mvbox_move, chat_msg, _expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
+    for (folder, mvbox_move, is_encrypted, _expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
         check_target_folder_combination(
             folder,
             *mvbox_move,
-            *chat_msg,
+            *is_encrypted,
             if folder == &"Spam" { "INBOX" } else { folder }, // Never move setup messages, except if they are in "Spam"
             false,
             true,

src/imap/session.rs

@@ -14,17 +14,20 @@ use crate::tools;
 /// Prefetch:
 /// - Message-ID to check if we already have the message.
 /// - In-Reply-To and References to check if message is a reply to chat message.
-/// - Chat-Version to check if a message is a chat message
 /// - Autocrypt-Setup-Message to check if a message is an autocrypt setup message,
 ///   not necessarily sent by Delta Chat.
+///
+/// NB: We don't look at Chat-Version as we don't want any "better" handling for unencrypted
+/// messages.
 const PREFETCH_FLAGS: &str = "(UID INTERNALDATE RFC822.SIZE BODY.PEEK[HEADER.FIELDS (\
                               MESSAGE-ID \
                               DATE \
                               X-MICROSOFT-ORIGINAL-MESSAGE-ID \
                               FROM \
                               IN-REPLY-TO REFERENCES \
-                              CHAT-VERSION \
-                              AUTO-SUBMITTED \
+                              CONTENT-TYPE \
+                              SECURE-JOIN \
+                              SUBJECT \
                               AUTOCRYPT-SETUP-MESSAGE\
                               )])";
 

src/mimefactory.rs

@@ -947,8 +947,7 @@ impl MimeFactory {
         //
         // These are standard headers such as Date, In-Reply-To, References, which cannot be placed
         // anywhere else according to the standard. Placing headers here also allows them to be fetched
-        // individually over IMAP without downloading the message body. This is why Chat-Version is
-        // placed here.
+        // individually over IMAP without downloading the message body.
         let mut unprotected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new();
 
         // Headers that MUST NOT (only) go into IMF header section:
@@ -1063,11 +1062,7 @@ impl MimeFactory {
                             mail_builder::headers::raw::Raw::new("[...]").into(),
                         ));
                     }
-                    "in-reply-to"
-                    | "references"
-                    | "auto-submitted"
-                    | "chat-version"
-                    | "autocrypt-setup-message" => {
+                    "autocrypt-setup-message" => {
                         unprotected_headers.push(header.clone());
                     }
                     _ => {