Be lenient to missing timestamp in signature (#1542)

Author: pierrec

gix-actor/src/signature/decode.rs

@@ -6,7 +6,7 @@ pub(crate) mod function {
     use winnow::error::{ErrMode, ErrorKind};
     use winnow::stream::Stream;
     use winnow::{
-        combinator::{alt, separated_pair, terminated},
+        combinator::{alt, opt, separated_pair, terminated},
         error::{AddContext, ParserError, StrContext},
         prelude::*,
         stream::AsChar,
@@ -21,8 +21,8 @@ pub(crate) mod function {
     ) -> PResult<SignatureRef<'a>, E> {
         separated_pair(
             identity,
-            b" ",
-            (
+            opt(b" "),
+            opt((
                 terminated(take_until(0.., SPACE), take(1usize))
                     .verify_map(|v| to_signed::<SecondsSinceUnixEpoch>(v).ok())
                     .context(StrContext::Expected("<timestamp>".into())),
@@ -38,8 +38,9 @@ pub(crate) mod function {
                     .verify_map(|v| to_signed::<OffsetInSeconds>(v).ok())
                     .context(StrContext::Expected("MM".into())),
                 take_while(0.., AsChar::is_dec_digit).map(|v: &[u8]| v),
-            )
-                .map(|(time, sign, hours, minutes, trailing_digits)| {
+            ))
+            .map(|maybe_timestamp| {
+                if let Some((time, sign, hours, minutes, trailing_digits)) = maybe_timestamp {
                     let offset = if trailing_digits.is_empty() {
                         (hours * 3600 + minutes * 60) * if sign == Sign::Minus { -1 } else { 1 }
                     } else {
@@ -50,7 +51,10 @@ pub(crate) mod function {
                         offset,
                         sign,
                     }
-                }),
+                } else {
+                    Time::new(0, 0)
+                }
+            }),
         )
         .context(StrContext::Expected("<name> <<email>> <timestamp> <+|-><HHMM>".into()))
         .map(|(identity, time)| SignatureRef {
@@ -206,12 +210,9 @@ mod tests {
         #[test]
         fn invalid_time() {
             assert_eq!(
-                        decode.parse_peek(b"hello <> abc -1215")
-                            .map_err(to_bstr_err)
-                            .expect_err("parse fails as > is missing")
-                            .to_string(),
-                        "in predicate verification at 'abc -1215'\n  0: expected `<timestamp>` at 'abc -1215'\n  1: expected `<name> <<email>> <timestamp> <+|-><HHMM>` at 'hello <> abc -1215'\n"
-                    );
+                decode.parse_peek(b"hello <> abc -1215").expect("parse to work").1,
+                signature("hello", "", 0, Sign::Plus, 0)
+            );
         }
     }
 }

gix-actor/tests/signature/mod.rs

@@ -108,11 +108,14 @@ fn parse_timestamp_with_trailing_digits() {
 
 #[test]
 fn parse_missing_timestamp() {
+    let signature = gix_actor::SignatureRef::from_bytes::<()>(b"first last <[email protected]>")
+        .expect("deal with missing timestamp in signature by zeroing it");
     assert_eq!(
-        format!(
-            "{:?}",
-            gix_actor::SignatureRef::from_bytes::<()>(b"first last <[email protected]>")
-        ),
-        "Err(Backtrack(()))".to_string()
+        signature,
+        SignatureRef {
+            name: "first last".into(),
+            email: "[email protected]".into(),
+            time: gix_actor::date::Time::new(0, 0),
+        }
     );
 }

gix-object/tests/tag/mod.rs

@@ -137,9 +137,11 @@ fn invalid() {
 }
 
 mod from_bytes {
+    use gix_actor::SignatureRef;
+    use gix_date::Time;
     use gix_object::{bstr::ByteSlice, Kind, TagRef};
 
-    use crate::{fixture_name, signature, tag::tag_fixture};
+    use crate::{fixture_name, signature, tag::tag_fixture, Sign};
 
     #[test]
     fn signed() -> crate::Result {
@@ -232,15 +234,24 @@ KLMHist5yj0sw1E4hDTyQa0=
 
     #[test]
     fn tagger_without_timestamp() -> crate::Result {
-        // Note: this behaviour is inconsistent with Git's
-        // Git would successfully interpret this tag and set the timestamp to 0
-        // See https://github.com/git/git/blob/3a7362eb9fad0c4838f5cfaa95ed3c51a4c18d93/tag.c#L116
         assert_eq!(
-            format!(
-                "{:?}",
-                TagRef::from_bytes(&fixture_name("tag", "tagger-without-timestamp.txt"))
-            ),
-            "Err(Error { inner: () })".to_string()
+            TagRef::from_bytes(&fixture_name("tag", "tagger-without-timestamp.txt"))?,
+            TagRef {
+                target: b"4fcd840c4935e4c7a5ea3552710a0f26b9178c24".as_bstr(),
+                name: b"ChangeLog".as_bstr(),
+                target_kind: Kind::Commit,
+                message: b"".as_bstr(),
+                tagger: Some(SignatureRef {
+                    name: b"shemminger".as_bstr(),
+                    email: b"shemminger".as_bstr(),
+                    time: Time {
+                        seconds: 0,
+                        offset: 0,
+                        sign: Sign::Plus
+                    }
+                }),
+                pgp_signature: None
+            }
         );
         Ok(())
     }