Skip to content

fix: Make email with spaces round-trip #1922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions gix-actor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,28 @@ pub mod signature;
#[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct Identity {
/// The actors name.
/// The actors name, potentially with whitespace as parsed.
///
/// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
pub name: BString,
/// The actor's email.
/// The actor's email, potentially with whitespace and garbage as parsed.
///
/// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
pub email: BString,
}

/// A person with name and email, as reference.
#[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct IdentityRef<'a> {
/// The actors name.
/// The actors name, potentially with whitespace as parsed.
///
/// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
#[cfg_attr(feature = "serde", serde(borrow))]
pub name: &'a BStr,
/// The actor's email.
/// The actor's email, potentially with whitespace and garbage as parsed.
///
/// Use [IdentityRef::trim()] or trim manually to be able to clean it up.
pub email: &'a BStr,
}

Expand All @@ -51,9 +59,13 @@ pub struct IdentityRef<'a> {
#[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct Signature {
/// The actors name.
/// The actors name, potentially with whitespace as parsed.
///
/// Use [SignatureRef::trim()] or trim manually to be able to clean it up.
pub name: BString,
/// The actor's email.
/// The actor's email, potentially with whitespace and garbage as parsed.
///
/// Use [SignatureRef::trim()] or trim manually to be able to clean it up.
pub email: BString,
/// The time stamp at which the signature is performed.
pub time: Time,
Expand All @@ -65,10 +77,14 @@ pub struct Signature {
#[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct SignatureRef<'a> {
/// The actor's name.
/// The actors name, potentially with whitespace as parsed.
///
/// Use [SignatureRef::trim()] or trim manually to be able to clean it up.
#[cfg_attr(feature = "serde", serde(borrow))]
pub name: &'a BStr,
/// The actor's email.
/// The actor's email, potentially with whitespace and garbage as parsed.
///
/// Use [SignatureRef::trim()] or trim manually to be able to clean it up.
pub email: &'a BStr,
/// The time stamp at which the signature was performed.
pub time: gix_date::Time,
Expand Down
22 changes: 13 additions & 9 deletions gix-actor/src/signature/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,15 @@ pub(crate) mod function {
StrContext::Label("Closing '>' not found"),
)))?;
let i_name_and_email = &i[..right_delim_idx];
let skip_from_right = i_name_and_email
.iter()
.rev()
.take_while(|b| b.is_ascii_whitespace() || **b == b'>')
.count();
let skip_from_right = i_name_and_email.iter().rev().take_while(|b| **b == b'>').count();
let left_delim_idx = i_name_and_email
.find_byte(b'<')
.ok_or(ErrMode::Cut(E::from_input(i).add_context(
&i_name_and_email,
&start,
StrContext::Label("Opening '<' not found"),
)))?;
let skip_from_left = i[left_delim_idx..]
.iter()
.take_while(|b| b.is_ascii_whitespace() || **b == b'<')
.count();
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();

Expand Down Expand Up @@ -164,6 +157,17 @@ mod tests {
);
}

#[test]
fn email_with_space() {
assert_eq!(
decode
.parse_peek(b"Sebastian Thiel <\[email protected] > 1528473343 +0230")
.expect("parse to work")
.1,
signature("Sebastian Thiel", "\[email protected] ", 1528473343, Sign::Plus, 9000)
);
}

#[test]
fn negative_offset_0000() {
assert_eq!(
Expand Down
19 changes: 14 additions & 5 deletions gix-actor/tests/identity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ use gix_actor::Identity;
fn round_trip() -> gix_testtools::Result {
static DEFAULTS: &[&[u8]] = &[
b"Sebastian Thiel <[email protected]>",
b"Sebastian Thiel < [email protected]>",
b"Sebastian Thiel <[email protected] >",
b"Sebastian Thiel <\[email protected] \t >",
".. ☺️Sebastian 王知明 Thiel🙌 .. <[email protected]>".as_bytes(),
b".. whitespace \t is explicitly allowed - unicode aware trimming must be done elsewhere <[email protected]>"
];
Expand All @@ -19,15 +22,21 @@ fn round_trip() -> gix_testtools::Result {

#[test]
fn lenient_parsing() -> gix_testtools::Result {
for input in [
"First Last<<fl <First Last<[email protected] >> >",
"First Last<fl <First Last<[email protected]>>\n",
for (input, expected_email) in [
(
"First Last<<fl <First Last<[email protected] >> >",
"fl <First Last<[email protected] >> ",
),
(
"First Last<fl <First Last<[email protected]>>\n",
"fl <First Last<[email protected]",
),
] {
let identity = gix_actor::IdentityRef::from_bytes::<()>(input.as_bytes()).unwrap();
assert_eq!(identity.name, "First Last");
assert_eq!(
identity.email, "fl <First Last<[email protected]",
"extra trailing and leading angled parens are stripped"
identity.email, expected_email,
"emails are parsed but left as is for round-tripping"
);
let signature: Identity = identity.into();
let mut output = Vec::new();
Expand Down
5 changes: 5 additions & 0 deletions gix-object/tests/fixtures/commit/email-with-space.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
tree 1b2dfb4ac5e42080b682fc676e9738c94ce6d54d
author Sebastian Thiel <[email protected] > 1592437401 +0800
committer Sebastian Thiel <[email protected]> 1592437401 +0800

In the author line, the email is followed by a space
1 change: 1 addition & 0 deletions gix-object/tests/object/encode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ mod commit {
round_trip!(
gix_object::Commit,
gix_object::CommitRef,
"commit/email-with-space.txt",
"commit/signed-whitespace.txt",
"commit/two-multiline-headers.txt",
"commit/mergetag.txt",
Expand Down
Loading