Skip to content

Commit

Permalink
fix: restore verbose error reporting capabilities when parsing of obj…
Browse files Browse the repository at this point in the history
…ects fails.

When `verbose-object-parsing-errors` is enabled, it will now once again
provide greater details as to where and why the parsing failed.
  • Loading branch information
Byron committed Nov 9, 2023
1 parent 3542cf5 commit 5d78ab3
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 7 deletions.
2 changes: 1 addition & 1 deletion gix-object/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ serde = ["dep:serde", "bstr/serde", "smallvec/serde", "gix-hash/serde", "gix-act
## details information about the error location will be collected.
## Use it in applications which expect broken or invalid objects or for debugging purposes. Incorrectly formatted objects aren't at all
## common otherwise.
verbose-object-parsing-errors = []
verbose-object-parsing-errors = ["winnow/std"]

[dependencies]
gix-features = { version = "^0.36.0", path = "../gix-features", features = ["rustsha1", "progress"] }
Expand Down
13 changes: 10 additions & 3 deletions gix-object/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,12 @@ pub mod decode {
self.inner.fmt(f)
}
}

impl std::error::Error for Error {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
self.inner.cause().map(|v| v as &(dyn std::error::Error + 'static))
}
}
}

///
Expand Down Expand Up @@ -318,14 +324,15 @@ pub mod decode {
}

impl std::fmt::Display for Error {
fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Ok(())
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("object parsing failed")
}
}

impl std::error::Error for Error {}
}
pub(crate) use _decode::empty_error;
pub use _decode::{Error, ParseError};
impl std::error::Error for Error {}

/// Returned by [`loose_header()`]
#[derive(Debug, thiserror::Error)]
Expand Down
4 changes: 2 additions & 2 deletions gix-object/src/tree/ref_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ impl<'a> Iterator for TreeRefIter<'a> {
Some(Ok(entry))
}
None => {
let failing = self.data;
self.data = &[];
let empty = &[] as &[u8];
#[allow(clippy::unit_arg)]
Some(Err(crate::decode::Error::with_err(
winnow::error::ErrMode::from_error_kind(&empty, winnow::error::ErrorKind::Verify),
winnow::error::ErrMode::from_error_kind(&failing, winnow::error::ErrorKind::Verify),
)))
}
}
Expand Down
24 changes: 24 additions & 0 deletions gix-object/tests/commit/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
use crate::fixture_name;
use gix_object::{CommitRef, CommitRefIter};

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 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 @@ -159,6 +162,27 @@ mod method {
}
}

#[test]
fn invalid() {
let fixture = fixture_name("commit", "unsigned.txt");
let partial_commit = &fixture[..fixture.len() / 2];
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>`"
} else {
"object parsing failed"
}
);
assert_eq!(
CommitRefIter::from_bytes(partial_commit)
.take_while(Result::is_ok)
.count(),
1,
"we can decode some fields before failing"
);
}

mod from_bytes;
mod iter;
mod message;
22 changes: 21 additions & 1 deletion gix-object/tests/tag/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::fixture_name;
use gix_date::time::Sign;
use gix_object::{bstr::ByteSlice, Kind, TagRef};
use gix_object::{bstr::ByteSlice, Kind, TagRef, TagRefIter};

mod method {
use gix_object::TagRef;
Expand Down Expand Up @@ -115,6 +116,25 @@ KLMHist5yj0sw1E4hDTyQa0=
}
}

#[test]
fn invalid() {
let fixture = fixture_name("tag", "whitespace.txt");
let partial_tag = &fixture[..fixture.len() / 2];
assert_eq!(
TagRef::from_bytes(partial_tag).unwrap_err().to_string(),
if cfg!(feature = "verbose-object-parsing-errors") {
""
} else {
"object parsing failed"
}
);
assert_eq!(
TagRefIter::from_bytes(partial_tag).take_while(Result::is_ok).count(),
4,
"we can decode some fields before failing"
);
}

mod from_bytes {
use gix_object::{bstr::ByteSlice, Kind, TagRef};

Expand Down
19 changes: 19 additions & 0 deletions gix-object/tests/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,25 @@ mod from_bytes {
Ok(())
}

#[test]
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"
}
);
assert_eq!(
TreeRefIter::from_bytes(partial_tree).take_while(Result::is_ok).count(),
9,
"we can decode about half of it before failing"
);
}

#[test]
fn special_trees() -> crate::Result {
for (name, expected_entry_count) in [
Expand Down

0 comments on commit 5d78ab3

Please sign in to comment.