Remove outdated comment

Envelope has the same Message-ID, but using other fields from the
envelope, such as From field, is error-prone. They may contain values
different from the message body. As we don't parse the envelope later on,
it is better not to fetch it during prefetch too.

Cargo.lock

@@ -99,7 +99,7 @@ dependencies = [
  "byte-pool 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "chrono 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "imap-proto 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "imap-proto 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "nom 5.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1326,7 +1326,7 @@ dependencies = [
 
 [[package]]
 name = "imap-proto"
-version = "0.10.0"
+version = "0.10.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "nom 5.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3405,7 +3405,7 @@ dependencies = [
 "checksum idna 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "02e2673c30ee86b5b96a9cb52ad15718aa1f966f5ab9ad54a8b95d5ca33120a9"
 "checksum image 0.22.4 (registry+https://github.com/rust-lang/crates.io-index)" = "53cb19c4e35102e5c6fb9ade5e0e236c5588424dc171a849af3141bf0b47768a"
 "checksum image-meta 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b00861cbbb254a627d8acc0cec786b484297d896ab8f20fdc8e28536a3e918ef"
-"checksum imap-proto 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)" = "11ba57a7d18ba7bca5a50d4723c16a3e532d2b3014d6fd3123910c266214d4f2"
+"checksum imap-proto 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)" = "16a6def1d5ac8975d70b3fd101d57953fe3278ef2ee5d7816cba54b1d1dfc22f"
 "checksum indexmap 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712d7b3ea5827fcb9d4fda14bf4da5f136f0db2ae9c8f4bd4e2d1c6fde4e6db2"
 "checksum inflate 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)" = "1cdb29978cc5797bd8dcc8e5bf7de604891df2a8dc576973d71a281e916db2ff"
 "checksum iovec 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "b2b3ea6ff95e175473f8ffe6a7eb7c00d054240321b84c57051175fe3c1e075e"

src/dc_receive_imf.rs

@@ -102,40 +102,13 @@ pub fn dc_receive_imf(
     // or if From: is equal to SELF (in this case, it is any outgoing messages,
     // we do not check Return-Path any more as this is unreliable, see
     // https://github.com/deltachat/deltachat-core/issues/150)
-    let mut from_id = 0;
-    let mut from_id_blocked = false;
-    let mut incoming = true;
-    let mut incoming_origin = Origin::Unknown;
-
-    if let Some(field_from) = mime_parser.get(HeaderDef::From_) {
-        let from_ids = dc_add_or_lookup_contacts_by_address_list(
-            context,
-            &field_from,
-            Origin::IncomingUnknownFrom,
-        )?;
-        if from_ids.contains(&DC_CONTACT_ID_SELF) {
-            incoming = false;
-            from_id = DC_CONTACT_ID_SELF;
-            incoming_origin = Origin::OutgoingBcc;
-        } else if !from_ids.is_empty() {
-            if from_ids.len() > 1 {
-                warn!(
-                    context,
-                    "mail has more than one From address, only using first: {:?}", field_from
-                );
-            }
-            from_id = from_ids.get_index(0).cloned().unwrap_or_default();
-            if let Ok(contact) = Contact::load_from_db(context, from_id) {
-                incoming_origin = contact.origin;
-                from_id_blocked = contact.blocked;
-            }
+    let (from_id, from_id_blocked, incoming_origin) =
+        if let Some(field_from) = mime_parser.get(HeaderDef::From_) {
+            from_field_to_contact_id(context, field_from)?
         } else {
-            warn!(context, "mail has an empty From header: {:?}", field_from);
-            // if there is no from given, from_id stays 0 which is just fine. These messages
-            // are very rare, however, we have to add them to the database (they go to the
-            // "deaddrop" chat) to avoid a re-download from the server. See also [**]
-        }
-    }
+            (0, false, Origin::Unknown)
+        };
+    let incoming = from_id != DC_CONTACT_ID_SELF;
 
     let mut to_ids = ContactIds::new();
     for header_def in &[HeaderDef::To, HeaderDef::Cc] {
@@ -250,6 +223,47 @@ pub fn dc_receive_imf(
     Ok(())
 }
 
+/// Converts "From" field to contact id.
+///
+/// Also returns whether it is blocked or not and its origin.
+pub fn from_field_to_contact_id(
+    context: &Context,
+    field_from: &str,
+) -> Result<(u32, bool, Origin)> {
+    let from_ids = dc_add_or_lookup_contacts_by_address_list(
+        context,
+        &field_from,
+        Origin::IncomingUnknownFrom,
+    )?;
+
+    if from_ids.contains(&DC_CONTACT_ID_SELF) {
+        Ok((DC_CONTACT_ID_SELF, false, Origin::OutgoingBcc))
+    } else if !from_ids.is_empty() {
+        if from_ids.len() > 1 {
+            warn!(
+                context,
+                "mail has more than one From address, only using first: {:?}", field_from
+            );
+        }
+        let from_id = from_ids.get_index(0).cloned().unwrap_or_default();
+
+        let mut from_id_blocked = false;
+        let mut incoming_origin = Origin::Unknown;
+        if let Ok(contact) = Contact::load_from_db(context, from_id) {
+            from_id_blocked = contact.blocked;
+            incoming_origin = contact.origin;
+        }
+        Ok((from_id, from_id_blocked, incoming_origin))
+    } else {
+        warn!(context, "mail has an empty From header: {:?}", field_from);
+        // if there is no from given, from_id stays 0 which is just fine. These messages
+        // are very rare, however, we have to add them to the database (they go to the
+        // "deaddrop" chat) to avoid a re-download from the server. See also [**]
+
+        Ok((0, false, Origin::Unknown))
+    }
+}
+
 #[allow(clippy::too_many_arguments, clippy::cognitive_complexity)]
 fn add_parts(
     context: &Context,
@@ -300,8 +314,7 @@ fn add_parts(
     } else {
         MessengerMessage::No
     };
-    // incoming non-chat messages may be discarded;
-    // maybe this can be optimized later, by checking the state before the message body is downloaded
+    // incoming non-chat messages may be discarded
     let mut allow_creation = true;
     let show_emails =
         ShowEmails::from_i32(context.get_config_int(Config::ShowEmails)).unwrap_or_default();
@@ -309,11 +322,13 @@ fn add_parts(
         && msgrmsg == MessengerMessage::No
     {
         // this message is a classic email not a chat-message nor a reply to one
-        if show_emails == ShowEmails::Off {
-            *chat_id = ChatId::new(DC_CHAT_ID_TRASH);
-            allow_creation = false
-        } else if show_emails == ShowEmails::AcceptedContacts {
-            allow_creation = false
+        match show_emails {
+            ShowEmails::Off => {
+                *chat_id = ChatId::new(DC_CHAT_ID_TRASH);
+                allow_creation = false;
+            }
+            ShowEmails::AcceptedContacts => allow_creation = false,
+            ShowEmails::All => {}
         }
     }
 
@@ -1510,7 +1525,7 @@ fn is_reply_to_messenger_message(context: &Context, mime_parser: &MimeMessage) -
     false
 }
 
-fn is_msgrmsg_rfc724_mid_in_list(context: &Context, mid_list: &str) -> bool {
+pub fn is_msgrmsg_rfc724_mid_in_list(context: &Context, mid_list: &str) -> bool {
     if let Ok(ids) = mailparse::addrparse(mid_list) {
         for id in ids.iter() {
             if is_msgrmsg_rfc724_mid(context, id) {

src/imap/mod.rs

@@ -5,6 +5,8 @@
 
 use std::sync::atomic::{AtomicBool, Ordering};
 
+use num_traits::FromPrimitive;
+
 use async_imap::{
     error::Result as ImapResult,
     types::{Capability, Fetch, Flag, Mailbox, Name, NameAttribute},
@@ -12,10 +14,16 @@ use async_imap::{
 use async_std::sync::{Mutex, RwLock};
 use async_std::task;
 
+use mailparse::MailHeaderMap;
+
+use crate::config::*;
 use crate::constants::*;
 use crate::context::Context;
-use crate::dc_receive_imf::dc_receive_imf;
+use crate::dc_receive_imf::{
+    dc_receive_imf, from_field_to_contact_id, is_msgrmsg_rfc724_mid_in_list,
+};
 use crate::events::Event;
+use crate::headerdef::HeaderDef;
 use crate::imap_client::*;
 use crate::job::{job_add, Action};
 use crate::login_param::{CertificateChecks, LoginParam};
@@ -63,6 +71,9 @@ pub enum Error {
     #[fail(display = "IMAP select folder error")]
     SelectFolderError(#[cause] select_folder::Error),
 
+    #[fail(display = "Mail parse error")]
+    MailParseError(#[cause] mailparse::MailParseError),
+
     #[fail(display = "No mailbox selected, folder: {:?}", _0)]
     NoMailbox(String),
 
@@ -94,6 +105,12 @@ impl From<select_folder::Error> for Error {
     }
 }
 
+impl From<mailparse::MailParseError> for Error {
+    fn from(err: mailparse::MailParseError) -> Error {
+        Error::MailParseError(err)
+    }
+}
+
 #[derive(Debug, Display, Clone, Copy, PartialEq, Eq)]
 pub enum ImapActionResult {
     Failed,
@@ -102,7 +119,20 @@ pub enum ImapActionResult {
     Success,
 }
 
-const PREFETCH_FLAGS: &str = "(UID ENVELOPE)";
+/// 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.
+const PREFETCH_FLAGS: &str = "(UID BODY.PEEK[HEADER.FIELDS (\
+                              MESSAGE-ID \
+                              FROM \
+                              IN-REPLY-TO REFERENCES \
+                              CHAT-VERSION \
+                              AUTOCRYPT-SETUP-MESSAGE\
+                              )])";
+const DELETE_CHECK_FLAGS: &str = "(UID BODY.PEEK[HEADER.FIELDS (MESSAGE-ID)])";
 const JUST_UID: &str = "(UID)";
 const BODY_FLAGS: &str = "(FLAGS BODY.PEEK[])";
 const SELECT_ALL: &str = "1:*";
@@ -555,6 +585,9 @@ impl Imap {
         context: &Context,
         folder: S,
     ) -> Result<bool> {
+        let show_emails =
+            ShowEmails::from_i32(context.get_config_int(Config::ShowEmails)).unwrap_or_default();
+
         let (uid_validity, last_seen_uid) =
             self.select_with_uidvalidity(context, folder.as_ref())?;
 
@@ -580,8 +613,8 @@ impl Imap {
 
         list.sort_unstable_by_key(|msg| msg.uid.unwrap_or_default());
 
-        for msg in &list {
-            let cur_uid = msg.uid.unwrap_or_default();
+        for fetch in &list {
+            let cur_uid = fetch.uid.unwrap_or_default();
             if cur_uid <= last_seen_uid {
                 // If the mailbox is not empty, results always include
                 // at least one UID, even if last_seen_uid+1 is past
@@ -598,20 +631,9 @@ impl Imap {
             }
             read_cnt += 1;
 
-            let message_id = prefetch_get_message_id(msg).unwrap_or_default();
-
-            if !precheck_imf(context, &message_id, folder.as_ref(), cur_uid) {
-                // check passed, go fetch the rest
-                if self.fetch_single_msg(context, &folder, cur_uid).await == 0 {
-                    info!(
-                        context,
-                        "Read error for message {} from \"{}\", trying over later.",
-                        message_id,
-                        folder.as_ref()
-                    );
-                    read_errors += 1;
-                }
-            } else {
+            let headers = get_fetch_headers(fetch)?;
+            let message_id = prefetch_get_message_id(&headers).unwrap_or_default();
+            if precheck_imf(context, &message_id, folder.as_ref(), cur_uid) {
                 // we know the message-id already or don't want the message otherwise.
                 info!(
                     context,
@@ -619,6 +641,33 @@ impl Imap {
                     message_id,
                     folder.as_ref(),
                 );
+            } else {
+                let show = prefetch_should_download(context, &headers, show_emails)
+                    .map_err(|err| {
+                        warn!(context, "prefetch_should_download error: {}", err);
+                        err
+                    })
+                    .unwrap_or(true);
+
+                if !show {
+                    info!(
+                        context,
+                        "Ignoring new message {} from \"{}\".",
+                        message_id,
+                        folder.as_ref(),
+                    );
+                } else {
+                    // check passed, go fetch the rest
+                    if self.fetch_single_msg(context, &folder, cur_uid).await == 0 {
+                        info!(
+                            context,
+                            "Read error for message {} from \"{}\", trying over later.",
+                            message_id,
+                            folder.as_ref()
+                        );
+                        read_errors += 1;
+                    }
+                }
             }
             if read_errors == 0 {
                 new_last_seen_uid = cur_uid;
@@ -940,9 +989,11 @@ impl Imap {
             // double-check that we are deleting the correct message-id
             // this comes at the expense of another imap query
             if let Some(ref mut session) = &mut *self.session.lock().await {
-                match session.uid_fetch(set, PREFETCH_FLAGS).await {
+                match session.uid_fetch(set, DELETE_CHECK_FLAGS).await {
                     Ok(msgs) => {
-                        if msgs.is_empty() {
+                        let fetch = if let Some(fetch) = msgs.first() {
+                            fetch
+                        } else {
                             warn!(
                                 context,
                                 "Cannot delete on IMAP, {}: imap entry gone '{}'",
@@ -950,9 +1001,11 @@ impl Imap {
                                 message_id,
                             );
                             return ImapActionResult::Failed;
-                        }
-                        let remote_message_id =
-                            prefetch_get_message_id(msgs.first().unwrap()).unwrap_or_default();
+                        };
+
+                        let remote_message_id = get_fetch_headers(fetch)
+                            .and_then(|headers| prefetch_get_message_id(&headers))
+                            .unwrap_or_default();
 
                         if remote_message_id != message_id {
                             warn!(
@@ -1233,8 +1286,7 @@ fn precheck_imf(context: &Context, rfc724_mid: &str, server_folder: &str, server
     }
 }
 
-fn parse_message_id(message_id: &[u8]) -> crate::error::Result<String> {
-    let value = std::str::from_utf8(message_id)?;
+fn parse_message_id(value: &str) -> crate::error::Result<String> {
     let addrs = mailparse::addrparse(value)
         .map_err(|err| format_err!("failed to parse message id {:?}", err))?;
 
@@ -1245,19 +1297,78 @@ fn parse_message_id(message_id: &[u8]) -> crate::error::Result<String> {
     bail!("could not parse message_id: {}", value);
 }
 
-fn prefetch_get_message_id(prefetch_msg: &Fetch) -> Result<String> {
-    if prefetch_msg.envelope().is_none() {
-        return Err(Error::Other(
-            "prefectch: message has no envelope".to_string(),
-        ));
+fn get_fetch_headers(prefetch_msg: &Fetch) -> Result<Vec<mailparse::MailHeader>> {
+    let header_bytes = match prefetch_msg.header() {
+        Some(header_bytes) => header_bytes,
+        None => return Ok(Vec::new()),
+    };
+    let (headers, _) = mailparse::parse_headers(header_bytes)?;
+    Ok(headers)
+}
+
+fn prefetch_get_message_id(headers: &[mailparse::MailHeader]) -> Result<String> {
+    if let Some(message_id) = headers.get_first_value(&HeaderDef::MessageId.get_headername())? {
+        Ok(parse_message_id(&message_id)?)
+    } else {
+        Err(Error::Other("prefetch: No message ID found".to_string()))
+    }
+}
+
+/// Checks if fetch result contains a header
+fn prefetch_has_header(headers: &[mailparse::MailHeader], headerdef: HeaderDef) -> Result<bool> {
+    Ok(headers
+        .get_first_value(&headerdef.get_headername())?
+        .is_some())
+}
+
+fn prefetch_is_reply_to_chat_message(
+    context: &Context,
+    headers: &[mailparse::MailHeader],
+) -> Result<bool> {
+    if let Some(value) = headers.get_first_value(&HeaderDef::InReplyTo.get_headername())? {
+        if is_msgrmsg_rfc724_mid_in_list(context, &value) {
+            return Ok(true);
+        }
     }
 
-    let message_id = prefetch_msg.envelope().unwrap().message_id;
-    if message_id.is_none() {
-        return Err(Error::Other("prefetch: No message ID found".to_string()));
+    if let Some(value) = headers.get_first_value(&HeaderDef::References.get_headername())? {
+        if is_msgrmsg_rfc724_mid_in_list(context, &value) {
+            return Ok(true);
+        }
     }
 
-    parse_message_id(&message_id.unwrap()).map_err(Into::into)
+    Ok(false)
+}
+
+fn prefetch_should_download(
+    context: &Context,
+    headers: &[mailparse::MailHeader],
+    show_emails: ShowEmails,
+) -> Result<bool> {
+    let is_chat_message = prefetch_has_header(&headers, HeaderDef::ChatVersion)?;
+    let is_autocrypt_setup_message =
+        prefetch_has_header(&headers, HeaderDef::AutocryptSetupMessage)?;
+
+    // Autocrypt Setup Message should be shown even if it is from non-chat client.
+    let is_reply_to_chat_message = prefetch_is_reply_to_chat_message(context, &headers)?;
+
+    let from_field = headers
+        .get_first_value(&HeaderDef::From_.get_headername())?
+        .unwrap_or_default();
+
+    let (_contact_id, blocked_contact, origin) = from_field_to_contact_id(context, &from_field)?;
+    let accepted_contact = origin.is_known();
+
+    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
+            }
+            ShowEmails::All => true,
+        };
+    let show = show && !blocked_contact;
+    Ok(show)
 }
 
 #[cfg(test)]
@@ -1267,11 +1378,11 @@ mod tests {
     #[test]
     fn test_parse_message_id() {
         assert_eq!(
-            parse_message_id(b"Mr.PRUe8HJBoaO.3whNvLCMFU0@testrun.org").unwrap(),
+            parse_message_id("Mr.PRUe8HJBoaO.3whNvLCMFU0@testrun.org").unwrap(),
             "Mr.PRUe8HJBoaO.3whNvLCMFU0@testrun.org"
         );
         assert_eq!(
-            parse_message_id(b"<Mr.PRUe8HJBoaO.3whNvLCMFU0@testrun.org>").unwrap(),
+            parse_message_id("<Mr.PRUe8HJBoaO.3whNvLCMFU0@testrun.org>").unwrap(),
             "Mr.PRUe8HJBoaO.3whNvLCMFU0@testrun.org"
         );
     }

src/job_thread.rs

@@ -143,7 +143,7 @@ impl JobThread {
             if state.jobs_needed {
                 info!(
                     context,
-                    "{}-IDLE will not be started as it was interrupted while not ideling.",
+                    "{}-IDLE will not be started as it was interrupted while not idling.",
                     self.name,
                 );
                 state.jobs_needed = false;