Allow round-tripping of malformed actors with angle brackets in commit objects

gix-actor/src/signature/mod.rs

@@ -104,7 +104,7 @@ pub(crate) mod write {
     #[derive(Debug, thiserror::Error)]
     #[allow(missing_docs)]
     pub(crate) enum Error {
-        #[error(r"Signature name or email must not contain '<', '>' or \n")]
+        #[error(r"Signature name or email must not contain \n")]
         IllegalCharacter,
     }
 
@@ -144,7 +144,7 @@ pub(crate) mod write {
     }
 
     pub(crate) fn validated_token(name: &BStr) -> Result<&BStr, Error> {
-        if name.find_byteset(b"<>\n").is_some() {
+        if name.find_byte(b'\n').is_some() {
             return Err(Error::IllegalCharacter);
         }
         Ok(name)

gix-actor/tests/identity/mod.rs

@@ -1,5 +1,6 @@
 use bstr::ByteSlice;
 use gix_actor::Identity;
+use winnow::stream::AsBStr;
 
 #[test]
 fn round_trip() -> gix_testtools::Result {
@@ -40,12 +41,40 @@ fn lenient_parsing() -> gix_testtools::Result {
         );
         let signature: Identity = identity.into();
         let mut output = Vec::new();
-        let err = signature.write_to(&mut output).unwrap_err();
-        assert_eq!(
-            err.to_string(),
-            r"Signature name or email must not contain '<', '>' or \n",
-            "this isn't roundtrippable as the name is technically incorrect - must not contain brackets"
-        );
+        signature.write_to(&mut output).expect("write does not complain");
+
+        assert_eq!(output.as_bstr(), input, "round-tripping should keep these equivalent");
     }
     Ok(())
 }
+
+#[test]
+fn newlines_still_rejected() -> gix_testtools::Result {
+    // Test that newlines within the actual parsed name or email are still rejected
+    let identity = gix_actor::IdentityRef {
+        name: "First\nLast".into(),
+        email: "test@example.com".into(),
+    };
+    let signature: Identity = identity.into();
+    let mut output = Vec::new();
+    let err = signature.write_to(&mut output).unwrap_err();
+    assert_eq!(
+        err.to_string(),
+        r"Signature name or email must not contain \n",
+        "newlines within parsed fields should still be rejected"
+    );
+
+    let identity = gix_actor::IdentityRef {
+        name: "First Last".into(),
+        email: "test\n@example.com".into(),
+    };
+    let signature: Identity = identity.into();
+    let mut output = Vec::new();
+    let err = signature.write_to(&mut output).unwrap_err();
+    assert_eq!(
+        err.to_string(),
+        r"Signature name or email must not contain \n",
+        "newlines within parsed fields should still be rejected"
+    );
+    Ok(())
+}

gix-actor/tests/signature/mod.rs

@@ -4,29 +4,25 @@ mod write_to {
         use gix_date::Time;
 
         #[test]
-        fn name() {
+        fn name_with_angle_brackets() {
             let signature = Signature {
                 name: "invalid < middlename".into(),
                 email: "ok".into(),
                 time: Time::default(),
             };
-            assert_eq!(
-                format!("{:?}", signature.write_to(&mut Vec::new())),
-                "Err(Custom { kind: Other, error: IllegalCharacter })"
-            );
+            // This should now work - angle brackets are allowed for round-tripping
+            assert!(signature.write_to(&mut Vec::new()).is_ok());
         }
 
         #[test]
-        fn email() {
+        fn email_with_angle_brackets() {
             let signature = Signature {
                 name: "ok".into(),
                 email: "server>.example.com".into(),
                 time: Time::default(),
             };
-            assert_eq!(
-                format!("{:?}", signature.write_to(&mut Vec::new())),
-                "Err(Custom { kind: Other, error: IllegalCharacter })"
-            );
+            // This should now work - angle brackets are allowed for round-tripping
+            assert!(signature.write_to(&mut Vec::new()).is_ok());
         }
 
         #[test]

gix-object/tests/object/commit/from_bytes.rs

@@ -37,9 +37,14 @@ fn invalid_email_of_committer() {
         email: b"gh <Gregor Hartmann<gh@openoffice.org".as_bstr(),
         time: "1282910542 +0200",
     };
+    let fixture = fixture_name("commit", "invalid-actor.txt");
+    let commit_ref = CommitRef::from_bytes(&fixture).expect("ignore strangely formed actor format");
+    let mut buf = vec![];
+    let write_result = commit_ref.write_to(&mut buf);
+    assert!(write_result.is_ok(), "write result should be OK for round-tripping");
+
     assert_eq!(
-        CommitRef::from_bytes(&fixture_name("commit", "invalid-actor.txt"))
-            .expect("ignore strangely formed actor format"),
+        commit_ref,
         CommitRef {
             tree: b"220738fd4199e95a2b244465168366a73ebdf271".as_bstr(),
             parents: [b"209fbe2d632761b30b7b17422914e11b93692833".as_bstr()].into(),