fix(object): Report remaining input on error

gix-object/src/commit/mod.rs

@@ -1,6 +1,7 @@
 use std::ops::Range;
 
 use bstr::{BStr, BString, ByteSlice};
+use winnow::prelude::*;
 
 use crate::{Commit, CommitRef, TagRef};
 
@@ -60,7 +61,11 @@ mod write;
 impl<'a> CommitRef<'a> {
     /// Deserialize a commit from the given `data` bytes while avoiding most allocations.
     pub fn from_bytes(mut data: &'a [u8]) -> Result<CommitRef<'a>, crate::decode::Error> {
-        decode::commit(&mut data).map_err(crate::decode::Error::with_err)
+        let input = &mut data;
+        match decode::commit.parse_next(input) {
+            Ok(tag) => Ok(tag),
+            Err(err) => Err(crate::decode::Error::with_err(err, input)),
+        }
     }
 }
 

gix-object/src/commit/ref_iter.rs

@@ -154,44 +154,42 @@ fn missing_field() -> crate::decode::Error {
 
 impl<'a> CommitRefIter<'a> {
     #[inline]
-    fn next_inner(i: &'a [u8], state: &mut State) -> Result<(&'a [u8], Token<'a>), crate::decode::Error> {
-        Self::next_inner_(i, state).map_err(crate::decode::Error::with_err)
+    fn next_inner(mut i: &'a [u8], state: &mut State) -> Result<(&'a [u8], Token<'a>), crate::decode::Error> {
+        let input = &mut i;
+        match Self::next_inner_(input, state) {
+            Ok(token) => Ok((*input, token)),
+            Err(err) => Err(crate::decode::Error::with_err(err, input)),
+        }
     }
 
     fn next_inner_(
-        mut i: &'a [u8],
+        input: &mut &'a [u8],
         state: &mut State,
-    ) -> Result<(&'a [u8], Token<'a>), winnow::error::ErrMode<crate::decode::ParseError>> {
+    ) -> Result<Token<'a>, winnow::error::ErrMode<crate::decode::ParseError>> {
         use State::*;
         Ok(match state {
             Tree => {
                 let tree = (|i: &mut _| parse::header_field(i, b"tree", parse::hex_hash))
                     .context(StrContext::Expected("tree <40 lowercase hex char>".into()))
-                    .parse_next(&mut i)?;
+                    .parse_next(input)?;
                 *state = State::Parents;
-                (
-                    i,
-                    Token::Tree {
-                        id: ObjectId::from_hex(tree).expect("parsing validation"),
-                    },
-                )
+                Token::Tree {
+                    id: ObjectId::from_hex(tree).expect("parsing validation"),
+                }
             }
             Parents => {
                 let parent = opt(|i: &mut _| parse::header_field(i, b"parent", parse::hex_hash))
                     .context(StrContext::Expected("commit <40 lowercase hex char>".into()))
-                    .parse_next(&mut i)?;
+                    .parse_next(input)?;
                 match parent {
-                    Some(parent) => (
-                        i,
-                        Token::Parent {
-                            id: ObjectId::from_hex(parent).expect("parsing validation"),
-                        },
-                    ),
+                    Some(parent) => Token::Parent {
+                        id: ObjectId::from_hex(parent).expect("parsing validation"),
+                    },
                     None => {
                         *state = State::Signature {
                             of: SignatureKind::Author,
                         };
-                        return Self::next_inner_(i, state);
+                        Self::next_inner_(input, state)?
                     }
                 }
             }
@@ -209,23 +207,20 @@ impl<'a> CommitRefIter<'a> {
                 };
                 let signature = (|i: &mut _| parse::header_field(i, field_name, parse::signature))
                     .context(StrContext::Expected(err_msg.into()))
-                    .parse_next(&mut i)?;
-                (
-                    i,
-                    match who {
-                        SignatureKind::Author => Token::Author { signature },
-                        SignatureKind::Committer => Token::Committer { signature },
-                    },
-                )
+                    .parse_next(input)?;
+                match who {
+                    SignatureKind::Author => Token::Author { signature },
+                    SignatureKind::Committer => Token::Committer { signature },
+                }
             }
             Encoding => {
                 let encoding = opt(|i: &mut _| parse::header_field(i, b"encoding", take_till1(NL)))
                     .context(StrContext::Expected("encoding <encoding>".into()))
-                    .parse_next(&mut i)?;
+                    .parse_next(input)?;
                 *state = State::ExtraHeaders;
                 match encoding {
-                    Some(encoding) => (i, Token::Encoding(encoding.as_bstr())),
-                    None => return Self::next_inner_(i, state),
+                    Some(encoding) => Token::Encoding(encoding.as_bstr()),
+                    None => Self::next_inner_(input, state)?,
                 }
             }
             ExtraHeaders => {
@@ -237,22 +232,22 @@ impl<'a> CommitRefIter<'a> {
                     },
                 )))
                 .context(StrContext::Expected("<field> <single-line|multi-line>".into()))
-                .parse_next(&mut i)?;
+                .parse_next(input)?;
                 match extra_header {
-                    Some(extra_header) => (i, Token::ExtraHeader(extra_header)),
+                    Some(extra_header) => Token::ExtraHeader(extra_header),
                     None => {
                         *state = State::Message;
-                        return Self::next_inner_(i, state);
+                        Self::next_inner_(input, state)?
                     }
                 }
             }
             Message => {
-                let message = terminated(decode::message, eof).parse_next(&mut i)?;
+                let message = terminated(decode::message, eof).parse_next(input)?;
                 debug_assert!(
-                    i.is_empty(),
+                    input.is_empty(),
                     "we should have consumed all data - otherwise iter may go forever"
                 );
-                return Ok((i, Token::Message(message)));
+                Token::Message(message)
             }
         })
     }

gix-object/src/lib.rs

@@ -267,6 +267,7 @@ pub mod decode {
         pub(crate) fn empty_error() -> Error {
             Error {
                 inner: winnow::error::ContextError::new(),
+                remaining: Default::default(),
             }
         }
 
@@ -275,19 +276,27 @@ pub mod decode {
         pub struct Error {
             /// The actual error
             pub inner: ParseError,
+            /// Where the error occurred
+            pub remaining: Vec<u8>,
         }
 
         impl Error {
-            pub(crate) fn with_err(err: winnow::error::ErrMode<ParseError>) -> Self {
+            pub(crate) fn with_err(err: winnow::error::ErrMode<ParseError>, remaining: &[u8]) -> Self {
                 Self {
                     inner: err.into_inner().expect("we don't have streaming parsers"),
+                    remaining: remaining.to_owned(),
                 }
             }
         }
 
         impl std::fmt::Display for Error {
             fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-                self.inner.fmt(f)
+                write!(f, "object parsing failed at `{}`", bstr::BStr::new(&self.remaining))?;
+                if self.inner.context().next().is_some() {
+                    writeln!(f)?;
+                    self.inner.fmt(f)?;
+                }
+                Ok(())
             }
         }
 
@@ -316,7 +325,7 @@ pub mod decode {
         }
 
         impl Error {
-            pub(crate) fn with_err(err: winnow::error::ErrMode<ParseError>) -> Self {
+            pub(crate) fn with_err(err: winnow::error::ErrMode<ParseError>, _remaining: &[u8]) -> Self {
                 Self {
                     inner: err.into_inner().expect("we don't have streaming parsers"),
                 }

gix-object/src/tag/mod.rs

@@ -1,3 +1,5 @@
+use winnow::prelude::*;
+
 use crate::TagRef;
 
 mod decode;
@@ -11,7 +13,11 @@ pub mod ref_iter;
 impl<'a> TagRef<'a> {
     /// Deserialize a tag from `data`.
     pub fn from_bytes(mut data: &'a [u8]) -> Result<TagRef<'a>, crate::decode::Error> {
-        decode::git_tag(&mut data).map_err(crate::decode::Error::with_err)
+        let input = &mut data;
+        match decode::git_tag.parse_next(input) {
+            Ok(tag) => Ok(tag),
+            Err(err) => Err(crate::decode::Error::with_err(err, input)),
+        }
     }
     /// The object this tag points to as `Id`.
     pub fn target(&self) -> gix_hash::ObjectId {

gix-object/src/tag/ref_iter.rs

@@ -59,58 +59,59 @@ fn missing_field() -> crate::decode::Error {
 
 impl<'a> TagRefIter<'a> {
     #[inline]
-    fn next_inner(i: &'a [u8], state: &mut State) -> Result<(&'a [u8], Token<'a>), crate::decode::Error> {
-        Self::next_inner_(i, state).map_err(crate::decode::Error::with_err)
+    fn next_inner(mut i: &'a [u8], state: &mut State) -> Result<(&'a [u8], Token<'a>), crate::decode::Error> {
+        let input = &mut i;
+        match Self::next_inner_(input, state) {
+            Ok(token) => Ok((*input, token)),
+            Err(err) => Err(crate::decode::Error::with_err(err, input)),
+        }
     }
 
     fn next_inner_(
-        mut i: &'a [u8],
+        input: &mut &'a [u8],
         state: &mut State,
-    ) -> Result<(&'a [u8], Token<'a>), winnow::error::ErrMode<crate::decode::ParseError>> {
+    ) -> Result<Token<'a>, winnow::error::ErrMode<crate::decode::ParseError>> {
         use State::*;
         Ok(match state {
             Target => {
                 let target = (|i: &mut _| parse::header_field(i, b"object", parse::hex_hash))
                     .context(StrContext::Expected("object <40 lowercase hex char>".into()))
-                    .parse_next(&mut i)?;
+                    .parse_next(input)?;
                 *state = TargetKind;
-                (
-                    i,
-                    Token::Target {
-                        id: ObjectId::from_hex(target).expect("parsing validation"),
-                    },
-                )
+                Token::Target {
+                    id: ObjectId::from_hex(target).expect("parsing validation"),
+                }
             }
             TargetKind => {
                 let kind = (|i: &mut _| parse::header_field(i, b"type", take_while(1.., AsChar::is_alpha)))
                     .context(StrContext::Expected("type <object kind>".into()))
-                    .parse_next(&mut i)?;
+                    .parse_next(input)?;
                 let kind = Kind::from_bytes(kind)
-                    .map_err(|_| winnow::error::ErrMode::from_error_kind(&i, winnow::error::ErrorKind::Verify))?;
+                    .map_err(|_| winnow::error::ErrMode::from_error_kind(input, winnow::error::ErrorKind::Verify))?;
                 *state = Name;
-                (i, Token::TargetKind(kind))
+                Token::TargetKind(kind)
             }
             Name => {
                 let tag_version = (|i: &mut _| parse::header_field(i, b"tag", take_while(1.., |b| b != NL[0])))
                     .context(StrContext::Expected("tag <version>".into()))
-                    .parse_next(&mut i)?;
+                    .parse_next(input)?;
                 *state = Tagger;
-                (i, Token::Name(tag_version.as_bstr()))
+                Token::Name(tag_version.as_bstr())
             }
             Tagger => {
                 let signature = opt(|i: &mut _| parse::header_field(i, b"tagger", parse::signature))
                     .context(StrContext::Expected("tagger <signature>".into()))
-                    .parse_next(&mut i)?;
+                    .parse_next(input)?;
                 *state = Message;
-                (i, Token::Tagger(signature))
+                Token::Tagger(signature)
             }
             Message => {
-                let (message, pgp_signature) = terminated(decode::message, eof).parse_next(&mut i)?;
+                let (message, pgp_signature) = terminated(decode::message, eof).parse_next(input)?;
                 debug_assert!(
-                    i.is_empty(),
+                    input.is_empty(),
                     "we should have consumed all data - otherwise iter may go forever"
                 );
-                return Ok((i, Token::Body { message, pgp_signature }));
+                Token::Body { message, pgp_signature }
             }
         })
     }

gix-object/src/tree/ref_iter.rs

@@ -2,6 +2,7 @@ use std::convert::TryFrom;
 
 use bstr::BStr;
 use winnow::error::ParserError;
+use winnow::prelude::*;
 
 use crate::{tree, tree::EntryRef, TreeRef, TreeRefIter};
 
@@ -15,7 +16,11 @@ impl<'a> TreeRefIter<'a> {
 impl<'a> TreeRef<'a> {
     /// Deserialize a Tree from `data`.
     pub fn from_bytes(mut data: &'a [u8]) -> Result<TreeRef<'a>, crate::decode::Error> {
-        decode::tree(&mut data).map_err(crate::decode::Error::with_err)
+        let input = &mut data;
+        match decode::tree.parse_next(input) {
+            Ok(tag) => Ok(tag),
+            Err(err) => Err(crate::decode::Error::with_err(err, input)),
+        }
     }
 
     /// Find an entry named `name` knowing if the entry is a directory or not, using a binary search.
@@ -72,6 +77,7 @@ impl<'a> Iterator for TreeRefIter<'a> {
                 #[allow(clippy::unit_arg)]
                 Some(Err(crate::decode::Error::with_err(
                     winnow::error::ErrMode::from_error_kind(&failing, winnow::error::ErrorKind::Verify),
+                    failing,
                 )))
             }
         }

gix-object/tests/commit/mod.rs

@@ -169,7 +169,7 @@ fn invalid() {
     assert_eq!(
         CommitRef::from_bytes(partial_commit).unwrap_err().to_string(),
         if cfg!(feature = "verbose-object-parsing-errors") {
-            "expected `<timestamp>`, `<name> <<email>> <timestamp> <+|-><HHMM>`, `author <signature>`"
+            "object parsing failed at `1`\nexpected `<timestamp>`, `<name> <<email>> <timestamp> <+|-><HHMM>`, `author <signature>`"
         } else {
             "object parsing failed"
         }

gix-object/tests/tag/mod.rs

@@ -123,7 +123,7 @@ fn invalid() {
     assert_eq!(
         TagRef::from_bytes(partial_tag).unwrap_err().to_string(),
         if cfg!(feature = "verbose-object-parsing-errors") {
-            ""
+            "object parsing failed at `tagger Sebasti`"
         } else {
             "object parsing failed"
         }

gix-object/tests/tree/mod.rs

@@ -111,14 +111,12 @@ mod from_bytes {
     fn invalid() {
         let fixture = fixture_name("tree", "definitely-special.tree");
         let partial_tree = &fixture[..fixture.len() / 2];
-        assert_eq!(
-            TreeRef::from_bytes(partial_tree).unwrap_err().to_string(),
-            if cfg!(feature = "verbose-object-parsing-errors") {
-                ""
-            } else {
-                "object parsing failed"
-            }
-        );
+        let err = TreeRef::from_bytes(partial_tree).unwrap_err().to_string();
+        if cfg!(feature = "verbose-object-parsing-errors") {
+            assert!(err.starts_with("object parsing failed at `100644"), "{err}");
+        } else {
+            assert_eq!(err, "object parsing failed");
+        }
         assert_eq!(
             TreeRefIter::from_bytes(partial_tree).take_while(Result::is_ok).count(),
             9,