Improve handling of quoted-pairs

src/addrparse.rs

@@ -3,6 +3,10 @@ use std::fmt;
 use crate::header::HeaderToken;
 use crate::{MailHeader, MailParseError};
 
+fn escape(text: &str) -> String {
+    text.replace('\\', "\\\\").replace('"', r#"\""#)
+}
+
 /// A representation of a single mailbox. Each mailbox has
 /// a routing address `addr` and an optional display name.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
@@ -29,7 +33,7 @@ impl SingleInfo {
 impl fmt::Display for SingleInfo {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if let Some(name) = &self.display_name {
-            write!(f, r#""{}" <{}>"#, name.replace('"', r#"\""#), self.addr)
+            write!(f, r#""{}" <{}>"#, escape(name), self.addr)
         } else {
             write!(f, "{}", self.addr)
         }
@@ -55,7 +59,7 @@ impl GroupInfo {
 
 impl fmt::Display for GroupInfo {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, r#""{}":"#, self.group_name.replace('"', r#"\""#))?;
+        write!(f, r#""{}":"#, escape(&self.group_name))?;
         for (i, addr) in self.addrs.iter().enumerate() {
             if i == 0 {
                 write!(f, " ")?;
@@ -91,6 +95,7 @@ enum AddrParseState {
     Unquoted,
     NameWithEncodedWord,
     Comment,
+    CommentEscapedChar,
 }
 
 /// A simple wrapper around `Vec<MailAddr>`. This is primarily here so we can
@@ -625,6 +630,8 @@ fn addrparse_inner(
                     HeaderTokenItem::Char(c) => {
                         if c == ')' {
                             state = comment_return.take().unwrap();
+                        } else if c == '\\' {
+                            state = AddrParseState::CommentEscapedChar;
                         }
                     }
                     HeaderTokenItem::Whitespace(_) => {
@@ -638,6 +645,9 @@ fn addrparse_inner(
                     }
                 }
             }
+            AddrParseState::CommentEscapedChar => {
+                state = AddrParseState::Comment;
+            }
         }
 
         hti = match it.next() {
@@ -656,6 +666,7 @@ fn addrparse_inner(
         | AddrParseState::AfterQuotedName
         | AddrParseState::BracketedAddr
         | AddrParseState::Comment
+        | AddrParseState::CommentEscapedChar
         | AddrParseState::NameWithEncodedWord => Err(MailParseError::Generic(
             "Address string unexpectedly terminated",
         )),
@@ -716,15 +727,18 @@ mod tests {
 
     #[test]
     fn parse_backslashes() {
+        let parsed = SingleInfo::new(
+            Some("First \"nick\" Last".to_string()),
+            "user@host.tld".to_string(),
+        )
+        .unwrap();
         assert_eq!(
             addrparse(r#" "First \"nick\" Last" <user@host.tld> "#).unwrap(),
-            MailAddrList(vec![MailAddr::Single(
-                SingleInfo::new(
-                    Some("First \"nick\" Last".to_string()),
-                    "user@host.tld".to_string()
-                )
-                .unwrap()
-            )])
+            MailAddrList(vec![MailAddr::Single(parsed.clone())])
+        );
+        assert_eq!(
+            parsed.to_string(),
+            r#""First \"nick\" Last" <user@host.tld>"#
         );
         assert_eq!(
             addrparse(r#" First \"nick\" Last <user@host.tld> "#).unwrap(),
@@ -924,6 +938,21 @@ mod tests {
             addrparse(&tc.to_string()).unwrap(),
             MailAddrList(vec![MailAddr::Single(tc)])
         );
+
+        let tc =
+            SingleInfo::new(Some(r"John \ Doe".to_string()), "john@doe.com".to_string()).unwrap();
+        assert_eq!(tc.to_string(), r#""John \\ Doe" <john@doe.com>"#);
+        assert_eq!(
+            addrparse(&tc.to_string()).unwrap(),
+            MailAddrList(vec![MailAddr::Single(tc)])
+        );
+
+        let tc = SingleInfo::new(Some(r#"a\b"c"#.to_string()), "john@doe.com".to_string()).unwrap();
+        assert_eq!(tc.to_string(), r#""a\\b\"c" <john@doe.com>"#);
+        assert_eq!(
+            addrparse(&tc.to_string()).unwrap(),
+            MailAddrList(vec![MailAddr::Single(tc)])
+        );
     }
 
     #[test]
@@ -954,6 +983,13 @@ mod tests {
             addrparse(&tc.to_string()).unwrap(),
             MailAddrList(vec![MailAddr::Group(tc)])
         );
+
+        let tc = GroupInfo::new(r"group-with\backslash".to_string(), vec![]);
+        assert_eq!(tc.to_string(), r#""group-with\\backslash":;"#);
+        assert_eq!(
+            addrparse(&tc.to_string()).unwrap(),
+            MailAddrList(vec![MailAddr::Group(tc)])
+        );
     }
 
     #[test]
@@ -1141,4 +1177,42 @@ mod tests {
             )])
         );
     }
+
+    #[test]
+    fn parse_escaped_comment() {
+        // A quoted-pair inside a comment does not end it (RFC 5322 3.2.2).
+        assert_eq!(
+            addrparse(r"x@y.com (a\)b)").unwrap(),
+            MailAddrList(vec![MailAddr::Single(
+                SingleInfo::new(None, "x@y.com".to_string()).unwrap()
+            )])
+        );
+        assert_eq!(
+            addrparse(r"x@y.com (a\(b)").unwrap(),
+            MailAddrList(vec![MailAddr::Single(
+                SingleInfo::new(None, "x@y.com".to_string()).unwrap()
+            )])
+        );
+        assert_eq!(
+            addrparse(r"x@y.com (a\\b)").unwrap(),
+            MailAddrList(vec![MailAddr::Single(
+                SingleInfo::new(None, "x@y.com".to_string()).unwrap()
+            )])
+        );
+        assert_eq!(
+            addrparse(r"x@y.com (plain)").unwrap(),
+            MailAddrList(vec![MailAddr::Single(
+                SingleInfo::new(None, "x@y.com".to_string()).unwrap()
+            )])
+        );
+        assert_eq!(
+            addrparse(r"x@y.com (a\)b) (c\)d)").unwrap(),
+            MailAddrList(vec![MailAddr::Single(
+                SingleInfo::new(None, "x@y.com".to_string()).unwrap()
+            )])
+        );
+        // An unterminated comment is still an error, not a silent truncation.
+        assert!(addrparse(r"x@y.com (a\)b").is_err());
+        assert!(addrparse(r"x@y.com (ab\)").is_err());
+    }
 }

src/lib.rs

@@ -1069,6 +1069,31 @@ fn split_continuation_index(key: &str) -> Option<(&str, usize)> {
     Some((base, index.parse().ok()?))
 }
 
+fn unescape(text: &str) -> String {
+    if !(text.starts_with('"') && text.ends_with('"') && text.len() > 1) {
+        return text.to_string();
+    }
+    let unquoted = &text[1..text.len() - 1];
+    if unquoted.find('\\').is_none() {
+        return unquoted.to_string();
+    }
+    let mut unescaped = String::with_capacity(unquoted.len());
+    let mut escape = false;
+    for c in unquoted.chars() {
+        if escape {
+            // Windows file paths may need to be handled specially here
+            // since they (incorrectly) contain unescaped '\' characters.
+            // If so, restore the '\' if c is none of [backslash, quote, semicolon].
+            escape = false;
+        } else if c == '\\' {
+            escape = true;
+            continue;
+        }
+        unescaped.push(c);
+    }
+    return unescaped;
+}
+
 /// Parse parameterized header values such as that for Content-Type
 /// e.g. `multipart/alternative; boundary=foobar`
 /// Note: this function is not made public as it may require
@@ -1086,11 +1111,8 @@ fn parse_param_content(content: &str) -> ParamContent {
         .filter_map(|kv| {
             kv.find('=').map(|idx| {
                 let key = kv[0..idx].trim().to_lowercase();
-                let mut value = kv[idx + 1..].trim();
-                if value.starts_with('"') && value.ends_with('"') && value.len() > 1 {
-                    value = &value[1..value.len() - 1];
-                }
-                (key, value.to_string())
+                let value = kv[idx + 1..].trim();
+                (key, unescape(value))
             })
         })
         .collect();
@@ -1952,6 +1974,41 @@ mod tests {
         assert_eq!(parsed.params["charset"], "utf-8");
     }
 
+    #[test]
+    fn test_parameter_quoted_pairs() {
+        let parsed = parse_param_content(r#"attachment; filename="a\"b.txt""#);
+        assert_eq!(parsed.params["filename"], r#"a"b.txt"#);
+
+        let parsed = parse_param_content(r#"attachment; filename="a\\b.txt""#);
+        assert_eq!(parsed.params["filename"], r#"a\b.txt"#);
+
+        let parsed = parse_param_content(r#"attachment; filename="\\\\""#);
+        assert_eq!(parsed.params["filename"], r#"\\"#);
+
+        // technically malformed but we handle it gracefully by dropping the
+        // last backslash
+        let parsed = parse_param_content(r#" attachment; filename="\\\" "#);
+        assert_eq!(parsed.params["filename"], r#"\"#);
+
+        let parsed = parse_param_content(r#"attachment; filename=a\b.txt"#);
+        assert_eq!(parsed.params["filename"], r#"a\b.txt"#);
+
+        let parsed = parse_param_content(r#"attachment; filename=a\"b.txt"#);
+        assert_eq!(parsed.params["filename"], r#"a\"b.txt"#);
+
+        // Same values reached through the two public accessors.
+        let cd = parse_content_disposition(r#"attachment; filename="a\"b.txt""#);
+        assert_eq!(cd.params["filename"], r#"a"b.txt"#);
+
+        let ct = parse_content_type(r#"text/plain; name="a\\b"; boundary="x\"y""#);
+        assert_eq!(ct.params["name"], r"a\b");
+        assert_eq!(ct.params["boundary"], r#"x"y"#);
+
+        // A quoted-pair also survives the RFC 2231 continuation join.
+        let parsed = parse_param_content(r#"attachment; filename*0="a\\"; filename*1="b.txt""#);
+        assert_eq!(parsed.params["filename"], r"a\b.txt");
+    }
+
     #[test]
     fn test_default_content_encoding() {
         let mail = parse_mail(b"Content-Type: text/plain; charset=UTF-7\r\n\r\n+JgM-").unwrap();