diff --git a/CHANGELOG.md b/CHANGELOG.md index b754ae5ed..6383f3f7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ ### Fixes - fix: only send contact changed event for recently seen if it is relevant (not too old to matter) #3938 - Immediately save `accounts.toml` if it was modified by a migration from absolute paths to relative paths #3943 +- Do not treat invalid email addresses as an exception #3942 + ## 1.105.0 diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index 07da89134..954812d3b 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -939,6 +939,20 @@ def test_dont_show_emails(acfactory, lp): ac1.get_config("configured_addr") ), ) + ac1.direct_imap.append( + "Spam", + """ + From: delta + Subject: subj + To: {} + Message-ID: + Content-Type: text/plain; charset=utf-8 + + Unknown & malformed message in Spam + """.format( + ac1.get_config("configured_addr") + ), + ) ac1.direct_imap.append( "Spam", """ diff --git a/src/chat.rs b/src/chat.rs index 618319cb1..8a2fb2a42 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4693,7 +4693,9 @@ mod tests { assert!(!shall_attach_selfavatar(&t, chat_id).await?); let (contact_id, _) = - Contact::add_or_lookup(&t, "", "foo@bar.org", Origin::IncomingUnknownTo).await?; + Contact::add_or_lookup(&t, "", "foo@bar.org", Origin::IncomingUnknownTo) + .await? + .unwrap(); add_contact_to_chat(&t, chat_id, contact_id).await?; assert!(!shall_attach_selfavatar(&t, chat_id).await?); t.set_config(Config::Selfavatar, None).await?; // setting to None also forces re-sending @@ -4940,7 +4942,9 @@ mod tests { bob.set_config(Config::ShowEmails, Some("2")).await?; let (contact_id, _) = - Contact::add_or_lookup(&alice, "", "bob@example.net", Origin::ManuallyCreated).await?; + Contact::add_or_lookup(&alice, "", "bob@example.net", Origin::ManuallyCreated) + .await? + .unwrap(); let alice_chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "grp").await?; let alice_chat = Chat::load_from_db(&alice, alice_chat_id).await?; @@ -5651,7 +5655,9 @@ mod tests { async fn test_create_for_contact_with_blocked() -> Result<()> { let t = TestContext::new().await; let (contact_id, _) = - Contact::add_or_lookup(&t, "", "foo@bar.org", Origin::ManuallyCreated).await?; + Contact::add_or_lookup(&t, "", "foo@bar.org", Origin::ManuallyCreated) + .await? + .unwrap(); // create a blocked chat let chat_id_orig = diff --git a/src/contact.rs b/src/contact.rs index 4f2189467..b0e33a52c 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -376,12 +376,14 @@ impl Contact { /// May result in a `#DC_EVENT_CONTACTS_CHANGED` event. pub async fn create(context: &Context, name: &str, addr: &str) -> Result { let name = improve_single_line_input(name); - ensure!(!addr.is_empty(), "Cannot create contact with empty address"); let (name, addr) = sanitize_name_and_addr(&name, addr); let (contact_id, sth_modified) = - Contact::add_or_lookup(context, &name, &addr, Origin::ManuallyCreated).await?; + Contact::add_or_lookup(context, &name, &addr, Origin::ManuallyCreated) + .await + .context("add_or_lookup")? + .with_context(|| format!("can't create a contact with address {:?}", addr))?; let blocked = Contact::is_blocked_load(context, contact_id).await?; match sth_modified { Modifier::None => {} @@ -466,12 +468,14 @@ impl Contact { /// Depending on the origin, both, "row_name" and "row_authname" are updated from "name". /// /// Returns the contact_id and a `Modifier` value indicating if a modification occurred. + /// + /// Returns None if the contact with such address cannot exist. pub(crate) async fn add_or_lookup( context: &Context, name: &str, addr: &str, mut origin: Origin, - ) -> Result<(ContactId, Modifier)> { + ) -> Result> { let mut sth_modified = Modifier::None; ensure!(!addr.is_empty(), "Can not add_or_lookup empty address"); @@ -480,7 +484,7 @@ impl Contact { let addr = addr_normalize(addr).to_string(); if context.is_self_addr(&addr).await? { - return Ok((ContactId::SELF, sth_modified)); + return Ok(Some((ContactId::SELF, sth_modified))); } if !may_be_valid_addr(&addr) { @@ -490,7 +494,7 @@ impl Contact { addr, if !name.is_empty() { name } else { "" }, ); - bail!("Bad address supplied: {:?}", addr); + return Ok(None); } let mut name = name; @@ -653,7 +657,7 @@ impl Contact { } } - Ok((ContactId::new(row_id), sth_modified)) + Ok(Some((ContactId::new(row_id), sth_modified))) } /// Add a number of contacts. @@ -686,7 +690,10 @@ impl Contact { "Failed to add address {} from address book: {}", addr, err ); } - Ok((_, modified)) => { + Ok(None) => { + warn!(context, "Cannot create contact with address {}.", addr); + } + Ok(Some((_, modified))) => { if modified != Modifier::None { modify_cnt += 1 } @@ -1697,7 +1704,8 @@ mod tests { "user@example.org", Origin::IncomingReplyTo, ) - .await?; + .await? + .unwrap(); assert_ne!(id, ContactId::UNDEFINED); let contact = Contact::load_from_db(&context.ctx, id).await.unwrap(); @@ -1725,7 +1733,8 @@ mod tests { "user@example.org", Origin::ManuallyCreated, ) - .await?; + .await? + .unwrap(); assert_eq!(contact_bob_id, id); assert_eq!(modified, Modifier::Modified); let contact = Contact::load_from_db(&context.ctx, id).await.unwrap(); @@ -1775,6 +1784,7 @@ mod tests { let (contact_id, sth_modified) = Contact::add_or_lookup(&t, "bla foo", "one@eins.org", Origin::IncomingUnknownTo) .await + .unwrap() .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::Modified); @@ -1790,6 +1800,7 @@ mod tests { let (contact_id_test, sth_modified) = Contact::add_or_lookup(&t, "Real one", " one@eins.org ", Origin::ManuallyCreated) .await + .unwrap() .unwrap(); assert_eq!(contact_id, contact_id_test); assert_eq!(sth_modified, Modifier::Modified); @@ -1802,6 +1813,7 @@ mod tests { let (contact_id, sth_modified) = Contact::add_or_lookup(&t, "", "three@drei.sam", Origin::IncomingUnknownTo) .await + .unwrap() .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::None); @@ -1819,6 +1831,7 @@ mod tests { Origin::IncomingUnknownFrom, ) .await + .unwrap() .unwrap(); assert_eq!(contact_id, contact_id_test); assert_eq!(sth_modified, Modifier::Modified); @@ -1830,6 +1843,7 @@ mod tests { let (contact_id_test, sth_modified) = Contact::add_or_lookup(&t, "schnucki", "three@drei.sam", Origin::ManuallyCreated) .await + .unwrap() .unwrap(); assert_eq!(contact_id, contact_id_test); assert_eq!(sth_modified, Modifier::Modified); @@ -1842,6 +1856,7 @@ mod tests { let (contact_id, sth_modified) = Contact::add_or_lookup(&t, "", "alice@w.de", Origin::IncomingUnknownTo) .await + .unwrap() .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::None); @@ -1979,7 +1994,8 @@ mod tests { // Create Bob contact let (contact_id, _) = Contact::add_or_lookup(&alice, "Bob", "bob@example.net", Origin::ManuallyCreated) - .await?; + .await? + .unwrap(); let chat = alice .create_chat_with_contact("Bob", "bob@example.net") .await; @@ -2055,6 +2071,7 @@ mod tests { let (contact_id, sth_modified) = Contact::add_or_lookup(&t, "bob1", "bob@example.org", Origin::IncomingUnknownFrom) .await + .unwrap() .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::Created); @@ -2067,6 +2084,7 @@ mod tests { let (contact_id, sth_modified) = Contact::add_or_lookup(&t, "bob2", "bob@example.org", Origin::IncomingUnknownFrom) .await + .unwrap() .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::Modified); @@ -2089,6 +2107,7 @@ mod tests { let (contact_id, sth_modified) = Contact::add_or_lookup(&t, "bob4", "bob@example.org", Origin::IncomingUnknownFrom) .await + .unwrap() .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::Modified); @@ -2118,6 +2137,7 @@ mod tests { Origin::IncomingUnknownFrom, ) .await + .unwrap() .unwrap(); assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); @@ -2134,6 +2154,7 @@ mod tests { Origin::IncomingUnknownFrom, ) .await + .unwrap() .unwrap(); assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); @@ -2154,7 +2175,8 @@ mod tests { // Incoming message from Bob. let (contact_id, sth_modified) = Contact::add_or_lookup(&t, "Bob", "bob@example.org", Origin::IncomingUnknownFrom) - .await?; + .await? + .unwrap(); assert_eq!(sth_modified, Modifier::Created); let contact = Contact::load_from_db(&t, contact_id).await?; assert_eq!(contact.get_display_name(), "Bob"); @@ -2162,7 +2184,8 @@ mod tests { // Incoming message from someone else with "Not Bob" in the "To:" field. let (contact_id_same, sth_modified) = Contact::add_or_lookup(&t, "Not Bob", "bob@example.org", Origin::IncomingUnknownTo) - .await?; + .await? + .unwrap(); assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::load_from_db(&t, contact_id).await?; @@ -2171,7 +2194,8 @@ mod tests { // Incoming message from Bob, changing the name back. let (contact_id_same, sth_modified) = Contact::add_or_lookup(&t, "Bob", "bob@example.org", Origin::IncomingUnknownFrom) - .await?; + .await? + .unwrap(); assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); // This was None until the bugfix let contact = Contact::load_from_db(&t, contact_id).await?; @@ -2312,7 +2336,8 @@ mod tests { let (contact_bob_id, _modified) = Contact::add_or_lookup(&alice, "Bob", "bob@example.net", Origin::ManuallyCreated) - .await?; + .await? + .unwrap(); let encrinfo = Contact::get_encrinfo(&alice, contact_bob_id).await?; assert_eq!(encrinfo, "No encryption"); @@ -2471,7 +2496,8 @@ CCCB 5AA9 F6E1 141C 9431 let (contact_id, _) = Contact::add_or_lookup(&alice, "Bob", "bob@example.net", Origin::ManuallyCreated) - .await?; + .await? + .unwrap(); let contact = Contact::load_from_db(&alice, contact_id).await?; assert_eq!(contact.last_seen(), 0); diff --git a/src/imap.rs b/src/imap.rs index 8bf71b4c3..c2bcd1f7a 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -1737,7 +1737,19 @@ async fn should_move_out_of_spam( }; // No chat found. let (from_id, blocked_contact, _origin) = - from_field_to_contact_id(context, &from, true).await?; + match from_field_to_contact_id(context, &from, true) + .await + .context("from_field_to_contact_id")? + { + Some(res) => res, + None => { + warn!( + context, + "Contact with From address {:?} cannot exist, not moving out of spam", from + ); + return Ok(false); + } + }; if blocked_contact { // Contact is blocked, leave the message in spam. return Ok(false); @@ -2027,7 +2039,10 @@ pub(crate) async fn prefetch_should_download( None => return Ok(false), }; let (_from_id, blocked_contact, origin) = - from_field_to_contact_id(context, &from, true).await?; + match from_field_to_contact_id(context, &from, true).await? { + Some(res) => res, + None => return Ok(false), + }; // 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()) @@ -2347,14 +2362,14 @@ async fn add_all_recipients_as_contacts( .await .with_context(|| format!("could not select {}", mailbox))?; - let contacts = imap + let recipients = imap .get_all_recipients(context) .await .context("could not get recipients")?; let mut any_modified = false; - for contact in contacts { - let display_name_normalized = contact + for recipient in recipients { + let display_name_normalized = recipient .display_name .as_ref() .map(|s| normalize_name(s)) @@ -2363,17 +2378,20 @@ async fn add_all_recipients_as_contacts( match Contact::add_or_lookup( context, &display_name_normalized, - &contact.addr, + &recipient.addr, Origin::OutgoingTo, ) - .await + .await? { - Ok((_, modified)) => { + Some((_, modified)) => { if modified != Modifier::None { any_modified = true; } } - Err(e) => warn!(context, "Could not add recipient: {}", e), + None => warn!( + context, + "Could not add contact for recipient with address {:?}", recipient.addr + ), } } if any_modified { diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 5d7d51fe8..bde304869 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1821,6 +1821,7 @@ mod tests { Contact::add_or_lookup(&t, "Dave", "dave@example.com", Origin::ManuallyCreated) .await .unwrap() + .unwrap() .0; let chat_id = ChatId::create_for_contact(&t, contact_id).await.unwrap(); diff --git a/src/peerstate.rs b/src/peerstate.rs index 2699bf6eb..6224118d9 100644 --- a/src/peerstate.rs +++ b/src/peerstate.rs @@ -542,14 +542,17 @@ impl Peerstate { if (chat.typ == Chattype::Group && chat.is_protected()) || chat.typ == Chattype::Broadcast { - chat::remove_from_chat_contacts_table(context, *chat_id, contact_id).await?; - - let (new_contact_id, _) = + if let Some((new_contact_id, _)) = Contact::add_or_lookup(context, "", new_addr, Origin::IncomingUnknownFrom) + .await? + { + chat::remove_from_chat_contacts_table(context, *chat_id, contact_id) + .await?; + chat::add_to_chat_contacts_table(context, *chat_id, &[new_contact_id]) .await?; - chat::add_to_chat_contacts_table(context, *chat_id, &[new_contact_id]).await?; - context.emit_event(EventType::ChatModified(*chat_id)); + context.emit_event(EventType::ChatModified(*chat_id)); + } } } diff --git a/src/qr.rs b/src/qr.rs index 2813c762d..c1e250211 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -221,10 +221,15 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { .context("Can't load peerstate")?; if let (Some(addr), Some(invitenumber), Some(authcode)) = (&addr, invitenumber, authcode) { - let contact_id = Contact::add_or_lookup(context, &name, addr, Origin::UnhandledQrScan) + let (contact_id, _) = Contact::add_or_lookup(context, &name, addr, Origin::UnhandledQrScan) .await - .map(|(id, _)| id) - .with_context(|| format!("failed to add or lookup contact for address {:?}", addr))?; + .with_context(|| format!("failed to add or lookup contact for address {:?}", addr))? + .with_context(|| { + format!( + "do not want to lookup contact for invalid address {:?}", + addr + ) + })?; if let (Some(grpid), Some(grpname)) = (grpid, grpname) { if context @@ -287,10 +292,13 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { } } else if let Some(addr) = addr { if let Some(peerstate) = peerstate { - let contact_id = + let (contact_id, _) = Contact::add_or_lookup(context, &name, &peerstate.addr, Origin::UnhandledQrScan) .await - .map(|(id, _)| id)?; + .context("add_or_lookup")? + .with_context(|| { + format!("Not looking up contact for address {:?}", &peerstate.addr) + })?; let chat = ChatIdBlocked::get_for_contact(context, contact_id, Blocked::Request) .await .context("Failed to create (new) chat for contact")?; @@ -620,7 +628,9 @@ impl Qr { draft: Option, ) -> Result { let (contact_id, _) = - Contact::add_or_lookup(context, &name, &addr, Origin::UnhandledQrScan).await?; + Contact::add_or_lookup(context, &name, &addr, Origin::UnhandledQrScan) + .await? + .context("QR code does not contain a valid address")?; Ok(Qr::Addr { contact_id, draft }) } } diff --git a/src/reaction.rs b/src/reaction.rs index 72a096030..6777c5375 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -368,6 +368,7 @@ Can we chat at 1pm pacific, today?" let bob_id = Contact::add_or_lookup(&alice, "", "bob@example.net", Origin::ManuallyCreated) .await? + .unwrap() .0; let bob_reaction = reactions.get(bob_id); assert!(bob_reaction.is_empty()); // Bob has not reacted to message yet. diff --git a/src/receive_imf.rs b/src/receive_imf.rs index e8ad015af..c22cfb348 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -170,7 +170,16 @@ 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, &mime_parser.from, prevent_rename).await?; + match from_field_to_contact_id(context, &mime_parser.from, prevent_rename).await? { + Some(contact_id_res) => contact_id_res, + None => { + warn!( + context, + "receive_imf: From field does not contain an acceptable address" + ); + return Ok(None); + } + }; let incoming = from_id != ContactId::SELF; @@ -373,22 +382,28 @@ pub async fn from_field_to_contact_id( context: &Context, from: &SingleInfo, prevent_rename: bool, -) -> Result<(ContactId, bool, Origin)> { +) -> Result> { let display_name = if prevent_rename { Some("") } else { from.display_name.as_deref() }; - let from_id = add_or_lookup_contact_by_addr( + let from_id = if let Some(from_id) = add_or_lookup_contact_by_addr( context, display_name, &from.addr, Origin::IncomingUnknownFrom, ) - .await?; + .await + .context("add_or_lookup_contact_by_addr")? + { + from_id + } else { + return Ok(None); + }; if from_id == ContactId::SELF { - Ok((ContactId::SELF, false, Origin::OutgoingBcc)) + Ok(Some((ContactId::SELF, false, Origin::OutgoingBcc))) } else { let mut from_id_blocked = false; let mut incoming_origin = Origin::Unknown; @@ -396,7 +411,7 @@ pub async fn from_field_to_contact_id( from_id_blocked = contact.blocked; incoming_origin = contact.origin; } - Ok((from_id, from_id_blocked, incoming_origin)) + Ok(Some((from_id, from_id_blocked, incoming_origin))) } } @@ -1928,8 +1943,11 @@ async fn apply_mailinglist_changes( } let listid = &chat.grpid; - let (contact_id, _) = - Contact::add_or_lookup(context, "", list_post, Origin::Hidden).await?; + let contact_id = + match Contact::add_or_lookup(context, "", list_post, Origin::Hidden).await? { + Some((contact_id, _)) => contact_id, + None => return Ok(()), + }; let mut contact = Contact::load_from_db(context, contact_id).await?; if contact.param.get(Param::ListId) != Some(listid) { contact.param.set(Param::ListId, listid); @@ -2268,28 +2286,39 @@ async fn add_or_lookup_contacts_by_address_list( continue; } let display_name = info.display_name.as_deref(); - contact_ids - .insert(add_or_lookup_contact_by_addr(context, display_name, addr, origin).await?); + if let Some(contact_id) = + add_or_lookup_contact_by_addr(context, display_name, addr, origin).await? + { + contact_ids.insert(contact_id); + } else { + warn!(context, "Contact with address {:?} cannot exist.", addr); + } } Ok(contact_ids.into_iter().collect::>()) } /// Add contacts to database on receiving messages. +/// +/// Returns `None` if the address can't be a valid email address. async fn add_or_lookup_contact_by_addr( context: &Context, display_name: Option<&str>, addr: &str, origin: Origin, -) -> Result { +) -> Result> { if context.is_self_addr(addr).await? { - return Ok(ContactId::SELF); + return Ok(Some(ContactId::SELF)); } let display_name_normalized = display_name.map(normalize_name).unwrap_or_default(); - let (row_id, _modified) = - Contact::add_or_lookup(context, &display_name_normalized, addr, origin).await?; - Ok(row_id) + if let Some((contact_id, _modified)) = + Contact::add_or_lookup(context, &display_name_normalized, addr, origin).await? + { + Ok(Some(contact_id)) + } else { + Ok(None) + } } #[cfg(test)] diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index a92c8aead..12a3901b0 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -429,6 +429,7 @@ async fn test_escaped_recipients() { Contact::add_or_lookup(&t, "Carl", "carl@host.tld", Origin::IncomingUnknownFrom) .await .unwrap() + .unwrap() .0; receive_imf( @@ -471,6 +472,7 @@ async fn test_cc_to_contact() { Contact::add_or_lookup(&t, "garabage", "carl@host.tld", Origin::IncomingUnknownFrom) .await .unwrap() + .unwrap() .0; receive_imf( @@ -2058,6 +2060,7 @@ async fn test_duplicate_message() -> Result<()> { Origin::IncomingUnknownFrom, ) .await? + .unwrap() .0; let first_message = b"Received: from [127.0.0.1] @@ -2111,6 +2114,7 @@ async fn test_ignore_footer_status_from_mailinglist() -> Result<()> { t.set_config(Config::ShowEmails, Some("2")).await?; let bob_id = Contact::add_or_lookup(&t, "", "bob@example.net", Origin::IncomingUnknownCc) .await? + .unwrap() .0; let bob = Contact::load_from_db(&t, bob_id).await?; assert_eq!(bob.get_status(), ""); @@ -2529,7 +2533,8 @@ Second thread."#; "fiona@example.net", Origin::IncomingUnknownTo, ) - .await?; + .await? + .unwrap(); chat::add_contact_to_chat(&alice, alice_first_msg.chat_id, alice_fiona_contact_id).await?; let alice_first_invite = alice.pop_sent_msg().await; diff --git a/src/securejoin.rs b/src/securejoin.rs index 77d743c17..f335f2e27 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -1006,7 +1006,8 @@ mod tests { "bob@example.net", Origin::ManuallyCreated, ) - .await?; + .await? + .unwrap(); let contact_bob = Contact::load_from_db(&alice.ctx, contact_bob_id).await?; assert_eq!( contact_bob.is_verified(&alice.ctx).await?, diff --git a/src/test_utils.rs b/src/test_utils.rs index cf19dafd8..4ae732591 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -529,7 +529,11 @@ impl TestContext { let (contact_id, modified) = Contact::add_or_lookup(self, &name, &addr, Origin::MailinglistAddress) .await - .unwrap(); + .expect("add_or_lookup") + .expect(&format!( + "contact with address {:?} cannot be created", + &addr + )); match modified { Modifier::None => (), Modifier::Modified => warn!(&self.ctx, "Contact {} modified by TestContext", &addr),