fix: make actor parsing even more lenient (#1438)

Author: Byron

gix-actor/src/signature/decode.rs

@@ -1,7 +1,10 @@
 pub(crate) mod function {
+    use crate::{IdentityRef, SignatureRef};
     use bstr::ByteSlice;
     use gix_date::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch, Time};
     use gix_utils::btoi::to_signed;
+    use winnow::error::{ErrMode, ErrorKind};
+    use winnow::stream::Stream;
     use winnow::{
         combinator::{alt, separated_pair, terminated},
         error::{AddContext, ParserError, StrContext},
@@ -10,8 +13,6 @@ pub(crate) mod function {
         token::{take, take_until, take_while},
     };
 
-    use crate::{IdentityRef, SignatureRef};
-
     const SPACE: &[u8] = b" ";
 
     /// Parse a signature from the bytes input `i` using `nom`.
@@ -64,16 +65,47 @@ pub(crate) mod function {
     pub fn identity<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>(
         i: &mut &'a [u8],
     ) -> PResult<IdentityRef<'a>, E> {
-        (
-            terminated(take_until(0.., &b" <"[..]), take(2usize)).context(StrContext::Expected("<name>".into())),
-            terminated(take_until(0.., &b">"[..]), take(1usize)).context(StrContext::Expected("<email>".into())),
-        )
-            .map(|(name, email): (&[u8], &[u8])| IdentityRef {
-                name: name.as_bstr(),
-                email: email.as_bstr(),
-            })
-            .context(StrContext::Expected("<name> <<email>>".into()))
-            .parse_next(i)
+        let start = i.checkpoint();
+        let eol_idx = i.find_byte(b'\n').unwrap_or(i.len());
+        let right_delim_idx =
+            i[..eol_idx]
+                .rfind_byte(b'>')
+                .ok_or(ErrMode::Cut(E::from_error_kind(i, ErrorKind::Eof).add_context(
+                    i,
+                    &start,
+                    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 left_delim_idx =
+            i_name_and_email
+                .find_byte(b'<')
+                .ok_or(ErrMode::Cut(E::from_error_kind(i, ErrorKind::Eof).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 mut name = i[..left_delim_idx].as_bstr();
+        name = name.strip_suffix(b" ").unwrap_or(name).as_bstr();
+
+        let email = i
+            .get(left_delim_idx + skip_from_left..right_delim_idx - skip_from_right)
+            .ok_or(ErrMode::Cut(E::from_error_kind(i, ErrorKind::Eof).add_context(
+                &i_name_and_email,
+                &start,
+                StrContext::Label("Skipped parts run into each other"),
+            )))?
+            .as_bstr();
+        *i = i.get(right_delim_idx + 1..).unwrap_or(&[]);
+        Ok(IdentityRef { name, email })
     }
 }
 pub use function::identity;
@@ -167,7 +199,7 @@ mod tests {
                             .map_err(to_bstr_err)
                             .expect_err("parse fails as > is missing")
                             .to_string(),
-                        "in slice at ' 12345 -1215'\n  0: expected `<email>` at ' 12345 -1215'\n  1: expected `<name> <<email>>` at 'hello < 12345 -1215'\n  2: expected `<name> <<email>> <timestamp> <+|-><HHMM>` at 'hello < 12345 -1215'\n"
+                        "in end of file at 'hello < 12345 -1215'\n  0: invalid Closing '>' not found at 'hello < 12345 -1215'\n  1: expected `<name> <<email>> <timestamp> <+|-><HHMM>` at 'hello < 12345 -1215'\n"
                     );
         }
 

gix-actor/tests/identity/mod.rs

@@ -16,3 +16,27 @@ fn round_trip() -> gix_testtools::Result {
     }
     Ok(())
 }
+
+#[test]
+fn lenient_parsing() -> gix_testtools::Result {
+    for input in [
+        "First Last<<fl <First Last<[email protected] >> >",
+        "First Last<fl <First Last<[email protected]>>\n",
+    ] {
+        let identity = gix_actor::IdentityRef::from_bytes::<()>(input.as_bytes()).unwrap();
+        assert_eq!(identity.name, "First Last");
+        assert_eq!(
+            identity.email, "fl <First Last<[email protected]",
+            "extra trailing and leading angled parens are stripped"
+        );
+        let signature: Identity = identity.into();
+        let mut output = Vec::new();
+        let err = signature.write_to(&mut output).unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "Signature name or email must not contain '<', '>' or \\n",
+            "this isn't roundtrippable as the name is technically incorrect - must not contain brackets"
+        );
+    }
+    Ok(())
+}