From 994600eb07d6ebc259cc6b876b93e449341cc5cc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 20 Sep 2025 05:21:02 +0200 Subject: [PATCH 1/2] Fix CommitRef round-tripping for malformed actors with angle brackets - Allow angle brackets in actor names and emails for round-tripping - Keep newline restriction to preserve git format integrity - Update tests to reflect the new behavior - Resolves issue #2177 Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-actor/src/signature/mod.rs | 4 +- gix-actor/tests/identity/mod.rs | 43 +++++++++++++++++--- gix-actor/tests/signature/mod.rs | 16 +++----- gix-object/tests/object/commit/from_bytes.rs | 9 +++- 4 files changed, 52 insertions(+), 20 deletions(-) diff --git a/gix-actor/src/signature/mod.rs b/gix-actor/src/signature/mod.rs index ecb702dbd10..723c0f1f09b 100644 --- a/gix-actor/src/signature/mod.rs +++ b/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) diff --git a/gix-actor/tests/identity/mod.rs b/gix-actor/tests/identity/mod.rs index d3787d9a732..7900b984178 100644 --- a/gix-actor/tests/identity/mod.rs +++ b/gix-actor/tests/identity/mod.rs @@ -40,12 +40,43 @@ 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" - ); + let result = signature.write_to(&mut output); + + // Both test cases should now work since angle brackets are allowed + // and the parser strips newlines from the input + assert!(result.is_ok(), + "angle brackets should be allowed for round-tripping, and newlines are stripped during parsing"); } 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(()) +} diff --git a/gix-actor/tests/signature/mod.rs b/gix-actor/tests/signature/mod.rs index a007ad871a7..65f15ee9f20 100644 --- a/gix-actor/tests/signature/mod.rs +++ b/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] diff --git a/gix-object/tests/object/commit/from_bytes.rs b/gix-object/tests/object/commit/from_bytes.rs index b89213394da..18b7d1412cc 100644 --- a/gix-object/tests/object/commit/from_bytes.rs +++ b/gix-object/tests/object/commit/from_bytes.rs @@ -37,9 +37,14 @@ fn invalid_email_of_committer() { email: b"gh Date: Sat, 20 Sep 2025 05:21:30 +0200 Subject: [PATCH 2/2] refactor --- gix-actor/tests/identity/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/gix-actor/tests/identity/mod.rs b/gix-actor/tests/identity/mod.rs index 7900b984178..a2a9b216cdf 100644 --- a/gix-actor/tests/identity/mod.rs +++ b/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,9 @@ fn lenient_parsing() -> gix_testtools::Result { ); let signature: Identity = identity.into(); let mut output = Vec::new(); - let result = signature.write_to(&mut output); - - // Both test cases should now work since angle brackets are allowed - // and the parser strips newlines from the input - assert!(result.is_ok(), - "angle brackets should be allowed for round-tripping, and newlines are stripped during parsing"); + signature.write_to(&mut output).expect("write does not complain"); + + assert_eq!(output.as_bstr(), input, "round-tripping should keep these equivalent"); } Ok(()) }