diff --git a/src/patch/error.rs b/src/patch/error.rs index f55f4969..eecf8767 100644 --- a/src/patch/error.rs +++ b/src/patch/error.rs @@ -110,6 +110,9 @@ pub(crate) enum ParsePatchErrorKind { /// Orphaned hunk header found after trailing content. OrphanedHunkHeader, + + /// Filename contains invalid UTF-8 when parsing as text. + InvalidUtf8Path, } impl fmt::Display for ParsePatchErrorKind { @@ -136,6 +139,7 @@ impl fmt::Display for ParsePatchErrorKind { Self::UnexpectedHunkLine => "unexpected line in hunk body", Self::MissingNewline => "missing newline", Self::OrphanedHunkHeader => "orphaned hunk header after trailing content", + Self::InvalidUtf8Path => "filename is not valid UTF-8", }; write!(f, "{msg}") } diff --git a/src/patch/parse.rs b/src/patch/parse.rs index dd74b15c..3d1fb26c 100644 --- a/src/patch/parse.rs +++ b/src/patch/parse.rs @@ -12,6 +12,43 @@ use std::borrow::Cow; type Result = std::result::Result; +/// Options that control parsing behavior. +/// +/// Defaults match the [`parse`]/[`parse_bytes`] behavior. +#[derive(Clone, Copy)] +pub(crate) struct ParseOpts { + skip_preamble: bool, + reject_orphaned_hunks: bool, +} + +impl Default for ParseOpts { + fn default() -> Self { + Self { + skip_preamble: true, + reject_orphaned_hunks: false, + } + } +} + +impl ParseOpts { + /// Don't skip preamble lines before `---`/`+++`/`@@`. + /// + /// Useful when the caller has already positioned the input + /// at the start of the patch content. + #[allow(dead_code)] // will be used by patch_set parser + pub(crate) fn no_skip_preamble(mut self) -> Self { + self.skip_preamble = false; + self + } + + /// Reject orphaned `@@ ` hunk headers after parsed hunks, + /// matching `git apply` behavior. + pub(crate) fn reject_orphaned_hunks(mut self) -> Self { + self.reject_orphaned_hunks = true; + self + } +} + struct Parser<'a, T: Text + ?Sized> { lines: std::iter::Peekable>, offset: usize, @@ -53,8 +90,28 @@ impl<'a, T: Text + ?Sized> Parser<'a, T> { } } +// TODO: make a better API for lib consumers +// +// Too many different variants of `parse*` functions here. +// And that also propogate to `Patch::from_{str,bytes}{,_strict}`. + pub fn parse(input: &str) -> Result> { - let (result, _consumed) = parse_one(input); + let (result, _consumed) = parse_one(input, ParseOpts::default()); + result +} + +pub fn parse_strict(input: &str) -> Result> { + let (result, _consumed) = parse_one(input, ParseOpts::default().reject_orphaned_hunks()); + result +} + +pub fn parse_bytes(input: &[u8]) -> Result> { + let (result, _consumed) = parse_one(input, ParseOpts::default()); + result +} + +pub fn parse_bytes_strict(input: &[u8]) -> Result> { + let (result, _consumed) = parse_one(input, ParseOpts::default().reject_orphaned_hunks()); result } @@ -62,10 +119,13 @@ pub fn parse(input: &str) -> Result> { /// /// Always returns consumed bytes alongside the result /// so callers can advance past the parsed or partially parsed content. -pub(crate) fn parse_one(input: &str) -> (Result>, usize) { +pub(crate) fn parse_one( + input: &T, + opts: ParseOpts, +) -> (Result>, usize) { let mut parser = Parser::new(input); - let header = match patch_header(&mut parser) { + let header = match patch_header(&mut parser, &opts) { Ok(h) => h, Err(e) => return (Err(e), parser.offset()), }; @@ -73,58 +133,23 @@ pub(crate) fn parse_one(input: &str) -> (Result>, usize) { Ok(h) => h, Err(e) => return (Err(e), parser.offset()), }; - - let patch = Patch::new( - header.0.map(convert_cow_to_str), - header.1.map(convert_cow_to_str), - hunks, - ); - (Ok(patch), parser.offset()) -} - -pub fn parse_strict(input: &str) -> Result> { - let mut parser = Parser::new(input); - let header = patch_header(&mut parser)?; - let hunks = hunks(&mut parser)?; - reject_orphaned_hunk_headers(&mut parser)?; - - Ok(Patch::new( - header.0.map(convert_cow_to_str), - header.1.map(convert_cow_to_str), - hunks, - )) -} - -pub fn parse_bytes(input: &[u8]) -> Result> { - let mut parser = Parser::new(input); - let header = patch_header(&mut parser)?; - let hunks = hunks(&mut parser)?; - - Ok(Patch::new(header.0, header.1, hunks)) -} - -pub fn parse_bytes_strict(input: &[u8]) -> Result> { - let mut parser = Parser::new(input); - let header = patch_header(&mut parser)?; - let hunks = hunks(&mut parser)?; - reject_orphaned_hunk_headers(&mut parser)?; - - Ok(Patch::new(header.0, header.1, hunks)) -} - -// This is only used when the type originated as a utf8 string -fn convert_cow_to_str(cow: Cow<'_, [u8]>) -> Cow<'_, str> { - match cow { - Cow::Borrowed(b) => std::str::from_utf8(b).unwrap().into(), - Cow::Owned(o) => String::from_utf8(o).unwrap().into(), + if opts.reject_orphaned_hunks { + if let Err(e) = reject_orphaned_hunk_headers(&mut parser) { + return (Err(e), parser.offset()); + } } + + (Ok(Patch::new(header.0, header.1, hunks)), parser.offset()) } #[allow(clippy::type_complexity)] -fn patch_header<'a, T: Text + ToOwned + ?Sized>( +fn patch_header<'a, T: Text + ?Sized>( parser: &mut Parser<'a, T>, -) -> Result<(Option>, Option>)> { - skip_header_preamble(parser)?; + opts: &ParseOpts, +) -> Result<(Option>, Option>)> { + if opts.skip_preamble { + skip_header_preamble(parser)?; + } let mut filename1 = None; let mut filename2 = None; @@ -161,10 +186,7 @@ fn skip_header_preamble(parser: &mut Parser<'_, T>) -> Result< Ok(()) } -fn parse_filename<'a, T: Text + ToOwned + ?Sized>( - prefix: &str, - line: &'a T, -) -> Result> { +fn parse_filename<'a, T: Text + ?Sized>(prefix: &str, line: &'a T) -> Result> { let line = line .strip_prefix(prefix) .ok_or(ParsePatchErrorKind::InvalidFilename)?; diff --git a/src/patch/tests.rs b/src/patch/tests.rs index 19671fd5..362dc373 100644 --- a/src/patch/tests.rs +++ b/src/patch/tests.rs @@ -618,6 +618,24 @@ fn plain_filename_roundtrip() { assert_eq!(p.modified(), p2.modified()); } +// Octal escape \377 decodes to 0xFF, which is not valid UTF-8. +// When parsing into `Patch<'_, str>`, this returns a parse error +// instead of panicking. +#[test] +fn non_utf8_escaped_filename_returns_error_on_str_parse() { + let s = r#"\ +--- "a/foo\377" ++++ "b/foo\377" +@@ -1 +1 @@ +-x ++y +"#; + assert_eq!( + parse(s).unwrap_err().kind, + ParsePatchErrorKind::InvalidUtf8Path, + ); +} + mod error_display { use crate::patch::error::ParsePatchErrorKind; use crate::Patch; diff --git a/src/patch_set/parse.rs b/src/patch_set/parse.rs index c4864f95..cec612b2 100644 --- a/src/patch_set/parse.rs +++ b/src/patch_set/parse.rs @@ -83,7 +83,8 @@ impl<'a> PatchSet<'a> { let patch_input = &remaining[patch_start..]; - let (result, consumed) = parse_one(patch_input); + let opts = crate::patch::parse::ParseOpts::default(); + let (result, consumed) = parse_one(patch_input, opts); // Always advance so the iterator makes progress even on error. let abs_patch_start = self.offset + patch_start; self.offset += patch_start + consumed; diff --git a/src/utils.rs b/src/utils.rs index c3b15944..c4914566 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -132,7 +132,7 @@ impl<'a, T: Text + ?Sized> Iterator for LineIter<'a, T> { /// A helper trait for processing text like `str` and `[u8]` /// Useful for abstracting over those types for parsing as well as breaking input into lines -pub trait Text: Eq + Hash { +pub trait Text: Eq + Hash + ToOwned { fn is_empty(&self) -> bool; fn len(&self) -> usize; fn starts_with(&self, prefix: &str) -> bool; @@ -148,6 +148,12 @@ pub trait Text: Eq + Hash { #[allow(unused)] fn lines(&self) -> LineIter<'_, Self>; + /// Converts raw bytes into `Self::Owned`. + /// + /// Returns `None` if the bytes are not valid for this type + /// (e.g. non-UTF-8 bytes for `str`). + fn owned_from_bytes(bytes: Vec) -> Option; + fn parse(&self) -> Option { self.as_str().and_then(|s| s.parse().ok()) } @@ -158,6 +164,10 @@ impl Text for str { self.is_empty() } + fn owned_from_bytes(bytes: Vec) -> Option { + String::from_utf8(bytes).ok() + } + fn len(&self) -> usize { self.len() } @@ -209,6 +219,10 @@ impl Text for [u8] { self.is_empty() } + fn owned_from_bytes(bytes: Vec) -> Option> { + Some(bytes) + } + fn len(&self) -> usize { self.len() } @@ -292,27 +306,29 @@ fn find_byte(haystack: &[u8], byte: u8) -> Option { /// /// See [`byte_needs_quoting`] for the set of characters that /// require quoting. -pub(crate) fn escaped_filename( +pub(crate) fn escaped_filename( filename: &T, -) -> Result, ParsePatchError> { +) -> Result, ParsePatchError> { if let Some(inner) = filename .strip_prefix("\"") .and_then(|s| s.strip_suffix("\"")) { - decode_escaped(inner) + match decode_escaped(inner.as_bytes())? { + None => Ok(Cow::Borrowed(inner)), + Some(bytes) => T::owned_from_bytes(bytes) + .map(Cow::Owned) + .ok_or_else(|| ParsePatchErrorKind::InvalidUtf8Path.into()), + } } else { let bytes = filename.as_bytes(); if bytes.iter().any(|b| byte_needs_quoting(*b)) { return Err(ParsePatchErrorKind::InvalidCharInUnquotedFilename.into()); } - Ok(bytes.into()) + Ok(Cow::Borrowed(filename)) } } -fn decode_escaped( - escaped: &T, -) -> Result, ParsePatchError> { - let bytes = escaped.as_bytes(); +fn decode_escaped(bytes: &[u8]) -> Result>, ParsePatchError> { let mut result = Vec::new(); let mut i = 0; let mut last_copy = 0; @@ -365,8 +381,8 @@ fn decode_escaped( if needs_allocation { result.extend_from_slice(&bytes[last_copy..]); - Ok(Cow::Owned(result)) + Ok(Some(result)) } else { - Ok(Cow::Borrowed(bytes)) + Ok(None) } }