Bugfix: Don't let malformed From: headers block the receiving pipeline (#3769)

That's a bug which @Simon-Laux and probably also @hpk42 had, where one malformed incoming (Spam-) mail blocked the receiving of all emails coming after it.

The problem was that from_field_to_contact_id() returned ContactId::UNDEFINED, and then lookup_by_contact() returned Err.

CHANGELOG.md

@@ -13,6 +13,7 @@
 ### Fixes
 - fix detection of "All mail", "Trash", "Junk" etc folders. #3760
 - fetch messages sequentially to fix reactions on partially downloaded messages #3688
+- Fix a bug where one malformed message blocked receiving any further messages #3769
 
 
 ## 1.101.0

python/tests/test_1_online.py

@@ -901,6 +901,34 @@ def test_dont_show_emails(acfactory, lp):
             ac1.get_config("configured_addr")
         ),
     )
+    ac1.direct_imap.append(
+        "Spam",
+        """
+        From: unknown.address@junk.org, unkwnown.add@junk.org
+        Subject: subj
+        To: {}
+        Message-ID: <spam.message2@junk.org>
+        Content-Type: text/plain; charset=utf-8
+
+        Unknown & malformed message in Spam
+    """.format(
+            ac1.get_config("configured_addr")
+        ),
+    )
+    ac1.direct_imap.append(
+        "Spam",
+        """
+        From: alice@example.org
+        Subject: subj
+        To: {}
+        Message-ID: <spam.message3@junk.org>
+        Content-Type: text/plain; charset=utf-8
+
+        Actually interesting message in Spam
+    """.format(
+            ac1.get_config("configured_addr")
+        ),
+    )
     ac1.direct_imap.append(
         "Junk",
         """
@@ -926,7 +954,9 @@ def test_dont_show_emails(acfactory, lp):
     ac1._evtracker.wait_idle_inbox_ready()
 
     assert msg.text == "subj – message in Sent"
-    assert len(msg.chat.get_messages()) == 1
+    chat_msgs = msg.chat.get_messages()
+    assert len(chat_msgs) == 2
+    assert any(msg.text == "subj – Actually interesting message in Spam" for msg in chat_msgs)
 
     assert not any("unknown.address" in c.get_name() for c in ac1.get_chats())
     ac1.direct_imap.select_folder("Spam")
@@ -942,7 +972,7 @@ def test_dont_show_emails(acfactory, lp):
     msg2 = ac1._evtracker.wait_next_messages_changed()
 
     assert msg2.text == "subj – message in Drafts that is moved to Sent later"
-    assert len(msg.chat.get_messages()) == 2
+    assert len(msg.chat.get_messages()) == 3
 
 
 def test_no_old_msg_is_fresh(acfactory, lp):

src/imap.rs

@@ -1754,9 +1754,13 @@ async fn should_move_out_of_spam(
             return Ok(false);
         }
     } else {
+        let from = match mimeparser::get_from(headers) {
+            Some(f) => f,
+            None => return Ok(false),
+        };
         // No chat found.
         let (from_id, blocked_contact, _origin) =
-            from_field_to_contact_id(context, mimeparser::get_from(headers).as_ref(), true).await?;
+            from_field_to_contact_id(context, &from, true).await?;
         if blocked_contact {
             // Contact is blocked, leave the message in spam.
             return Ok(false);
@@ -2041,8 +2045,12 @@ pub(crate) async fn prefetch_should_download(
         .get_header_value(HeaderDef::AutocryptSetupMessage)
         .is_some();
 
+    let from = match mimeparser::get_from(headers) {
+        Some(f) => f,
+        None => return Ok(false),
+    };
     let (_from_id, blocked_contact, origin) =
-        from_field_to_contact_id(context, mimeparser::get_from(headers).as_ref(), true).await?;
+        from_field_to_contact_id(context, &from, true).await?;
     // prevent_rename=true as this might be a mailing list message and in this case it would be bad if we rename the contact.
     // (prevent_rename is the last argument of from_field_to_contact_id())
 

src/receive_imf.rs

@@ -172,7 +172,7 @@ pub(crate) async fn receive_imf_inner(
     // If this is a mailing list email (i.e. list_id_header is some), don't change the displayname because in
     // a mailing list the sender displayname sometimes does not belong to the sender email address.
     let (from_id, _from_id_blocked, incoming_origin) =
-        from_field_to_contact_id(context, Some(&mime_parser.from), prevent_rename).await?;
+        from_field_to_contact_id(context, &mime_parser.from, prevent_rename).await?;
 
     let incoming = from_id != ContactId::SELF;
 
@@ -370,17 +370,9 @@ pub(crate) async fn receive_imf_inner(
 /// * `prevent_rename`: passed through to `add_or_lookup_contacts_by_address_list()`
 pub async fn from_field_to_contact_id(
     context: &Context,
-    from: Option<&SingleInfo>,
+    from: &SingleInfo,
     prevent_rename: bool,
 ) -> Result<(ContactId, bool, Origin)> {
-    let from = match from {
-        Some(f) => f,
-        None => {
-            warn!(context, "mail has an empty From header");
-            return Ok((ContactId::UNDEFINED, false, Origin::Unknown));
-        }
-    };
-
     let display_name = if prevent_rename {
         Some("")
     } else {