reproduce failure when parsing malformed commit (#1438)

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<fl@openoffice.org >> >",
+        "First Last<fl <First Last<fl@openoffice.org>>\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<fl@openoffice.org",
+            "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(())
+}

gix-object/tests/commit/from_bytes.rs

@@ -35,6 +35,32 @@ fn invalid_timestsamp() {
     );
 }
 
+#[test]
+fn invalid_email_of_committer() {
+    let actor = gix_actor::SignatureRef {
+        name: b"Gregor Hartmann".as_bstr(),
+        email: b"gh <Gregor Hartmann<gh@openoffice.org".as_bstr(),
+        time: Time {
+            seconds: 1282910542,
+            offset: 2 * 60 * 60,
+            sign: Sign::Plus,
+        },
+    };
+    assert_eq!(
+        CommitRef::from_bytes(&fixture_name("commit", "invalid-actor.txt"))
+            .expect("ignore strangely formed actor format"),
+        CommitRef {
+            tree: b"220738fd4199e95a2b244465168366a73ebdf271".as_bstr(),
+            parents: [b"209fbe2d632761b30b7b17422914e11b93692833".as_bstr()].into(),
+            author: actor,
+            committer: actor,
+            encoding: None,
+            message: b"build breakers".as_bstr(),
+            extra_headers: vec![]
+        }
+    );
+}
+
 #[test]
 fn unsigned() -> crate::Result {
     assert_eq!(

gix-object/tests/fixtures/commit/invalid-actor.txt

@@ -0,0 +1,6 @@
+tree 220738fd4199e95a2b244465168366a73ebdf271
+parent 209fbe2d632761b30b7b17422914e11b93692833
+author Gregor Hartmann<gh <Gregor Hartmann<gh@openoffice.org>> 1282910542 +0200
+committer Gregor Hartmann<gh <Gregor Hartmann<gh@openoffice.org>> 1282910542 +0200
+
+build breakers

gix-object/tests/tag/mod.rs

@@ -124,14 +124,14 @@ fn invalid() {
     assert_eq!(
         TagRef::from_bytes(partial_tag).unwrap_err().to_string(),
         if cfg!(feature = "verbose-object-parsing-errors") {
-            "object parsing failed at `tagger Sebasti`"
+            "object parsing failed at `Sebasti`\ninvalid Closing '>' not found\nexpected `<name> <<email>> <timestamp> <+|-><HHMM>`, `tagger <signature>`"
         } else {
             "object parsing failed"
         }
     );
     assert_eq!(
         TagRefIter::from_bytes(partial_tag).take_while(Result::is_ok).count(),
-        4,
+        3,
         "we can decode some fields before failing"
     );
 }