Enable SignatureRef/IdentityRef to preserve raw actor bytes for round-tripping malformed commits

Author: Pingasmaster

gitoxide-core/src/repository/mailmap.rs

@@ -85,17 +85,20 @@ pub fn check(
                 gix::actor::IdentityRef {
                     name: "".into(),
                     email: email.into(),
+                    raw: None,
                 }
             }
         };
         let resolved = mailmap.resolve_cow(gix::actor::SignatureRef {
             name: actor.name,
             email: actor.email,
             time: Default::default(),
+            raw: None,
         });
         let resolved = gix::actor::IdentityRef {
             name: resolved.name.as_ref(),
             email: resolved.email.as_ref(),
+            raw: None,
         };
         buf.clear();
         resolved.write_to(&mut buf)?;

gix-actor/src/identity.rs

@@ -25,6 +25,7 @@ impl<'a> IdentityRef<'a> {
         IdentityRef {
             name: self.name.trim().as_bstr(),
             email: self.email.trim().as_bstr(),
+            raw: None,
         }
     }
 }
@@ -43,6 +44,10 @@ mod write {
     impl IdentityRef<'_> {
         /// Serialize this instance to `out` in the git serialization format for signatures (but without timestamp).
         pub fn write_to(&self, out: &mut dyn std::io::Write) -> std::io::Result<()> {
+            if let Some(raw) = self.raw {
+                out.write_all(raw.as_ref())?;
+                return Ok(());
+            }
             out.write_all(validated_token(self.name)?)?;
             out.write_all(b" ")?;
             out.write_all(b"<")?;
@@ -61,13 +66,14 @@ mod impls {
             IdentityRef {
                 name: self.name.as_ref(),
                 email: self.email.as_ref(),
+                raw: None,
             }
         }
     }
 
     impl From<IdentityRef<'_>> for Identity {
         fn from(other: IdentityRef<'_>) -> Identity {
-            let IdentityRef { name, email } = other;
+            let IdentityRef { name, email, .. } = other;
             Identity {
                 name: name.to_owned(),
                 email: email.to_owned(),
@@ -88,8 +94,15 @@ mod impls {
     }
 
     impl<'a> From<SignatureRef<'a>> for IdentityRef<'a> {
-        fn from(SignatureRef { name, email, time: _ }: SignatureRef<'a>) -> Self {
-            IdentityRef { name, email }
+        fn from(
+            SignatureRef {
+                name,
+                email,
+                raw,
+                time: _,
+            }: SignatureRef<'a>,
+        ) -> Self {
+            IdentityRef { name, email, raw }
         }
     }
 }

gix-actor/src/lib.rs

@@ -49,6 +49,12 @@ pub struct IdentityRef<'a> {
     ///
     /// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
     pub email: &'a BStr,
+    /// The raw representation of the identity as encountered in serialized data.
+    ///
+    /// When present this allows lossless round-tripping of malformed identities which would otherwise
+    /// trigger the writer's strict validation of `<` and `>` characters.
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub raw: Option<&'a BStr>,
 }
 
 /// A mutable signature that is created by an actor at a certain time.
@@ -92,4 +98,9 @@ pub struct SignatureRef<'a> {
     ///
     /// Use [`SignatureRef::time()`] to decode.
     pub time: &'a str,
+    /// Raw identity part of this signature (`name <email>`), when available.
+    ///
+    /// See [`IdentityRef::raw`] for details.
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub raw: Option<&'a BStr>,
 }

gix-actor/src/signature/decode.rs

@@ -37,6 +37,7 @@ pub(crate) mod function {
             name: identity.name,
             email: identity.email,
             time,
+            raw: identity.raw,
         })
         .parse_next(i)
     }
@@ -66,6 +67,14 @@ pub(crate) mod function {
         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();
+        let raw_slice =
+            i.get(..=right_delim_idx)
+                .map(ByteSlice::as_bstr)
+                .ok_or(ErrMode::Cut(E::from_input(i).add_context(
+                    i,
+                    &start,
+                    StrContext::Label("Identity slice out of bounds"),
+                )))?;
 
         let email = i
             .get(left_delim_idx + skip_from_left..right_delim_idx - skip_from_right)
@@ -75,8 +84,13 @@ pub(crate) mod function {
                 StrContext::Label("Skipped parts run into each other"),
             )))?
             .as_bstr();
+        let raw = if name.find_byteset(b"<>\n").is_some() || email.find_byteset(b"<>\n").is_some() {
+            Some(raw_slice)
+        } else {
+            None
+        };
         *i = i.get(right_delim_idx + 1..).unwrap_or(&[]);
-        Ok(IdentityRef { name, email })
+        Ok(IdentityRef { name, email, raw })
     }
 }
 pub use function::identity;
@@ -100,6 +114,7 @@ mod tests {
                 name: name.into(),
                 email: email.into(),
                 time,
+                raw: None,
             }
         }
 

gix-actor/src/signature/mod.rs

@@ -32,6 +32,7 @@ mod _ref {
                 name: self.name.trim().as_bstr(),
                 email: self.email.trim().as_bstr(),
                 time: self.time.trim(),
+                raw: None,
             }
         }
 
@@ -40,6 +41,7 @@ mod _ref {
             IdentityRef {
                 name: self.name,
                 email: self.email,
+                raw: self.raw,
             }
         }
 
@@ -78,6 +80,7 @@ mod convert {
                 name: self.name.as_ref(),
                 email: self.email.as_ref(),
                 time: self.time.to_str(time_buf),
+                raw: None,
             }
         }
     }
@@ -130,16 +133,26 @@ pub(crate) mod write {
     impl SignatureRef<'_> {
         /// Serialize this instance to `out` in the git serialization format for actors.
         pub fn write_to(&self, out: &mut dyn std::io::Write) -> std::io::Result<()> {
-            out.write_all(validated_token(self.name)?)?;
+            if let Some(raw) = self.raw {
+                out.write_all(raw.as_ref())?;
+            } else {
+                out.write_all(validated_token(self.name)?)?;
+                out.write_all(b" ")?;
+                out.write_all(b"<")?;
+                out.write_all(validated_token(self.email)?)?;
+                out.write_all(b">")?;
+            }
             out.write_all(b" ")?;
-            out.write_all(b"<")?;
-            out.write_all(validated_token(self.email)?)?;
-            out.write_all(b"> ")?;
             out.write_all(validated_token(self.time.into())?)
         }
         /// Computes the number of bytes necessary to serialize this signature
         pub fn size(&self) -> usize {
-            self.name.len() + 2 /* space <*/ + self.email.len() +  2 /* > space */ + self.time.len()
+            let identity_len = if let Some(raw) = self.raw {
+                raw.len()
+            } else {
+                self.name.len() + 2 /* space <*/ + self.email.len() + 1 /* > */
+            };
+            identity_len + 1 /* space */ + self.time.len()
         }
     }
 

gix-actor/tests/signature/mod.rs

@@ -83,6 +83,16 @@ fn signature_ref_round_trips_with_seconds_in_offset() -> Result<(), Box<dyn std:
     Ok(())
 }
 
+#[test]
+fn signature_ref_round_trips_with_malformed_identity() -> Result<(), Box<dyn std::error::Error>> {
+    let input = b"Gregor Hartmann<gh <Gregor Hartmann<[email protected]>> 1282910542 +0200";
+    let signature: SignatureRef = gix_actor::SignatureRef::from_bytes::<()>(input).unwrap();
+    let mut output = Vec::new();
+    signature.write_to(&mut output)?;
+    assert_eq!(output.as_bstr(), input.as_bstr());
+    Ok(())
+}
+
 #[test]
 fn parse_timestamp_with_trailing_digits() {
     let signature = gix_actor::SignatureRef::from_bytes::<()>(b"first last <[email protected]> 1312735823 +051800")
@@ -93,6 +103,7 @@ fn parse_timestamp_with_trailing_digits() {
             name: "first last".into(),
             email: "[email protected]".into(),
             time: "1312735823 +051800",
+            raw: None,
         }
     );
 
@@ -104,6 +115,7 @@ fn parse_timestamp_with_trailing_digits() {
             name: "first last".into(),
             email: "[email protected]".into(),
             time: "1312735823 +0518",
+            raw: None,
         }
     );
 }
@@ -117,7 +129,8 @@ fn parse_missing_timestamp() {
         SignatureRef {
             name: "first last".into(),
             email: "[email protected]".into(),
-            time: ""
+            time: "",
+            raw: None,
         }
     );
 }

gix-mailmap/src/snapshot/mod.rs

@@ -154,7 +154,7 @@ impl Snapshot {
 }
 
 fn enriched_signature<'a>(
-    SignatureRef { name, email, time }: SignatureRef<'a>,
+    SignatureRef { name, email, time, .. }: SignatureRef<'a>,
     new: ResolvedSignature<'_>,
 ) -> Signature<'a> {
     match (new.email, new.name) {

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

@@ -13,6 +13,7 @@ fn invalid_timestsamp() {
         name: b"Name".as_bstr(),
         email: b"[email protected]".as_bstr(),
         time: "1312735823 +051800",
+        raw: None,
     };
     assert_eq!(
         CommitRef::from_bytes(&fixture_name("commit", "invalid-timestamp.txt"))
@@ -36,10 +37,12 @@ fn invalid_email_of_committer() {
         name: b"Gregor Hartmann".as_bstr(),
         email: b"gh <Gregor Hartmann<[email protected]".as_bstr(),
         time: "1282910542 +0200",
+        raw: Some(b"Gregor Hartmann<gh <Gregor Hartmann<[email protected]>>".as_bstr()),
     };
+    let fixture = fixture_name("commit", "invalid-actor.txt");
+    let commit_ref = CommitRef::from_bytes(&fixture).expect("ignore strangely formed actor format and round-trip");
     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(),
@@ -50,6 +53,11 @@ fn invalid_email_of_committer() {
             extra_headers: vec![]
         }
     );
+    let mut buf = Vec::new();
+    commit_ref
+        .write_to(&mut buf)
+        .expect("serialized commits with malformed actors should succeed");
+    assert_eq!(buf, fixture);
 }
 
 #[test]
@@ -184,6 +192,7 @@ fn pre_epoch() -> crate::Result {
         name: "Législateur".into(),
         email: "".into(),
         time: "-5263834140 +0009",
+        raw: None,
     };
     assert_eq!(
         CommitRef::from_bytes(&fixture_name("commit", "pre-epoch.txt"))?,
@@ -206,6 +215,7 @@ fn double_dash_special_time_offset() -> crate::Result {
         name: "name".into(),
         email: "[email protected]".into(),
         time: "1288373970 --700",
+        raw: None,
     };
     assert_eq!(
         CommitRef::from_bytes(&fixture_name("commit", "double-dash-date-offset.txt"))?,
@@ -228,6 +238,7 @@ fn with_trailer() -> crate::Result {
         name: "Kim Altintop".into(),
         email: "[email protected]".into(),
         time: "1631514803 +0200",
+        raw: None,
     };
     let backing = fixture_name("commit", "message-with-footer.txt");
     let commit = CommitRef::from_bytes(&backing)?;

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

@@ -236,6 +236,7 @@ mod summary {
             name: "name".into(),
             email: "email".into(),
             time: "0 0000",
+            raw: None,
         };
         assert_eq!(
             CommitRef {

gix-object/tests/object/main.rs

@@ -96,6 +96,7 @@ fn signature(time: &str) -> gix_actor::SignatureRef<'_> {
         name: b"Sebastian Thiel".as_bstr(),
         email: b"[email protected]".as_bstr(),
         time,
+        raw: None,
     }
 }
 
@@ -105,5 +106,6 @@ fn linus_signature(time: &str) -> gix_actor::SignatureRef<'_> {
         name: b"Linus Torvalds".as_bstr(),
         email: b"[email protected]".as_bstr(),
         time,
+        raw: None,
     }
 }