Skip to content

Commit 1efbe16

Browse files
committed
change!: migrate hash verification to the common interface
This mostly just affects return types – using `git_hash::verify::Error` instead of bespoke duplicated versions thereof, and occasionally returning an `ObjectId` instead of `()` for convenience.
1 parent 917a762 commit 1efbe16

File tree

18 files changed

+90
-152
lines changed

18 files changed

+90
-152
lines changed

gitoxide-core/src/pack/explode.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ enum Error {
7878
},
7979
#[error("Object didn't verify after right after writing it")]
8080
Verify(#[from] objs::data::verify::Error),
81-
#[error("{kind} object {expected} wasn't re-encoded without change - new hash is {actual}")]
81+
#[error("{kind} object wasn't re-encoded without change")]
8282
ObjectEncodeMismatch {
83+
#[source]
84+
source: gix::hash::verify::Error,
8385
kind: object::Kind,
84-
actual: ObjectId,
85-
expected: ObjectId,
8686
},
8787
#[error("The recently written file for loose object {id} could not be found")]
8888
WrittenFileMissing { id: ObjectId },
@@ -201,17 +201,16 @@ pub fn pack_or_pack_index(
201201
kind: object_kind,
202202
id: index_entry.oid,
203203
})?;
204-
if written_id != index_entry.oid {
204+
if let Err(err) = written_id.verify(&index_entry.oid) {
205205
if let object::Kind::Tree = object_kind {
206206
progress.info(format!(
207207
"The tree in pack named {} was written as {} due to modes 100664 and 100640 rewritten as 100644.",
208208
index_entry.oid, written_id
209209
));
210210
} else {
211211
return Err(Error::ObjectEncodeMismatch {
212+
source: err,
212213
kind: object_kind,
213-
actual: index_entry.oid,
214-
expected: written_id,
215214
});
216215
}
217216
}

gix-commitgraph/src/file/verify.rs

+20-19
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,8 @@ pub enum Error<E: std::error::Error + 'static> {
2828
Filename(String),
2929
#[error("commit {id} has invalid generation {generation}")]
3030
Generation { generation: u32, id: gix_hash::ObjectId },
31-
#[error("checksum mismatch: expected {expected}, got {actual}")]
32-
Mismatch {
33-
actual: gix_hash::ObjectId,
34-
expected: gix_hash::ObjectId,
35-
},
31+
#[error(transparent)]
32+
Checksum(#[from] checksum::Error),
3633
#[error("{0}")]
3734
Processor(#[source] E),
3835
#[error("commit {id} has invalid root tree ID {root_tree_id}")]
@@ -42,6 +39,17 @@ pub enum Error<E: std::error::Error + 'static> {
4239
},
4340
}
4441

42+
///
43+
pub mod checksum {
44+
/// The error used in [`super::File::verify_checksum()`].
45+
#[derive(thiserror::Error, Debug)]
46+
#[allow(missing_docs)]
47+
pub enum Error {
48+
#[error(transparent)]
49+
Verify(#[from] gix_hash::verify::Error),
50+
}
51+
}
52+
4553
/// The positive result of [`File::traverse()`] providing some statistical information.
4654
#[derive(Clone, Debug, Eq, PartialEq)]
4755
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
@@ -73,8 +81,7 @@ impl File {
7381
E: std::error::Error + 'static,
7482
Processor: FnMut(&file::Commit<'a>) -> Result<(), E>,
7583
{
76-
self.verify_checksum()
77-
.map_err(|(actual, expected)| Error::Mismatch { actual, expected })?;
84+
self.verify_checksum()?;
7885
verify_split_chain_filename_hash(&self.path, self.checksum()).map_err(Error::Filename)?;
7986

8087
let null_id = self.object_hash().null_ref();
@@ -138,23 +145,17 @@ impl File {
138145
/// Assure the [`checksum`][File::checksum()] matches the actual checksum over all content of this file, excluding the trailing
139146
/// checksum itself.
140147
///
141-
/// Return the actual checksum on success or `(actual checksum, expected checksum)` if there is a mismatch.
142-
pub fn verify_checksum(&self) -> Result<gix_hash::ObjectId, (gix_hash::ObjectId, gix_hash::ObjectId)> {
143-
// Even though we could use gix_hash::bytes_of_file(…), this would require using our own
144-
// Error type to support io::Error and Mismatch. As we only gain progress, there probably isn't much value
145-
// as these files are usually small enough to process them in less than a second, even for the large ones.
148+
/// Return the actual checksum on success or [`checksum::Error`] if there is a mismatch.
149+
pub fn verify_checksum(&self) -> Result<gix_hash::ObjectId, checksum::Error> {
150+
// Even though we could use gix_hash::bytes_of_file(…), this would require extending our
151+
// Error type to support io::Error. As we only gain progress, there probably isn't much value // as these files are usually small enough to process them in less than a second, even for the large ones.
146152
// But it's possible, once a progress instance is passed.
147153
let data_len_without_trailer = self.data.len() - self.hash_len;
148154
let mut hasher = gix_hash::hasher(self.object_hash());
149155
hasher.update(&self.data[..data_len_without_trailer]);
150156
let actual = hasher.digest();
151-
152-
let expected = self.checksum();
153-
if actual == expected {
154-
Ok(actual)
155-
} else {
156-
Err((actual, expected.into()))
157-
}
157+
actual.verify(self.checksum())?;
158+
Ok(actual)
158159
}
159160
}
160161

gix-commitgraph/src/verify.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,7 @@ impl Graph {
164164
file::verify::Error::RootTreeId { id, root_tree_id } => {
165165
file::verify::Error::RootTreeId { id, root_tree_id }
166166
}
167-
file::verify::Error::Mismatch { actual, expected } => {
168-
file::verify::Error::Mismatch { actual, expected }
169-
}
167+
file::verify::Error::Checksum(err) => file::verify::Error::Checksum(err),
170168
file::verify::Error::Generation { generation, id } => {
171169
file::verify::Error::Generation { generation, id }
172170
}

gix-index/src/decode/mod.rs

+3-11
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,8 @@ mod error {
2222
Extension(#[from] extension::decode::Error),
2323
#[error("Index trailer should have been {expected} bytes long, but was {actual}")]
2424
UnexpectedTrailerLength { expected: usize, actual: usize },
25-
#[error("Shared index checksum was {actual_checksum} but should have been {expected_checksum}")]
26-
ChecksumMismatch {
27-
actual_checksum: gix_hash::ObjectId,
28-
expected_checksum: gix_hash::ObjectId,
29-
},
25+
#[error("Shared index checksum mismatch")]
26+
Verify(#[from] gix_hash::verify::Error),
3027
}
3128
}
3229
pub use error::Error;
@@ -217,12 +214,7 @@ impl State {
217214
let checksum = gix_hash::ObjectId::from_bytes_or_panic(data);
218215
let checksum = (!checksum.is_null()).then_some(checksum);
219216
if let Some((expected_checksum, actual_checksum)) = expected_checksum.zip(checksum) {
220-
if actual_checksum != expected_checksum {
221-
return Err(Error::ChecksumMismatch {
222-
actual_checksum,
223-
expected_checksum,
224-
});
225-
}
217+
actual_checksum.verify(&expected_checksum)?;
226218
}
227219
let EntriesOutcome {
228220
entries,

gix-index/src/extension/end_of_index_entry/decode.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option<usize> {
2929

3030
let (offset, checksum) = ext_data.split_at(4);
3131
let offset = from_be_u32(offset) as usize;
32-
if offset < header::SIZE || offset > start_of_eoie || checksum.len() != gix_hash::Kind::Sha1.len_in_bytes() {
32+
let Ok(checksum) = gix_hash::oid::try_from_bytes(checksum) else {
33+
return None;
34+
};
35+
if offset < header::SIZE || offset > start_of_eoie || checksum.kind() != gix_hash::Kind::Sha1 {
3336
return None;
3437
}
3538

@@ -41,7 +44,7 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option<usize> {
4144
last_chunk = Some(chunk);
4245
}
4346

44-
if hasher.digest().as_slice() != checksum {
47+
if hasher.digest().verify(checksum).is_err() {
4548
return None;
4649
}
4750
// The last-to-this chunk ends where ours starts

gix-index/src/file/init.rs

+4-9
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,15 @@ impl File {
7575
let _span = gix_features::trace::detail!("gix::open_index::hash_index", path = ?path);
7676
let meta = file.metadata()?;
7777
let num_bytes_to_hash = meta.len() - object_hash.len_in_bytes() as u64;
78-
let actual_hash = gix_hash::bytes(
78+
gix_hash::bytes(
7979
&mut file,
8080
num_bytes_to_hash,
8181
object_hash,
8282
&mut gix_features::progress::Discard,
8383
&Default::default(),
84-
)?;
85-
86-
if actual_hash != expected {
87-
return Err(Error::Decode(decode::Error::ChecksumMismatch {
88-
actual_checksum: actual_hash,
89-
expected_checksum: expected,
90-
}));
91-
}
84+
)?
85+
.verify(&expected)
86+
.map_err(decode::Error::from)?;
9287
}
9388
}
9489

gix-index/src/file/verify.rs

+6-13
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@ mod error {
99
pub enum Error {
1010
#[error("Could not read index file to generate hash")]
1111
Io(#[from] std::io::Error),
12-
#[error("Index checksum should have been {expected}, but was {actual}")]
13-
ChecksumMismatch {
14-
actual: gix_hash::ObjectId,
15-
expected: gix_hash::ObjectId,
16-
},
12+
#[error("Index checksum mismatch")]
13+
Verify(#[from] gix_hash::verify::Error),
1714
}
1815
}
1916
pub use error::Error;
@@ -25,19 +22,15 @@ impl File {
2522
if let Some(checksum) = self.checksum {
2623
let num_bytes_to_hash = self.path.metadata()?.len() - checksum.as_bytes().len() as u64;
2724
let should_interrupt = AtomicBool::new(false);
28-
let actual = gix_hash::bytes_of_file(
25+
gix_hash::bytes_of_file(
2926
&self.path,
3027
num_bytes_to_hash,
3128
checksum.kind(),
3229
&mut gix_features::progress::Discard,
3330
&should_interrupt,
34-
)?;
35-
(actual == checksum).then_some(()).ok_or(Error::ChecksumMismatch {
36-
actual,
37-
expected: checksum,
38-
})
39-
} else {
40-
Ok(())
31+
)?
32+
.verify(&checksum)?;
4133
}
34+
Ok(())
4235
}
4336
}

gix-index/tests/index/file/read.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ fn v2_split_index_recursion_is_handled_gracefully() {
328328
let err = try_file("v2_split_index_recursive").expect_err("recursion fails gracefully");
329329
assert!(matches!(
330330
err,
331-
gix_index::file::init::Error::Decode(gix_index::decode::Error::ChecksumMismatch { .. })
331+
gix_index::file::init::Error::Decode(gix_index::decode::Error::Verify(_))
332332
));
333333
}
334334

gix-object/src/data.rs

+7-16
Original file line numberDiff line numberDiff line change
@@ -51,31 +51,22 @@ impl<'a> Data<'a> {
5151

5252
/// Types supporting object hash verification
5353
pub mod verify {
54-
5554
/// Returned by [`crate::Data::verify_checksum()`]
5655
#[derive(Debug, thiserror::Error)]
5756
#[allow(missing_docs)]
5857
pub enum Error {
59-
#[error("Object expected to have id {desired}, but actual id was {actual}")]
60-
ChecksumMismatch {
61-
desired: gix_hash::ObjectId,
62-
actual: gix_hash::ObjectId,
63-
},
58+
#[error(transparent)]
59+
Verify(#[from] gix_hash::verify::Error),
6460
}
6561

6662
impl crate::Data<'_> {
67-
/// Compute the checksum of `self` and compare it with the `desired` hash.
63+
/// Compute the checksum of `self` and compare it with the `expected` hash.
6864
/// If the hashes do not match, an [`Error`] is returned, containing the actual
6965
/// hash of `self`.
70-
pub fn verify_checksum(&self, desired: &gix_hash::oid) -> Result<(), Error> {
71-
let actual_id = crate::compute_hash(desired.kind(), self.kind, self.data);
72-
if desired != actual_id {
73-
return Err(Error::ChecksumMismatch {
74-
desired: desired.into(),
75-
actual: actual_id,
76-
});
77-
}
78-
Ok(())
66+
pub fn verify_checksum(&self, expected: &gix_hash::oid) -> Result<gix_hash::ObjectId, Error> {
67+
let actual = crate::compute_hash(expected.kind(), self.kind, self.data);
68+
actual.verify(expected)?;
69+
Ok(actual)
7970
}
8071
}
8172
}

gix-odb/src/store_impls/loose/verify.rs

+10-11
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ pub mod integrity {
1919
kind: gix_object::Kind,
2020
id: gix_hash::ObjectId,
2121
},
22-
#[error("{kind} object {expected} wasn't re-encoded without change - new hash is {actual}")]
23-
ObjectHashMismatch {
22+
#[error("{kind} object wasn't re-encoded without change")]
23+
ObjectEncodeMismatch {
24+
#[source]
25+
source: gix_hash::verify::Error,
2426
kind: gix_object::Kind,
25-
actual: gix_hash::ObjectId,
26-
expected: gix_hash::ObjectId,
2727
},
2828
#[error("Objects were deleted during iteration - try again")]
2929
Retry,
@@ -77,14 +77,13 @@ impl Store {
7777
.try_find(&id, &mut buf)
7878
.map_err(|_| integrity::Error::Retry)?
7979
.ok_or(integrity::Error::Retry)?;
80-
let actual_id = sink.write_buf(object.kind, object.data).expect("sink never fails");
81-
if actual_id != id {
82-
return Err(integrity::Error::ObjectHashMismatch {
80+
sink.write_buf(object.kind, object.data)
81+
.expect("sink never fails")
82+
.verify(&id)
83+
.map_err(|err| integrity::Error::ObjectEncodeMismatch {
84+
source: err,
8385
kind: object.kind,
84-
actual: actual_id,
85-
expected: id,
86-
});
87-
}
86+
})?;
8887
object.decode().map_err(|err| integrity::Error::ObjectDecode {
8988
source: err,
9089
kind: object.kind,

gix-pack/src/data/input/bytes_to_entries.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,8 @@ where
180180
let actual_id = hash.digest();
181181
if self.mode == input::Mode::Restore {
182182
id = actual_id;
183-
}
184-
if id != actual_id {
185-
return Err(input::Error::ChecksumMismatch {
186-
actual: actual_id,
187-
expected: id,
188-
});
183+
} else {
184+
actual_id.verify(&id)?;
189185
}
190186
}
191187
Some(id)

gix-pack/src/data/input/types.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@ pub enum Error {
99
Io(#[from] io::Error),
1010
#[error(transparent)]
1111
PackParse(#[from] crate::data::header::decode::Error),
12-
#[error("pack checksum in trailer was {expected}, but actual checksum was {actual}")]
13-
ChecksumMismatch {
14-
expected: gix_hash::ObjectId,
15-
actual: gix_hash::ObjectId,
16-
},
12+
#[error("Failed to verify pack checksum in trailer")]
13+
Verify(#[from] gix_hash::verify::Error),
1714
#[error("pack is incomplete: it was decompressed into {actual} bytes but {expected} bytes where expected.")]
1815
IncompletePack { actual: u64, expected: u64 },
1916
#[error("The object {object_id} could not be decoded or wasn't found")]

gix-pack/src/index/traverse/error.rs

+6-10
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,15 @@ pub enum Error<E: std::error::Error + Send + Sync + 'static> {
2020
offset: u64,
2121
source: crate::data::decode::Error,
2222
},
23-
#[error("The packfiles checksum didn't match the index file checksum: expected {expected}, got {actual}")]
24-
PackMismatch {
25-
expected: gix_hash::ObjectId,
26-
actual: gix_hash::ObjectId,
27-
},
23+
#[error("The packfiles checksum didn't match the index file checksum")]
24+
PackMismatch(#[source] gix_hash::verify::Error),
2825
#[error("Failed to verify pack file checksum")]
2926
PackVerify(#[source] crate::verify::checksum::Error),
30-
#[error("The hash of {kind} object at offset {offset} didn't match the checksum in the index file: expected {expected}, got {actual}")]
31-
PackObjectMismatch {
32-
expected: gix_hash::ObjectId,
33-
actual: gix_hash::ObjectId,
27+
#[error("Error verifying object at offset {offset} against checksum in the index file")]
28+
PackObjectVerify {
3429
offset: u64,
35-
kind: gix_object::Kind,
30+
#[source]
31+
source: gix_object::data::verify::Error,
3632
},
3733
#[error(
3834
"The CRC32 of {kind} object at offset {offset} didn't match the checksum in the index file: expected {expected}, got {actual}"

0 commit comments

Comments
 (0)