Merge pull request #1922 from pierrechevalier83/make_email_with_spaces_roundtrip

fix: Make email with spaces round-trip

Author: Byron

gix-actor/src/lib.rs

@@ -28,20 +28,28 @@ pub mod signature;
 #[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Identity {
-    /// The actors name.
+    /// The actors name, potentially with whitespace as parsed.
+    ///
+    /// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
     pub name: BString,
-    /// The actor's email.
+    /// The actor's email, potentially with whitespace and garbage as parsed.
+    ///
+    /// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
     pub email: BString,
 }
 
 /// A person with name and email, as reference.
 #[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct IdentityRef<'a> {
-    /// The actors name.
+    /// The actors name, potentially with whitespace as parsed.
+    ///
+    /// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
     #[cfg_attr(feature = "serde", serde(borrow))]
     pub name: &'a BStr,
-    /// The actor's email.
+    /// The actor's email, potentially with whitespace and garbage as parsed.
+    ///
+    /// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
     pub email: &'a BStr,
 }
 
@@ -51,9 +59,13 @@ pub struct IdentityRef<'a> {
 #[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Signature {
-    /// The actors name.
+    /// The actors name, potentially with whitespace as parsed.
+    ///
+    /// Use [SignatureRef::trim()] or trim manually to be able to clean it up.
     pub name: BString,
-    /// The actor's email.
+    /// The actor's email, potentially with whitespace and garbage as parsed.
+    ///
+    /// Use [SignatureRef::trim()] or trim manually to be able to clean it up.
     pub email: BString,
     /// The time stamp at which the signature is performed.
     pub time: Time,
@@ -65,10 +77,14 @@ pub struct Signature {
 #[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct SignatureRef<'a> {
-    /// The actor's name.
+    /// The actors name, potentially with whitespace as parsed.
+    ///
+    /// Use [SignatureRef::trim()] or trim manually to be able to clean it up.
     #[cfg_attr(feature = "serde", serde(borrow))]
     pub name: &'a BStr,
-    /// The actor's email.
+    /// The actor's email, potentially with whitespace and garbage as parsed.
+    ///
+    /// Use [SignatureRef::trim()] or trim manually to be able to clean it up.
     pub email: &'a BStr,
     /// The time stamp at which the signature was performed.
     pub time: gix_date::Time,

gix-actor/src/signature/decode.rs

@@ -79,22 +79,15 @@ pub(crate) mod function {
                 StrContext::Label("Closing '>' not found"),
             )))?;
         let i_name_and_email = &i[..right_delim_idx];
-        let skip_from_right = i_name_and_email
-            .iter()
-            .rev()
-            .take_while(|b| b.is_ascii_whitespace() || **b == b'>')
-            .count();
+        let skip_from_right = i_name_and_email.iter().rev().take_while(|b| **b == b'>').count();
         let left_delim_idx = i_name_and_email
             .find_byte(b'<')
             .ok_or(ErrMode::Cut(E::from_input(i).add_context(
                 &i_name_and_email,
                 &start,
                 StrContext::Label("Opening '<' not found"),
             )))?;
-        let skip_from_left = i[left_delim_idx..]
-            .iter()
-            .take_while(|b| b.is_ascii_whitespace() || **b == b'<')
-            .count();
+        let skip_from_left = i[left_delim_idx..].iter().take_while(|b| **b == b'<').count();
         let mut name = i[..left_delim_idx].as_bstr();
         name = name.strip_suffix(b" ").unwrap_or(name).as_bstr();
 
@@ -164,6 +157,17 @@ mod tests {
             );
         }
 
+        #[test]
+        fn email_with_space() {
+            assert_eq!(
+                decode
+                    .parse_peek(b"Sebastian Thiel <\tbyronimo@gmail.com > 1528473343 +0230")
+                    .expect("parse to work")
+                    .1,
+                signature("Sebastian Thiel", "\tbyronimo@gmail.com ", 1528473343, Sign::Plus, 9000)
+            );
+        }
+
         #[test]
         fn negative_offset_0000() {
             assert_eq!(

gix-actor/tests/identity/mod.rs

@@ -5,6 +5,9 @@ use gix_actor::Identity;
 fn round_trip() -> gix_testtools::Result {
     static DEFAULTS: &[&[u8]] =     &[
         b"Sebastian Thiel <byronimo@gmail.com>",
+        b"Sebastian Thiel < byronimo@gmail.com>",
+        b"Sebastian Thiel <byronimo@gmail.com  >",
+        b"Sebastian Thiel <\tbyronimo@gmail.com \t >",
         ".. ☺️Sebastian 王知明 Thiel🙌 .. <byronimo@gmail.com>".as_bytes(),
         b".. whitespace  \t  is explicitly allowed    - unicode aware trimming must be done elsewhere  <byronimo@gmail.com>"
     ];
@@ -19,15 +22,21 @@ fn round_trip() -> gix_testtools::Result {
 
 #[test]
 fn lenient_parsing() -> gix_testtools::Result {
-    for input in [
-        "First Last<<fl <First Last<fl@openoffice.org >> >",
-        "First Last<fl <First Last<fl@openoffice.org>>\n",
+    for (input, expected_email) in [
+        (
+            "First Last<<fl <First Last<fl@openoffice.org >> >",
+            "fl <First Last<fl@openoffice.org >> ",
+        ),
+        (
+            "First Last<fl <First Last<fl@openoffice.org>>\n",
+            "fl <First Last<fl@openoffice.org",
+        ),
     ] {
         let identity = gix_actor::IdentityRef::from_bytes::<()>(input.as_bytes()).unwrap();
         assert_eq!(identity.name, "First Last");
         assert_eq!(
-            identity.email, "fl <First Last<fl@openoffice.org",
-            "extra trailing and leading angled parens are stripped"
+            identity.email, expected_email,
+            "emails are parsed but left as is for round-tripping"
         );
         let signature: Identity = identity.into();
         let mut output = Vec::new();

gix-object/tests/fixtures/commit/email-with-space.txt

@@ -0,0 +1,5 @@
+tree 1b2dfb4ac5e42080b682fc676e9738c94ce6d54d
+author Sebastian Thiel <sebastian.thiel@icloud.com > 1592437401 +0800
+committer Sebastian Thiel <sebastian.thiel@icloud.com> 1592437401 +0800
+
+In the author line, the email is followed by a space

gix-object/tests/object/encode/mod.rs

@@ -87,6 +87,7 @@ mod commit {
     round_trip!(
         gix_object::Commit,
         gix_object::CommitRef,
+        "commit/email-with-space.txt",
         "commit/signed-whitespace.txt",
         "commit/two-multiline-headers.txt",
         "commit/mergetag.txt",