diff --git a/src/chat.rs b/src/chat.rs index 8a2fb2a42..cef50d3d1 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3629,7 +3629,7 @@ mod tests { use crate::chatlist::{get_archived_cnt, Chatlist}; use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS}; - use crate::contact::Contact; + use crate::contact::{Contact, ContactAddress}; use crate::receive_imf::receive_imf; use crate::test_utils::TestContext; @@ -4692,10 +4692,13 @@ mod tests { let chat_id = create_group_chat(&t, ProtectionStatus::Unprotected, "foo").await?; assert!(!shall_attach_selfavatar(&t, chat_id).await?); - let (contact_id, _) = - Contact::add_or_lookup(&t, "", "foo@bar.org", Origin::IncomingUnknownTo) - .await? - .unwrap(); + let (contact_id, _) = Contact::add_or_lookup( + &t, + "", + ContactAddress::new("foo@bar.org")?, + Origin::IncomingUnknownTo, + ) + .await?; 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 @@ -4941,10 +4944,8 @@ mod tests { alice.set_config(Config::ShowEmails, Some("2")).await?; bob.set_config(Config::ShowEmails, Some("2")).await?; - let (contact_id, _) = - Contact::add_or_lookup(&alice, "", "bob@example.net", Origin::ManuallyCreated) - .await? - .unwrap(); + let alice_bob_contact = alice.add_or_lookup_contact(&bob).await; + let contact_id = alice_bob_contact.id; let alice_chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "grp").await?; let alice_chat = Chat::load_from_db(&alice, alice_chat_id).await?; @@ -5654,10 +5655,13 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] 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? - .unwrap(); + let (contact_id, _) = Contact::add_or_lookup( + &t, + "", + ContactAddress::new("foo@bar.org")?, + Origin::ManuallyCreated, + ) + .await?; // create a blocked chat let chat_id_orig = diff --git a/src/contact.rs b/src/contact.rs index b0e33a52c..1f5233fbf 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -4,6 +4,7 @@ use std::cmp::Reverse; use std::collections::BinaryHeap; use std::convert::{TryFrom, TryInto}; use std::fmt; +use std::ops::Deref; use std::path::PathBuf; use std::time::{SystemTime, UNIX_EPOCH}; @@ -36,6 +37,51 @@ use crate::{chat, stock_str}; /// Time during which a contact is considered as seen recently. const SEEN_RECENTLY_SECONDS: i64 = 600; +/// Valid contact address. +#[derive(Debug, Clone, Copy)] +pub(crate) struct ContactAddress<'a>(&'a str); + +impl Deref for ContactAddress<'_> { + type Target = str; + + fn deref(&self) -> &Self::Target { + self.0 + } +} + +impl AsRef for ContactAddress<'_> { + fn as_ref(&self) -> &str { + self.0 + } +} + +impl fmt::Display for ContactAddress<'_> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl<'a> ContactAddress<'a> { + /// Constructs a new contact address from string, + /// normalizing and validating it. + pub fn new(s: &'a str) -> Result { + let addr = addr_normalize(s); + if !may_be_valid_addr(addr) { + bail!("invalid address {:?}", s); + } + Ok(Self(addr)) + } +} + +/// Allow converting [`ContactAddress`] to an SQLite type. +impl rusqlite::types::ToSql for ContactAddress<'_> { + fn to_sql(&self) -> rusqlite::Result { + let val = rusqlite::types::Value::Text(self.0.to_string()); + let out = rusqlite::types::ToSqlOutput::Owned(val); + Ok(out) + } +} + /// Contact ID, including reserved IDs. /// /// Some contact IDs are reserved to identify special contacts. This @@ -378,12 +424,12 @@ impl Contact { let name = improve_single_line_input(name); let (name, addr) = sanitize_name_and_addr(&name, addr); + let addr = ContactAddress::new(&addr)?; let (contact_id, sth_modified) = - Contact::add_or_lookup(context, &name, &addr, Origin::ManuallyCreated) + 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))?; + .context("add_or_lookup")?; let blocked = Contact::is_blocked_load(context, contact_id).await?; match sth_modified { Modifier::None => {} @@ -473,28 +519,16 @@ impl Contact { pub(crate) async fn add_or_lookup( context: &Context, name: &str, - addr: &str, + addr: ContactAddress<'_>, mut origin: Origin, - ) -> Result> { + ) -> Result<(ContactId, Modifier)> { let mut sth_modified = Modifier::None; ensure!(!addr.is_empty(), "Can not add_or_lookup empty address"); ensure!(origin != Origin::Unknown, "Missing valid origin"); - let addr = addr_normalize(addr).to_string(); - if context.is_self_addr(&addr).await? { - return Ok(Some((ContactId::SELF, sth_modified))); - } - - if !may_be_valid_addr(&addr) { - warn!( - context, - "Bad address \"{}\" for contact \"{}\".", - addr, - if !name.is_empty() { name } else { "" }, - ); - return Ok(None); + return Ok((ContactId::SELF, sth_modified)); } let mut name = name; @@ -555,7 +589,7 @@ impl Contact { || row_authname.is_empty()); row_id = u32::try_from(id)?; - if origin as i32 >= row_origin as i32 && addr != row_addr { + if origin >= row_origin && addr.as_ref() != row_addr { update_addr = true; } if update_name || update_authname || update_addr || origin > row_origin { @@ -657,7 +691,7 @@ impl Contact { } } - Ok(Some((ContactId::new(row_id), sth_modified))) + Ok((ContactId::new(row_id), sth_modified)) } /// Add a number of contacts. @@ -683,21 +717,25 @@ impl Contact { for (name, addr) in split_address_book(addr_book).into_iter() { let (name, addr) = sanitize_name_and_addr(name, addr); let name = normalize_name(&name); - match Contact::add_or_lookup(context, &name, &addr, Origin::AddressBook).await { - Err(err) => { - warn!( - context, - "Failed to add address {} from address book: {}", addr, err - ); - } - Ok(None) => { - warn!(context, "Cannot create contact with address {}.", addr); - } - Ok(Some((_, modified))) => { - if modified != Modifier::None { - modify_cnt += 1 + match ContactAddress::new(&addr) { + Ok(addr) => { + match Contact::add_or_lookup(context, &name, addr, Origin::AddressBook).await { + Ok((_, modified)) => { + if modified != Modifier::None { + modify_cnt += 1 + } + } + Err(err) => { + warn!( + context, + "Failed to add address {} from address book: {}", addr, err + ); + } } } + Err(err) => { + warn!(context, "{:#}.", err); + } } } if modify_cnt > 0 { @@ -1701,11 +1739,10 @@ mod tests { let (id, _modified) = Contact::add_or_lookup( &context.ctx, "bob", - "user@example.org", + ContactAddress::new("user@example.org")?, Origin::IncomingReplyTo, ) - .await? - .unwrap(); + .await?; assert_ne!(id, ContactId::UNDEFINED); let contact = Contact::load_from_db(&context.ctx, id).await.unwrap(); @@ -1730,11 +1767,10 @@ mod tests { let (contact_bob_id, modified) = Contact::add_or_lookup( &context.ctx, "someone", - "user@example.org", + ContactAddress::new("user@example.org")?, Origin::ManuallyCreated, ) - .await? - .unwrap(); + .await?; assert_eq!(contact_bob_id, id); assert_eq!(modified, Modifier::Modified); let contact = Contact::load_from_db(&context.ctx, id).await.unwrap(); @@ -1766,6 +1802,18 @@ mod tests { Ok(()) } + #[test] + fn test_contact_address() -> Result<()> { + let alice_addr = "alice@example.org"; + let contact_address = ContactAddress::new(alice_addr)?; + assert_eq!(contact_address.as_ref(), alice_addr); + + let invalid_addr = "<> foobar"; + assert!(ContactAddress::new(invalid_addr).is_err()); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_add_or_lookup() { // add some contacts, this also tests add_address_book() @@ -1781,11 +1829,14 @@ mod tests { assert_eq!(Contact::add_address_book(&t, book).await.unwrap(), 4); // check first added contact, this modifies authname because it is empty - let (contact_id, sth_modified) = - Contact::add_or_lookup(&t, "bla foo", "one@eins.org", Origin::IncomingUnknownTo) - .await - .unwrap() - .unwrap(); + let (contact_id, sth_modified) = Contact::add_or_lookup( + &t, + "bla foo", + ContactAddress::new("one@eins.org").unwrap(), + Origin::IncomingUnknownTo, + ) + .await + .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); @@ -1797,11 +1848,14 @@ mod tests { assert_eq!(contact.get_name_n_addr(), "Name one (one@eins.org)"); // modify first added contact - let (contact_id_test, sth_modified) = - Contact::add_or_lookup(&t, "Real one", " one@eins.org ", Origin::ManuallyCreated) - .await - .unwrap() - .unwrap(); + let (contact_id_test, sth_modified) = Contact::add_or_lookup( + &t, + "Real one", + ContactAddress::new(" one@eins.org ").unwrap(), + Origin::ManuallyCreated, + ) + .await + .unwrap(); assert_eq!(contact_id, contact_id_test); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); @@ -1810,11 +1864,14 @@ mod tests { assert!(!contact.is_blocked()); // check third added contact (contact without name) - let (contact_id, sth_modified) = - Contact::add_or_lookup(&t, "", "three@drei.sam", Origin::IncomingUnknownTo) - .await - .unwrap() - .unwrap(); + let (contact_id, sth_modified) = Contact::add_or_lookup( + &t, + "", + ContactAddress::new("three@drei.sam").unwrap(), + Origin::IncomingUnknownTo, + ) + .await + .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::None); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); @@ -1827,11 +1884,10 @@ mod tests { let (contact_id_test, sth_modified) = Contact::add_or_lookup( &t, "m. serious", - "three@drei.sam", + ContactAddress::new("three@drei.sam").unwrap(), Origin::IncomingUnknownFrom, ) .await - .unwrap() .unwrap(); assert_eq!(contact_id, contact_id_test); assert_eq!(sth_modified, Modifier::Modified); @@ -1840,11 +1896,14 @@ mod tests { assert!(!contact.is_blocked()); // manually edit name of third contact (does not changed authorized name) - let (contact_id_test, sth_modified) = - Contact::add_or_lookup(&t, "schnucki", "three@drei.sam", Origin::ManuallyCreated) - .await - .unwrap() - .unwrap(); + let (contact_id_test, sth_modified) = Contact::add_or_lookup( + &t, + "schnucki", + ContactAddress::new("three@drei.sam").unwrap(), + Origin::ManuallyCreated, + ) + .await + .unwrap(); assert_eq!(contact_id, contact_id_test); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); @@ -1853,11 +1912,14 @@ mod tests { assert!(!contact.is_blocked()); // Fourth contact: - let (contact_id, sth_modified) = - Contact::add_or_lookup(&t, "", "alice@w.de", Origin::IncomingUnknownTo) - .await - .unwrap() - .unwrap(); + let (contact_id, sth_modified) = Contact::add_or_lookup( + &t, + "", + ContactAddress::new("alice@w.de").unwrap(), + Origin::IncomingUnknownTo, + ) + .await + .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::None); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); @@ -1992,10 +2054,13 @@ mod tests { assert!(Contact::delete(&alice, ContactId::SELF).await.is_err()); // Create Bob contact - let (contact_id, _) = - Contact::add_or_lookup(&alice, "Bob", "bob@example.net", Origin::ManuallyCreated) - .await? - .unwrap(); + let (contact_id, _) = Contact::add_or_lookup( + &alice, + "Bob", + ContactAddress::new("bob@example.net")?, + Origin::ManuallyCreated, + ) + .await?; let chat = alice .create_chat_with_contact("Bob", "bob@example.net") .await; @@ -2068,11 +2133,14 @@ mod tests { let t = TestContext::new().await; // incoming mail `From: bob1 ` - this should init authname - let (contact_id, sth_modified) = - Contact::add_or_lookup(&t, "bob1", "bob@example.org", Origin::IncomingUnknownFrom) - .await - .unwrap() - .unwrap(); + let (contact_id, sth_modified) = Contact::add_or_lookup( + &t, + "bob1", + ContactAddress::new("bob@example.org").unwrap(), + Origin::IncomingUnknownFrom, + ) + .await + .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::Created); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); @@ -2081,11 +2149,14 @@ mod tests { assert_eq!(contact.get_display_name(), "bob1"); // incoming mail `From: bob2 ` - this should update authname - let (contact_id, sth_modified) = - Contact::add_or_lookup(&t, "bob2", "bob@example.org", Origin::IncomingUnknownFrom) - .await - .unwrap() - .unwrap(); + let (contact_id, sth_modified) = Contact::add_or_lookup( + &t, + "bob2", + ContactAddress::new("bob@example.org").unwrap(), + Origin::IncomingUnknownFrom, + ) + .await + .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); @@ -2104,11 +2175,14 @@ mod tests { assert_eq!(contact.get_display_name(), "bob3"); // incoming mail `From: bob4 ` - this should update authname, manually given name is still "bob3" - let (contact_id, sth_modified) = - Contact::add_or_lookup(&t, "bob4", "bob@example.org", Origin::IncomingUnknownFrom) - .await - .unwrap() - .unwrap(); + let (contact_id, sth_modified) = Contact::add_or_lookup( + &t, + "bob4", + ContactAddress::new("bob@example.org").unwrap(), + Origin::IncomingUnknownFrom, + ) + .await + .unwrap(); assert!(!contact_id.is_special()); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); @@ -2133,11 +2207,10 @@ mod tests { let (contact_id_same, sth_modified) = Contact::add_or_lookup( &t, "claire1", - "claire@example.org", + ContactAddress::new("claire@example.org").unwrap(), Origin::IncomingUnknownFrom, ) .await - .unwrap() .unwrap(); assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); @@ -2150,11 +2223,10 @@ mod tests { let (contact_id_same, sth_modified) = Contact::add_or_lookup( &t, "claire2", - "claire@example.org", + ContactAddress::new("claire@example.org").unwrap(), Origin::IncomingUnknownFrom, ) .await - .unwrap() .unwrap(); assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); @@ -2173,29 +2245,38 @@ mod tests { let t = TestContext::new().await; // Incoming message from Bob. - let (contact_id, sth_modified) = - Contact::add_or_lookup(&t, "Bob", "bob@example.org", Origin::IncomingUnknownFrom) - .await? - .unwrap(); + let (contact_id, sth_modified) = Contact::add_or_lookup( + &t, + "Bob", + ContactAddress::new("bob@example.org")?, + Origin::IncomingUnknownFrom, + ) + .await?; assert_eq!(sth_modified, Modifier::Created); let contact = Contact::load_from_db(&t, contact_id).await?; assert_eq!(contact.get_display_name(), "Bob"); // 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? - .unwrap(); + let (contact_id_same, sth_modified) = Contact::add_or_lookup( + &t, + "Not Bob", + ContactAddress::new("bob@example.org")?, + Origin::IncomingUnknownTo, + ) + .await?; assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::load_from_db(&t, contact_id).await?; assert_eq!(contact.get_display_name(), "Not Bob"); // 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? - .unwrap(); + let (contact_id_same, sth_modified) = Contact::add_or_lookup( + &t, + "Bob", + ContactAddress::new("bob@example.org")?, + Origin::IncomingUnknownFrom, + ) + .await?; 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?; @@ -2218,9 +2299,14 @@ mod tests { assert_eq!(contact.get_display_name(), "dave1"); // incoming mail `From: dave2 ` - this should update authname - Contact::add_or_lookup(&t, "dave2", "dave@example.org", Origin::IncomingUnknownFrom) - .await - .unwrap(); + Contact::add_or_lookup( + &t, + "dave2", + ContactAddress::new("dave@example.org").unwrap(), + Origin::IncomingUnknownFrom, + ) + .await + .unwrap(); let contact = Contact::load_from_db(&t, contact_id).await.unwrap(); assert_eq!(contact.get_authname(), "dave2"); assert_eq!(contact.get_name(), "dave1"); @@ -2334,10 +2420,13 @@ mod tests { let encrinfo = Contact::get_encrinfo(&alice, ContactId::DEVICE).await; assert!(encrinfo.is_err()); - let (contact_bob_id, _modified) = - Contact::add_or_lookup(&alice, "Bob", "bob@example.net", Origin::ManuallyCreated) - .await? - .unwrap(); + let (contact_bob_id, _modified) = Contact::add_or_lookup( + &alice, + "Bob", + ContactAddress::new("bob@example.net")?, + Origin::ManuallyCreated, + ) + .await?; let encrinfo = Contact::get_encrinfo(&alice, contact_bob_id).await?; assert_eq!(encrinfo, "No encryption"); @@ -2494,10 +2583,13 @@ CCCB 5AA9 F6E1 141C 9431 async fn test_last_seen() -> Result<()> { let alice = TestContext::new_alice().await; - let (contact_id, _) = - Contact::add_or_lookup(&alice, "Bob", "bob@example.net", Origin::ManuallyCreated) - .await? - .unwrap(); + let (contact_id, _) = Contact::add_or_lookup( + &alice, + "Bob", + ContactAddress::new("bob@example.net")?, + Origin::ManuallyCreated, + ) + .await?; 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 c2bcd1f7a..fe8e3a1fb 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -22,7 +22,7 @@ use crate::config::Config; use crate::constants::{ Blocked, Chattype, ShowEmails, DC_FETCH_EXISTING_MSGS_COUNT, DC_FOLDERS_CONFIGURED_VERSION, }; -use crate::contact::{normalize_name, Contact, ContactId, Modifier, Origin}; +use crate::contact::{normalize_name, Contact, ContactAddress, ContactId, Modifier, Origin}; use crate::context::Context; use crate::events::EventType; use crate::headerdef::{HeaderDef, HeaderDefMap}; @@ -2375,23 +2375,23 @@ async fn add_all_recipients_as_contacts( .map(|s| normalize_name(s)) .unwrap_or_default(); - match Contact::add_or_lookup( - context, - &display_name_normalized, - &recipient.addr, - Origin::OutgoingTo, - ) - .await? - { - Some((_, modified)) => { + match ContactAddress::new(&recipient.addr) { + Err(err) => warn!( + context, + "Could not add contact for recipient with address {:?}: {:#}", recipient.addr, err + ), + Ok(recipient_addr) => { + let (_, modified) = Contact::add_or_lookup( + context, + &display_name_normalized, + recipient_addr, + Origin::OutgoingTo, + ) + .await?; if modified != Modifier::None { any_modified = true; } } - 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 bde304869..7025ad52c 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1490,7 +1490,7 @@ mod tests { ProtectionStatus, }; use crate::chatlist::Chatlist; - use crate::contact::Origin; + use crate::contact::{ContactAddress, Origin}; use crate::mimeparser::MimeMessage; use crate::receive_imf::receive_imf; use crate::test_utils::{get_chat_msg, TestContext}; @@ -1817,12 +1817,15 @@ mod tests { } async fn first_subject_str(t: TestContext) -> String { - let contact_id = - Contact::add_or_lookup(&t, "Dave", "dave@example.com", Origin::ManuallyCreated) - .await - .unwrap() - .unwrap() - .0; + let contact_id = Contact::add_or_lookup( + &t, + "Dave", + ContactAddress::new("dave@example.com").unwrap(), + Origin::ManuallyCreated, + ) + .await + .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 6224118d9..761fec50f 100644 --- a/src/peerstate.rs +++ b/src/peerstate.rs @@ -9,7 +9,7 @@ use crate::aheader::{Aheader, EncryptPreference}; use crate::chat::{self, Chat}; use crate::chatlist::Chatlist; use crate::constants::Chattype; -use crate::contact::{addr_cmp, Contact, Origin}; +use crate::contact::{addr_cmp, Contact, ContactAddress, Origin}; use crate::context::Context; use crate::events::EventType; use crate::key::{DcKey, Fingerprint, SignedPublicKey}; @@ -542,16 +542,30 @@ impl Peerstate { if (chat.typ == Chattype::Group && chat.is_protected()) || chat.typ == Chattype::Broadcast { - 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]) + match ContactAddress::new(new_addr) { + Ok(new_addr) => { + let (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?; - context.emit_event(EventType::ChatModified(*chat_id)); + context.emit_event(EventType::ChatModified(*chat_id)); + } + Err(err) => { + warn!( + context, + "New address {:?} is not vaild, not doing AEAP: {:#}.", + new_addr, + err + ) + } } } } diff --git a/src/qr.rs b/src/qr.rs index 7cd4fc28c..c6b5e18b7 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -14,7 +14,9 @@ use std::collections::BTreeMap; use crate::chat::{self, get_chat_id_by_grpid, ChatIdBlocked}; use crate::config::Config; use crate::constants::Blocked; -use crate::contact::{addr_normalize, may_be_valid_addr, Contact, ContactId, Origin}; +use crate::contact::{ + addr_normalize, may_be_valid_addr, Contact, ContactAddress, ContactId, Origin, +}; use crate::context::Context; use crate::key::Fingerprint; use crate::message::Message; @@ -221,19 +223,14 @@ 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 addr = ContactAddress::new(addr)?; let (contact_id, _) = Contact::add_or_lookup(context, &name, addr, Origin::UnhandledQrScan) .await - .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 - ) - })?; + .with_context(|| format!("failed to add or lookup contact for address {:?}", addr))?; if let (Some(grpid), Some(grpname)) = (grpid, grpname) { if context - .is_self_addr(addr) + .is_self_addr(&addr) .await .with_context(|| format!("can't check if address {:?} is our address", addr))? { @@ -266,7 +263,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { authcode, }) } - } else if context.is_self_addr(addr).await? { + } else if context.is_self_addr(&addr).await? { if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await { Ok(Qr::WithdrawVerifyContact { contact_id, @@ -292,13 +289,11 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { } } else if let Some(addr) = addr { if let Some(peerstate) = peerstate { + let peerstate_addr = ContactAddress::new(&peerstate.addr)?; let (contact_id, _) = - Contact::add_or_lookup(context, &name, &peerstate.addr, Origin::UnhandledQrScan) + Contact::add_or_lookup(context, &name, peerstate_addr, Origin::UnhandledQrScan) .await - .context("add_or_lookup")? - .with_context(|| { - format!("Not looking up contact for address {:?}", &peerstate.addr) - })?; + .context("add_or_lookup")?; let chat = ChatIdBlocked::get_for_contact(context, contact_id, Blocked::Request) .await .context("Failed to create (new) chat for contact")?; @@ -538,11 +533,11 @@ async fn decode_mailto(context: &Context, qr: &str) -> Result { }; let addr = normalize_address(addr)?; - let name = "".to_string(); + let name = ""; Qr::from_address( context, name, - addr, + &addr, if draft.is_empty() { None } else { Some(draft) }, ) .await @@ -562,8 +557,8 @@ async fn decode_smtp(context: &Context, qr: &str) -> Result { }; let addr = normalize_address(addr)?; - let name = "".to_string(); - Qr::from_address(context, name, addr, None).await + let name = ""; + Qr::from_address(context, name, &addr, None).await } /// Extract address for the matmsg scheme. @@ -587,8 +582,8 @@ async fn decode_matmsg(context: &Context, qr: &str) -> Result { }; let addr = normalize_address(addr)?; - let name = "".to_string(); - Qr::from_address(context, name, addr, None).await + let name = ""; + Qr::from_address(context, name, &addr, None).await } static VCARD_NAME_RE: Lazy = @@ -617,20 +612,19 @@ async fn decode_vcard(context: &Context, qr: &str) -> Result { bail!("Bad e-mail address"); }; - Qr::from_address(context, name, addr, None).await + Qr::from_address(context, &name, &addr, None).await } impl Qr { pub async fn from_address( context: &Context, - name: String, - addr: String, + name: &str, + addr: &str, draft: Option, ) -> Result { + let addr = ContactAddress::new(addr)?; let (contact_id, _) = - Contact::add_or_lookup(context, &name, &addr, Origin::UnhandledQrScan) - .await? - .context("QR code does not contain a valid address")?; + Contact::add_or_lookup(context, name, addr, Origin::UnhandledQrScan).await?; Ok(Qr::Addr { contact_id, draft }) } } diff --git a/src/reaction.rs b/src/reaction.rs index 6777c5375..f858f6172 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -286,11 +286,11 @@ pub async fn get_msg_reactions(context: &Context, msg_id: MsgId) -> Result from_addr, + Err(err) => { + warn!( + context, + "Cannot create a contact for the given From field: {:#}.", err + ); + return Ok(None); + } + }; + + let from_id = add_or_lookup_contact_by_addr( context, display_name, - &from.addr, + from_addr, Origin::IncomingUnknownFrom, ) - .await - .context("add_or_lookup_contact_by_addr")? - { - from_id - } else { - return Ok(None); - }; + .await?; if from_id == ContactId::SELF { Ok(Some((ContactId::SELF, false, Origin::OutgoingBcc))) @@ -1943,11 +1950,15 @@ async fn apply_mailinglist_changes( } let listid = &chat.grpid; - let contact_id = - match Contact::add_or_lookup(context, "", list_post, Origin::Hidden).await? { - Some((contact_id, _)) => contact_id, - None => return Ok(()), - }; + let list_post = match ContactAddress::new(list_post) { + Ok(list_post) => list_post, + Err(err) => { + warn!(context, "Invalid List-Post: {:#}.", err); + return Ok(()); + } + }; + let (contact_id, _) = + Contact::add_or_lookup(context, "", list_post, Origin::Hidden).await?; 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); @@ -1955,7 +1966,7 @@ async fn apply_mailinglist_changes( } if let Some(old_list_post) = chat.param.get(Param::ListPost) { - if list_post != old_list_post { + if list_post.as_ref() != old_list_post { // Apparently the mailing list is using a different List-Post header in each message. // Make the mailing list read-only because we would't know which message the user wants to reply to. chat.param.remove(Param::ListPost); @@ -2286,9 +2297,9 @@ async fn add_or_lookup_contacts_by_address_list( continue; } let display_name = info.display_name.as_deref(); - if let Some(contact_id) = - add_or_lookup_contact_by_addr(context, display_name, addr, origin).await? - { + if let Ok(addr) = ContactAddress::new(addr) { + let 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); @@ -2299,26 +2310,20 @@ async fn add_or_lookup_contacts_by_address_list( } /// 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, + addr: ContactAddress<'_>, origin: Origin, -) -> Result> { - if context.is_self_addr(addr).await? { - return Ok(Some(ContactId::SELF)); +) -> Result { + if context.is_self_addr(&addr).await? { + return Ok(ContactId::SELF); } let display_name_normalized = display_name.map(normalize_name).unwrap_or_default(); - if let Some((contact_id, _modified)) = - Contact::add_or_lookup(context, &display_name_normalized, addr, origin).await? - { - Ok(Some(contact_id)) - } else { - Ok(None) - } + let (contact_id, _modified) = + Contact::add_or_lookup(context, &display_name_normalized, addr, origin).await?; + Ok(contact_id) } #[cfg(test)] diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index 12a3901b0..cd5dcd30e 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -425,12 +425,15 @@ async fn test_escaped_recipients() { .await .unwrap(); - let carl_contact_id = - Contact::add_or_lookup(&t, "Carl", "carl@host.tld", Origin::IncomingUnknownFrom) - .await - .unwrap() - .unwrap() - .0; + let carl_contact_id = Contact::add_or_lookup( + &t, + "Carl", + ContactAddress::new("carl@host.tld").unwrap(), + Origin::IncomingUnknownFrom, + ) + .await + .unwrap() + .0; receive_imf( &t, @@ -468,12 +471,15 @@ async fn test_cc_to_contact() { .await .unwrap(); - let carl_contact_id = - Contact::add_or_lookup(&t, "garabage", "carl@host.tld", Origin::IncomingUnknownFrom) - .await - .unwrap() - .unwrap() - .0; + let carl_contact_id = Contact::add_or_lookup( + &t, + "garabage", + ContactAddress::new("carl@host.tld").unwrap(), + Origin::IncomingUnknownFrom, + ) + .await + .unwrap() + .0; receive_imf( &t, @@ -2056,11 +2062,10 @@ async fn test_duplicate_message() -> Result<()> { let bob_contact_id = Contact::add_or_lookup( &alice, "Bob", - "bob@example.org", + ContactAddress::new("bob@example.org").unwrap(), Origin::IncomingUnknownFrom, ) .await? - .unwrap() .0; let first_message = b"Received: from [127.0.0.1] @@ -2112,10 +2117,14 @@ Second signature"; async fn test_ignore_footer_status_from_mailinglist() -> Result<()> { let t = TestContext::new_alice().await; 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_id = Contact::add_or_lookup( + &t, + "", + ContactAddress::new("bob@example.net").unwrap(), + Origin::IncomingUnknownCc, + ) + .await? + .0; let bob = Contact::load_from_db(&t, bob_id).await?; assert_eq!(bob.get_status(), ""); assert_eq!(Chatlist::try_load(&t, 0, None, None).await?.len(), 0); @@ -2527,14 +2536,8 @@ Second thread."#; // Alice adds Fiona to both ad hoc groups. let fiona = TestContext::new_fiona().await; - let (alice_fiona_contact_id, _) = Contact::add_or_lookup( - &alice, - "Fiona", - "fiona@example.net", - Origin::IncomingUnknownTo, - ) - .await? - .unwrap(); + let alice_fiona_contact = alice.add_or_lookup_contact(&fiona).await; + let alice_fiona_contact_id = alice_fiona_contact.id; 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 f335f2e27..420966ceb 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -710,6 +710,7 @@ mod tests { use crate::chat::ProtectionStatus; use crate::chatlist::Chatlist; use crate::constants::{Chattype, DC_GCM_ADDDAYMARKER}; + use crate::contact::ContactAddress; use crate::peerstate::Peerstate; use crate::receive_imf::receive_imf; use crate::test_utils::{TestContext, TestContextManager}; @@ -1003,11 +1004,10 @@ mod tests { let (contact_bob_id, _modified) = Contact::add_or_lookup( &alice.ctx, "Bob", - "bob@example.net", + ContactAddress::new("bob@example.net")?, Origin::ManuallyCreated, ) - .await? - .unwrap(); + .await?; 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 d8542fe2f..f5bce1cf3 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -23,7 +23,7 @@ use crate::chatlist::Chatlist; use crate::config::Config; use crate::constants::Chattype; use crate::constants::{DC_GCL_NO_SPECIALS, DC_GCM_ADDDAYMARKER, DC_MSG_ID_DAYMARKER}; -use crate::contact::{Contact, ContactId, Modifier, Origin}; +use crate::contact::{Contact, ContactAddress, ContactId, Modifier, Origin}; use crate::context::Context; use crate::events::{Event, EventType, Events}; use crate::key::{self, DcKey, KeyPair, KeyPairUse}; @@ -523,14 +523,14 @@ impl TestContext { .await .unwrap_or_default() .unwrap_or_default(); - let addr = other.ctx.get_primary_self_addr().await.unwrap(); + let primary_self_addr = other.ctx.get_primary_self_addr().await.unwrap(); + let addr = ContactAddress::new(&primary_self_addr).unwrap(); // MailinglistAddress is the lowest allowed origin, we'd prefer to not modify the // origin when creating this contact. let (contact_id, modified) = - Contact::add_or_lookup(self, &name, &addr, Origin::MailinglistAddress) + Contact::add_or_lookup(self, &name, addr, Origin::MailinglistAddress) .await - .expect("add_or_lookup") - .unwrap_or_else(|| panic!("contact with address {:?} cannot be created", &addr)); + .expect("add_or_lookup"); match modified { Modifier::None => (), Modifier::Modified => warn!(&self.ctx, "Contact {} modified by TestContext", &addr),