From 743b9796e3c9e9a6ebc83524620475f4e5f1a91f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 21 Dec 2024 14:55:12 +0100 Subject: [PATCH 1/2] add failing test to show the issue --- gix-ref/tests/refs/file/log.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/gix-ref/tests/refs/file/log.rs b/gix-ref/tests/refs/file/log.rs index 17e030a1cf7..ba3e939e59e 100644 --- a/gix-ref/tests/refs/file/log.rs +++ b/gix-ref/tests/refs/file/log.rs @@ -29,6 +29,23 @@ mod line { Ok(()) } } + + mod parse { + use gix_ref::file::log; + + #[test] + fn angle_bracket_in_comment() -> crate::Result { + let line = log::LineRef::from_bytes(b"7b114132d03c468a9cd97836901553658c9792de 306cdbab5457c323d1201aa8a59b3639f600a758 First Last 1727013187 +0200\trebase (pick): Replace Into> by From")?; + assert_eq!(line.signature.name, "First Last"); + assert_eq!(line.signature.email, "first.last@example.com"); + assert_eq!(line.signature.time.seconds, 1727013187); + assert_eq!( + line.message, + "rebase (pick): Replace Into> by From" + ); + Ok(()) + } + } } mod iter { From 6abee881024568416cdb2d8feac2c52d4c3af0ad Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 21 Dec 2024 15:34:00 +0100 Subject: [PATCH 2/2] fix: Properly parse reflog messages with `>` in them. Previously these confused the signature parser, who gladly took the entire message as part of the email. --- gix-ref/src/store/file/log/line.rs | 60 ++++++++++++++++++++---------- gix-ref/tests/refs/file/log.rs | 2 +- 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/gix-ref/src/store/file/log/line.rs b/gix-ref/src/store/file/log/line.rs index ab53314bfbd..27e3be797b0 100644 --- a/gix-ref/src/store/file/log/line.rs +++ b/gix-ref/src/store/file/log/line.rs @@ -73,6 +73,7 @@ impl<'a> From> for Line { /// pub mod decode { + use crate::{file::log::LineRef, parse::hex_hash}; use gix_object::bstr::{BStr, ByteSlice}; use winnow::{ combinator::{alt, eof, fail, opt, preceded, rest, terminated}, @@ -81,8 +82,6 @@ pub mod decode { token::take_while, }; - use crate::{file::log::LineRef, parse::hex_hash}; - /// mod error { use gix_object::bstr::{BString, ByteSlice}; @@ -135,34 +134,57 @@ pub mod decode { fn one<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>( bytes: &mut &'a [u8], ) -> PResult, E> { - ( - ( + let mut tokens = bytes.splitn(2, |b| *b == b'\t'); + if let (Some(mut first), Some(mut second)) = (tokens.next(), tokens.next()) { + let (old, new, signature) = ( terminated(hex_hash, b" ").context(StrContext::Expected("".into())), terminated(hex_hash, b" ").context(StrContext::Expected("".into())), gix_actor::signature::decode.context(StrContext::Expected(" <> ".into())), ) .context(StrContext::Expected( " <> \\t".into(), - )), - alt(( - preceded( - b'\t', - message.context(StrContext::Expected("".into())), - ), - b'\n'.value(Default::default()), - eof.value(Default::default()), - fail.context(StrContext::Expected( - "log message must be separated from signature with whitespace".into(), - )), - )), - ) - .map(|((old, new, signature), message)| LineRef { + )) + .parse_next(&mut first)?; + + // forward the buffer🤦‍♂️ + message.parse_next(bytes)?; + let message = message(&mut second)?; + Ok(LineRef { previous_oid: old, new_oid: new, signature, message, }) - .parse_next(bytes) + } else { + ( + ( + terminated(hex_hash, b" ").context(StrContext::Expected("".into())), + terminated(hex_hash, b" ").context(StrContext::Expected("".into())), + gix_actor::signature::decode.context(StrContext::Expected(" <> ".into())), + ) + .context(StrContext::Expected( + " <> \\t".into(), + )), + alt(( + preceded( + b'\t', + message.context(StrContext::Expected("".into())), + ), + b'\n'.value(Default::default()), + eof.value(Default::default()), + fail.context(StrContext::Expected( + "log message must be separated from signature with whitespace".into(), + )), + )), + ) + .map(|((old, new, signature), message)| LineRef { + previous_oid: old, + new_oid: new, + signature, + message, + }) + .parse_next(bytes) + } } #[cfg(test)] diff --git a/gix-ref/tests/refs/file/log.rs b/gix-ref/tests/refs/file/log.rs index ba3e939e59e..b87b55fe5b9 100644 --- a/gix-ref/tests/refs/file/log.rs +++ b/gix-ref/tests/refs/file/log.rs @@ -221,7 +221,7 @@ mod iter { let mut iter = gix_ref::file::log::iter::forward(log_first_broken.as_bytes()); let err = iter.next().expect("error is not none").expect_err("the line is broken"); - assert_eq!(err.to_string(), "In line 1: \"134385fbroken7062102c6a483440bfda2a03 committer 946771200 +0000\\tcommit\" did not match ' <> \\t'"); + assert_eq!(err.to_string(), "In line 1: \"0000000000000000000000000000000000000000 134385fbroken7062102c6a483440bfda2a03 committer 946771200 +0000\\tcommit\" did not match ' <> \\t'"); assert!(iter.next().expect("a second line").is_ok(), "line parses ok"); assert!(iter.next().is_none(), "iterator exhausted"); }