Fix #1474 "Sending message to contact with < or > in Recipient gets treated as "Sent" but is not received" (#1476)

Fix #1474 "Sending message to contact with < or > in Recipient gets treated as "Sent" but is not received".

As I was at it, I also extracted the correct name and address from addresses like Mueller, Dave <dave@domain.com>.

Cargo.toml

@@ -50,7 +50,7 @@ bitflags = "1.1.0"
 debug_stub_derive = "0.3.0"
 sanitize-filename = "0.2.1"
 stop-token = { version = "0.1.1", features = ["unstable"] }
-mailparse = "0.12.0"
+mailparse = "0.12.1"
 encoded-words = { git = "https://github.com/async-email/encoded-words", branch="master" }
 native-tls = "0.2.3"
 image = { version = "0.22.4", default-features=false, features = ["gif_codec", "jpeg", "ico", "png_codec", "pnm", "webp", "bmp"] }

src/contact.rs

@@ -3,6 +3,8 @@
 use async_std::path::PathBuf;
 use deltachat_derive::*;
 use itertools::Itertools;
+use lazy_static::lazy_static;
+use regex::Regex;
 
 use crate::aheader::EncryptPreference;
 use crate::chat::ChatId;
@@ -238,6 +240,8 @@ impl Contact {
             "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?;
         let blocked = Contact::is_blocked_load(context, contact_id).await;
@@ -1030,6 +1034,24 @@ pub fn addr_normalize(addr: &str) -> &str {
     norm
 }
 
+fn sanitize_name_and_addr(name: impl AsRef<str>, addr: impl AsRef<str>) -> (String, String) {
+    lazy_static! {
+        static ref ADDR_WITH_NAME_REGEX: Regex = Regex::new("(.*)<(.*)>").unwrap();
+    }
+    if let Some(captures) = ADDR_WITH_NAME_REGEX.captures(addr.as_ref()) {
+        (
+            if name.as_ref().is_empty() {
+                normalize_name(&captures[1])
+            } else {
+                name.as_ref().to_string()
+            },
+            captures[2].to_string(),
+        )
+    } else {
+        (name.as_ref().to_string(), addr.as_ref().to_string())
+    }
+}
+
 async fn set_block_contact(context: &Context, contact_id: u32, new_blocking: bool) {
     if contact_id <= DC_CONTACT_ID_LAST_SPECIAL {
         return;
@@ -1204,6 +1226,10 @@ mod tests {
         assert_eq!(may_be_valid_addr("u@d.tt"), true);
         assert_eq!(may_be_valid_addr("u@.tt"), false);
         assert_eq!(may_be_valid_addr("@d.tt"), false);
+        assert_eq!(may_be_valid_addr("<da@d.tt"), false);
+        assert_eq!(may_be_valid_addr("sk <@d.tt>"), false);
+        assert_eq!(may_be_valid_addr("as@sd.de>"), false);
+        assert_eq!(may_be_valid_addr("ask dkl@dd.tt"), false);
     }
 
     #[test]
@@ -1530,4 +1556,50 @@ mod tests {
         assert!(addr_cmp(" aa@aa.ORG ", "AA@AA.ORG"));
         assert!(addr_cmp(" mailto:AA@AA.ORG", "Aa@Aa.orG"));
     }
+
+    #[async_std::test]
+    async fn test_name_in_address() {
+        let t = dummy_context().await;
+
+        let contact_id = Contact::create(&t.ctx, "", "<dave@example.org>")
+            .await
+            .unwrap();
+        let contact = Contact::load_from_db(&t.ctx, contact_id).await.unwrap();
+        assert_eq!(contact.get_name(), "");
+        assert_eq!(contact.get_addr(), "dave@example.org");
+
+        let contact_id = Contact::create(&t.ctx, "", "Mueller, Dave <dave@example.org>")
+            .await
+            .unwrap();
+        let contact = Contact::load_from_db(&t.ctx, contact_id).await.unwrap();
+        assert_eq!(contact.get_name(), "Dave Mueller");
+        assert_eq!(contact.get_addr(), "dave@example.org");
+
+        let contact_id = Contact::create(&t.ctx, "name1", "name2 <dave@example.org>")
+            .await
+            .unwrap();
+        let contact = Contact::load_from_db(&t.ctx, contact_id).await.unwrap();
+        assert_eq!(contact.get_name(), "name1");
+        assert_eq!(contact.get_addr(), "dave@example.org");
+
+        assert!(Contact::create(&t.ctx, "", "<dskjfdslk@sadklj.dk")
+            .await
+            .is_err());
+        assert!(Contact::create(&t.ctx, "", "<dskjf>dslk@sadklj.dk>")
+            .await
+            .is_err());
+        assert!(Contact::create(&t.ctx, "", "dskjfdslksadklj.dk")
+            .await
+            .is_err());
+        assert!(Contact::create(&t.ctx, "", "dskjfdslk@sadklj.dk>")
+            .await
+            .is_err());
+        assert!(Contact::create(&t.ctx, "", "dskjf@dslk@sadkljdk")
+            .await
+            .is_err());
+        assert!(Contact::create(&t.ctx, "", "dskjf dslk@d.e").await.is_err());
+        assert!(Contact::create(&t.ctx, "", "<dskjf dslk@sadklj.dk")
+            .await
+            .is_err());
+    }
 }

src/dc_tools.rs

@@ -482,15 +482,6 @@ pub struct InvalidEmailError {
     addr: String,
 }
 
-impl InvalidEmailError {
-    fn new(msg: impl Into<String>, addr: impl Into<String>) -> InvalidEmailError {
-        InvalidEmailError {
-            message: msg.into(),
-            addr: addr.into(),
-        }
-    }
-}
-
 /// Very simple email address wrapper.
 ///
 /// Represents an email address, right now just the `name@domain` portion.
@@ -530,17 +521,24 @@ impl FromStr for EmailAddress {
 
     /// Performs a dead-simple parse of an email address.
     fn from_str(input: &str) -> Result<EmailAddress, InvalidEmailError> {
-        if input.is_empty() {
-            return Err(InvalidEmailError::new("empty string is not valid", input));
-        }
-        let parts: Vec<&str> = input.rsplitn(2, '@').collect();
-
         let err = |msg: &str| {
             Err(InvalidEmailError {
                 message: msg.to_string(),
                 addr: input.to_string(),
             })
         };
+        if input.is_empty() {
+            return err("empty string is not valid");
+        }
+        let parts: Vec<&str> = input.rsplitn(2, '@').collect();
+
+        if input
+            .chars()
+            .any(|c| c.is_whitespace() || c == '<' || c == '>')
+        {
+            return err("Email must not contain whitespaces, '>' or '<'");
+        }
+
         match &parts[..] {
             [domain, local] => {
                 if local.is_empty() {