Skip to content

Commit

Permalink
fix: keep multi-line header value newlines verbatim
Browse files Browse the repository at this point in the history
Previously they were normalized to \n which made round-trips
degenerate information.

It's notable that we explicitly do NOT normalize the newline
at the very last line of the multi-line header as its part
of that line, even though it's also relevant to make the header field
valid. The reason is that these *can* also be written with \r\n, and
we don't want to degenerate that information.
  • Loading branch information
Byron committed Oct 8, 2024
1 parent 9db3f9c commit 4a6bbb1
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 34 deletions.
8 changes: 4 additions & 4 deletions gix-object/src/commit/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ impl crate::WriteTo for Commit {
.extra_headers
.iter()
.map(|(name, value)| {
// each header *value* is preceded by a space and followed by a newline
name.len() + value.split_str("\n").map(|s| s.len() + 2).sum::<usize>()
// each header *value* is preceded by a space, and it starts right after the name.
name.len() + value.lines_with_terminator().map(|s| s.len() + 1).sum::<usize>() + usize::from(!value.ends_with_str(b"\n"))
})
.sum::<usize>()
+ 1 /* nl */
Expand Down Expand Up @@ -87,8 +87,8 @@ impl<'a> crate::WriteTo for CommitRef<'a> {
.extra_headers
.iter()
.map(|(name, value)| {
// each header *value* is preceded by a space and followed by a newline
name.len() + value.split_str("\n").map(|s| s.len() + 2).sum::<usize>()
// each header *value* is preceded by a space, and it starts right after the name.
name.len() + value.lines_with_terminator().map(|s| s.len() + 1).sum::<usize>() + usize::from(!value.ends_with_str(b"\n"))
})
.sum::<usize>()
+ 1 /* nl */
Expand Down
8 changes: 6 additions & 2 deletions gix-object/src/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ impl From<Error> for io::Error {
}

pub(crate) fn header_field_multi_line(name: &[u8], value: &[u8], out: &mut dyn io::Write) -> io::Result<()> {
let mut lines = value.as_bstr().split_str(b"\n");
trusted_header_field(name, lines.next().ok_or(Error::EmptyValue)?, out)?;
let mut lines = value.as_bstr().lines_with_terminator();
out.write_all(name)?;
out.write_all(SPACE)?;
out.write_all(lines.next().ok_or(Error::EmptyValue)?)?;
for line in lines {
out.write_all(SPACE)?;
out.write_all(line)?;
}
if !value.ends_with_str(b"\n") {
out.write_all(NL)?;
}
Ok(())
Expand Down
3 changes: 1 addition & 2 deletions gix-object/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ pub(crate) fn any_header_field_multi_line<'a, E: ParserError<&'a [u8]> + AddCont
.map(|o: &[u8]| {
let bytes = o.as_bstr();
let mut out = BString::from(Vec::with_capacity(bytes.len()));
let mut lines = bytes.lines();
let mut lines = bytes.lines_with_terminator();
out.push_str(lines.next().expect("first line"));
for line in lines {
out.push(b'\n');
out.push_str(&line[1..]); // cut leading space
}
out
Expand Down
4 changes: 2 additions & 2 deletions gix-object/tests/commit/from_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ fn signed() -> crate::Result {
committer: signature(1592391367),
encoding: None,
message: b"update tasks\n".as_bstr(),
extra_headers: vec![(b"gpgsig".as_bstr(), b"-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCAAdFiEEdjYp/sh4j8NRKLX27gKdHl60AwAFAl7p9tgACgkQ7gKdHl60\nAwBpegf+KQciv9AOIN7+yPmowecGxBnSfpKWTDzFxnyGR8dq63SpWT8WEKG5mf3a\nG6iUqpsDWaMHlzihaMKRvgRpZxFRbjnNPFBj6F4RRqfE+5R7k6DRSLUV5PqnsdSH\nuccfIDWi1imhsm7AaP5trwl1t+83U2JhHqPcPVFLMODYwWeO6NLR/JCzGSTQRa8t\nRgaVMKI19O/fge5OT5Ua8D47VKEhsJX0LfmkP5RfZQ8JJvNd40TupqKRdlv0sAzP\nya7NXkSHXCavHNR6kA+KpWxn900UoGK8/IDlwU6MeOkpPVawb3NFMqnc7KJDaC2p\nSMzpuEG8LTrCx2YSpHNLqHyzvQ1CZA==\n=5ITV\n-----END PGP SIGNATURE-----".as_bstr().into())]
extra_headers: vec![(b"gpgsig".as_bstr(), b"-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCAAdFiEEdjYp/sh4j8NRKLX27gKdHl60AwAFAl7p9tgACgkQ7gKdHl60\nAwBpegf+KQciv9AOIN7+yPmowecGxBnSfpKWTDzFxnyGR8dq63SpWT8WEKG5mf3a\nG6iUqpsDWaMHlzihaMKRvgRpZxFRbjnNPFBj6F4RRqfE+5R7k6DRSLUV5PqnsdSH\nuccfIDWi1imhsm7AaP5trwl1t+83U2JhHqPcPVFLMODYwWeO6NLR/JCzGSTQRa8t\nRgaVMKI19O/fge5OT5Ua8D47VKEhsJX0LfmkP5RfZQ8JJvNd40TupqKRdlv0sAzP\nya7NXkSHXCavHNR6kA+KpWxn900UoGK8/IDlwU6MeOkpPVawb3NFMqnc7KJDaC2p\nSMzpuEG8LTrCx2YSpHNLqHyzvQ1CZA==\n=5ITV\n-----END PGP SIGNATURE-----\n".as_bstr().into())]
}
);
Ok(())
Expand Down Expand Up @@ -271,7 +271,7 @@ instead of depending directly on the lower-level crates.
Signed-off-by: Sebastian Thiel <[email protected]>
Signed-off-by: Kim Altintop <[email protected]>"
.as_bstr(),
extra_headers: vec![(b"gpgsig".as_bstr(), b"-----BEGIN PGP SIGNATURE-----\n\niHUEABYIAB0WIQSuZwcGWSQItmusNgR5URpSUCnwXQUCYT7xpAAKCRB5URpSUCnw\nXWB3AP9q323HlxnI8MyqszNOeYDwa7Y3yEZaUM2y/IRjz+z4YQEAq0yr1Syt3mrK\nOSFCqL2vDm3uStP+vF31f6FnzayhNg0=\n=Mhpp\n-----END PGP SIGNATURE-----".as_bstr().into())]
extra_headers: vec![(b"gpgsig".as_bstr(), b"-----BEGIN PGP SIGNATURE-----\n\niHUEABYIAB0WIQSuZwcGWSQItmusNgR5URpSUCnwXQUCYT7xpAAKCRB5URpSUCnw\nXWB3AP9q323HlxnI8MyqszNOeYDwa7Y3yEZaUM2y/IRjz+z4YQEAq0yr1Syt3mrK\nOSFCqL2vDm3uStP+vF31f6FnzayhNg0=\n=Mhpp\n-----END PGP SIGNATURE-----\n".as_bstr().into())]
}
);
let message = commit.message();
Expand Down
8 changes: 4 additions & 4 deletions gix-object/tests/commit/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,15 +224,15 @@ mod method {
let fixture_data = fixture_name("commit", fixture);

let (actual_signature, actual_signed_data) = CommitRefIter::signature(&fixture_data)?.expect("sig present");
assert_eq!(actual_signature, expected_signature);

let expected_signed_data: BString = fixture_data
.lines_with_terminator()
.enumerate()
.filter_map(|(i, line)| (!signature_lines.contains(&i)).then_some(line))
.collect();

assert_eq!(actual_signed_data.to_bstring(), expected_signed_data);
assert_eq!(actual_signature, expected_signature);

Ok(())
}

Expand All @@ -243,7 +243,7 @@ mod method {

#[test]
fn signed() -> crate::Result {
validate("signed.txt", b"-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCAAdFiEEdjYp/sh4j8NRKLX27gKdHl60AwAFAl7p9tgACgkQ7gKdHl60\nAwBpegf+KQciv9AOIN7+yPmowecGxBnSfpKWTDzFxnyGR8dq63SpWT8WEKG5mf3a\nG6iUqpsDWaMHlzihaMKRvgRpZxFRbjnNPFBj6F4RRqfE+5R7k6DRSLUV5PqnsdSH\nuccfIDWi1imhsm7AaP5trwl1t+83U2JhHqPcPVFLMODYwWeO6NLR/JCzGSTQRa8t\nRgaVMKI19O/fge5OT5Ua8D47VKEhsJX0LfmkP5RfZQ8JJvNd40TupqKRdlv0sAzP\nya7NXkSHXCavHNR6kA+KpWxn900UoGK8/IDlwU6MeOkpPVawb3NFMqnc7KJDaC2p\nSMzpuEG8LTrCx2YSpHNLqHyzvQ1CZA==\n=5ITV\n-----END PGP SIGNATURE-----", 4..=14)
validate("signed.txt", b"-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCAAdFiEEdjYp/sh4j8NRKLX27gKdHl60AwAFAl7p9tgACgkQ7gKdHl60\nAwBpegf+KQciv9AOIN7+yPmowecGxBnSfpKWTDzFxnyGR8dq63SpWT8WEKG5mf3a\nG6iUqpsDWaMHlzihaMKRvgRpZxFRbjnNPFBj6F4RRqfE+5R7k6DRSLUV5PqnsdSH\nuccfIDWi1imhsm7AaP5trwl1t+83U2JhHqPcPVFLMODYwWeO6NLR/JCzGSTQRa8t\nRgaVMKI19O/fge5OT5Ua8D47VKEhsJX0LfmkP5RfZQ8JJvNd40TupqKRdlv0sAzP\nya7NXkSHXCavHNR6kA+KpWxn900UoGK8/IDlwU6MeOkpPVawb3NFMqnc7KJDaC2p\nSMzpuEG8LTrCx2YSpHNLqHyzvQ1CZA==\n=5ITV\n-----END PGP SIGNATURE-----\n", 4..=14)
}

#[test]
Expand All @@ -253,7 +253,7 @@ mod method {

#[test]
fn msg_footer() -> crate::Result {
validate("message-with-footer.txt", b"-----BEGIN PGP SIGNATURE-----\n\niHUEABYIAB0WIQSuZwcGWSQItmusNgR5URpSUCnwXQUCYT7xpAAKCRB5URpSUCnw\nXWB3AP9q323HlxnI8MyqszNOeYDwa7Y3yEZaUM2y/IRjz+z4YQEAq0yr1Syt3mrK\nOSFCqL2vDm3uStP+vF31f6FnzayhNg0=\n=Mhpp\n-----END PGP SIGNATURE-----", 4..=10)
validate("message-with-footer.txt", b"-----BEGIN PGP SIGNATURE-----\n\niHUEABYIAB0WIQSuZwcGWSQItmusNgR5URpSUCnwXQUCYT7xpAAKCRB5URpSUCnw\nXWB3AP9q323HlxnI8MyqszNOeYDwa7Y3yEZaUM2y/IRjz+z4YQEAq0yr1Syt3mrK\nOSFCqL2vDm3uStP+vF31f6FnzayhNg0=\n=Mhpp\n-----END PGP SIGNATURE-----\n", 4..=10)
}

#[test]
Expand Down
8 changes: 5 additions & 3 deletions gix-object/tests/commit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use gix_object::{CommitRef, CommitRefIter};

use crate::fixture_name;

const SIGNATURE: & [u8; 487] = b"-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCAAdFiEEdjYp/sh4j8NRKLX27gKdHl60AwAFAl7q2DsACgkQ7gKdHl60\nAwDvewgAkL5UjEztzeVXlzceom0uCrAkCw9wSGLTmYcMKW3JwEaTRgQ4FX+sDuFT\nLZ8DoPu3UHUP0QnKrUwHulTTlKcOAvsczHbVPIKtXCxo6QpUfhsJQwz/J29kiE4L\nsOd+lqKGnn4oati/de2xwqNGi081fO5KILX75z6KfsAe7Qz7R3jxRF4uzHI033O+\nJc2Y827XeaELxW40SmzoLanWgEcdreXf3PstXEWW77CAu0ozXmvYj56vTviVybxx\nG7bc8lwc+SSKVe2VVB+CCfVbs0i541gmghUpZfMhUgaqttcCH8ysrUJDhne1BLG8\nCrOJIWTwAeEDtomV1p76qrMeqr1GFg==\n=qlSN\n-----END PGP SIGNATURE-----";
const SIGNATURE: & [u8] = b"-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCAAdFiEEdjYp/sh4j8NRKLX27gKdHl60AwAFAl7q2DsACgkQ7gKdHl60\nAwDvewgAkL5UjEztzeVXlzceom0uCrAkCw9wSGLTmYcMKW3JwEaTRgQ4FX+sDuFT\nLZ8DoPu3UHUP0QnKrUwHulTTlKcOAvsczHbVPIKtXCxo6QpUfhsJQwz/J29kiE4L\nsOd+lqKGnn4oati/de2xwqNGi081fO5KILX75z6KfsAe7Qz7R3jxRF4uzHI033O+\nJc2Y827XeaELxW40SmzoLanWgEcdreXf3PstXEWW77CAu0ozXmvYj56vTviVybxx\nG7bc8lwc+SSKVe2VVB+CCfVbs0i541gmghUpZfMhUgaqttcCH8ysrUJDhne1BLG8\nCrOJIWTwAeEDtomV1p76qrMeqr1GFg==\n=qlSN\n-----END PGP SIGNATURE-----\n";

const LONG_MESSAGE: &str = "Merge tag 'thermal-v5.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux
Expand Down Expand Up @@ -133,9 +133,10 @@ O53SmJAwS5zxLOd8QA5nfSWP9FYYMuCR2AHj8BUCmxiAjXZLPNB/Hz2RRBr7q0MF
zRo/4HJ04mSQYp0kluP/EBhz9g2wM/htIPyWRveB/ByKEYt3UNKjB++PJmPbu5UG
dS3aXZhRfaPqpdsWrMB9fY7ll+oyfw==
=T+RI
-----END PGP SIGNATURE-----";
-----END PGP SIGNATURE-----
";

const OTHER_SIGNATURE: &[u8; 455] = b"-----BEGIN PGP SIGNATURE-----
const OTHER_SIGNATURE: &[u8] = b"-----BEGIN PGP SIGNATURE-----
wsBcBAABCAAQBQJeqxW4CRBK7hj4Ov3rIwAAdHIIAFD98qgN/k8ybukCLf6kpzvi
5V8gf6BflONXc/oIDySurW7kfS9/r6jOgu08UN8KlQx4Q4g8yY7PROABhwGI70B3
Expand All @@ -145,6 +146,7 @@ k7D0LqGSXjU5wrQrKnemC7nWhmQsqaXDe89XXmliClCAx4/bepPiXK0eT/DNIKUr
iyBBl69jASy41Ug/BlFJbw4+ItkShpXwkJKuBBV/JExChmvbxYWaS7QnyYC9UO0=
=HLmy
-----END PGP SIGNATURE-----
";

mod method {
Expand Down
24 changes: 18 additions & 6 deletions gix-object/tests/encode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ macro_rules! round_trip {
use gix_object::{ObjectRef, Object, WriteTo};
use bstr::ByteSlice;

for input in &[
for input_name in &[
$( $files ),*
] {
let input = fixture_bytes(input);
let input = fixture_bytes(input_name);
// Test the parse->borrowed->owned->write chain for an object kind
let mut output = Vec::new();
let item = <$borrowed>::from_bytes(&input)?;
item.write_to(&mut output)?;
assert_eq!(output.as_bstr(), input.as_bstr(), "borrowed");
assert_eq!(output.as_bstr(), input.as_bstr(), "borrowed: {input_name}");

let item: $owned = item.into();
output.clear();
Expand All @@ -44,14 +44,26 @@ macro_rules! round_trip {

// Test the loose serialisation -> parse chain for an object kind
let item = <$borrowed>::from_bytes(&input)?;
// serialise a borowed item to a tagged loose object
output.clear();
{
let w = &mut output;
w.write_all(&item.loose_header())?;
item.write_to(w)?;
let parsed = ObjectRef::from_loose(&output)?;
let item2 = <$borrowed>::try_from(parsed).or(Err(super::Error::TryFromError))?;
assert_eq!(item2, item, "object-ref loose: {input_name} {:?}\n{:?}", output.as_bstr(), input.as_bstr());
}

let item: $owned = item.into();
// serialise an owned to a tagged loose object
output.clear();
// serialise to a tagged loose object
let w = &mut output;
w.write_all(&item.loose_header())?;
item.write_to(w)?;
let parsed = ObjectRef::from_loose(&output)?;
let item2 = <$borrowed>::try_from(parsed).or(Err(super::Error::TryFromError))?;
assert_eq!(item2, item, "object-ref looose");
let item2: $owned = <$borrowed>::try_from(parsed).or(Err(super::Error::TryFromError))?.into();
assert_eq!(item2, item, "object-ref loose owned: {input_name} {:?}\n{:?}", output.as_bstr(), input.as_bstr());
}
Ok(())
}
Expand Down
2 changes: 2 additions & 0 deletions gix-object/tests/fixtures/.gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# assure newlines aren't butchered as they matter for testing
**/*.txt text crlf=input eol=lf
22 changes: 11 additions & 11 deletions gix-object/tests/fixtures/commit/subtle.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ tree de33f909dd559dd60753264ccf9e80183a32c9cc
parent 2305aad7ac23331e75465eccf425f899fa53a8d8
author Brezak <[email protected]> 1724180029 +0200
committer Brezak <[email protected]> 1728251787 +0200
gpgsig -----BEGIN PGP SIGNATURE-----
iQEzBAABCAAdFiEE6+MMXTYlBWl/3EAJyziR4rEnnXwFAmcDB4sACgkQyziR4rEn
nXy8owf9EkHTtmxUKhXiZ8ThGlpqwzcUaaYOBSX6Pmwacz+mqsA7fwLN/AxyN/Wv
d7zVhCYX01aZi7lutN4rVa08S9ZoXeZOBHm1u2YtrtETcjiBpVIVt33spkLendvB
1C5UyuVhZ4m8bHG5A3kmJlg0Mf9cwKvNTZiozb+lG79p1b3rc8PJHilHdAMmaUkb
58/7dUXM1B/LLd9p4IyJdLVA0jZmAagm+akiH/A1lji7zSvLJqhr1xOlD6L/K/Ao
TM6Kk0i2+eido5vaoutUFuKx+u4j4OtA3lDohvmu36RwyZbfa3OSAg+TWChgYBc/
ctOcWsB0EKL3XKz7X8Sw2EIS1Fcd4w==
=aZq+
-----END PGP SIGNATURE-----
gpgsig -----BEGIN PGP SIGNATURE-----

iQEzBAABCAAdFiEE6+MMXTYlBWl/3EAJyziR4rEnnXwFAmcDB4sACgkQyziR4rEn
nXy8owf9EkHTtmxUKhXiZ8ThGlpqwzcUaaYOBSX6Pmwacz+mqsA7fwLN/AxyN/Wv
d7zVhCYX01aZi7lutN4rVa08S9ZoXeZOBHm1u2YtrtETcjiBpVIVt33spkLendvB
1C5UyuVhZ4m8bHG5A3kmJlg0Mf9cwKvNTZiozb+lG79p1b3rc8PJHilHdAMmaUkb
58/7dUXM1B/LLd9p4IyJdLVA0jZmAagm+akiH/A1lji7zSvLJqhr1xOlD6L/K/Ao
TM6Kk0i2+eido5vaoutUFuKx+u4j4OtA3lDohvmu36RwyZbfa3OSAg+TWChgYBc/
ctOcWsB0EKL3XKz7X8Sw2EIS1Fcd4w==
=aZq+
-----END PGP SIGNATURE-----


Check that `#[pointee]` is applied only to generic arguments

0 comments on commit 4a6bbb1

Please sign in to comment.