review suggestions

deltachat-contact-tools/Cargo.toml

@@ -4,7 +4,6 @@ version = "0.0.0" # No semver-stable versioning
 edition = "2021"
 description = "Contact-related tools, like parsing vcards and sanitizing name and address. Meant for internal use in the deltachat crate."
 license = "MPL-2.0"
-# TODO maybe it should be called "deltachat-text-utils" or similar?
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 

deltachat-contact-tools/src/lib.rs

@@ -29,7 +29,6 @@ use std::fmt;
 use std::ops::Deref;
 
 use anyhow::bail;
-use anyhow::format_err;
 use anyhow::Context as _;
 use anyhow::Result;
 use chrono::DateTime;
@@ -47,14 +46,15 @@ pub struct VcardContact {
     pub addr: String,
     /// The contact's display name, vcard property `fn`
     pub display_name: String,
-    /// The contact's public PGP key, vcard property `key`
+    /// The contact's public PGP key in Base64, vcard property `key`
     pub key: Option<String>,
-    /// The contact's profile photo (=avatar), vcard property `photo`
+    /// The contact's profile photo (=avatar) in Base64, vcard property `photo`
     pub profile_photo: Option<String>,
     /// The timestamp when the vcard was created / last updated, vcard property `rev`
     pub timestamp: Result<u64>,
 }
 
+/// Parses `VcardContact`s from a given `String`.
 pub fn parse_vcard(vcard: String) -> Result<Vec<VcardContact>> {
     fn remove_prefix<'a>(s: &'a str, prefix: &str) -> Option<&'a str> {
         let start_of_s = s.get(..prefix.len())?;
@@ -69,12 +69,18 @@ pub fn parse_vcard(vcard: String) -> Result<Vec<VcardContact>> {
         let remainder = remove_prefix(s, property)?;
 
         // TODO this doesn't handle the case where there are quotes around a colon
-        let (_params, value) = remainder.split_once(':')?;
+        let (params, value) = remainder.split_once(':')?;
+        if params
+            .chars()
+            .next()
+            .filter(|c| !c.is_ascii_punctuation() || *c == '_')
+            .is_some()
+        {
+            return None;
+        }
         Some(value)
     }
-    fn parse_datetime(datetime: Option<&str>) -> Result<u64> {
-        let datetime = datetime.context("No timestamp in vcard")?;
-
+    fn parse_datetime(datetime: &str) -> Result<u64> {
         // According to https://www.rfc-editor.org/rfc/rfc6350#section-4.3.5, the timestamp
         // is in ISO.8601.2004 format. DateTime::parse_from_rfc3339() apparently parses
         // ISO.8601, but fails to parse any of the examples given.
@@ -97,42 +103,45 @@ pub fn parse_vcard(vcard: String) -> Result<Vec<VcardContact>> {
             }
         }
 
-        let mut display_name = "";
-        let mut addr = "";
+        let mut display_name = None;
+        let mut addr = None;
         let mut key = None;
         let mut photo = None;
         let mut datetime = None;
 
         for line in lines.by_ref() {
             if let Some(email) = vcard_property(line, "email") {
-                addr = email;
+                addr.get_or_insert(email);
             } else if let Some(name) = vcard_property(line, "fn") {
-                display_name = name;
+                display_name.get_or_insert(name);
             } else if let Some(k) = remove_prefix(line, "KEY;PGP;ENCODING=BASE64:")
                 .or_else(|| remove_prefix(line, "KEY;TYPE=PGP;ENCODING=b:"))
                 .or_else(|| remove_prefix(line, "KEY:data:application/pgp-keys;base64,"))
             {
-                key = Some(key.unwrap_or(k));
+                key.get_or_insert(k);
             } else if let Some(p) = remove_prefix(line, "PHOTO;JPEG;ENCODING=BASE64:")
                 .or_else(|| remove_prefix(line, "PHOTO;TYPE=JPEG;ENCODING=b:"))
                 .or_else(|| remove_prefix(line, "PHOTO;ENCODING=BASE64;TYPE=JPEG:"))
             {
-                photo = Some(photo.unwrap_or(p));
+                photo.get_or_insert(p);
             } else if let Some(rev) = vcard_property(line, "rev") {
-                datetime = Some(datetime.unwrap_or(rev));
+                datetime.get_or_insert(rev);
             } else if line.eq_ignore_ascii_case("END:VCARD") {
                 break;
             }
         }
 
-        let (display_name, addr) = sanitize_name_and_addr(display_name, addr);
+        let (display_name, addr) =
+            sanitize_name_and_addr(display_name.unwrap_or(""), addr.unwrap_or(""));
 
         contacts.push(VcardContact {
             display_name,
             addr,
             key: key.map(|s| s.to_string()),
             profile_photo: photo.map(|s| s.to_string()),
-            timestamp: parse_datetime(datetime),
+            timestamp: datetime
+                .context("No timestamp in vcard")
+                .and_then(parse_datetime),
         });
     }
 
@@ -345,8 +354,6 @@ impl rusqlite::types::ToSql for EmailAddress {
 
 #[cfg(test)]
 mod tests {
-    use chrono::NaiveDateTime;
-
     use super::*;
 
     #[test]