fix!: expose raw commit/tag actor headers for round-tripping

Author: Pingasmaster

gix-object/src/commit/decode.rs

@@ -40,9 +40,9 @@ pub fn commit<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>(
             .context(StrContext::Expected(
                 "zero or more 'parent <40 lowercase hex char>'".into(),
             )),
-        (|i: &mut _| parse::header_field(i, b"author", parse::signature))
+        (|i: &mut _| parse::header_field(i, b"author", parse::signature_with_raw))
             .context(StrContext::Expected("author <signature>".into())),
-        (|i: &mut _| parse::header_field(i, b"committer", parse::signature))
+        (|i: &mut _| parse::header_field(i, b"committer", parse::signature_with_raw))
             .context(StrContext::Expected("committer <signature>".into())),
         opt(|i: &mut _| parse::header_field(i, b"encoding", take_till(0.., NL)))
             .context(StrContext::Expected("encoding <encoding>".into())),
@@ -60,14 +60,18 @@ pub fn commit<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>(
         terminated(message, eof),
     )
         .map(
-            |(tree, parents, author, committer, encoding, extra_headers, message)| CommitRef {
-                tree,
-                parents: SmallVec::from(parents),
-                author,
-                committer,
-                encoding: encoding.map(ByteSlice::as_bstr),
-                message,
-                extra_headers,
+            |(tree, parents, (author, author_raw), (committer, committer_raw), encoding, extra_headers, message)| {
+                CommitRef {
+                    tree,
+                    parents: SmallVec::from(parents),
+                    author,
+                    author_raw,
+                    committer,
+                    committer_raw,
+                    encoding: encoding.map(ByteSlice::as_bstr),
+                    message,
+                    extra_headers,
+                }
             },
         )
         .parse_next(i)

gix-object/src/commit/write.rs

@@ -4,6 +4,31 @@ use bstr::ByteSlice;
 
 use crate::{encode, encode::NL, Commit, CommitRef, Kind};
 
+fn signature_requires_raw(signature: &gix_actor::SignatureRef<'_>) -> bool {
+    signature.name.find_byteset(b"<>\n").is_some() || signature.email.find_byteset(b"<>\n").is_some()
+}
+
+fn signature_len(signature: &gix_actor::SignatureRef<'_>, raw: &bstr::BStr) -> usize {
+    if signature_requires_raw(signature) {
+        raw.len()
+    } else {
+        signature.size()
+    }
+}
+
+fn write_signature(
+    mut out: &mut dyn io::Write,
+    field: &[u8],
+    signature: &gix_actor::SignatureRef<'_>,
+    raw: &bstr::BStr,
+) -> io::Result<()> {
+    if signature_requires_raw(signature) {
+        encode::trusted_header_field(field, raw.as_ref(), &mut out)
+    } else {
+        encode::trusted_header_signature(field, signature, &mut out)
+    }
+}
+
 impl crate::WriteTo for Commit {
     /// Serializes this instance to `out` in the git serialization format.
     fn write_to(&self, mut out: &mut dyn io::Write) -> io::Result<()> {
@@ -58,8 +83,8 @@ impl crate::WriteTo for CommitRef<'_> {
         for parent in self.parents() {
             encode::trusted_header_id(b"parent", &parent, &mut out)?;
         }
-        encode::trusted_header_signature(b"author", &self.author, &mut out)?;
-        encode::trusted_header_signature(b"committer", &self.committer, &mut out)?;
+        write_signature(&mut out, b"author", &self.author, self.author_raw)?;
+        write_signature(&mut out, b"committer", &self.committer, self.committer_raw)?;
         if let Some(encoding) = self.encoding.as_ref() {
             encode::header_field(b"encoding", encoding, &mut out)?;
         }
@@ -78,8 +103,8 @@ impl crate::WriteTo for CommitRef<'_> {
         let hash_in_hex = self.tree().kind().len_in_hex();
         (b"tree".len() + 1 /* space */ + hash_in_hex + 1 /* nl */
             + self.parents.iter().count() * (b"parent".len() + 1 /* space */ + hash_in_hex + 1 /* nl */)
-            + b"author".len() + 1 /* space */ + self.author.size() + 1 /* nl */
-            + b"committer".len() + 1 /* space */ + self.committer.size() + 1 /* nl */
+            + b"author".len() + 1 /* space */ + signature_len(&self.author, self.author_raw) + 1 /* nl */
+            + b"committer".len() + 1 /* space */ + signature_len(&self.committer, self.committer_raw) + 1 /* nl */
             + self
                 .encoding
                 .as_ref()

gix-object/src/lib.rs

@@ -92,13 +92,19 @@ pub struct CommitRef<'a> {
     ///
     /// Use the [`author()`](CommitRef::author()) method to received a trimmed version of it.
     pub author: gix_actor::SignatureRef<'a>,
+    /// The raw author header as encountered during parsing.
+    ///
+    /// This is used to preserve otherwise invalid identities during serialization.
+    pub author_raw: &'a BStr,
     /// Who committed this commit. Name and email might contain whitespace and are not trimmed to ensure round-tripping.
     ///
     /// Use the [`committer()`](CommitRef::committer()) method to received a trimmed version of it.
     ///
     /// This may be different from the `author` in case the author couldn't write to the repository themselves and
     /// is commonly encountered with contributed commits.
     pub committer: gix_actor::SignatureRef<'a>,
+    /// The raw committer header as encountered during parsing.
+    pub committer_raw: &'a BStr,
     /// The name of the message encoding, otherwise [UTF-8 should be assumed](https://github.com/git/git/blob/e67fbf927dfdf13d0b21dc6ea15dc3c7ef448ea0/commit.c#L1493:L1493).
     pub encoding: Option<&'a BStr>,
     /// The commit message documenting the change.
@@ -152,6 +158,8 @@ pub struct TagRef<'a> {
     pub name: &'a BStr,
     /// The author of the tag.
     pub tagger: Option<gix_actor::SignatureRef<'a>>,
+    /// The raw tagger header as encountered during parsing.
+    pub tagger_raw: Option<&'a BStr>,
     /// The message describing this release.
     pub message: &'a BStr,
     /// A cryptographic signature over the entire content of the serialized tag object thus far.

gix-object/src/object/convert.rs

@@ -8,6 +8,7 @@ impl From<TagRef<'_>> for Tag {
             target_kind,
             message,
             tagger: signature,
+            tagger_raw: _,
             pgp_signature,
         } = other;
         Tag {
@@ -27,7 +28,9 @@ impl From<CommitRef<'_>> for Commit {
             tree,
             parents,
             author,
+            author_raw: _,
             committer,
+            committer_raw: _,
             encoding,
             message,
             extra_headers,

gix-object/src/parse.rs

@@ -69,5 +69,15 @@ pub fn hex_hash<'a, E: ParserError<&'a [u8]>>(i: &mut &'a [u8]) -> ModalResult<&
 pub(crate) fn signature<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>(
     i: &mut &'a [u8],
 ) -> ModalResult<gix_actor::SignatureRef<'a>, E> {
-    gix_actor::signature::decode(i)
+    signature_with_raw(i).map(|(signature, _)| signature)
+}
+
+pub(crate) fn signature_with_raw<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>(
+    i: &mut &'a [u8],
+) -> ModalResult<(gix_actor::SignatureRef<'a>, &'a BStr), E> {
+    let original = *i;
+    gix_actor::signature::decode(i).map(|signature| {
+        let consumed = original.len() - i.len();
+        (signature, original[..consumed].as_bstr())
+    })
 }

gix-object/src/tag/decode.rs

@@ -19,20 +19,24 @@ pub fn git_tag<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>(
             .context(StrContext::Expected("type <object kind>".into())),
         (|i: &mut _| parse::header_field(i, b"tag", take_while(1.., |b| b != NL[0])))
             .context(StrContext::Expected("tag <version>".into())),
-        opt(|i: &mut _| parse::header_field(i, b"tagger", parse::signature))
+        opt(|i: &mut _| parse::header_field(i, b"tagger", parse::signature_with_raw))
             .context(StrContext::Expected("tagger <signature>".into())),
         terminated(message, eof),
     )
-        .map(
-            |(target, kind, tag_version, signature, (message, pgp_signature))| TagRef {
+        .map(|(target, kind, tag_version, signature, (message, pgp_signature))| {
+            let (tagger, tagger_raw) = signature
+                .map(|(sig, raw)| (Some(sig), Some(raw)))
+                .unwrap_or((None, None));
+            TagRef {
                 target,
                 name: tag_version.as_bstr(),
                 target_kind: kind,
                 message,
-                tagger: signature,
+                tagger,
+                tagger_raw,
                 pgp_signature,
-            },
-        )
+            }
+        })
         .parse_next(i)
 }
 

gix-object/src/tag/write.rs

@@ -1,10 +1,22 @@
 use std::io;
 
-use bstr::BStr;
+use bstr::{BStr, ByteSlice};
 use gix_date::parse::TimeBuf;
 
 use crate::{encode, encode::NL, Kind, Tag, TagRef};
 
+fn signature_requires_raw(signature: &gix_actor::SignatureRef<'_>) -> bool {
+    signature.name.find_byteset(b"<>\n").is_some() || signature.email.find_byteset(b"<>\n").is_some()
+}
+
+fn signature_len(signature: &gix_actor::SignatureRef<'_>, raw: &BStr) -> usize {
+    if signature_requires_raw(signature) {
+        raw.len()
+    } else {
+        signature.size()
+    }
+}
+
 /// An Error used in [`Tag::write_to()`][crate::WriteTo::write_to()].
 #[derive(Debug, thiserror::Error)]
 #[allow(missing_docs)]
@@ -64,8 +76,15 @@ impl crate::WriteTo for TagRef<'_> {
         encode::trusted_header_field(b"object", self.target, &mut out)?;
         encode::trusted_header_field(b"type", self.target_kind.as_bytes(), &mut out)?;
         encode::header_field(b"tag", validated_name(self.name)?, &mut out)?;
-        if let Some(tagger) = &self.tagger {
-            encode::trusted_header_signature(b"tagger", tagger, &mut out)?;
+        if let Some(tagger) = self.tagger {
+            let raw = self
+                .tagger_raw
+                .expect("raw tagger data should be present whenever a tagger exists");
+            if signature_requires_raw(&tagger) {
+                encode::trusted_header_field(b"tagger", raw.as_ref(), &mut out)?;
+            } else {
+                encode::trusted_header_signature(b"tagger", &tagger, &mut out)?;
+            }
         }
 
         if !self.message.iter().all(|b| *b == b'\n') {
@@ -87,10 +106,13 @@ impl crate::WriteTo for TagRef<'_> {
         (b"object".len() + 1 /* space */ + self.target().kind().len_in_hex() + 1 /* nl */
             + b"type".len() + 1 /* space */ + self.target_kind.as_bytes().len() + 1 /* nl */
             + b"tag".len() + 1 /* space */ + self.name.len() + 1 /* nl */
-            + self
-                .tagger
-                .as_ref()
-                .map_or(0, |t| b"tagger".len() + 1 /* space */ + t.size() + 1 /* nl */)
+            + match (self.tagger, self.tagger_raw) {
+                (Some(tagger), Some(raw)) => {
+                    b"tagger".len() + 1 /* space */ + signature_len(&tagger, raw) + 1 /* nl */
+                }
+                (None, None) => 0,
+                _ => unreachable!("tagger raw data must be present if tagger exists"),
+            }
             + if self.message.iter().all(|b| *b == b'\n') { 0 } else { 1 /* nl */ } + self.message.len()
             + self.pgp_signature.as_ref().map_or(0, |m| 1 /* nl */ + m.len())) as u64
     }