diff --git a/src/imap.rs b/src/imap.rs index 5614f6593a..73d4452f18 100644 --- a/src/imap.rs +++ b/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; @@ -1965,21 +1965,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 () + // 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 { - 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); } @@ -2038,7 +2041,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? @@ -2091,18 +2095,6 @@ async fn needs_move_to_mvbox( context: &Context, headers: &[mailparse::MailHeader<'_>], ) -> Result { - 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() - && let Some(from) = mimeparser::get_from(headers) - && context.is_self_addr(&from.addr).await? - { - return Ok(true); - } if !context.get_config_bool(Config::MvboxMove).await? { return Ok(false); } @@ -2115,17 +2107,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. @@ -2238,21 +2220,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> { - 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, @@ -2271,15 +2238,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? - && 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 +2267,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) diff --git a/src/imap/imap_tests.rs b/src/imap/imap_tests.rs index 304b9b5e20..03a4702dc6 100644 --- a/src/imap/imap_tests.rs +++ b/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: \n\ {}\ Date: Sun, 22 Mar 2020 22:37:57 +0000\n\ @@ -135,7 +134,12 @@ async fn check_target_folder_combination( } else { "From: bob@example.net\nTo: alice@example.org\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, diff --git a/src/imap/session.rs b/src/imap/session.rs index 8cf0a17de0..a01118b077 100644 --- a/src/imap/session.rs +++ b/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\ )])"; diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 3febeac50b..6717a4a364 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -979,7 +979,7 @@ pub(crate) async fn receive_imf_inner( if let Some(is_bot) = mime_parser.is_bot { // If the message is auto-generated and was generated by Delta Chat, // mark the contact as a bot. - if mime_parser.get_header(HeaderDef::ChatVersion).is_some() { + if mime_parser.has_chat_version() { from_id.mark_bot(context, is_bot).await?; } } @@ -2924,7 +2924,7 @@ async fn apply_group_changes( } // Allow non-Delta Chat MUAs to add members. - if mime_parser.get_header(HeaderDef::ChatVersion).is_none() { + if !mime_parser.has_chat_version() { // Don't delete any members locally, but instead add absent ones to provide group // membership consistency for all members: new_members.extend(to_ids_flat.iter());