From 25ff995fb1f7dcee4ce0fc45908df9e77b6a1a84 Mon Sep 17 00:00:00 2001 From: Emily Date: Thu, 27 Mar 2025 22:22:09 +0000 Subject: [PATCH 01/22] =?UTF-8?q?add=20more=20tests=20for=20SHA=E2=80=901?= =?UTF-8?q?=20vectors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- gix-hash/tests/fixtures/shambles/messageA | Bin 0 -> 640 bytes gix-hash/tests/fixtures/shambles/messageB | Bin 0 -> 640 bytes gix-hash/tests/object_id/mod.rs | 52 ++++++++++++++++++++-- 3 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 gix-hash/tests/fixtures/shambles/messageA create mode 100644 gix-hash/tests/fixtures/shambles/messageB diff --git a/gix-hash/tests/fixtures/shambles/messageA b/gix-hash/tests/fixtures/shambles/messageA new file mode 100644 index 0000000000000000000000000000000000000000..5a7c30e97646c66422abe0a9793a5fcb9f1cf8d6 GIT binary patch literal 640 zcmV-`0)PFP1Pug#=of$iAOQbMWqBZJb0BbGa&#bXW*}i8V{dG1X>)0BZXqB^bSHBl zVIXvJVQ?XN#v1Ui%mqai*(XkO2VzSd$NM9gi@4s4S6#Y$o~tpzXG?9DLwKksb1(IU z9Co7S2XeKfeBtWE3%QfQEsSvDN>7bn&F#UnES&M4F|Q;kb)7=w-`g>9pICMy?o}x{ zw%puBpUP8JJ8<}Z-Y}v^>N@tvS)%d_G7WYOwomkV2v5_@v(40lV%ch(Lk1Vh|7Z|qdJ4b`+{(G#SY32i+svyyDTet0$KULm2lmSL^q=mU$6JW%D|e^Qf38QClE(f7mlv~ zf?6!9D$ljvWX^U$+*zeTsr;OEXIA3kJ;xKs!c3Qts%s87r}bYHMJgPkg$>=6V*Q#J ztwKp^sc;DQMsoI!^kM6Wu$eQ~Cban&bezB^{oQOrU&JA3HP91H6)0P)EVqQD_sg{V zQB6zm_9J}o3Z9=6E1CvwZ_$5jLYQ=TSa0@Gua`F!H%LccW0YmW1WR(AfS%&6t}-k`d&K|M|24(A@=9~LXyZ62@LCFZWWu4*+- z@qF?Hqy+oh68uF?LH*fW@4;KF^V3*EX*WC9JhR6N@N literal 0 HcmV?d00001 diff --git a/gix-hash/tests/fixtures/shambles/messageB b/gix-hash/tests/fixtures/shambles/messageB new file mode 100644 index 0000000000000000000000000000000000000000..fe39178400a7ebeedca8ccfd0f3a64ceecdb9cda GIT binary patch literal 640 zcmV-`0)PFP0}TX!=of$i7y$oJa$#e1X=7n*AX7*|EioWtXm4|6ZY^+fWoBu3AY*TA zY-w|8Z*CzSCv0n`Vc`T0AANRz2VzOx$NMuZi@3tKS6#lxo~tY2XG?vvLwK~yb1&hE z9Cn~42XdtleBt)!3%QqlEsSVVN>80%&F#$#ES%wzF|Q`+b)8aB-`g!spICCA?o~4x zw%p8NpUO|UJ8N@6gS)%SIG7WOEwomNI2v5Vzv(4B$V%d6-Lk1NJ|7<35 zH{~PZC_#JPZ|qLZc|){HUso(1yEyL=_TUS^5YQfwt$3rpWEq#-j*SgEc%LN1Z{g~? ze>Jqn`+{<|#SYrKi+t8qyDTro0$Jmqm2l0eL^r?vU$5iO%D|WsQf3wCClFhC7mlh! zf?6~hD$k|fWX^i|+*!3vsr;U`XIAo7J;x99!c3Das%sP&r}a-TMJfPJg$>D>V*Qxd ztwLGesc;nAMso5p^kMTju$d#)CbadSbezyD{oQVYU&JxoHP8=n6)0a9EVrJ7_sgW; zQB7AI_9JNg3Z9+QE1DMCZ_#5wLYQ#GSa0s(ua;zgv?jGbE1H<-20vSSGfdF3Q1~Xi z#3xk?8Fa}Y{3r>A=<)x+8&TgB=D)@po37@;)T-&7S4sfGk8R3a6*_}KB9oi-l8>0e z*)~2t`F!$ILcb{WYmWCjR(9lt%&7I#-k_#lK|NXQ4(A*o9~L`rZ634rCFZIIu4*m* z@qFS1qy+b468u#`LH*mD@;KFUF3*EN#WC9(5s63Pa literal 0 HcmV?d00001 diff --git a/gix-hash/tests/object_id/mod.rs b/gix-hash/tests/object_id/mod.rs index 2ccc53abe9e..c2539100239 100644 --- a/gix-hash/tests/object_id/mod.rs +++ b/gix-hash/tests/object_id/mod.rs @@ -42,7 +42,9 @@ mod from_hex { } } -mod empty { +mod sha1 { + use std::str::FromStr as _; + use gix_features::hash::hasher; use gix_hash::{Kind, ObjectId}; @@ -53,12 +55,56 @@ mod empty { } #[test] - fn blob() { + fn empty_blob() { assert_eq!(ObjectId::empty_blob(Kind::Sha1), hash_contents(b"blob 0\0")); } #[test] - fn tree() { + fn empty_tree() { assert_eq!(ObjectId::empty_tree(Kind::Sha1), hash_contents(b"tree 0\0")); } + + /// Check the test vectors from RFC 3174. + #[test] + fn rfc_3174() { + let fixtures: &[(&[u8], &str)] = &[ + (b"abc", "A9 99 3E 36 47 06 81 6A BA 3E 25 71 78 50 C2 6C 9C D0 D8 9D"), + ( + b"abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq", + "84 98 3E 44 1C 3B D2 6E BA AE 4A A1 F9 51 29 E5 E5 46 70 F1", + ), + ( + &b"a".repeat(1000000), + "34 AA 97 3C D4 C4 DA A4 F6 1E EB 2B DB AD 27 31 65 34 01 6F", + ), + ( + &b"0123456701234567012345670123456701234567012345670123456701234567".repeat(10), + "DE A3 56 A2 CD DD 90 C7 A7 EC ED C5 EB B5 63 93 4F 46 04 52", + ), + ]; + for (input, output) in fixtures { + assert_eq!( + hash_contents(input), + ObjectId::from_str(&output.to_lowercase().replace(' ', "")).expect("RFC digests to be valid"), + ); + } + } + + /// Check the “SHA‐1 is a Shambles” chosen‐prefix collision. + /// + /// See . + /// + /// We test these and not the earlier SHAttered PDFs because they are much smaller. + #[test] + fn shambles() { + let message_a = include_bytes!("../fixtures/shambles/messageA"); + let message_b = include_bytes!("../fixtures/shambles/messageB"); + assert_ne!(message_a, message_b); + + // BUG: These should be detected as a collision attack. + let expected = + ObjectId::from_str("8ac60ba76f1999a1ab70223f225aefdc78d4ddc0").expect("Shambles digest to be valid"); + assert_eq!(hash_contents(message_a), expected); + assert_eq!(hash_contents(message_b), expected); + } } From cf261d5b0ef647d389fce7c901d076cd9f9dd8e2 Mon Sep 17 00:00:00 2001 From: Emily Date: Sat, 22 Mar 2025 19:09:23 +0000 Subject: [PATCH 02/22] factor out private `gix_object::object_hasher` helper --- gix-object/src/lib.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index 398374bee16..22706b041b1 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -398,15 +398,17 @@ pub mod decode { } } +fn object_hasher(hash_kind: gix_hash::Kind, object_kind: Kind, object_size: u64) -> gix_features::hash::Hasher { + let mut hasher = gix_features::hash::hasher(hash_kind); + hasher.update(&encode::loose_header(object_kind, object_size)); + hasher +} + /// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its `data`. #[doc(alias = "hash_object", alias = "git2")] pub fn compute_hash(hash_kind: gix_hash::Kind, object_kind: Kind, data: &[u8]) -> gix_hash::ObjectId { - let header = encode::loose_header(object_kind, data.len() as u64); - - let mut hasher = gix_features::hash::hasher(hash_kind); - hasher.update(&header); + let mut hasher = object_hasher(hash_kind, object_kind, data.len() as u64); hasher.update(data); - hasher.digest().into() } @@ -423,9 +425,6 @@ pub fn compute_stream_hash( progress: &mut dyn gix_features::progress::Progress, should_interrupt: &std::sync::atomic::AtomicBool, ) -> std::io::Result { - let header = encode::loose_header(object_kind, stream_len); - let mut hasher = gix_features::hash::hasher(hash_kind); - - hasher.update(&header); + let hasher = object_hasher(hash_kind, object_kind, stream_len); gix_features::hash::bytes_with_hasher(stream, stream_len, hasher, progress, should_interrupt) } From 7805ffe7ac0af6a413b491efeacdd7e4616df86b Mon Sep 17 00:00:00 2001 From: Emily Date: Sat, 22 Mar 2025 19:09:23 +0000 Subject: [PATCH 03/22] use `gix_object::compute_hash` in a test and benchmark --- gix-object/benches/edit_tree.rs | 6 +----- gix-object/tests/object/tree/editor.rs | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/gix-object/benches/edit_tree.rs b/gix-object/benches/edit_tree.rs index 41f1e5f9839..270a77dde75 100644 --- a/gix-object/benches/edit_tree.rs +++ b/gix-object/benches/edit_tree.rs @@ -144,11 +144,7 @@ fn new_inmemory_writes() -> (TreeStore, impl FnMut(&Tree) -> Result {} diff --git a/gix-object/tests/object/tree/editor.rs b/gix-object/tests/object/tree/editor.rs index bd9ca96fd80..df79683fb0c 100644 --- a/gix-object/tests/object/tree/editor.rs +++ b/gix-object/tests/object/tree/editor.rs @@ -805,11 +805,7 @@ mod utils { move |tree: &Tree| { buf.clear(); tree.write_to(&mut buf)?; - let header = gix_object::encode::loose_header(gix_object::Kind::Tree, buf.len() as u64); - let mut hasher = gix_features::hash::hasher(gix_hash::Kind::Sha1); - hasher.update(&header); - hasher.update(&buf); - let id = hasher.digest().into(); + let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf); store.borrow_mut().insert(id, tree.clone()); let old = num_writes.get(); num_writes.set(old + 1); From f253f02a6658b3b7612a50d56c71f5ae4da4ca21 Mon Sep 17 00:00:00 2001 From: Emily Date: Tue, 1 Apr 2025 21:55:16 +0100 Subject: [PATCH 04/22] =?UTF-8?q?feat!:=20detect=20SHA=E2=80=901=20collisi?= =?UTF-8?q?on=20attacks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix [GHSA-2frx-2596-x5r6]. [GHSA-2frx-2596-x5r6]: https://github.com/GitoxideLabs/gitoxide/security/advisories/GHSA-2frx-2596-x5r6 This uses the `sha1-checked` crate from the RustCrypto project. It’s a pure Rust implementation, with no SIMD or assembly code. The hashing implementation moves to `gix-hash`, as it no longer depends on any feature configuration. I wasn’t sure the ideal crate to put this in, but after checking reverse dependencies on crates.io, it seems like there’s essentially no user of `gix-hash` that wouldn’t be pulling in a hashing implementation anyway, so I think this is a fine and logical place for it to be. A fallible API seems better than killing the process as Git does, since we’re in a library context and it would be bad if you could perform denial‐of‐service attacks on a server by sending it hash collisions. (Although there are probably cheaper ways to mount a denial‐of‐service attack.) The new API also returns an `ObjectId` rather than `[u8; 20]`; the vast majority of `Hasher::digest()` users immediately convert the result to `ObjectId`, so this will help eliminate a lot of cruft across the tree. `ObjectId` also has nicer `Debug` and `Display` instances than `[u8; 20]`, and should theoretically make supporting the hash function transition easier, although I suspect further API changes will be required for that anyway. I wasn’t sure whether this would be a good change, as not every digest identifies an entry in the Git object database, but even many of the existing uses for non‐object digests across the tree used the `ObjectId` API anyway. Perhaps it would be best to have a separate non‐alias `Digest` type that `ObjectId` wraps, but this seems like the pragmatic choice for now that sticks with current practice. The old API remains in this commit, as well as a temporary non‐fallible but `ObjectId`‐returning `Hasher::finalize()`, pending the migration of all in‐tree callers. I named the module `gix_hash::hasher` since `gix_hash::hash` seemed like it would be confusing. This does mean that there is a function and module with the same name, which is permitted but perhaps a little strange. Everything is re‐exported directly other than `gix_features::hash::Write`, which moves along with the I/O convenience functions into a new public submodule and becomes `gix_hash::hasher::io::Write`, as that seems like a clearer name to me, being akin to the `gix_hash::hasher` function but as an `std::io::Write` wrapper. Raw hashing is somewhere around 0.25× to 0.65× the speed of the previous implementation, depending on the feature configuration and whether the CPU supports hardware‐accelerated hashing. (The more portable assembly in `sha1-asm` that doesn’t require the SHA instruction set doesn’t seem to speed things up that much; in fact, `sha1_smol` somehow regularly beats the assembly code used by `sha1` on my i9‐9880H MacBook Pro! Presumably this is why that path was removed in newer versions of the `sha1` crate.) Performance on an end‐to‐end `gix no-repo pack verify` benchmark using pack files from the Linux kernel Git server measures around 0.41× to 0.44× compared to the base commit on an M2 Max and a Ryzen 7 5800X, both of which have hardware instructions for SHA‐1 acceleration that the previous implementation uses but this one does not. On the i9‐9880H, it’s around 0.58× to 0.60× the speed; the slowdown is reduced by the older hardware’s lack of SHA‐1 instructions. The `sha1collisiondetection` crate from the Sequoia PGP project, based on a modified C2Rust translation of the library used by Git, was also considered; although its raw hashing performance seems to measure around 1.12–1.15× the speed of `sha1-checked` on x86, it’s indistinguishable from noise on the end‐to‐end benchmark, and on an M2 Max `sha1-checked` is consistently around 1.03× the speed of `sha1collisiondetection` on that benchmark. The `sha1collisiondetection` crate has also had a soundness issue in the past due to the automatic C translation, whereas `sha1-checked` has only one trivial `unsafe` block. On the other hand, `sha1collisiondetection` is used by both Sequoia itself and the `gitoid` crate, whereas rPGP is the only major user of `sha1-checked`. I don’t think there’s a clear winner here. The performance regression is very unfortunate, but the [SHAttered] attack demonstrated a collision back in 2017, and the 2020 [SHA‐1 is a Shambles] attack demonstrated a practical chosen‐prefix collision that broke the use of SHA‐1 in OpenPGP, costing $75k to perform, with an estimate of $45k to replicate at the time of publication and $11k for a classical collision. [SHAttered]: https://shattered.io/ [SHA‐1 is a Shambles]: https://sha-mbles.github.io/ Given the increase in GPU performance and production since then, that puts the Git object format squarely at risk. Git mitigated this attack in 2017; the algorithm is fairly general and detects all the existing public collisions. My understanding is that an entirely new cryptanalytic approach would be required to develop a collision attack for SHA‐1 that would not be detected with very high probability. I believe that the speed penalty could be mitigated, although not fully eliminated, by implementing a version of the hardened SHA‐1 function that makes use of SIMD. For instance, the assembly code used by `openssl speed sha1` on my i9‐9880H measures around 830 MiB/s, compared to the winning 580 MiB/s of `sha1_smol`; adding collision detection support to that would surely incur a performance penalty, but it is likely that it could be much more competitive with the performance before this commit than the 310 MiB/s I get with `sha1-checked`. I haven’t been able to find any existing work on this; it seems that more or less everyone just uses the original C library that Git does, presumably because nothing except Git and OpenPGP is still relying on SHA‐1 anyway… The performance will never compete with the >2 GiB/s that can be achieved with the x86 SHA instruction set extension, as the `SHA1RNDS4` instruction sadly runs four rounds at a time while the collision detection algorithm requires checks after every round, but I believe SIMD would still offer a significant improvement, and the AArch64 extension seems like it may be more flexible. I know that these days the Git codebase has an additional faster unsafe API without these checks that it tries to carefully use only for operations that do not depend on hashing results for correctness or safety. I personally believe that’s not a terribly good idea, as it seems easy to misuse in a case where correctness actually does matter, but maybe that’s just my Rust safety bias talking. I think it would be better to focus on improving the performance of the safer algorithm, as I think that many of the operations where the performance penalty is the most painful are dealing with untrusted input anyway. The `Hasher` struct gets a lot bigger; I don’t know if this is an issue or not, but if it is, it could potentially be boxed. Closes: #585 --- Cargo.lock | 14 +-- Cargo.toml | 3 +- SHORTCOMINGS.md | 4 - gix-features/Cargo.toml | 33 ++---- gix-features/src/hash.rs | 183 ++------------------------------ gix-features/tests/hash.rs | 16 --- gix-hash/Cargo.toml | 6 ++ gix-hash/src/hasher/io.rs | 138 ++++++++++++++++++++++++ gix-hash/src/hasher/mod.rs | 90 ++++++++++++++++ gix-hash/src/lib.rs | 5 + gix-hash/tests/hash.rs | 1 + gix-hash/tests/hasher/mod.rs | 9 ++ gix-hash/tests/object_id/mod.rs | 32 ++++-- 13 files changed, 292 insertions(+), 242 deletions(-) delete mode 100644 gix-features/tests/hash.rs create mode 100644 gix-hash/src/hasher/io.rs create mode 100644 gix-hash/src/hasher/mod.rs create mode 100644 gix-hash/tests/hasher/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 4bd5b1712ad..54982eb8e4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1853,8 +1853,6 @@ dependencies = [ "once_cell", "parking_lot", "prodash 29.0.1", - "sha1", - "sha1_smol", "thiserror 2.0.12", "walkdir", ] @@ -1966,7 +1964,9 @@ dependencies = [ "faster-hex", "gix-features 0.40.0", "gix-testtools", + "prodash 29.0.1", "serde", + "sha1-checked", "thiserror 2.0.12", ] @@ -4872,16 +4872,16 @@ dependencies = [ "cfg-if", "cpufeatures", "digest", - "sha1-asm", ] [[package]] -name = "sha1-asm" -version = "0.5.3" +name = "sha1-checked" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "286acebaf8b67c1130aedffad26f594eff0c1292389158135327d2e23aed582b" +checksum = "89f599ac0c323ebb1c6082821a54962b839832b03984598375bff3975b804423" dependencies = [ - "cc", + "digest", + "sha1", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 5b8963a9661..eb4c9991729 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -205,8 +205,7 @@ gix-hash = { opt-level = 3 } gix-actor = { opt-level = 3 } gix-config = { opt-level = 3 } miniz_oxide = { opt-level = 3 } -sha1 = { opt-level = 3 } -sha1_smol = { opt-level = 3 } +sha1-checked = { opt-level = 3 } [profile.release] overflow-checks = false diff --git a/SHORTCOMINGS.md b/SHORTCOMINGS.md index f25e320698e..3bdf20aa43d 100644 --- a/SHORTCOMINGS.md +++ b/SHORTCOMINGS.md @@ -35,7 +35,3 @@ This file is for tracking features that are less well implemented or less powerf * **gix-url** _might_ be more restrictive than what git allows as for the most part, it uses a browser grade URL parser. * Thus far there is no proof for this, and as _potential remedy_ we could certainly re-implement exactly what git does to handle its URLs. - -### `gix-features` - -* **sha1** isn't hardened (i.e. doesn't have collision detection). Needs [to be contributed](https://github.com/GitoxideLabs/gitoxide/issues/585). diff --git a/gix-features/Cargo.toml b/gix-features/Cargo.toml index 67983ee4f34..4eb7e0bae97 100644 --- a/gix-features/Cargo.toml +++ b/gix-features/Cargo.toml @@ -24,7 +24,7 @@ progress = ["prodash"] ## Provide human-readable numbers as well as easier to read byte units for progress bars. progress-unit-human-numbers = ["prodash?/unit-human"] ## Provide human readable byte units for progress bars. -progress-unit-bytes = ["dep:bytesize", "prodash?/unit-bytes"] +progress-unit-bytes = ["dep:bytesize", "prodash?/unit-bytes", "gix-hash/progress-unit-bytes"] ## Provide utilities suitable for working with the `std::fs::read_dir()`. fs-read-dir = ["dep:gix-utils"] @@ -77,14 +77,9 @@ zlib-stock = ["zlib", "flate2?/zlib"] ## may build in environments where other backends don't. zlib-rust-backend = ["zlib", "flate2?/rust_backend"] -#! ### Mutually Exclusive SHA1 -## A fast SHA1 implementation is critical to `gitoxide's` object database performance -## A multi-crate implementation that can use hardware acceleration, thus bearing the potential for up to 2Gb/s throughput on -## CPUs that support it, like AMD Ryzen or Intel Core i3, as well as Apple Silicon like M1. -## Takes precedence over `rustsha1` if both are specified. -fast-sha1 = ["dep:sha1"] -## A standard and well performing pure Rust implementation of Sha1. Will significantly slow down various git operations. -rustsha1 = ["dep:sha1_smol"] +# TODO: Remove these. +fast-sha1 = [] +rustsha1 = [] #! ### Other @@ -92,25 +87,19 @@ rustsha1 = ["dep:sha1_smol"] ## Caches implement this by default, which costs nothing unless this feature is enabled cache-efficiency-debug = [] -[[test]] -name = "hash" -path = "tests/hash.rs" -required-features = ["rustsha1"] - [[test]] name = "parallel" path = "tests/parallel_threaded.rs" -required-features = ["parallel", "rustsha1"] +required-features = ["parallel"] [[test]] name = "multi-threaded" path = "tests/parallel_shared_threaded.rs" -required-features = ["parallel", "rustsha1"] +required-features = ["parallel"] [[test]] name = "single-threaded" path = "tests/parallel_shared.rs" -required-features = ["rustsha1"] [[test]] name = "pipe" @@ -130,10 +119,8 @@ parking_lot = { version = "0.12.0", default-features = false, optional = true } walkdir = { version = "2.3.2", optional = true } # used when parallel is off -# hashing and 'fast-sha1' feature -sha1_smol = { version = "1.0.0", optional = true } +# hashing crc32fast = { version = "1.2.1", optional = true } -sha1 = { version = "0.10.0", optional = true } # progress prodash = { version = "29.0.1", optional = true } @@ -156,12 +143,6 @@ libc = { version = "0.2.119" } [dev-dependencies] bstr = { version = "1.3.0", default-features = false } - -# Assembly doesn't yet compile on MSVC on windows, but does on GNU, see https://github.com/RustCrypto/asm-hashes/issues/17 -# At this time, only aarch64, x86 and x86_64 are supported. -[target.'cfg(all(any(target_arch = "aarch64", target_arch = "x86", target_arch = "x86_64"), not(target_os = "windows")))'.dependencies] -sha1 = { version = "0.10.0", optional = true, features = ["asm"] } - [package.metadata.docs.rs] all-features = true features = ["document-features"] diff --git a/gix-features/src/hash.rs b/gix-features/src/hash.rs index eebc40fa075..719c57927c2 100644 --- a/gix-features/src/hash.rs +++ b/gix-features/src/hash.rs @@ -1,54 +1,12 @@ //! Hash functions and hash utilities -//! -//! With the `fast-sha1` feature, the `Sha1` hash type will use a more elaborate implementation utilizing hardware support -//! in case it is available. Otherwise the `rustsha1` feature should be set. `fast-sha1` will take precedence. -//! Otherwise, a minimal yet performant implementation is used instead for a decent trade-off between compile times and run-time performance. -#[cfg(all(feature = "rustsha1", not(feature = "fast-sha1")))] -mod _impl { - use super::Digest; - - /// A implementation of the Sha1 hash, which can be used once. - #[derive(Default, Clone)] - pub struct Sha1(sha1_smol::Sha1); - - impl Sha1 { - /// Digest the given `bytes`. - pub fn update(&mut self, bytes: &[u8]) { - self.0.update(bytes); - } - /// Finalize the hash and produce a digest. - pub fn digest(self) -> Digest { - self.0.digest().bytes() - } - } -} - -/// A hash-digest produced by a [`Hasher`] hash implementation. -#[cfg(any(feature = "fast-sha1", feature = "rustsha1"))] -pub type Digest = [u8; 20]; - -#[cfg(feature = "fast-sha1")] -mod _impl { - use sha1::Digest; - - /// A implementation of the Sha1 hash, which can be used once. - #[derive(Default, Clone)] - pub struct Sha1(sha1::Sha1); - - impl Sha1 { - /// Digest the given `bytes`. - pub fn update(&mut self, bytes: &[u8]) { - self.0.update(bytes); - } - /// Finalize the hash and produce a digest. - pub fn digest(self) -> super::Digest { - self.0.finalize().into() - } - } -} +// TODO: Remove this. #[cfg(any(feature = "rustsha1", feature = "fast-sha1"))] -pub use _impl::Sha1 as Hasher; +pub use gix_hash::hasher::{ + hasher, + io::{bytes, bytes_of_file, bytes_with_hasher, Write}, + Digest, Hasher, +}; /// Compute a CRC32 hash from the given `bytes`, returning the CRC32 hash. /// @@ -71,132 +29,3 @@ pub fn crc32(bytes: &[u8]) -> u32 { h.update(bytes); h.finalize() } - -/// Produce a hasher suitable for the given kind of hash. -#[cfg(any(feature = "rustsha1", feature = "fast-sha1"))] -pub fn hasher(kind: gix_hash::Kind) -> Hasher { - match kind { - gix_hash::Kind::Sha1 => Hasher::default(), - } -} - -/// Compute the hash of `kind` for the bytes in the file at `path`, hashing only the first `num_bytes_from_start` -/// while initializing and calling `progress`. -/// -/// `num_bytes_from_start` is useful to avoid reading trailing hashes, which are never part of the hash itself, -/// denoting the amount of bytes to hash starting from the beginning of the file. -/// -/// # Note -/// -/// * Only available with the `gix-object` feature enabled due to usage of the [`gix_hash::Kind`] enum and the -/// [`gix_hash::ObjectId`] return value. -/// * [Interrupts][crate::interrupt] are supported. -#[cfg(all(feature = "progress", any(feature = "rustsha1", feature = "fast-sha1")))] -pub fn bytes_of_file( - path: &std::path::Path, - num_bytes_from_start: u64, - kind: gix_hash::Kind, - progress: &mut dyn crate::progress::Progress, - should_interrupt: &std::sync::atomic::AtomicBool, -) -> std::io::Result { - bytes( - &mut std::fs::File::open(path)?, - num_bytes_from_start, - kind, - progress, - should_interrupt, - ) -} - -/// Similar to [`bytes_of_file`], but operates on a stream of bytes. -#[cfg(all(feature = "progress", any(feature = "rustsha1", feature = "fast-sha1")))] -pub fn bytes( - read: &mut dyn std::io::Read, - num_bytes_from_start: u64, - kind: gix_hash::Kind, - progress: &mut dyn crate::progress::Progress, - should_interrupt: &std::sync::atomic::AtomicBool, -) -> std::io::Result { - bytes_with_hasher(read, num_bytes_from_start, hasher(kind), progress, should_interrupt) -} - -/// Similar to [`bytes()`], but takes a `hasher` instead of a hash kind. -#[cfg(all(feature = "progress", any(feature = "rustsha1", feature = "fast-sha1")))] -pub fn bytes_with_hasher( - read: &mut dyn std::io::Read, - num_bytes_from_start: u64, - mut hasher: Hasher, - progress: &mut dyn crate::progress::Progress, - should_interrupt: &std::sync::atomic::AtomicBool, -) -> std::io::Result { - let start = std::time::Instant::now(); - // init progress before the possibility for failure, as convenience in case people want to recover - progress.init( - Some(num_bytes_from_start as prodash::progress::Step), - crate::progress::bytes(), - ); - - const BUF_SIZE: usize = u16::MAX as usize; - let mut buf = [0u8; BUF_SIZE]; - let mut bytes_left = num_bytes_from_start; - - while bytes_left > 0 { - let out = &mut buf[..BUF_SIZE.min(bytes_left as usize)]; - read.read_exact(out)?; - bytes_left -= out.len() as u64; - progress.inc_by(out.len()); - hasher.update(out); - if should_interrupt.load(std::sync::atomic::Ordering::SeqCst) { - return Err(std::io::Error::new(std::io::ErrorKind::Other, "Interrupted")); - } - } - - let id = gix_hash::ObjectId::from(hasher.digest()); - progress.show_throughput(start); - Ok(id) -} - -#[cfg(any(feature = "rustsha1", feature = "fast-sha1"))] -mod write { - use crate::hash::Hasher; - - /// A utility to automatically generate a hash while writing into an inner writer. - pub struct Write { - /// The hash implementation. - pub hash: Hasher, - /// The inner writer. - pub inner: T, - } - - impl std::io::Write for Write - where - T: std::io::Write, - { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - let written = self.inner.write(buf)?; - self.hash.update(&buf[..written]); - Ok(written) - } - - fn flush(&mut self) -> std::io::Result<()> { - self.inner.flush() - } - } - - impl Write - where - T: std::io::Write, - { - /// Create a new hash writer which hashes all bytes written to `inner` with a hash of `kind`. - pub fn new(inner: T, object_hash: gix_hash::Kind) -> Self { - match object_hash { - gix_hash::Kind::Sha1 => Write { - inner, - hash: Hasher::default(), - }, - } - } - } -} -#[cfg(any(feature = "rustsha1", feature = "fast-sha1"))] -pub use write::Write; diff --git a/gix-features/tests/hash.rs b/gix-features/tests/hash.rs deleted file mode 100644 index 7e7f6654dbd..00000000000 --- a/gix-features/tests/hash.rs +++ /dev/null @@ -1,16 +0,0 @@ -use gix_features::hash::Hasher; - -#[cfg(not(feature = "fast-sha1"))] -#[test] -fn size_of_sha1() { - assert_eq!(std::mem::size_of::(), 96); -} - -#[cfg(feature = "fast-sha1")] -#[test] -fn size_of_sha1() { - assert_eq!( - std::mem::size_of::(), - if cfg!(target_arch = "x86") { 96 } else { 104 } - ); -} diff --git a/gix-hash/Cargo.toml b/gix-hash/Cargo.toml index b037e9fa7c9..93bd885d6ef 100644 --- a/gix-hash/Cargo.toml +++ b/gix-hash/Cargo.toml @@ -16,13 +16,19 @@ doctest = false test = false [features] +# Temporary, to avoid a circular dependency on `gix-features`. +progress-unit-bytes = ["prodash/unit-bytes"] ## Data structures implement `serde::Serialize` and `serde::Deserialize`. serde = ["dep:serde"] [dependencies] +# Temporary, to avoid a circular dependency on `gix-features`. +prodash = "29.0.1" + thiserror = "2.0.0" faster-hex = { version = "0.9.0" } serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } +sha1-checked = { version = "0.10.0", default-features = false } document-features = { version = "0.2.0", optional = true } diff --git a/gix-hash/src/hasher/io.rs b/gix-hash/src/hasher/io.rs new file mode 100644 index 00000000000..ec582d9f9f1 --- /dev/null +++ b/gix-hash/src/hasher/io.rs @@ -0,0 +1,138 @@ +use crate::{hasher, Hasher}; + +// Temporary, to avoid a circular dependency on `gix-features`. +/// +mod gix_features { + /// + pub mod progress { + pub use prodash::{self, unit, Progress, Unit}; + + /// + #[cfg(feature = "progress-unit-bytes")] + pub fn bytes() -> Option { + Some(unit::dynamic_and_mode( + unit::Bytes, + unit::display::Mode::with_throughput().and_percentage(), + )) + } + + /// + #[cfg(not(feature = "progress-unit-bytes"))] + pub fn bytes() -> Option { + Some(unit::label_and_mode( + "B", + unit::display::Mode::with_throughput().and_percentage(), + )) + } + } +} + +/// Compute the hash of `kind` for the bytes in the file at `path`, hashing only the first `num_bytes_from_start` +/// while initializing and calling `progress`. +/// +/// `num_bytes_from_start` is useful to avoid reading trailing hashes, which are never part of the hash itself, +/// denoting the amount of bytes to hash starting from the beginning of the file. +/// +/// # Note +/// +/// * Interrupts are supported. +// TODO: Fix link to `gix_features::interrupt`. +pub fn bytes_of_file( + path: &std::path::Path, + num_bytes_from_start: u64, + kind: crate::Kind, + progress: &mut dyn gix_features::progress::Progress, + should_interrupt: &std::sync::atomic::AtomicBool, +) -> std::io::Result { + bytes( + &mut std::fs::File::open(path)?, + num_bytes_from_start, + kind, + progress, + should_interrupt, + ) +} + +/// Similar to [`bytes_of_file`], but operates on a stream of bytes. +pub fn bytes( + read: &mut dyn std::io::Read, + num_bytes_from_start: u64, + kind: crate::Kind, + progress: &mut dyn gix_features::progress::Progress, + should_interrupt: &std::sync::atomic::AtomicBool, +) -> std::io::Result { + bytes_with_hasher(read, num_bytes_from_start, hasher(kind), progress, should_interrupt) +} + +/// Similar to [`bytes()`], but takes a `hasher` instead of a hash kind. +pub fn bytes_with_hasher( + read: &mut dyn std::io::Read, + num_bytes_from_start: u64, + mut hasher: Hasher, + progress: &mut dyn gix_features::progress::Progress, + should_interrupt: &std::sync::atomic::AtomicBool, +) -> std::io::Result { + let start = std::time::Instant::now(); + // init progress before the possibility for failure, as convenience in case people want to recover + progress.init( + Some(num_bytes_from_start as gix_features::progress::prodash::progress::Step), + gix_features::progress::bytes(), + ); + + const BUF_SIZE: usize = u16::MAX as usize; + let mut buf = [0u8; BUF_SIZE]; + let mut bytes_left = num_bytes_from_start; + + while bytes_left > 0 { + let out = &mut buf[..BUF_SIZE.min(bytes_left as usize)]; + read.read_exact(out)?; + bytes_left -= out.len() as u64; + progress.inc_by(out.len()); + hasher.update(out); + if should_interrupt.load(std::sync::atomic::Ordering::SeqCst) { + return Err(std::io::Error::new(std::io::ErrorKind::Other, "Interrupted")); + } + } + + let id = crate::ObjectId::from(hasher.digest()); + progress.show_throughput(start); + Ok(id) +} + +/// A utility to automatically generate a hash while writing into an inner writer. +pub struct Write { + /// The hash implementation. + pub hash: Hasher, + /// The inner writer. + pub inner: T, +} + +impl std::io::Write for Write +where + T: std::io::Write, +{ + fn write(&mut self, buf: &[u8]) -> std::io::Result { + let written = self.inner.write(buf)?; + self.hash.update(&buf[..written]); + Ok(written) + } + + fn flush(&mut self) -> std::io::Result<()> { + self.inner.flush() + } +} + +impl Write +where + T: std::io::Write, +{ + /// Create a new hash writer which hashes all bytes written to `inner` with a hash of `kind`. + pub fn new(inner: T, object_hash: crate::Kind) -> Self { + match object_hash { + crate::Kind::Sha1 => Write { + inner, + hash: Hasher::default(), + }, + } + } +} diff --git a/gix-hash/src/hasher/mod.rs b/gix-hash/src/hasher/mod.rs new file mode 100644 index 00000000000..db4350b5cf4 --- /dev/null +++ b/gix-hash/src/hasher/mod.rs @@ -0,0 +1,90 @@ +use sha1_checked::CollisionResult; + +/// A hash-digest produced by a [`Hasher`] hash implementation. +pub type Digest = [u8; 20]; + +/// The error returned by [`Hasher::try_finalize()`]. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error("Detected SHA-1 collision attack with digest {digest}")] + CollisionAttack { digest: crate::ObjectId }, +} + +/// A implementation of the Sha1 hash, which can be used once. +/// +/// We use [`sha1_checked`] to implement the same collision detection +/// algorithm as Git. +#[derive(Clone)] +pub struct Hasher(sha1_checked::Sha1); + +impl Default for Hasher { + #[inline] + fn default() -> Self { + // This matches the configuration used by Git, which only uses + // the collision detection to bail out, rather than computing + // alternate “safe hashes” for inputs where a collision attack + // was detected. + Self(sha1_checked::Builder::default().safe_hash(false).build()) + } +} + +impl Hasher { + /// Digest the given `bytes`. + pub fn update(&mut self, bytes: &[u8]) { + use sha1_checked::Digest; + self.0.update(bytes); + } + + /// Finalize the hash and produce an object ID. + /// + /// Returns [`Error`] if a collision attack is detected. + #[inline] + pub fn try_finalize(self) -> Result { + match self.0.try_finalize() { + CollisionResult::Ok(digest) => Ok(crate::ObjectId::Sha1(digest.into())), + CollisionResult::Mitigated(_) => { + // SAFETY: `CollisionResult::Mitigated` is only + // returned when `safe_hash()` is on. `Hasher`’s field + // is private, and we only construct it in the + // `Default` instance, which turns `safe_hash()` off. + // + // As of Rust 1.84.1, the compiler can’t figure out + // this function cannot panic without this. + #[allow(unsafe_code)] + unsafe { + std::hint::unreachable_unchecked() + } + } + CollisionResult::Collision(digest) => Err(Error::CollisionAttack { + digest: crate::ObjectId::Sha1(digest.into()), + }), + } + } + + /// Finalize the hash and produce an object ID. + #[inline] + pub fn finalize(self) -> crate::ObjectId { + self.try_finalize().expect("Detected SHA-1 collision attack") + } + + /// Finalize the hash and produce a digest. + #[inline] + pub fn digest(self) -> Digest { + self.finalize() + .as_slice() + .try_into() + .expect("SHA-1 object ID to be 20 bytes long") + } +} + +/// Produce a hasher suitable for the given kind of hash. +#[inline] +pub fn hasher(kind: crate::Kind) -> Hasher { + match kind { + crate::Kind::Sha1 => Hasher::default(), + } +} + +/// Hashing utilities for I/O operations. +pub mod io; diff --git a/gix-hash/src/lib.rs b/gix-hash/src/lib.rs index 20cb9c4327e..924681d0607 100644 --- a/gix-hash/src/lib.rs +++ b/gix-hash/src/lib.rs @@ -13,6 +13,11 @@ mod borrowed; pub use borrowed::{oid, Error}; +/// Hash functions and hash utilities +pub mod hasher; +pub use hasher::io::{bytes, bytes_of_file, bytes_with_hasher}; +pub use hasher::{hasher, Hasher}; + mod object_id; pub use object_id::{decode, ObjectId}; diff --git a/gix-hash/tests/hash.rs b/gix-hash/tests/hash.rs index 9cdd947657e..766b53619b7 100644 --- a/gix-hash/tests/hash.rs +++ b/gix-hash/tests/hash.rs @@ -1,5 +1,6 @@ use gix_hash::ObjectId; +mod hasher; mod kind; mod object_id; mod oid; diff --git a/gix-hash/tests/hasher/mod.rs b/gix-hash/tests/hasher/mod.rs new file mode 100644 index 00000000000..1dd100ffa74 --- /dev/null +++ b/gix-hash/tests/hasher/mod.rs @@ -0,0 +1,9 @@ +use gix_hash::Hasher; + +#[test] +fn size_of_sha1() { + assert_eq!( + std::mem::size_of::(), + if cfg!(target_arch = "x86") { 820 } else { 824 }, + ); +} diff --git a/gix-hash/tests/object_id/mod.rs b/gix-hash/tests/object_id/mod.rs index c2539100239..a91aef4b2bc 100644 --- a/gix-hash/tests/object_id/mod.rs +++ b/gix-hash/tests/object_id/mod.rs @@ -45,23 +45,28 @@ mod from_hex { mod sha1 { use std::str::FromStr as _; - use gix_features::hash::hasher; - use gix_hash::{Kind, ObjectId}; + use gix_hash::{hasher, Kind, ObjectId}; - fn hash_contents(s: &[u8]) -> ObjectId { + fn hash_contents(s: &[u8]) -> Result { let mut hasher = hasher(Kind::Sha1); hasher.update(s); - ObjectId::Sha1(hasher.digest()) + hasher.try_finalize() } #[test] fn empty_blob() { - assert_eq!(ObjectId::empty_blob(Kind::Sha1), hash_contents(b"blob 0\0")); + assert_eq!( + ObjectId::empty_blob(Kind::Sha1), + hash_contents(b"blob 0\0").expect("empty blob to not collide"), + ); } #[test] fn empty_tree() { - assert_eq!(ObjectId::empty_tree(Kind::Sha1), hash_contents(b"tree 0\0")); + assert_eq!( + ObjectId::empty_tree(Kind::Sha1), + hash_contents(b"tree 0\0").expect("empty tree to not collide"), + ); } /// Check the test vectors from RFC 3174. @@ -84,7 +89,7 @@ mod sha1 { ]; for (input, output) in fixtures { assert_eq!( - hash_contents(input), + hash_contents(input).expect("RFC inputs to not collide"), ObjectId::from_str(&output.to_lowercase().replace(' ', "")).expect("RFC digests to be valid"), ); } @@ -101,10 +106,17 @@ mod sha1 { let message_b = include_bytes!("../fixtures/shambles/messageB"); assert_ne!(message_a, message_b); - // BUG: These should be detected as a collision attack. let expected = ObjectId::from_str("8ac60ba76f1999a1ab70223f225aefdc78d4ddc0").expect("Shambles digest to be valid"); - assert_eq!(hash_contents(message_a), expected); - assert_eq!(hash_contents(message_b), expected); + + let Err(hasher::Error::CollisionAttack { digest }) = hash_contents(message_a) else { + panic!("expected Shambles input to collide"); + }; + assert_eq!(digest, expected); + + let Err(hasher::Error::CollisionAttack { digest }) = hash_contents(message_b) else { + panic!("expected Shambles input to collide"); + }; + assert_eq!(digest, expected); } } From baa1430aed0dc8160a71cc675e2626780a2de052 Mon Sep 17 00:00:00 2001 From: Emily Date: Tue, 1 Apr 2025 23:33:32 +0100 Subject: [PATCH 05/22] migrate all hashing API users to `gix_hash` --- Cargo.lock | 2 -- gix-commitgraph/Cargo.toml | 1 - gix-commitgraph/src/file/verify.rs | 4 ++-- gix-hash/Cargo.toml | 1 - gix-index/Cargo.toml | 1 - gix-index/src/extension/end_of_index_entry/decode.rs | 2 +- gix-index/src/extension/end_of_index_entry/write.rs | 2 +- gix-index/src/file/init.rs | 2 +- gix-index/src/file/verify.rs | 2 +- gix-index/src/file/write.rs | 4 ++-- gix-index/tests/Cargo.toml | 2 +- gix-object/Cargo.toml | 1 - gix-object/src/lib.rs | 6 +++--- gix-odb/Cargo.toml | 2 +- gix-odb/src/sink.rs | 2 +- gix-odb/src/store_impls/loose/write.rs | 9 +++++---- gix-pack/Cargo.toml | 2 +- gix-pack/src/data/input/bytes_to_entries.rs | 6 +++--- gix-pack/src/data/input/entries_to_bytes.rs | 4 +--- gix-pack/src/data/output/bytes.rs | 6 +++--- gix-pack/src/index/encode.rs | 8 +++----- gix-pack/src/index/write/mod.rs | 2 +- gix-pack/src/multi_index/write.rs | 3 ++- gix-pack/src/verify.rs | 4 ++-- 24 files changed, 35 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 54982eb8e4d..dfebd88d57f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1615,7 +1615,6 @@ dependencies = [ "document-features", "gix-chunk 0.4.11", "gix-date 0.9.3", - "gix-features 0.40.0", "gix-hash 0.16.0", "gix-testtools", "memmap2", @@ -1962,7 +1961,6 @@ version = "0.16.0" dependencies = [ "document-features", "faster-hex", - "gix-features 0.40.0", "gix-testtools", "prodash 29.0.1", "serde", diff --git a/gix-commitgraph/Cargo.toml b/gix-commitgraph/Cargo.toml index ec5ca164c96..585dbf1b415 100644 --- a/gix-commitgraph/Cargo.toml +++ b/gix-commitgraph/Cargo.toml @@ -20,7 +20,6 @@ doctest = false serde = ["dep:serde", "gix-hash/serde", "bstr/serde"] [dependencies] -gix-features = { version = "^0.40.0", path = "../gix-features", features = ["rustsha1"] } gix-hash = { version = "^0.16.0", path = "../gix-hash" } gix-chunk = { version = "^0.4.11", path = "../gix-chunk" } diff --git a/gix-commitgraph/src/file/verify.rs b/gix-commitgraph/src/file/verify.rs index 5bff7309504..df7be631862 100644 --- a/gix-commitgraph/src/file/verify.rs +++ b/gix-commitgraph/src/file/verify.rs @@ -140,12 +140,12 @@ impl File { /// /// Return the actual checksum on success or `(actual checksum, expected checksum)` if there is a mismatch. pub fn verify_checksum(&self) -> Result { - // Even though we could use gix_features::hash::bytes_of_file(…), this would require using our own + // Even though we could use gix_hash::bytes_of_file(…), this would require using our own // Error type to support io::Error and Mismatch. 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. // But it's possible, once a progress instance is passed. let data_len_without_trailer = self.data.len() - self.hash_len; - let mut hasher = gix_features::hash::hasher(self.object_hash()); + let mut hasher = gix_hash::hasher(self.object_hash()); hasher.update(&self.data[..data_len_without_trailer]); let actual = gix_hash::ObjectId::from_bytes_or_panic(hasher.digest().as_ref()); diff --git a/gix-hash/Cargo.toml b/gix-hash/Cargo.toml index 93bd885d6ef..8b86e88efa0 100644 --- a/gix-hash/Cargo.toml +++ b/gix-hash/Cargo.toml @@ -34,7 +34,6 @@ document-features = { version = "0.2.0", optional = true } [dev-dependencies] gix-testtools = { path = "../tests/tools" } -gix-features = { path = "../gix-features", features = ["rustsha1"] } [package.metadata.docs.rs] all-features = true diff --git a/gix-index/Cargo.toml b/gix-index/Cargo.toml index 060e86de52b..0d6753592e0 100644 --- a/gix-index/Cargo.toml +++ b/gix-index/Cargo.toml @@ -23,7 +23,6 @@ serde = ["dep:serde", "smallvec/serde", "gix-hash/serde"] [dependencies] gix-features = { version = "^0.40.0", path = "../gix-features", features = [ - "rustsha1", "progress", ] } gix-hash = { version = "^0.16.0", path = "../gix-hash" } diff --git a/gix-index/src/extension/end_of_index_entry/decode.rs b/gix-index/src/extension/end_of_index_entry/decode.rs index 4acc0be84bb..b923e20224d 100644 --- a/gix-index/src/extension/end_of_index_entry/decode.rs +++ b/gix-index/src/extension/end_of_index_entry/decode.rs @@ -33,7 +33,7 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option { return None; } - let mut hasher = gix_features::hash::hasher(gix_hash::Kind::Sha1); + let mut hasher = gix_hash::hasher(gix_hash::Kind::Sha1); let mut last_chunk = None; for (signature, chunk) in extension::Iter::new(&data[offset..data.len() - MIN_SIZE_WITH_HEADER - hash_len]) { hasher.update(&signature); diff --git a/gix-index/src/extension/end_of_index_entry/write.rs b/gix-index/src/extension/end_of_index_entry/write.rs index 6752bd23866..13e91a13063 100644 --- a/gix-index/src/extension/end_of_index_entry/write.rs +++ b/gix-index/src/extension/end_of_index_entry/write.rs @@ -18,7 +18,7 @@ pub fn write_to( out.write_all(&offset_to_extensions.to_be_bytes())?; - let mut hasher = gix_features::hash::hasher(hash_kind); + let mut hasher = gix_hash::hasher(hash_kind); for (signature, size) in prior_extensions { hasher.update(&signature); hasher.update(&size.to_be_bytes()); diff --git a/gix-index/src/file/init.rs b/gix-index/src/file/init.rs index 10c01015fcb..7ba91cbb4a2 100644 --- a/gix-index/src/file/init.rs +++ b/gix-index/src/file/init.rs @@ -75,7 +75,7 @@ impl File { let _span = gix_features::trace::detail!("gix::open_index::hash_index", path = ?path); let meta = file.metadata()?; let num_bytes_to_hash = meta.len() - object_hash.len_in_bytes() as u64; - let actual_hash = gix_features::hash::bytes( + let actual_hash = gix_hash::bytes( &mut file, num_bytes_to_hash, object_hash, diff --git a/gix-index/src/file/verify.rs b/gix-index/src/file/verify.rs index 1c0dc7539b6..9d49d579ea8 100644 --- a/gix-index/src/file/verify.rs +++ b/gix-index/src/file/verify.rs @@ -25,7 +25,7 @@ impl File { if let Some(checksum) = self.checksum { let num_bytes_to_hash = self.path.metadata()?.len() - checksum.as_bytes().len() as u64; let should_interrupt = AtomicBool::new(false); - let actual = gix_features::hash::bytes_of_file( + let actual = gix_hash::bytes_of_file( &self.path, num_bytes_to_hash, checksum.kind(), diff --git a/gix-index/src/file/write.rs b/gix-index/src/file/write.rs index 0623f6c59c3..0f6941b0625 100644 --- a/gix-index/src/file/write.rs +++ b/gix-index/src/file/write.rs @@ -1,4 +1,4 @@ -use gix_features::hash; +use gix_hash::hasher; use crate::{write, File, Version}; @@ -28,7 +28,7 @@ impl File { let version = self.state.write_to(out, options)?; (version, self.state.object_hash.null()) } else { - let mut hasher = hash::Write::new(&mut out, self.state.object_hash); + let mut hasher = hasher::io::Write::new(&mut out, self.state.object_hash); let out: &mut dyn std::io::Write = &mut hasher; let version = self.state.write_to(out, options)?; (version, gix_hash::ObjectId::from(hasher.hash.digest())) diff --git a/gix-index/tests/Cargo.toml b/gix-index/tests/Cargo.toml index 8d707b1ed80..85356da7dd2 100644 --- a/gix-index/tests/Cargo.toml +++ b/gix-index/tests/Cargo.toml @@ -20,7 +20,7 @@ gix-features-parallel = ["gix-features/parallel"] [dev-dependencies] gix-index = { path = ".." } -gix-features = { path = "../../gix-features", features = ["rustsha1", "progress"] } +gix-features = { path = "../../gix-features", features = ["progress"] } gix-testtools = { path = "../../tests/tools" } gix-odb = { path = "../../gix-odb" } gix-object = { path = "../../gix-object" } diff --git a/gix-object/Cargo.toml b/gix-object/Cargo.toml index f05f05fc592..e6713b15a6d 100644 --- a/gix-object/Cargo.toml +++ b/gix-object/Cargo.toml @@ -42,7 +42,6 @@ verbose-object-parsing-errors = ["winnow/std"] [dependencies] gix-features = { version = "^0.40.0", path = "../gix-features", features = [ - "rustsha1", "progress", ] } gix-hash = { version = "^0.16.0", path = "../gix-hash" } diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index 22706b041b1..e851f08c9be 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -398,8 +398,8 @@ pub mod decode { } } -fn object_hasher(hash_kind: gix_hash::Kind, object_kind: Kind, object_size: u64) -> gix_features::hash::Hasher { - let mut hasher = gix_features::hash::hasher(hash_kind); +fn object_hasher(hash_kind: gix_hash::Kind, object_kind: Kind, object_size: u64) -> gix_hash::Hasher { + let mut hasher = gix_hash::hasher(hash_kind); hasher.update(&encode::loose_header(object_kind, object_size)); hasher } @@ -426,5 +426,5 @@ pub fn compute_stream_hash( should_interrupt: &std::sync::atomic::AtomicBool, ) -> std::io::Result { let hasher = object_hasher(hash_kind, object_kind, stream_len); - gix_features::hash::bytes_with_hasher(stream, stream_len, hasher, progress, should_interrupt) + gix_hash::bytes_with_hasher(stream, stream_len, hasher, progress, should_interrupt) } diff --git a/gix-odb/Cargo.toml b/gix-odb/Cargo.toml index 6215d48ed8c..70ac0492509 100644 --- a/gix-odb/Cargo.toml +++ b/gix-odb/Cargo.toml @@ -20,7 +20,7 @@ doctest = false serde = ["dep:serde", "gix-hash/serde", "gix-object/serde", "gix-pack/serde"] [dependencies] -gix-features = { version = "^0.40.0", path = "../gix-features", features = ["rustsha1", "walkdir", "zlib", "crc32"] } +gix-features = { version = "^0.40.0", path = "../gix-features", features = ["walkdir", "zlib", "crc32"] } gix-hashtable = { version = "^0.7.0", path = "../gix-hashtable" } gix-hash = { version = "^0.16.0", path = "../gix-hash" } gix-date = { version = "^0.9.3", path = "../gix-date" } diff --git a/gix-odb/src/sink.rs b/gix-odb/src/sink.rs index 46077ff37cf..dff75bacb8d 100644 --- a/gix-odb/src/sink.rs +++ b/gix-odb/src/sink.rs @@ -36,7 +36,7 @@ impl gix_object::Write for Sink { Ok(()) }; - let mut hasher = gix_features::hash::hasher(self.object_hash); + let mut hasher = gix_hash::hasher(self.object_hash); hasher.update(&header); possibly_compress(&header).map_err(Box::new)?; diff --git a/gix-odb/src/store_impls/loose/write.rs b/gix-odb/src/store_impls/loose/write.rs index bdef0b90005..d969eec261c 100644 --- a/gix-odb/src/store_impls/loose/write.rs +++ b/gix-odb/src/store_impls/loose/write.rs @@ -1,6 +1,7 @@ use std::{fs, io, io::Write, path::PathBuf}; -use gix_features::{hash, zlib::stream::deflate}; +use gix_features::zlib::stream::deflate; +use gix_hash::hasher; use gix_object::WriteTo; use tempfile::NamedTempFile; @@ -106,7 +107,7 @@ impl Store { } impl Store { - fn dest(&self) -> Result, Error> { + fn dest(&self) -> Result, Error> { #[cfg_attr(not(unix), allow(unused_mut))] let mut builder = tempfile::Builder::new(); #[cfg(unix)] @@ -115,7 +116,7 @@ impl Store { let perms = std::fs::Permissions::from_mode(0o444); builder.permissions(perms); } - Ok(hash::Write::new( + Ok(hasher::io::Write::new( deflate::Write::new(builder.tempfile_in(&self.path).map_err(|err| Error::Io { source: err, message: "create named temp file in", @@ -127,7 +128,7 @@ impl Store { fn finalize_object( &self, - hash::Write { hash, inner: file }: hash::Write, + hasher::io::Write { hash, inner: file }: hasher::io::Write, ) -> Result { let id = gix_hash::ObjectId::from(hash.digest()); let object_path = loose::hash_path(&id, self.path.clone()); diff --git a/gix-pack/Cargo.toml b/gix-pack/Cargo.toml index 2552f95f742..e6449a3e64e 100644 --- a/gix-pack/Cargo.toml +++ b/gix-pack/Cargo.toml @@ -34,7 +34,7 @@ serde = ["dep:serde", "gix-object/serde"] wasm = ["gix-diff?/wasm"] [dependencies] -gix-features = { version = "^0.40.0", path = "../gix-features", features = ["crc32", "rustsha1", "progress", "zlib"] } +gix-features = { version = "^0.40.0", path = "../gix-features", features = ["crc32", "progress", "zlib"] } gix-path = { version = "^0.10.14", path = "../gix-path" } gix-hash = { version = "^0.16.0", path = "../gix-hash" } gix-chunk = { version = "^0.4.11", path = "../gix-chunk" } diff --git a/gix-pack/src/data/input/bytes_to_entries.rs b/gix-pack/src/data/input/bytes_to_entries.rs index f8f7aa926bd..b08f26a5415 100644 --- a/gix-pack/src/data/input/bytes_to_entries.rs +++ b/gix-pack/src/data/input/bytes_to_entries.rs @@ -1,7 +1,7 @@ use std::{fs, io}; -use gix_features::{hash::Hasher, zlib::Decompress}; -use gix_hash::ObjectId; +use gix_features::zlib::Decompress; +use gix_hash::{Hasher, ObjectId}; use crate::data::input; @@ -69,7 +69,7 @@ where version, objects_left: num_objects, hash: (mode != input::Mode::AsIs).then(|| { - let mut hash = gix_features::hash::hasher(object_hash); + let mut hash = gix_hash::hasher(object_hash); hash.update(&header_data); hash }), diff --git a/gix-pack/src/data/input/entries_to_bytes.rs b/gix-pack/src/data/input/entries_to_bytes.rs index dc2ac58848a..d2e0d0f3ee1 100644 --- a/gix-pack/src/data/input/entries_to_bytes.rs +++ b/gix-pack/src/data/input/entries_to_bytes.rs @@ -1,7 +1,5 @@ use std::iter::Peekable; -use gix_features::hash; - use crate::data::input; /// An implementation of [`Iterator`] to write [encoded entries][input::Entry] to an inner implementation each time @@ -95,7 +93,7 @@ where self.output.rewind()?; let interrupt_never = std::sync::atomic::AtomicBool::new(false); - let digest = hash::bytes( + let digest = gix_hash::bytes( &mut self.output, num_bytes_written, self.object_hash, diff --git a/gix-pack/src/data/output/bytes.rs b/gix-pack/src/data/output/bytes.rs index 849da6af6cf..195c41790b9 100644 --- a/gix-pack/src/data/output/bytes.rs +++ b/gix-pack/src/data/output/bytes.rs @@ -1,6 +1,6 @@ use std::io::Write; -use gix_features::hash; +use gix_hash::hasher; use crate::data::output; use crate::exact_vec; @@ -24,7 +24,7 @@ pub struct FromEntriesIter { /// An iterator for input [`output::Entry`] instances pub input: I, /// A way of writing encoded bytes. - output: hash::Write, + output: hasher::io::Write, /// Our trailing hash when done writing all input entries trailer: Option, /// The amount of objects in the iteration and the version of the packfile to be written. @@ -71,7 +71,7 @@ where ); FromEntriesIter { input, - output: hash::Write::new(output, object_hash), + output: hasher::io::Write::new(output, object_hash), trailer: None, entry_version: version, pack_offsets_and_validity: exact_vec(num_entries as usize), diff --git a/gix-pack/src/index/encode.rs b/gix-pack/src/index/encode.rs index 61c6f6de77a..ef436a4f022 100644 --- a/gix-pack/src/index/encode.rs +++ b/gix-pack/src/index/encode.rs @@ -36,10 +36,8 @@ pub(crate) fn fanout(iter: &mut dyn ExactSizeIterator) -> [u32; 256] mod function { use std::io; - use gix_features::{ - hash, - progress::{self, DynNestedProgress}, - }; + use gix_features::progress::{self, DynNestedProgress}; + use gix_hash::hasher; use super::{fanout, HIGH_BIT, LARGE_OFFSET_THRESHOLD}; use crate::index::V2_SIGNATURE; @@ -87,7 +85,7 @@ mod function { // Write header let mut out = Count::new(std::io::BufWriter::with_capacity( 8 * 4096, - hash::Write::new(out, kind.hash()), + hasher::io::Write::new(out, kind.hash()), )); out.write_all(V2_SIGNATURE)?; out.write_all(&(kind as u32).to_be_bytes())?; diff --git a/gix-pack/src/index/write/mod.rs b/gix-pack/src/index/write/mod.rs index 0f07bf7a932..d77e4c00617 100644 --- a/gix-pack/src/index/write/mod.rs +++ b/gix-pack/src/index/write/mod.rs @@ -225,7 +225,7 @@ impl crate::index::File { Some(ph) => ph, None if num_objects == 0 => { let header = crate::data::header::encode(pack_version, 0); - let mut hasher = gix_features::hash::hasher(object_hash); + let mut hasher = gix_hash::hasher(object_hash); hasher.update(&header); gix_hash::ObjectId::from(hasher.digest()) } diff --git a/gix-pack/src/multi_index/write.rs b/gix-pack/src/multi_index/write.rs index 8054f3571d6..67a0a917b1c 100644 --- a/gix-pack/src/multi_index/write.rs +++ b/gix-pack/src/multi_index/write.rs @@ -5,6 +5,7 @@ use std::{ }; use gix_features::progress::{Count, DynNestedProgress, Progress}; +use gix_hash::hasher; use crate::multi_index; @@ -83,7 +84,7 @@ impl multi_index::File { should_interrupt: &AtomicBool, Options { object_hash }: Options, ) -> Result { - let out = gix_features::hash::Write::new(out, object_hash); + let out = hasher::io::Write::new(out, object_hash); let (index_paths_sorted, index_filenames_sorted) = { index_paths.sort(); let file_names = index_paths diff --git a/gix-pack/src/verify.rs b/gix-pack/src/verify.rs index fff99c75d12..9656c255c23 100644 --- a/gix-pack/src/verify.rs +++ b/gix-pack/src/verify.rs @@ -37,7 +37,7 @@ pub fn checksum_on_disk_or_mmap( should_interrupt: &AtomicBool, ) -> Result { let data_len_without_trailer = data.len() - object_hash.len_in_bytes(); - let actual = match gix_features::hash::bytes_of_file( + let actual = match gix_hash::bytes_of_file( data_path, data_len_without_trailer as u64, object_hash, @@ -48,7 +48,7 @@ pub fn checksum_on_disk_or_mmap( Err(err) if err.kind() == std::io::ErrorKind::Interrupted => return Err(checksum::Error::Interrupted), Err(_io_err) => { let start = std::time::Instant::now(); - let mut hasher = gix_features::hash::hasher(object_hash); + let mut hasher = gix_hash::hasher(object_hash); hasher.update(&data[..data_len_without_trailer]); progress.inc_by(data_len_without_trailer); progress.show_throughput(start); From e4439aa9a6969e14b7a03bea6b0b771534510edd Mon Sep 17 00:00:00 2001 From: Emily Date: Tue, 1 Apr 2025 23:41:24 +0100 Subject: [PATCH 06/22] change!: move hashing API to `gix_hash` --- Cargo.lock | 3 +-- gix-features/Cargo.toml | 3 +-- gix-features/src/hash.rs | 8 -------- gix-hash/Cargo.toml | 5 +---- gix-hash/src/hasher/io.rs | 30 +----------------------------- 5 files changed, 4 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dfebd88d57f..0649859d58b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1845,7 +1845,6 @@ dependencies = [ "crossbeam-channel", "document-features", "flate2", - "gix-hash 0.16.0", "gix-trace 0.1.12", "gix-utils 0.1.14", "libc", @@ -1961,8 +1960,8 @@ version = "0.16.0" dependencies = [ "document-features", "faster-hex", + "gix-features 0.40.0", "gix-testtools", - "prodash 29.0.1", "serde", "sha1-checked", "thiserror 2.0.12", diff --git a/gix-features/Cargo.toml b/gix-features/Cargo.toml index 4eb7e0bae97..468ff326113 100644 --- a/gix-features/Cargo.toml +++ b/gix-features/Cargo.toml @@ -24,7 +24,7 @@ progress = ["prodash"] ## Provide human-readable numbers as well as easier to read byte units for progress bars. progress-unit-human-numbers = ["prodash?/unit-human"] ## Provide human readable byte units for progress bars. -progress-unit-bytes = ["dep:bytesize", "prodash?/unit-bytes", "gix-hash/progress-unit-bytes"] +progress-unit-bytes = ["dep:bytesize", "prodash?/unit-bytes"] ## Provide utilities suitable for working with the `std::fs::read_dir()`. fs-read-dir = ["dep:gix-utils"] @@ -107,7 +107,6 @@ path = "tests/pipe.rs" required-features = ["io-pipe"] [dependencies] -gix-hash = { version = "^0.16.0", path = "../gix-hash" } gix-trace = { version = "^0.1.12", path = "../gix-trace" } # for walkdir diff --git a/gix-features/src/hash.rs b/gix-features/src/hash.rs index 719c57927c2..24e7b36881b 100644 --- a/gix-features/src/hash.rs +++ b/gix-features/src/hash.rs @@ -1,13 +1,5 @@ //! Hash functions and hash utilities -// TODO: Remove this. -#[cfg(any(feature = "rustsha1", feature = "fast-sha1"))] -pub use gix_hash::hasher::{ - hasher, - io::{bytes, bytes_of_file, bytes_with_hasher, Write}, - Digest, Hasher, -}; - /// Compute a CRC32 hash from the given `bytes`, returning the CRC32 hash. /// /// When calling this function for the first time, `previous_value` should be `0`. Otherwise it diff --git a/gix-hash/Cargo.toml b/gix-hash/Cargo.toml index 8b86e88efa0..94a1cd5ad59 100644 --- a/gix-hash/Cargo.toml +++ b/gix-hash/Cargo.toml @@ -16,14 +16,11 @@ doctest = false test = false [features] -# Temporary, to avoid a circular dependency on `gix-features`. -progress-unit-bytes = ["prodash/unit-bytes"] ## Data structures implement `serde::Serialize` and `serde::Deserialize`. serde = ["dep:serde"] [dependencies] -# Temporary, to avoid a circular dependency on `gix-features`. -prodash = "29.0.1" +gix-features = { version = "^0.40.0", path = "../gix-features", features = ["progress"] } thiserror = "2.0.0" faster-hex = { version = "0.9.0" } diff --git a/gix-hash/src/hasher/io.rs b/gix-hash/src/hasher/io.rs index ec582d9f9f1..4634dc1b8d0 100644 --- a/gix-hash/src/hasher/io.rs +++ b/gix-hash/src/hasher/io.rs @@ -1,32 +1,5 @@ use crate::{hasher, Hasher}; -// Temporary, to avoid a circular dependency on `gix-features`. -/// -mod gix_features { - /// - pub mod progress { - pub use prodash::{self, unit, Progress, Unit}; - - /// - #[cfg(feature = "progress-unit-bytes")] - pub fn bytes() -> Option { - Some(unit::dynamic_and_mode( - unit::Bytes, - unit::display::Mode::with_throughput().and_percentage(), - )) - } - - /// - #[cfg(not(feature = "progress-unit-bytes"))] - pub fn bytes() -> Option { - Some(unit::label_and_mode( - "B", - unit::display::Mode::with_throughput().and_percentage(), - )) - } - } -} - /// Compute the hash of `kind` for the bytes in the file at `path`, hashing only the first `num_bytes_from_start` /// while initializing and calling `progress`. /// @@ -35,8 +8,7 @@ mod gix_features { /// /// # Note /// -/// * Interrupts are supported. -// TODO: Fix link to `gix_features::interrupt`. +/// * [Interrupts][gix_features::interrupt] are supported. pub fn bytes_of_file( path: &std::path::Path, num_bytes_from_start: u64, From fd12ef89af29bf0684fc1df3e7b76ff367dee994 Mon Sep 17 00:00:00 2001 From: Emily Date: Sat, 22 Mar 2025 19:26:35 +0000 Subject: [PATCH 07/22] =?UTF-8?q?change!:=20drop=20obsolete=20SHA=E2=80=90?= =?UTF-8?q?1=20features?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hashing API has moved to `gix_hash::hasher`, and we now use `sha1-checked` unconditionally. --- .github/workflows/ci.yml | 2 +- .github/workflows/release.yml | 3 +-- Cargo.toml | 17 +++++++---------- crate-status.md | 2 -- gix-features/Cargo.toml | 4 ---- gix/Cargo.toml | 8 ++------ justfile | 2 -- 7 files changed, 11 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 21db6b6374e..26d4755da19 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -391,7 +391,7 @@ jobs: - name: features of gix-features run: | set +x - for feature in progress parallel io-pipe crc32 zlib zlib-rust-backend fast-sha1 rustsha1 cache-efficiency-debug; do + for feature in progress parallel io-pipe crc32 zlib zlib-rust-backend cache-efficiency-debug; do (cd gix-features && cargo build --features "$feature" --target "$TARGET") done - name: crates with 'wasm' feature diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8de670112b9..0a2801a6e78 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -137,8 +137,7 @@ jobs: os: windows-latest - target: aarch64-pc-windows-msvc os: windows-latest - # on linux we build with musl which causes trouble with open-ssl. For now, just build max-pure there - # even though we could also build with `--features max-control,http-client-reqwest,gitoxide-core-blocking-client,gix-features/fast-sha1` for fast hashing. + # on linux we build with musl which causes trouble with open-ssl. For now, just build max-pure there. # It's a TODO. exclude: - target: x86_64-unknown-linux-musl diff --git a/Cargo.toml b/Cargo.toml index eb4c9991729..e367ddebcf0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ max = ["max-control", "fast", "gitoxide-core-tools-query", "gitoxide-core-tools- ## transports as it uses Rust's HTTP implementation. ## ## As fast as possible, with TUI progress, progress line rendering with auto-configuration, all transports available but less mature pure Rust HTTP implementation, all `ein` tools, CLI colors and local-time support, JSON output, regex support for rev-specs. -max-pure = ["max-control", "gix-features/rustsha1", "gix-features/zlib-rust-backend", "http-client-reqwest", "gitoxide-core-blocking-client"] +max-pure = ["max-control", "gix-features/zlib-rust-backend", "http-client-reqwest", "gitoxide-core-blocking-client"] ## Like `max`, but with more control for configuration. See the *Package Maintainers* headline for more information. max-control = ["tracing", "fast-safe", "pretty-cli", "gitoxide-core-tools", "prodash-render-line", "prodash-render-tui", "prodash/render-line-autoconfigure", "gix/revparse-regex"] @@ -60,7 +60,7 @@ lean = ["fast", "tracing", "pretty-cli", "http-client-curl", "gitoxide-core-tool ## This build is essentially limited to local operations without any fanciness. ## ## Optimized for size, no parallelism thus much slower, progress line rendering. -small = ["pretty-cli", "gix-features/rustsha1", "gix-features/zlib-rust-backend", "prodash-render-line", "is-terminal"] +small = ["pretty-cli", "gix-features/zlib-rust-backend", "prodash-render-line", "is-terminal"] ## Like lean, but uses Rusts async implementations for networking. ## @@ -74,7 +74,7 @@ small = ["pretty-cli", "gix-features/rustsha1", "gix-features/zlib-rust-backend" lean-async = ["fast", "tracing", "pretty-cli", "gitoxide-core-tools", "gitoxide-core-tools-query", "gitoxide-core-tools-corpus", "gitoxide-core-async-client", "prodash-render-line"] #! ### Package Maintainers -#! `*-control` features leave it to you to configure C libraries, involving choices for `zlib`, ! hashing and transport implementation. +#! `*-control` features leave it to you to configure C libraries, involving choices for `zlib` and transport implementation. #! #! Additional features *can* be provided with `--features` and are handled by the [`gix-features` crate](https://docs.rs/gix-features/latest). #! If nothing else is specified, the Rust implementation is used. ! Note that only one feature of each section can be enabled at a time. @@ -84,28 +84,25 @@ lean-async = ["fast", "tracing", "pretty-cli", "gitoxide-core-tools", "gitoxide- #! - `gix-features/zlib-ng-compat` #! - `gix-features/zlib-stock` #! - `gix-features/zlib-rust-backend` (*default if no choice is made*) -#! * **sha1** -#! - `gix-features/fast-sha1` -#! - `gix-features/rustsha1` (*default if no choice is made*) #! * **HTTP** - see the *Building Blocks for mutually exclusive networking* headline #! #! #### Examples #! #! * `cargo build --release --no-default-features --features max-control,gix-features/zlib-stock,gitoxide-core-blocking-client,http-client-curl` #! - Create a build just like `max`, but using the stock `zlib` library instead of `zlib-ng` -#! * `cargo build --release --no-default-features --features max-control,http-client-reqwest,gitoxide-core-blocking-client,gix-features/fast-sha1` -#! - Create a build just like `max-pure`, but with faster hashing due to `fast-sha1`. +#! * `cargo build --release --no-default-features --features max-control,http-client-reqwest,gitoxide-core-blocking-client,gix-features/zlib-ng` +#! - Create a build just like `max-pure`, but with faster compression due to `zlib-ng`. #! ### Building Blocks #! Typical combinations of features of our dependencies, some of which are referred to in the `gitoxide` crate's code for conditional compilation. ## Makes the crate execute as fast as possible by supporting parallel computation of otherwise long-running functions -## as well as fast, hardware accelerated hashing, along with a faster zlib backend. +## as well as a faster zlib backend. ## If disabled, the binary will be visibly smaller. fast = ["gix/max-performance", "gix/comfort"] ## Makes the crate execute as fast as possible by supporting parallel computation of otherwise long-running functions -## as well as fast, hardware accelerated hashing, along with a faster zlib backend. +## as well as a faster zlib backend. ## If disabled, the binary will be visibly smaller. fast-safe = ["gix/max-performance-safe", "gix/comfort"] diff --git a/crate-status.md b/crate-status.md index 3274407dd48..f5081e45826 100644 --- a/crate-status.md +++ b/crate-status.md @@ -894,8 +894,6 @@ See its [README.md](https://github.com/GitoxideLabs/gitoxide/blob/main/gix-lock/ * `in_parallel` * `join` * _When off all functions execute serially_ -* **fast-sha1** - * provides a faster SHA1 implementation using CPU intrinsics * [x] API documentation ### gix-tui diff --git a/gix-features/Cargo.toml b/gix-features/Cargo.toml index 468ff326113..ae089a62f9b 100644 --- a/gix-features/Cargo.toml +++ b/gix-features/Cargo.toml @@ -77,10 +77,6 @@ zlib-stock = ["zlib", "flate2?/zlib"] ## may build in environments where other backends don't. zlib-rust-backend = ["zlib", "flate2?/rust_backend"] -# TODO: Remove these. -fast-sha1 = [] -rustsha1 = [] - #! ### Other ## Count cache hits and misses and print that debug information on drop. diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 5e0a0d2a31a..14445293f98 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -217,7 +217,7 @@ blocking-http-transport-reqwest-native-tls = [ #! #### Performance #! #! The reason these features exist is to allow optimization for compile time and optimize for compatibility by default. This means that some performance options around -#! SHA1 and ZIP might not compile on all platforms, so it depends on the end-user who compiles the application to chose these based on their needs. +#! ZIP might not compile on all platforms, so it depends on the end-user who compiles the application to chose these based on their needs. ## Activate features that maximize performance, like using threads, but leave everything else that might affect compatibility out to allow users more fine-grained ## control over performance features like which `zlib*` implementation to use. @@ -249,11 +249,7 @@ pack-cache-lru-dynamic = ["gix-pack/pack-cache-lru-dynamic"] ## Activate other features that maximize performance, like usage of threads, `zlib-ng` and access to caching in object databases. ## Note that some platforms might suffer from compile failures, which is when `max-performance-safe` should be used. -max-performance = ["max-performance-safe", "zlib-ng", "fast-sha1"] - -## If enabled, use assembly versions of sha1 on supported platforms. -## This might cause compile failures as well which is why it can be turned off separately. -fast-sha1 = ["gix-features/fast-sha1"] +max-performance = ["max-performance-safe", "zlib-ng"] ## Use the C-based zlib-ng backend, which can compress and decompress significantly faster. ## Note that this will cause duplicate symbol errors if the application also depends on `zlib` - use `zlib-ng-compat` in that case. diff --git a/justfile b/justfile index da837e97e13..f143bc06abc 100755 --- a/justfile +++ b/justfile @@ -99,8 +99,6 @@ check: cargo check -p gix-features --all-features cargo check -p gix-features --features parallel cargo check -p gix-features --features fs-read-dir - cargo check -p gix-features --features rustsha1 - cargo check -p gix-features --features fast-sha1 cargo check -p gix-features --features progress cargo check -p gix-features --features io-pipe cargo check -p gix-features --features crc32 From 4e935ce167428581f7e0351768b705164f71179a Mon Sep 17 00:00:00 2001 From: Emily Date: Tue, 1 Apr 2025 15:36:24 +0100 Subject: [PATCH 08/22] migrate all hashing API users to `gix_hash::Hasher::finalize()` --- gix-commitgraph/src/file/verify.rs | 2 +- gix-hash/src/hasher/io.rs | 2 +- gix-index/src/extension/end_of_index_entry/decode.rs | 2 +- gix-index/src/extension/end_of_index_entry/write.rs | 2 +- gix-index/src/file/write.rs | 2 +- gix-object/src/lib.rs | 2 +- gix-odb/src/sink.rs | 2 +- gix-odb/src/store_impls/loose/write.rs | 2 +- gix-pack/src/data/input/bytes_to_entries.rs | 4 ++-- gix-pack/src/data/output/bytes.rs | 8 ++++---- gix-pack/src/index/encode.rs | 2 +- gix-pack/src/index/write/mod.rs | 2 +- gix-pack/src/multi_index/write.rs | 2 +- gix-pack/src/verify.rs | 2 +- 14 files changed, 18 insertions(+), 18 deletions(-) diff --git a/gix-commitgraph/src/file/verify.rs b/gix-commitgraph/src/file/verify.rs index df7be631862..dbfd0103b00 100644 --- a/gix-commitgraph/src/file/verify.rs +++ b/gix-commitgraph/src/file/verify.rs @@ -147,7 +147,7 @@ impl File { let data_len_without_trailer = self.data.len() - self.hash_len; let mut hasher = gix_hash::hasher(self.object_hash()); hasher.update(&self.data[..data_len_without_trailer]); - let actual = gix_hash::ObjectId::from_bytes_or_panic(hasher.digest().as_ref()); + let actual = hasher.finalize(); let expected = self.checksum(); if actual == expected { diff --git a/gix-hash/src/hasher/io.rs b/gix-hash/src/hasher/io.rs index 4634dc1b8d0..02d976bb437 100644 --- a/gix-hash/src/hasher/io.rs +++ b/gix-hash/src/hasher/io.rs @@ -66,7 +66,7 @@ pub fn bytes_with_hasher( } } - let id = crate::ObjectId::from(hasher.digest()); + let id = hasher.finalize(); progress.show_throughput(start); Ok(id) } diff --git a/gix-index/src/extension/end_of_index_entry/decode.rs b/gix-index/src/extension/end_of_index_entry/decode.rs index b923e20224d..c7904b58de7 100644 --- a/gix-index/src/extension/end_of_index_entry/decode.rs +++ b/gix-index/src/extension/end_of_index_entry/decode.rs @@ -41,7 +41,7 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option { last_chunk = Some(chunk); } - if hasher.digest() != checksum { + if hasher.finalize().as_slice() != checksum { return None; } // The last-to-this chunk ends where ours starts diff --git a/gix-index/src/extension/end_of_index_entry/write.rs b/gix-index/src/extension/end_of_index_entry/write.rs index 13e91a13063..ad432cc3ee4 100644 --- a/gix-index/src/extension/end_of_index_entry/write.rs +++ b/gix-index/src/extension/end_of_index_entry/write.rs @@ -23,7 +23,7 @@ pub fn write_to( hasher.update(&signature); hasher.update(&size.to_be_bytes()); } - out.write_all(&hasher.digest())?; + out.write_all(hasher.finalize().as_slice())?; Ok(()) } diff --git a/gix-index/src/file/write.rs b/gix-index/src/file/write.rs index 0f6941b0625..3c5775f1518 100644 --- a/gix-index/src/file/write.rs +++ b/gix-index/src/file/write.rs @@ -31,7 +31,7 @@ impl File { let mut hasher = hasher::io::Write::new(&mut out, self.state.object_hash); let out: &mut dyn std::io::Write = &mut hasher; let version = self.state.write_to(out, options)?; - (version, gix_hash::ObjectId::from(hasher.hash.digest())) + (version, hasher.hash.finalize()) }; out.write_all(hash.as_slice())?; Ok((version, hash)) diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index e851f08c9be..96c7216334f 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -409,7 +409,7 @@ fn object_hasher(hash_kind: gix_hash::Kind, object_kind: Kind, object_size: u64) pub fn compute_hash(hash_kind: gix_hash::Kind, object_kind: Kind, data: &[u8]) -> gix_hash::ObjectId { let mut hasher = object_hasher(hash_kind, object_kind, data.len() as u64); hasher.update(data); - hasher.digest().into() + hasher.finalize() } /// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its data read from `stream` diff --git a/gix-odb/src/sink.rs b/gix-odb/src/sink.rs index dff75bacb8d..d0952d1df8a 100644 --- a/gix-odb/src/sink.rs +++ b/gix-odb/src/sink.rs @@ -53,6 +53,6 @@ impl gix_object::Write for Sink { c.reset(); } - Ok(hasher.digest().into()) + Ok(hasher.finalize()) } } diff --git a/gix-odb/src/store_impls/loose/write.rs b/gix-odb/src/store_impls/loose/write.rs index d969eec261c..022a53f8b3b 100644 --- a/gix-odb/src/store_impls/loose/write.rs +++ b/gix-odb/src/store_impls/loose/write.rs @@ -130,7 +130,7 @@ impl Store { &self, hasher::io::Write { hash, inner: file }: hasher::io::Write, ) -> Result { - let id = gix_hash::ObjectId::from(hash.digest()); + let id = hash.finalize(); let object_path = loose::hash_path(&id, self.path.clone()); let object_dir = object_path .parent() diff --git a/gix-pack/src/data/input/bytes_to_entries.rs b/gix-pack/src/data/input/bytes_to_entries.rs index b08f26a5415..386f01fc706 100644 --- a/gix-pack/src/data/input/bytes_to_entries.rs +++ b/gix-pack/src/data/input/bytes_to_entries.rs @@ -177,7 +177,7 @@ where } if let Some(hash) = self.hash.take() { - let actual_id = gix_hash::ObjectId::from(hash.digest()); + let actual_id = hash.finalize(); if self.mode == input::Mode::Restore { id = actual_id; } @@ -191,7 +191,7 @@ where Some(id) } else if self.mode == input::Mode::Restore { let hash = self.hash.clone().expect("in restore mode a hash is set"); - Some(gix_hash::ObjectId::from(hash.digest())) + Some(hash.finalize()) } else { None }) diff --git a/gix-pack/src/data/output/bytes.rs b/gix-pack/src/data/output/bytes.rs index 195c41790b9..689ad383ec9 100644 --- a/gix-pack/src/data/output/bytes.rs +++ b/gix-pack/src/data/output/bytes.rs @@ -121,12 +121,12 @@ where } } None => { - let digest = self.output.hash.clone().digest(); - self.output.inner.write_all(&digest[..])?; - self.written += digest.len() as u64; + let digest = self.output.hash.clone().finalize(); + self.output.inner.write_all(digest.as_slice())?; + self.written += digest.as_slice().len() as u64; self.output.inner.flush()?; self.is_done = true; - self.trailer = Some(gix_hash::ObjectId::from(digest)); + self.trailer = Some(digest); } } Ok(self.written - previous_written) diff --git a/gix-pack/src/index/encode.rs b/gix-pack/src/index/encode.rs index ef436a4f022..91f03ab3bcb 100644 --- a/gix-pack/src/index/encode.rs +++ b/gix-pack/src/index/encode.rs @@ -137,7 +137,7 @@ mod function { let bytes_written_without_trailer = out.bytes; let out = out.inner.into_inner()?; - let index_hash: gix_hash::ObjectId = out.hash.digest().into(); + let index_hash = out.hash.finalize(); out.inner.write_all(index_hash.as_slice())?; out.inner.flush()?; diff --git a/gix-pack/src/index/write/mod.rs b/gix-pack/src/index/write/mod.rs index d77e4c00617..49ffe14184a 100644 --- a/gix-pack/src/index/write/mod.rs +++ b/gix-pack/src/index/write/mod.rs @@ -227,7 +227,7 @@ impl crate::index::File { let header = crate::data::header::encode(pack_version, 0); let mut hasher = gix_hash::hasher(object_hash); hasher.update(&header); - gix_hash::ObjectId::from(hasher.digest()) + hasher.finalize() } None => return Err(Error::IteratorInvariantTrailer), }; diff --git a/gix-pack/src/multi_index/write.rs b/gix-pack/src/multi_index/write.rs index 67a0a917b1c..887ac14aa0c 100644 --- a/gix-pack/src/multi_index/write.rs +++ b/gix-pack/src/multi_index/write.rs @@ -214,7 +214,7 @@ impl multi_index::File { } // write trailing checksum - let multi_index_checksum: gix_hash::ObjectId = out.inner.hash.digest().into(); + let multi_index_checksum = out.inner.hash.finalize(); out.inner.inner.write_all(multi_index_checksum.as_slice())?; out.progress.show_throughput(write_start); diff --git a/gix-pack/src/verify.rs b/gix-pack/src/verify.rs index 9656c255c23..6b1b3a3ec11 100644 --- a/gix-pack/src/verify.rs +++ b/gix-pack/src/verify.rs @@ -52,7 +52,7 @@ pub fn checksum_on_disk_or_mmap( hasher.update(&data[..data_len_without_trailer]); progress.inc_by(data_len_without_trailer); progress.show_throughput(start); - gix_hash::ObjectId::from(hasher.digest()) + hasher.finalize() } }; From 65aab56ebdc0b676b5ebf47fd3b102928b4ed4aa Mon Sep 17 00:00:00 2001 From: Emily Date: Tue, 1 Apr 2025 15:44:46 +0100 Subject: [PATCH 09/22] change!: drop `gix_hash::{hasher::Digest, Hasher::digest()}` Complete the transition to `ObjectId` returns. --- gix-hash/src/hasher/mod.rs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/gix-hash/src/hasher/mod.rs b/gix-hash/src/hasher/mod.rs index db4350b5cf4..6a226f65d49 100644 --- a/gix-hash/src/hasher/mod.rs +++ b/gix-hash/src/hasher/mod.rs @@ -1,7 +1,4 @@ -use sha1_checked::CollisionResult; - -/// A hash-digest produced by a [`Hasher`] hash implementation. -pub type Digest = [u8; 20]; +use sha1_checked::{CollisionResult, Digest}; /// The error returned by [`Hasher::try_finalize()`]. #[derive(Debug, thiserror::Error)] @@ -32,7 +29,6 @@ impl Default for Hasher { impl Hasher { /// Digest the given `bytes`. pub fn update(&mut self, bytes: &[u8]) { - use sha1_checked::Digest; self.0.update(bytes); } @@ -67,15 +63,6 @@ impl Hasher { pub fn finalize(self) -> crate::ObjectId { self.try_finalize().expect("Detected SHA-1 collision attack") } - - /// Finalize the hash and produce a digest. - #[inline] - pub fn digest(self) -> Digest { - self.finalize() - .as_slice() - .try_into() - .expect("SHA-1 object ID to be 20 bytes long") - } } /// Produce a hasher suitable for the given kind of hash. From 6c02b0a044edc123d95525ed8e8a6e2fb1e45213 Mon Sep 17 00:00:00 2001 From: Emily Date: Mon, 31 Mar 2025 00:16:39 +0100 Subject: [PATCH 10/22] change!: split index and pack checksum verification errors --- gix-pack/src/index/traverse/error.rs | 6 ++++-- gix-pack/src/index/traverse/mod.rs | 4 ++-- gix-pack/src/index/verify.rs | 2 +- gix-pack/src/multi_index/verify.rs | 3 ++- gix-pack/src/verify.rs | 2 +- .../no-repo/pack/explode/broken-delete-pack-to-sink-failure | 4 ++-- tests/snapshots/plumbing/no-repo/pack/verify/index-failure | 4 ++-- 7 files changed, 14 insertions(+), 11 deletions(-) diff --git a/gix-pack/src/index/traverse/error.rs b/gix-pack/src/index/traverse/error.rs index f6cc7506548..21c582d5f73 100644 --- a/gix-pack/src/index/traverse/error.rs +++ b/gix-pack/src/index/traverse/error.rs @@ -6,8 +6,8 @@ use crate::index; pub enum Error { #[error("One of the traversal processors failed")] Processor(#[source] E), - #[error("Index file, pack file or object verification failed")] - VerifyChecksum(#[from] index::verify::checksum::Error), + #[error("Failed to verify index file checksum")] + IndexVerify(#[source] index::verify::checksum::Error), #[error("The pack delta tree index could not be built")] Tree(#[from] crate::cache::delta::from_offsets::Error), #[error("The tree traversal failed")] @@ -25,6 +25,8 @@ pub enum Error { expected: gix_hash::ObjectId, actual: gix_hash::ObjectId, }, + #[error("Failed to verify pack file checksum")] + PackVerify(#[source] crate::verify::checksum::Error), #[error("The hash of {kind} object at offset {offset} didn't match the checksum in the index file: expected {expected}, got {actual}")] PackObjectMismatch { expected: gix_hash::ObjectId, diff --git a/gix-pack/src/index/traverse/mod.rs b/gix-pack/src/index/traverse/mod.rs index 50f0236b862..8b61c2ebca1 100644 --- a/gix-pack/src/index/traverse/mod.rs +++ b/gix-pack/src/index/traverse/mod.rs @@ -136,8 +136,8 @@ impl index::File { move || pack.verify_checksum(pack_progress, should_interrupt), move || self.verify_checksum(index_progress, should_interrupt), ); - pack_res?; - id? + pack_res.map_err(Error::PackVerify)?; + id.map_err(Error::IndexVerify)? } else { self.index_checksum() }) diff --git a/gix-pack/src/index/verify.rs b/gix-pack/src/index/verify.rs index c2c72f0faa5..c0ed279152a 100644 --- a/gix-pack/src/index/verify.rs +++ b/gix-pack/src/index/verify.rs @@ -220,7 +220,7 @@ impl index::File { .add_child_with_id("Sha1 of index".into(), integrity::ProgressId::ChecksumBytes.into()), should_interrupt, ) - .map_err(Into::into) + .map_err(index::traverse::Error::IndexVerify) .map(|id| integrity::Outcome { actual_index_checksum: id, pack_traverse_statistics: None, diff --git a/gix-pack/src/multi_index/verify.rs b/gix-pack/src/multi_index/verify.rs index fe02d2ebfb5..88b2a04011a 100644 --- a/gix-pack/src/multi_index/verify.rs +++ b/gix-pack/src/multi_index/verify.rs @@ -279,9 +279,10 @@ impl File { use index::traverse::Error::*; match err { Processor(err) => Processor(integrity::Error::IndexIntegrity(err)), - VerifyChecksum(err) => VerifyChecksum(err), + IndexVerify(err) => IndexVerify(err), Tree(err) => Tree(err), TreeTraversal(err) => TreeTraversal(err), + PackVerify(err) => PackVerify(err), PackDecode { id, offset, source } => PackDecode { id, offset, source }, PackMismatch { expected, actual } => PackMismatch { expected, actual }, EntryType(err) => EntryType(err), diff --git a/gix-pack/src/verify.rs b/gix-pack/src/verify.rs index 6b1b3a3ec11..f55b891f6f3 100644 --- a/gix-pack/src/verify.rs +++ b/gix-pack/src/verify.rs @@ -10,7 +10,7 @@ pub mod checksum { pub enum Error { #[error("Interrupted by user")] Interrupted, - #[error("index checksum mismatch: expected {expected}, got {actual}")] + #[error("expected {expected}, got {actual}")] Mismatch { expected: gix_hash::ObjectId, actual: gix_hash::ObjectId, diff --git a/tests/snapshots/plumbing/no-repo/pack/explode/broken-delete-pack-to-sink-failure b/tests/snapshots/plumbing/no-repo/pack/explode/broken-delete-pack-to-sink-failure index 2123cefd0f0..25e21cd7442 100644 --- a/tests/snapshots/plumbing/no-repo/pack/explode/broken-delete-pack-to-sink-failure +++ b/tests/snapshots/plumbing/no-repo/pack/explode/broken-delete-pack-to-sink-failure @@ -1,5 +1,5 @@ Error: Failed to explode the entire pack - some loose objects may have been created nonetheless Caused by: - 0: Index file, pack file or object verification failed - 1: index checksum mismatch: expected f1cd3cc7bc63a4a2b357a475a58ad49b40355470, got 337fe3b886fc5041a35313887d68feefeae52519 \ No newline at end of file + 0: Failed to verify pack file checksum + 1: expected f1cd3cc7bc63a4a2b357a475a58ad49b40355470, got 337fe3b886fc5041a35313887d68feefeae52519 \ No newline at end of file diff --git a/tests/snapshots/plumbing/no-repo/pack/verify/index-failure b/tests/snapshots/plumbing/no-repo/pack/verify/index-failure index b93e9895bb1..ecea05fdf51 100644 --- a/tests/snapshots/plumbing/no-repo/pack/verify/index-failure +++ b/tests/snapshots/plumbing/no-repo/pack/verify/index-failure @@ -2,5 +2,5 @@ Could not find matching pack file at 'index.pack' - only index file will be veri Error: Verification failure Caused by: - 0: Index file, pack file or object verification failed - 1: index checksum mismatch: expected 0eba66e6b391eb83efc3ec9fc8a3087788911c0a, got fa9a8a630eacc2d3df00aff604bec2451ccbc8ff \ No newline at end of file + 0: Failed to verify index file checksum + 1: expected 0eba66e6b391eb83efc3ec9fc8a3087788911c0a, got fa9a8a630eacc2d3df00aff604bec2451ccbc8ff \ No newline at end of file From b32f1f3243821f97f95350e21745c97fe7b1377e Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 19 Mar 2025 17:54:14 +0000 Subject: [PATCH 11/22] feat: add common interface for hash verification --- gix-hash/src/lib.rs | 3 +++ gix-hash/src/verify.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 gix-hash/src/verify.rs diff --git a/gix-hash/src/lib.rs b/gix-hash/src/lib.rs index 924681d0607..3c94ee8d6df 100644 --- a/gix-hash/src/lib.rs +++ b/gix-hash/src/lib.rs @@ -24,6 +24,9 @@ pub use object_id::{decode, ObjectId}; /// pub mod prefix; +/// +pub mod verify; + /// A partial, owned hash possibly identifying an object uniquely, whose non-prefix bytes are zeroed. /// /// An example would `0000000000000000000000000000000032bd3242`, where `32bd3242` is the prefix, diff --git a/gix-hash/src/verify.rs b/gix-hash/src/verify.rs new file mode 100644 index 00000000000..9cb7f6b19ef --- /dev/null +++ b/gix-hash/src/verify.rs @@ -0,0 +1,27 @@ +use crate::{oid, ObjectId}; + +/// The error returned by [`oid::verify()`]. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +#[error("Hash was {actual}, but should have been {expected}")] +pub struct Error { + actual: ObjectId, + expected: ObjectId, +} + +impl oid { + /// Verify that `self` matches the `expected` object ID. + /// + /// Returns an [`Error`] containing both object IDs if they differ. + #[inline] + pub fn verify(&self, expected: &oid) -> Result<(), Error> { + if self == expected { + Ok(()) + } else { + Err(Error { + actual: self.to_owned(), + expected: expected.to_owned(), + }) + } + } +} From 54e57649f0e0b15c0bd1d3233e41524cb91a8cb9 Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 19 Mar 2025 17:54:14 +0000 Subject: [PATCH 12/22] change!: adjust hash verification return types for the common interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- gitoxide-core/src/pack/explode.rs | 11 +++--- gix-commitgraph/src/file/verify.rs | 38 ++++++++++--------- gix-commitgraph/src/verify.rs | 4 +- gix-index/src/decode/mod.rs | 14 ++----- .../extension/end_of_index_entry/decode.rs | 7 +++- gix-index/src/file/init.rs | 13 ++----- gix-index/src/file/verify.rs | 19 +++------- gix-index/tests/index/file/read.rs | 2 +- gix-object/src/data.rs | 23 ++++------- gix-odb/src/store_impls/loose/verify.rs | 21 +++++----- gix-pack/src/data/input/bytes_to_entries.rs | 8 +--- gix-pack/src/data/input/types.rs | 7 +--- gix-pack/src/index/traverse/error.rs | 16 +++----- gix-pack/src/index/traverse/mod.rs | 22 ++++------- gix-pack/src/multi_index/verify.rs | 14 +------ gix-pack/src/verify.rs | 18 +++------ .../broken-delete-pack-to-sink-failure | 2 +- .../no-repo/pack/verify/index-failure | 2 +- 18 files changed, 90 insertions(+), 151 deletions(-) diff --git a/gitoxide-core/src/pack/explode.rs b/gitoxide-core/src/pack/explode.rs index 0ae360d95b5..30581b97cc4 100644 --- a/gitoxide-core/src/pack/explode.rs +++ b/gitoxide-core/src/pack/explode.rs @@ -78,11 +78,11 @@ enum Error { }, #[error("Object didn't verify after right after writing it")] Verify(#[from] objs::data::verify::Error), - #[error("{kind} object {expected} wasn't re-encoded without change - new hash is {actual}")] + #[error("{kind} object wasn't re-encoded without change")] ObjectEncodeMismatch { + #[source] + source: gix::hash::verify::Error, kind: object::Kind, - actual: ObjectId, - expected: ObjectId, }, #[error("The recently written file for loose object {id} could not be found")] WrittenFileMissing { id: ObjectId }, @@ -201,7 +201,7 @@ pub fn pack_or_pack_index( kind: object_kind, id: index_entry.oid, })?; - if written_id != index_entry.oid { + if let Err(err) = written_id.verify(&index_entry.oid) { if let object::Kind::Tree = object_kind { progress.info(format!( "The tree in pack named {} was written as {} due to modes 100664 and 100640 rewritten as 100644.", @@ -209,9 +209,8 @@ pub fn pack_or_pack_index( )); } else { return Err(Error::ObjectEncodeMismatch { + source: err, kind: object_kind, - actual: index_entry.oid, - expected: written_id, }); } } diff --git a/gix-commitgraph/src/file/verify.rs b/gix-commitgraph/src/file/verify.rs index dbfd0103b00..79e93f88b6f 100644 --- a/gix-commitgraph/src/file/verify.rs +++ b/gix-commitgraph/src/file/verify.rs @@ -28,11 +28,8 @@ pub enum Error { Filename(String), #[error("commit {id} has invalid generation {generation}")] Generation { generation: u32, id: gix_hash::ObjectId }, - #[error("checksum mismatch: expected {expected}, got {actual}")] - Mismatch { - actual: gix_hash::ObjectId, - expected: gix_hash::ObjectId, - }, + #[error(transparent)] + Checksum(#[from] checksum::Error), #[error("{0}")] Processor(#[source] E), #[error("commit {id} has invalid root tree ID {root_tree_id}")] @@ -42,6 +39,17 @@ pub enum Error { }, } +/// +pub mod checksum { + /// The error used in [`super::File::verify_checksum()`]. + #[derive(thiserror::Error, Debug)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Verify(#[from] gix_hash::verify::Error), + } +} + /// The positive result of [`File::traverse()`] providing some statistical information. #[derive(Clone, Debug, Eq, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] @@ -73,8 +81,7 @@ impl File { E: std::error::Error + 'static, Processor: FnMut(&file::Commit<'a>) -> Result<(), E>, { - self.verify_checksum() - .map_err(|(actual, expected)| Error::Mismatch { actual, expected })?; + self.verify_checksum()?; verify_split_chain_filename_hash(&self.path, self.checksum()).map_err(Error::Filename)?; let null_id = self.object_hash().null_ref(); @@ -138,23 +145,18 @@ impl File { /// Assure the [`checksum`][File::checksum()] matches the actual checksum over all content of this file, excluding the trailing /// checksum itself. /// - /// Return the actual checksum on success or `(actual checksum, expected checksum)` if there is a mismatch. - pub fn verify_checksum(&self) -> Result { - // Even though we could use gix_hash::bytes_of_file(…), this would require using our own - // Error type to support io::Error and Mismatch. As we only gain progress, there probably isn't much value + /// Return the actual checksum on success or [`checksum::Error`] if there is a mismatch. + pub fn verify_checksum(&self) -> Result { + // Even though we could use gix_hash::bytes_of_file(…), this would require extending our + // 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. // But it's possible, once a progress instance is passed. let data_len_without_trailer = self.data.len() - self.hash_len; let mut hasher = gix_hash::hasher(self.object_hash()); hasher.update(&self.data[..data_len_without_trailer]); let actual = hasher.finalize(); - - let expected = self.checksum(); - if actual == expected { - Ok(actual) - } else { - Err((actual, expected.into())) - } + actual.verify(self.checksum())?; + Ok(actual) } } diff --git a/gix-commitgraph/src/verify.rs b/gix-commitgraph/src/verify.rs index 9a37cff83ad..f1798514bb4 100644 --- a/gix-commitgraph/src/verify.rs +++ b/gix-commitgraph/src/verify.rs @@ -164,9 +164,7 @@ impl Graph { file::verify::Error::RootTreeId { id, root_tree_id } => { file::verify::Error::RootTreeId { id, root_tree_id } } - file::verify::Error::Mismatch { actual, expected } => { - file::verify::Error::Mismatch { actual, expected } - } + file::verify::Error::Checksum(err) => file::verify::Error::Checksum(err), file::verify::Error::Generation { generation, id } => { file::verify::Error::Generation { generation, id } } diff --git a/gix-index/src/decode/mod.rs b/gix-index/src/decode/mod.rs index aea189248bd..708df929ad1 100644 --- a/gix-index/src/decode/mod.rs +++ b/gix-index/src/decode/mod.rs @@ -22,11 +22,8 @@ mod error { Extension(#[from] extension::decode::Error), #[error("Index trailer should have been {expected} bytes long, but was {actual}")] UnexpectedTrailerLength { expected: usize, actual: usize }, - #[error("Shared index checksum was {actual_checksum} but should have been {expected_checksum}")] - ChecksumMismatch { - actual_checksum: gix_hash::ObjectId, - expected_checksum: gix_hash::ObjectId, - }, + #[error("Shared index checksum mismatch")] + Verify(#[from] gix_hash::verify::Error), } } pub use error::Error; @@ -217,12 +214,7 @@ impl State { let checksum = gix_hash::ObjectId::from_bytes_or_panic(data); let checksum = (!checksum.is_null()).then_some(checksum); if let Some((expected_checksum, actual_checksum)) = expected_checksum.zip(checksum) { - if actual_checksum != expected_checksum { - return Err(Error::ChecksumMismatch { - actual_checksum, - expected_checksum, - }); - } + actual_checksum.verify(&expected_checksum)?; } let EntriesOutcome { entries, diff --git a/gix-index/src/extension/end_of_index_entry/decode.rs b/gix-index/src/extension/end_of_index_entry/decode.rs index c7904b58de7..89b1c341a22 100644 --- a/gix-index/src/extension/end_of_index_entry/decode.rs +++ b/gix-index/src/extension/end_of_index_entry/decode.rs @@ -29,7 +29,10 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option { let (offset, checksum) = ext_data.split_at(4); let offset = from_be_u32(offset) as usize; - if offset < header::SIZE || offset > start_of_eoie || checksum.len() != gix_hash::Kind::Sha1.len_in_bytes() { + let Ok(checksum) = gix_hash::oid::try_from_bytes(checksum) else { + return None; + }; + if offset < header::SIZE || offset > start_of_eoie || checksum.kind() != gix_hash::Kind::Sha1 { return None; } @@ -41,7 +44,7 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option { last_chunk = Some(chunk); } - if hasher.finalize().as_slice() != checksum { + if hasher.finalize().verify(checksum).is_err() { return None; } // The last-to-this chunk ends where ours starts diff --git a/gix-index/src/file/init.rs b/gix-index/src/file/init.rs index 7ba91cbb4a2..654bab6c05b 100644 --- a/gix-index/src/file/init.rs +++ b/gix-index/src/file/init.rs @@ -75,20 +75,15 @@ impl File { let _span = gix_features::trace::detail!("gix::open_index::hash_index", path = ?path); let meta = file.metadata()?; let num_bytes_to_hash = meta.len() - object_hash.len_in_bytes() as u64; - let actual_hash = gix_hash::bytes( + gix_hash::bytes( &mut file, num_bytes_to_hash, object_hash, &mut gix_features::progress::Discard, &Default::default(), - )?; - - if actual_hash != expected { - return Err(Error::Decode(decode::Error::ChecksumMismatch { - actual_checksum: actual_hash, - expected_checksum: expected, - })); - } + )? + .verify(&expected) + .map_err(decode::Error::from)?; } } diff --git a/gix-index/src/file/verify.rs b/gix-index/src/file/verify.rs index 9d49d579ea8..1d3212e388d 100644 --- a/gix-index/src/file/verify.rs +++ b/gix-index/src/file/verify.rs @@ -9,11 +9,8 @@ mod error { pub enum Error { #[error("Could not read index file to generate hash")] Io(#[from] std::io::Error), - #[error("Index checksum should have been {expected}, but was {actual}")] - ChecksumMismatch { - actual: gix_hash::ObjectId, - expected: gix_hash::ObjectId, - }, + #[error("Index checksum mismatch")] + Verify(#[from] gix_hash::verify::Error), } } pub use error::Error; @@ -25,19 +22,15 @@ impl File { if let Some(checksum) = self.checksum { let num_bytes_to_hash = self.path.metadata()?.len() - checksum.as_bytes().len() as u64; let should_interrupt = AtomicBool::new(false); - let actual = gix_hash::bytes_of_file( + gix_hash::bytes_of_file( &self.path, num_bytes_to_hash, checksum.kind(), &mut gix_features::progress::Discard, &should_interrupt, - )?; - (actual == checksum).then_some(()).ok_or(Error::ChecksumMismatch { - actual, - expected: checksum, - }) - } else { - Ok(()) + )? + .verify(&checksum)?; } + Ok(()) } } diff --git a/gix-index/tests/index/file/read.rs b/gix-index/tests/index/file/read.rs index e514934b04a..df85fbff492 100644 --- a/gix-index/tests/index/file/read.rs +++ b/gix-index/tests/index/file/read.rs @@ -328,7 +328,7 @@ fn v2_split_index_recursion_is_handled_gracefully() { let err = try_file("v2_split_index_recursive").expect_err("recursion fails gracefully"); assert!(matches!( err, - gix_index::file::init::Error::Decode(gix_index::decode::Error::ChecksumMismatch { .. }) + gix_index::file::init::Error::Decode(gix_index::decode::Error::Verify(_)) )); } diff --git a/gix-object/src/data.rs b/gix-object/src/data.rs index cea807684d9..8251ed6eaa5 100644 --- a/gix-object/src/data.rs +++ b/gix-object/src/data.rs @@ -51,31 +51,22 @@ impl<'a> Data<'a> { /// Types supporting object hash verification pub mod verify { - /// Returned by [`crate::Data::verify_checksum()`] #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { - #[error("Object expected to have id {desired}, but actual id was {actual}")] - ChecksumMismatch { - desired: gix_hash::ObjectId, - actual: gix_hash::ObjectId, - }, + #[error(transparent)] + Verify(#[from] gix_hash::verify::Error), } impl crate::Data<'_> { - /// Compute the checksum of `self` and compare it with the `desired` hash. + /// Compute the checksum of `self` and compare it with the `expected` hash. /// If the hashes do not match, an [`Error`] is returned, containing the actual /// hash of `self`. - pub fn verify_checksum(&self, desired: &gix_hash::oid) -> Result<(), Error> { - let actual_id = crate::compute_hash(desired.kind(), self.kind, self.data); - if desired != actual_id { - return Err(Error::ChecksumMismatch { - desired: desired.into(), - actual: actual_id, - }); - } - Ok(()) + pub fn verify_checksum(&self, expected: &gix_hash::oid) -> Result { + let actual = crate::compute_hash(expected.kind(), self.kind, self.data); + actual.verify(expected)?; + Ok(actual) } } } diff --git a/gix-odb/src/store_impls/loose/verify.rs b/gix-odb/src/store_impls/loose/verify.rs index dd08e9cd9f2..0568a38db9e 100644 --- a/gix-odb/src/store_impls/loose/verify.rs +++ b/gix-odb/src/store_impls/loose/verify.rs @@ -19,11 +19,11 @@ pub mod integrity { kind: gix_object::Kind, id: gix_hash::ObjectId, }, - #[error("{kind} object {expected} wasn't re-encoded without change - new hash is {actual}")] - ObjectHashMismatch { + #[error("{kind} object wasn't re-encoded without change")] + ObjectEncodeMismatch { + #[source] + source: gix_hash::verify::Error, kind: gix_object::Kind, - actual: gix_hash::ObjectId, - expected: gix_hash::ObjectId, }, #[error("Objects were deleted during iteration - try again")] Retry, @@ -77,14 +77,13 @@ impl Store { .try_find(&id, &mut buf) .map_err(|_| integrity::Error::Retry)? .ok_or(integrity::Error::Retry)?; - let actual_id = sink.write_buf(object.kind, object.data).expect("sink never fails"); - if actual_id != id { - return Err(integrity::Error::ObjectHashMismatch { + sink.write_buf(object.kind, object.data) + .expect("sink never fails") + .verify(&id) + .map_err(|err| integrity::Error::ObjectEncodeMismatch { + source: err, kind: object.kind, - actual: actual_id, - expected: id, - }); - } + })?; object.decode().map_err(|err| integrity::Error::ObjectDecode { source: err, kind: object.kind, diff --git a/gix-pack/src/data/input/bytes_to_entries.rs b/gix-pack/src/data/input/bytes_to_entries.rs index 386f01fc706..76aa6d4098e 100644 --- a/gix-pack/src/data/input/bytes_to_entries.rs +++ b/gix-pack/src/data/input/bytes_to_entries.rs @@ -180,12 +180,8 @@ where let actual_id = hash.finalize(); if self.mode == input::Mode::Restore { id = actual_id; - } - if id != actual_id { - return Err(input::Error::ChecksumMismatch { - actual: actual_id, - expected: id, - }); + } else { + actual_id.verify(&id)?; } } Some(id) diff --git a/gix-pack/src/data/input/types.rs b/gix-pack/src/data/input/types.rs index 86852479faa..86ecb6ebdb1 100644 --- a/gix-pack/src/data/input/types.rs +++ b/gix-pack/src/data/input/types.rs @@ -9,11 +9,8 @@ pub enum Error { Io(#[from] io::Error), #[error(transparent)] PackParse(#[from] crate::data::header::decode::Error), - #[error("pack checksum in trailer was {expected}, but actual checksum was {actual}")] - ChecksumMismatch { - expected: gix_hash::ObjectId, - actual: gix_hash::ObjectId, - }, + #[error("Failed to verify pack checksum in trailer")] + Verify(#[from] gix_hash::verify::Error), #[error("pack is incomplete: it was decompressed into {actual} bytes but {expected} bytes where expected.")] IncompletePack { actual: u64, expected: u64 }, #[error("The object {object_id} could not be decoded or wasn't found")] diff --git a/gix-pack/src/index/traverse/error.rs b/gix-pack/src/index/traverse/error.rs index 21c582d5f73..a77b011af27 100644 --- a/gix-pack/src/index/traverse/error.rs +++ b/gix-pack/src/index/traverse/error.rs @@ -20,19 +20,15 @@ pub enum Error { offset: u64, source: crate::data::decode::Error, }, - #[error("The packfiles checksum didn't match the index file checksum: expected {expected}, got {actual}")] - PackMismatch { - expected: gix_hash::ObjectId, - actual: gix_hash::ObjectId, - }, + #[error("The packfiles checksum didn't match the index file checksum")] + PackMismatch(#[source] gix_hash::verify::Error), #[error("Failed to verify pack file checksum")] PackVerify(#[source] crate::verify::checksum::Error), - #[error("The hash of {kind} object at offset {offset} didn't match the checksum in the index file: expected {expected}, got {actual}")] - PackObjectMismatch { - expected: gix_hash::ObjectId, - actual: gix_hash::ObjectId, + #[error("Error verifying object at offset {offset} against checksum in the index file")] + PackObjectVerify { offset: u64, - kind: gix_object::Kind, + #[source] + source: gix_object::data::verify::Error, }, #[error( "The CRC32 of {kind} object at offset {offset} didn't match the checksum in the index file: expected {expected}, got {actual}" diff --git a/gix-pack/src/index/traverse/mod.rs b/gix-pack/src/index/traverse/mod.rs index 8b61c2ebca1..8f13774f0ab 100644 --- a/gix-pack/src/index/traverse/mod.rs +++ b/gix-pack/src/index/traverse/mod.rs @@ -126,12 +126,9 @@ impl index::File { E: std::error::Error + Send + Sync + 'static, { Ok(if check.file_checksum() { - if self.pack_checksum() != pack.checksum() { - return Err(Error::PackMismatch { - actual: pack.checksum(), - expected: self.pack_checksum(), - }); - } + pack.checksum() + .verify(&self.pack_checksum()) + .map_err(Error::PackMismatch)?; let (pack_res, id) = parallel::join( move || pack.verify_checksum(pack_progress, should_interrupt), move || self.verify_checksum(index_progress, should_interrupt), @@ -210,15 +207,12 @@ where E: std::error::Error + Send + Sync + 'static, { if check.object_checksum() { - let actual_oid = gix_object::compute_hash(index_entry.oid.kind(), object_kind, decompressed); - if actual_oid != index_entry.oid { - return Err(Error::PackObjectMismatch { - actual: actual_oid, - expected: index_entry.oid, + gix_object::Data::new(object_kind, decompressed) + .verify_checksum(&index_entry.oid) + .map_err(|source| Error::PackObjectVerify { offset: index_entry.pack_offset, - kind: object_kind, - }); - } + source, + })?; if let Some(desired_crc32) = index_entry.crc32 { let actual_crc32 = pack_entry_crc32(); if actual_crc32 != desired_crc32 { diff --git a/gix-pack/src/multi_index/verify.rs b/gix-pack/src/multi_index/verify.rs index 88b2a04011a..8ef4c61c2e2 100644 --- a/gix-pack/src/multi_index/verify.rs +++ b/gix-pack/src/multi_index/verify.rs @@ -284,19 +284,9 @@ impl File { TreeTraversal(err) => TreeTraversal(err), PackVerify(err) => PackVerify(err), PackDecode { id, offset, source } => PackDecode { id, offset, source }, - PackMismatch { expected, actual } => PackMismatch { expected, actual }, + PackMismatch(err) => PackMismatch(err), EntryType(err) => EntryType(err), - PackObjectMismatch { - expected, - actual, - offset, - kind, - } => PackObjectMismatch { - expected, - actual, - offset, - kind, - }, + PackObjectVerify { offset, source } => PackObjectVerify { offset, source }, Crc32Mismatch { expected, actual, diff --git a/gix-pack/src/verify.rs b/gix-pack/src/verify.rs index f55b891f6f3..75704c68746 100644 --- a/gix-pack/src/verify.rs +++ b/gix-pack/src/verify.rs @@ -10,11 +10,8 @@ pub mod checksum { pub enum Error { #[error("Interrupted by user")] Interrupted, - #[error("expected {expected}, got {actual}")] - Mismatch { - expected: gix_hash::ObjectId, - actual: gix_hash::ObjectId, - }, + #[error(transparent)] + Verify(#[from] gix_hash::verify::Error), } } @@ -26,8 +23,8 @@ pub fn fan(data: &[u32]) -> Option { } /// Calculate the hash of the given kind by trying to read the file from disk at `data_path` or falling back on the mapped content in `data`. -/// `Ok(desired_hash)` or `Err(Some(actual_hash))` is returned if the hash matches or mismatches. -/// If the `Err(None)` is returned, the operation was interrupted. +/// `Ok(expected)` or [`checksum::Error::Verify`] is returned if the hash matches or mismatches. +/// If the [`checksum::Error::Interrupted`] is returned, the operation was interrupted. pub fn checksum_on_disk_or_mmap( data_path: &Path, data: &[u8], @@ -56,9 +53,6 @@ pub fn checksum_on_disk_or_mmap( } }; - if actual == expected { - Ok(actual) - } else { - Err(checksum::Error::Mismatch { actual, expected }) - } + actual.verify(&expected)?; + Ok(actual) } diff --git a/tests/snapshots/plumbing/no-repo/pack/explode/broken-delete-pack-to-sink-failure b/tests/snapshots/plumbing/no-repo/pack/explode/broken-delete-pack-to-sink-failure index 25e21cd7442..90237d86cf9 100644 --- a/tests/snapshots/plumbing/no-repo/pack/explode/broken-delete-pack-to-sink-failure +++ b/tests/snapshots/plumbing/no-repo/pack/explode/broken-delete-pack-to-sink-failure @@ -2,4 +2,4 @@ Error: Failed to explode the entire pack - some loose objects may have been crea Caused by: 0: Failed to verify pack file checksum - 1: expected f1cd3cc7bc63a4a2b357a475a58ad49b40355470, got 337fe3b886fc5041a35313887d68feefeae52519 \ No newline at end of file + 1: Hash was 337fe3b886fc5041a35313887d68feefeae52519, but should have been f1cd3cc7bc63a4a2b357a475a58ad49b40355470 \ No newline at end of file diff --git a/tests/snapshots/plumbing/no-repo/pack/verify/index-failure b/tests/snapshots/plumbing/no-repo/pack/verify/index-failure index ecea05fdf51..3a7ec0aa154 100644 --- a/tests/snapshots/plumbing/no-repo/pack/verify/index-failure +++ b/tests/snapshots/plumbing/no-repo/pack/verify/index-failure @@ -3,4 +3,4 @@ Error: Verification failure Caused by: 0: Failed to verify index file checksum - 1: expected 0eba66e6b391eb83efc3ec9fc8a3087788911c0a, got fa9a8a630eacc2d3df00aff604bec2451ccbc8ff \ No newline at end of file + 1: Hash was fa9a8a630eacc2d3df00aff604bec2451ccbc8ff, but should have been 0eba66e6b391eb83efc3ec9fc8a3087788911c0a \ No newline at end of file From 6d5c89693faf8971bc13e022c57a396d0db0ce03 Mon Sep 17 00:00:00 2001 From: Emily Date: Sat, 22 Mar 2025 19:09:23 +0000 Subject: [PATCH 13/22] change!: use separate error types for the `eol` and `ident` filters Prepare for hashing becoming fallible. --- gix-filter/src/eol/convert_to_worktree.rs | 10 +++++++++- gix-filter/src/eol/mod.rs | 3 ++- gix-filter/src/ident.rs | 17 ++++++++++++----- gix-filter/src/pipeline/convert.rs | 6 ++++-- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/gix-filter/src/eol/convert_to_worktree.rs b/gix-filter/src/eol/convert_to_worktree.rs index bc3bc474813..a7b8efe828b 100644 --- a/gix-filter/src/eol/convert_to_worktree.rs +++ b/gix-filter/src/eol/convert_to_worktree.rs @@ -5,6 +5,14 @@ use crate::{ eol::{AttributesDigest, Configuration, Mode, Stats}, }; +/// The error produced by [`convert_to_worktree()`]. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error("Could not allocate buffer")] + OutOfMemory(#[from] std::collections::TryReserveError), +} + /// Convert all `\n` in `src` to `crlf` if `digest` and `config` indicate it, returning `true` if `buf` holds the result, or `false` /// if no change was made after all. pub fn convert_to_worktree( @@ -12,7 +20,7 @@ pub fn convert_to_worktree( digest: AttributesDigest, buf: &mut Vec, config: Configuration, -) -> Result { +) -> Result { if src.is_empty() || digest.to_eol(config) != Some(Mode::CrLf) { return Ok(false); } diff --git a/gix-filter/src/eol/mod.rs b/gix-filter/src/eol/mod.rs index ad1553826e0..a338e786630 100644 --- a/gix-filter/src/eol/mod.rs +++ b/gix-filter/src/eol/mod.rs @@ -2,7 +2,8 @@ pub mod convert_to_git; pub use convert_to_git::function::convert_to_git; -mod convert_to_worktree; +/// +pub mod convert_to_worktree; pub use convert_to_worktree::convert_to_worktree; mod utils; diff --git a/gix-filter/src/ident.rs b/gix-filter/src/ident.rs index a5c2e0bdae5..e915ace3c46 100644 --- a/gix-filter/src/ident.rs +++ b/gix-filter/src/ident.rs @@ -40,6 +40,17 @@ pub fn undo(src: &[u8], buf: &mut Vec) -> Result$` if present in `src` and write all changes to `buf`, /// with `object_hash` being used accordingly. Return `true` if `buf` was written to or `false` if no change was made /// (as there was nothing to do). @@ -48,11 +59,7 @@ pub fn undo(src: &[u8], buf: &mut Vec) -> Result$`, but we don't do that, sticking exactly to what ought to be done. /// The respective code is up to 16 years old and one might assume that `git` by now handles checking and checkout filters correctly. -pub fn apply( - src: &[u8], - object_hash: gix_hash::Kind, - buf: &mut Vec, -) -> Result { +pub fn apply(src: &[u8], object_hash: gix_hash::Kind, buf: &mut Vec) -> Result { const HASH_LEN: usize = ": ".len() + gix_hash::Kind::longest().len_in_hex(); let mut id = None; let mut ofs = 0; diff --git a/gix-filter/src/pipeline/convert.rs b/gix-filter/src/pipeline/convert.rs index 4253e91ec5c..a236fc26e40 100644 --- a/gix-filter/src/pipeline/convert.rs +++ b/gix-filter/src/pipeline/convert.rs @@ -49,14 +49,16 @@ pub mod to_worktree { #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { + #[error(transparent)] + Ident(#[from] crate::ident::apply::Error), + #[error(transparent)] + Eol(#[from] crate::eol::convert_to_worktree::Error), #[error(transparent)] Worktree(#[from] crate::worktree::encode_to_worktree::Error), #[error(transparent)] Driver(#[from] crate::driver::apply::Error), #[error(transparent)] Configuration(#[from] super::configuration::Error), - #[error("Could not allocate buffer")] - OutOfMemory(#[from] std::collections::TryReserveError), } } From 4f2b6496109573237f82ee30058cbb93abf3e338 Mon Sep 17 00:00:00 2001 From: Emily Date: Sun, 30 Mar 2025 15:58:15 +0100 Subject: [PATCH 14/22] change!: use separate error type for I/O hashing operations Prepare for hashing becoming fallible. --- gix-hash/src/hasher/io.rs | 16 +++++++++--- gix-index/src/file/init.rs | 7 +++++- gix-index/src/file/verify.rs | 7 ++++-- gix-index/src/file/write.rs | 6 ++--- gix-index/src/write.rs | 2 +- gix-object/benches/edit_tree.rs | 5 +++- gix-object/src/lib.rs | 2 +- gix-object/tests/object/tree/editor.rs | 2 +- gix-odb/src/store_impls/loose/write.rs | 16 ++++++------ gix-pack/src/data/input/bytes_to_entries.rs | 18 +++++++------ gix-pack/src/data/input/entries_to_bytes.rs | 25 +++++++++++-------- gix-pack/src/data/input/entry.rs | 2 +- gix-pack/src/data/input/types.rs | 4 +-- gix-pack/src/data/output/bytes.rs | 20 ++++++++++----- gix-pack/src/index/encode.rs | 4 +-- gix-pack/src/index/write/error.rs | 6 ++--- gix-pack/src/index/write/mod.rs | 3 ++- gix-pack/src/multi_index/write.rs | 25 ++++++++++++------- gix-pack/src/verify.rs | 7 ++++-- gix-status/src/index_as_worktree/function.rs | 12 +++++---- gix-status/src/index_as_worktree/traits.rs | 4 +-- gix-status/src/index_as_worktree/types.rs | 2 +- .../src/index_as_worktree_with_renames/mod.rs | 2 +- .../index_as_worktree_with_renames/types.rs | 2 +- 24 files changed, 124 insertions(+), 75 deletions(-) diff --git a/gix-hash/src/hasher/io.rs b/gix-hash/src/hasher/io.rs index 02d976bb437..2561382ffaa 100644 --- a/gix-hash/src/hasher/io.rs +++ b/gix-hash/src/hasher/io.rs @@ -1,5 +1,13 @@ use crate::{hasher, Hasher}; +/// The error type for I/O operations that compute hashes. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error(transparent)] + Io(#[from] std::io::Error), +} + /// Compute the hash of `kind` for the bytes in the file at `path`, hashing only the first `num_bytes_from_start` /// while initializing and calling `progress`. /// @@ -15,7 +23,7 @@ pub fn bytes_of_file( kind: crate::Kind, progress: &mut dyn gix_features::progress::Progress, should_interrupt: &std::sync::atomic::AtomicBool, -) -> std::io::Result { +) -> Result { bytes( &mut std::fs::File::open(path)?, num_bytes_from_start, @@ -32,7 +40,7 @@ pub fn bytes( kind: crate::Kind, progress: &mut dyn gix_features::progress::Progress, should_interrupt: &std::sync::atomic::AtomicBool, -) -> std::io::Result { +) -> Result { bytes_with_hasher(read, num_bytes_from_start, hasher(kind), progress, should_interrupt) } @@ -43,7 +51,7 @@ pub fn bytes_with_hasher( mut hasher: Hasher, progress: &mut dyn gix_features::progress::Progress, should_interrupt: &std::sync::atomic::AtomicBool, -) -> std::io::Result { +) -> Result { let start = std::time::Instant::now(); // init progress before the possibility for failure, as convenience in case people want to recover progress.init( @@ -62,7 +70,7 @@ pub fn bytes_with_hasher( progress.inc_by(out.len()); hasher.update(out); if should_interrupt.load(std::sync::atomic::Ordering::SeqCst) { - return Err(std::io::Error::new(std::io::ErrorKind::Other, "Interrupted")); + return Err(std::io::Error::new(std::io::ErrorKind::Other, "Interrupted").into()); } } diff --git a/gix-index/src/file/init.rs b/gix-index/src/file/init.rs index 654bab6c05b..6957fc77a37 100644 --- a/gix-index/src/file/init.rs +++ b/gix-index/src/file/init.rs @@ -2,6 +2,8 @@ use std::path::{Path, PathBuf}; +use gix_hash::hasher; + use crate::{decode, extension, File, State}; mod error { @@ -81,7 +83,10 @@ impl File { object_hash, &mut gix_features::progress::Discard, &Default::default(), - )? + ) + .map_err(|err| match err { + hasher::io::Error::Io(err) => Error::Io(err), + })? .verify(&expected) .map_err(decode::Error::from)?; } diff --git a/gix-index/src/file/verify.rs b/gix-index/src/file/verify.rs index 1d3212e388d..d4b31800350 100644 --- a/gix-index/src/file/verify.rs +++ b/gix-index/src/file/verify.rs @@ -1,5 +1,7 @@ use std::sync::atomic::AtomicBool; +use gix_hash::hasher; + use crate::File; mod error { @@ -8,7 +10,7 @@ mod error { #[allow(missing_docs)] pub enum Error { #[error("Could not read index file to generate hash")] - Io(#[from] std::io::Error), + Io(#[from] gix_hash::hasher::io::Error), #[error("Index checksum mismatch")] Verify(#[from] gix_hash::verify::Error), } @@ -20,7 +22,8 @@ impl File { pub fn verify_integrity(&self) -> Result<(), Error> { let _span = gix_features::trace::coarse!("gix_index::File::verify_integrity()"); if let Some(checksum) = self.checksum { - let num_bytes_to_hash = self.path.metadata()?.len() - checksum.as_bytes().len() as u64; + let num_bytes_to_hash = + self.path.metadata().map_err(hasher::io::Error::from)?.len() - checksum.as_bytes().len() as u64; let should_interrupt = AtomicBool::new(false); gix_hash::bytes_of_file( &self.path, diff --git a/gix-index/src/file/write.rs b/gix-index/src/file/write.rs index 3c5775f1518..a942ece5586 100644 --- a/gix-index/src/file/write.rs +++ b/gix-index/src/file/write.rs @@ -7,7 +7,7 @@ use crate::{write, File, Version}; #[allow(missing_docs)] pub enum Error { #[error(transparent)] - Io(#[from] std::io::Error), + Io(#[from] hasher::io::Error), #[error("Could not acquire lock for index file")] AcquireLock(#[from] gix_lock::acquire::Error), #[error("Could not commit lock for index file")] @@ -21,7 +21,7 @@ impl File { &self, mut out: impl std::io::Write, options: write::Options, - ) -> std::io::Result<(Version, gix_hash::ObjectId)> { + ) -> Result<(Version, gix_hash::ObjectId), hasher::io::Error> { let _span = gix_features::trace::detail!("gix_index::File::write_to()", skip_hash = options.skip_hash); let (version, hash) = if options.skip_hash { let out: &mut dyn std::io::Write = &mut out; @@ -49,7 +49,7 @@ impl File { let (version, digest) = self.write_to(&mut lock, options)?; match lock.into_inner() { Ok(lock) => lock.commit()?, - Err(err) => return Err(err.into_error().into()), + Err(err) => return Err(Error::Io(err.into_error().into())), }; self.state.version = version; self.checksum = Some(digest); diff --git a/gix-index/src/write.rs b/gix-index/src/write.rs index e0b9554c220..436b8c5898a 100644 --- a/gix-index/src/write.rs +++ b/gix-index/src/write.rs @@ -67,7 +67,7 @@ impl State { extensions, skip_hash: _, }: Options, - ) -> std::io::Result { + ) -> Result { let _span = gix_features::trace::detail!("gix_index::State::write()"); let version = self.detect_required_version(); diff --git a/gix-object/benches/edit_tree.rs b/gix-object/benches/edit_tree.rs index 270a77dde75..a083a3ff4be 100644 --- a/gix-object/benches/edit_tree.rs +++ b/gix-object/benches/edit_tree.rs @@ -136,7 +136,10 @@ criterion_main!(benches); type TreeStore = Rc>>; -fn new_inmemory_writes() -> (TreeStore, impl FnMut(&Tree) -> Result) { +fn new_inmemory_writes() -> ( + TreeStore, + impl FnMut(&Tree) -> Result, +) { let store = TreeStore::default(); let write_tree = { let store = store.clone(); diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index 96c7216334f..96843309902 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -424,7 +424,7 @@ pub fn compute_stream_hash( stream_len: u64, progress: &mut dyn gix_features::progress::Progress, should_interrupt: &std::sync::atomic::AtomicBool, -) -> std::io::Result { +) -> Result { let hasher = object_hasher(hash_kind, object_kind, stream_len); gix_hash::bytes_with_hasher(stream, stream_len, hasher, progress, should_interrupt) } diff --git a/gix-object/tests/object/tree/editor.rs b/gix-object/tests/object/tree/editor.rs index df79683fb0c..9065c5aad1c 100644 --- a/gix-object/tests/object/tree/editor.rs +++ b/gix-object/tests/object/tree/editor.rs @@ -793,7 +793,7 @@ mod utils { pub(super) fn new_inmemory_writes() -> ( TreeStore, - impl FnMut(&Tree) -> Result, + impl FnMut(&Tree) -> Result, impl Fn() -> usize, ) { let store = TreeStore::default(); diff --git a/gix-odb/src/store_impls/loose/write.rs b/gix-odb/src/store_impls/loose/write.rs index 022a53f8b3b..8dbd3fa1e3d 100644 --- a/gix-odb/src/store_impls/loose/write.rs +++ b/gix-odb/src/store_impls/loose/write.rs @@ -14,7 +14,7 @@ use crate::store_impls::loose; pub enum Error { #[error("Could not {message} '{path}'")] Io { - source: io::Error, + source: hasher::io::Error, message: &'static str, path: PathBuf, }, @@ -31,12 +31,12 @@ impl gix_object::Write for Store { fn write(&self, object: &dyn WriteTo) -> Result { let mut to = self.dest()?; to.write_all(&object.loose_header()).map_err(|err| Error::Io { - source: err, + source: err.into(), message: "write header to tempfile in", path: self.path.to_owned(), })?; object.write_to(&mut to).map_err(|err| Error::Io { - source: err, + source: err.into(), message: "stream all data into tempfile in", path: self.path.to_owned(), })?; @@ -51,13 +51,13 @@ impl gix_object::Write for Store { let mut to = self.dest().map_err(Box::new)?; to.write_all(&gix_object::encode::loose_header(kind, from.len() as u64)) .map_err(|err| Error::Io { - source: err, + source: err.into(), message: "write header to tempfile in", path: self.path.to_owned(), })?; to.write_all(from).map_err(|err| Error::Io { - source: err, + source: err.into(), message: "stream all data into tempfile in", path: self.path.to_owned(), })?; @@ -77,14 +77,14 @@ impl gix_object::Write for Store { let mut to = self.dest().map_err(Box::new)?; to.write_all(&gix_object::encode::loose_header(kind, size)) .map_err(|err| Error::Io { - source: err, + source: err.into(), message: "write header to tempfile in", path: self.path.to_owned(), })?; io::copy(&mut from, &mut to) .map_err(|err| Error::Io { - source: err, + source: err.into(), message: "stream all data into tempfile in", path: self.path.to_owned(), }) @@ -118,7 +118,7 @@ impl Store { } Ok(hasher::io::Write::new( deflate::Write::new(builder.tempfile_in(&self.path).map_err(|err| Error::Io { - source: err, + source: err.into(), message: "create named temp file in", path: self.path.to_owned(), })?), diff --git a/gix-pack/src/data/input/bytes_to_entries.rs b/gix-pack/src/data/input/bytes_to_entries.rs index 76aa6d4098e..42924808c1f 100644 --- a/gix-pack/src/data/input/bytes_to_entries.rs +++ b/gix-pack/src/data/input/bytes_to_entries.rs @@ -1,7 +1,7 @@ use std::{fs, io}; use gix_features::zlib::Decompress; -use gix_hash::{Hasher, ObjectId}; +use gix_hash::{hasher, Hasher, ObjectId}; use crate::data::input; @@ -52,7 +52,7 @@ where object_hash: gix_hash::Kind, ) -> Result, input::Error> { let mut header_data = [0u8; 12]; - read.read_exact(&mut header_data)?; + read.read_exact(&mut header_data).map_err(hasher::io::Error::from)?; let (version, num_objects) = crate::data::header::decode(&header_data)?; assert_eq!( @@ -97,7 +97,7 @@ where } None => crate::data::Entry::from_read(&mut self.read, self.offset, self.hash_len), } - .map_err(input::Error::from)?; + .map_err(hasher::io::Error::from)?; // Decompress object to learn its compressed bytes let compressed_buf = self.compressed_buf.take().unwrap_or_else(|| Vec::with_capacity(4096)); @@ -114,7 +114,7 @@ where decompressor: &mut self.decompressor, }; - let bytes_copied = io::copy(&mut decompressed_reader, &mut io::sink())?; + let bytes_copied = io::copy(&mut decompressed_reader, &mut io::sink()).map_err(hasher::io::Error::from)?; if bytes_copied != entry.decompressed_size { return Err(input::Error::IncompletePack { actual: bytes_copied, @@ -138,7 +138,10 @@ where let crc32 = if self.compressed.crc32() { let mut header_buf = [0u8; 12 + gix_hash::Kind::longest().len_in_bytes()]; - let header_len = entry.header.write_to(bytes_copied, &mut header_buf.as_mut())?; + let header_len = entry + .header + .write_to(bytes_copied, &mut header_buf.as_mut()) + .map_err(hasher::io::Error::from)?; let state = gix_features::hash::crc32_update(0, &header_buf[..header_len]); Some(gix_features::hash::crc32_update(state, &compressed)) } else { @@ -172,7 +175,7 @@ where let mut id = gix_hash::ObjectId::null(self.object_hash); if let Err(err) = self.read.read_exact(id.as_mut_slice()) { if self.mode != input::Mode::Restore { - return Err(err.into()); + return Err(input::Error::Io(err.into())); } } @@ -269,7 +272,8 @@ where impl crate::data::File { /// Returns an iterator over [`Entries`][crate::data::input::Entry], without making use of the memory mapping. pub fn streaming_iter(&self) -> Result, input::Error> { - let reader = io::BufReader::with_capacity(4096 * 8, fs::File::open(&self.path)?); + let reader = + io::BufReader::with_capacity(4096 * 8, fs::File::open(&self.path).map_err(hasher::io::Error::from)?); BytesToEntriesIter::new_from_header( reader, input::Mode::Verify, diff --git a/gix-pack/src/data/input/entries_to_bytes.rs b/gix-pack/src/data/input/entries_to_bytes.rs index d2e0d0f3ee1..2a428349010 100644 --- a/gix-pack/src/data/input/entries_to_bytes.rs +++ b/gix-pack/src/data/input/entries_to_bytes.rs @@ -1,5 +1,7 @@ use std::iter::Peekable; +use gix_hash::hasher; + use crate::data::input; /// An implementation of [`Iterator`] to write [encoded entries][input::Entry] to an inner implementation each time @@ -64,7 +66,7 @@ where self.trailer } - fn next_inner(&mut self, entry: input::Entry) -> Result { + fn next_inner(&mut self, entry: input::Entry) -> Result { if self.num_entries == 0 { let header_bytes = crate::data::header::encode(self.data_version, 0); self.output.write_all(&header_bytes[..])?; @@ -80,7 +82,7 @@ where Ok(entry) } - fn write_header_and_digest(&mut self, last_entry: Option<&mut input::Entry>) -> Result<(), input::Error> { + fn write_header_and_digest(&mut self, last_entry: Option<&mut input::Entry>) -> Result<(), hasher::io::Error> { let header_bytes = crate::data::header::encode(self.data_version, self.num_entries); let num_bytes_written = if last_entry.is_some() { self.output.stream_position()? @@ -127,13 +129,16 @@ where match self.input.next() { Some(res) => Some(match res { - Ok(entry) => self.next_inner(entry).and_then(|mut entry| { - if self.input.peek().is_none() { - self.write_header_and_digest(Some(&mut entry)).map(|_| entry) - } else { - Ok(entry) - } - }), + Ok(entry) => self + .next_inner(entry) + .and_then(|mut entry| { + if self.input.peek().is_none() { + self.write_header_and_digest(Some(&mut entry)).map(|_| entry) + } else { + Ok(entry) + } + }) + .map_err(input::Error::from), Err(err) => { self.is_done = true; Err(err) @@ -141,7 +146,7 @@ where }), None => match self.write_header_and_digest(None) { Ok(_) => None, - Err(err) => Some(Err(err)), + Err(err) => Some(Err(err.into())), }, } } diff --git a/gix-pack/src/data/input/entry.rs b/gix-pack/src/data/input/entry.rs index ad970aa0376..55fb439afc5 100644 --- a/gix-pack/src/data/input/entry.rs +++ b/gix-pack/src/data/input/entry.rs @@ -54,7 +54,7 @@ fn compress_data(obj: &gix_object::Data<'_>) -> Result, input::Error> { let mut out = gix_features::zlib::stream::deflate::Write::new(Vec::new()); if let Err(err) = std::io::copy(&mut &*obj.data, &mut out) { match err.kind() { - std::io::ErrorKind::Other => return Err(input::Error::Io(err)), + std::io::ErrorKind::Other => return Err(input::Error::Io(err.into())), err => { unreachable!("Should never see other errors than zlib, but got {:?}", err) } diff --git a/gix-pack/src/data/input/types.rs b/gix-pack/src/data/input/types.rs index 86ecb6ebdb1..6d066855028 100644 --- a/gix-pack/src/data/input/types.rs +++ b/gix-pack/src/data/input/types.rs @@ -1,4 +1,4 @@ -use std::io; +use gix_hash::hasher; /// Returned by [`BytesToEntriesIter::new_from_header()`][crate::data::input::BytesToEntriesIter::new_from_header()] and as part /// of `Item` of [`BytesToEntriesIter`][crate::data::input::BytesToEntriesIter]. @@ -6,7 +6,7 @@ use std::io; #[allow(missing_docs)] pub enum Error { #[error("An IO operation failed while streaming an entry")] - Io(#[from] io::Error), + Io(#[from] hasher::io::Error), #[error(transparent)] PackParse(#[from] crate::data::header::decode::Error), #[error("Failed to verify pack checksum in trailer")] diff --git a/gix-pack/src/data/output/bytes.rs b/gix-pack/src/data/output/bytes.rs index 689ad383ec9..4c2eb1c60bf 100644 --- a/gix-pack/src/data/output/bytes.rs +++ b/gix-pack/src/data/output/bytes.rs @@ -13,7 +13,7 @@ where E: std::error::Error + 'static, { #[error(transparent)] - Io(#[from] std::io::Error), + Io(#[from] hasher::io::Error), #[error(transparent)] Input(E), } @@ -98,7 +98,9 @@ where let previous_written = self.written; if let Some((version, num_entries)) = self.header_info.take() { let header_bytes = crate::data::header::encode(version, num_entries); - self.output.write_all(&header_bytes[..])?; + self.output + .write_all(&header_bytes[..]) + .map_err(hasher::io::Error::from)?; self.written += header_bytes.len() as u64; } match self.input.next() { @@ -116,15 +118,21 @@ where } self.written - base_offset }); - self.written += header.write_to(entry.decompressed_size as u64, &mut self.output)? as u64; - self.written += std::io::copy(&mut &*entry.compressed_data, &mut self.output)?; + self.written += header + .write_to(entry.decompressed_size as u64, &mut self.output) + .map_err(hasher::io::Error::from)? as u64; + self.written += std::io::copy(&mut &*entry.compressed_data, &mut self.output) + .map_err(hasher::io::Error::from)?; } } None => { let digest = self.output.hash.clone().finalize(); - self.output.inner.write_all(digest.as_slice())?; + self.output + .inner + .write_all(digest.as_slice()) + .map_err(hasher::io::Error::from)?; self.written += digest.as_slice().len() as u64; - self.output.inner.flush()?; + self.output.inner.flush().map_err(hasher::io::Error::from)?; self.is_done = true; self.trailer = Some(digest); } diff --git a/gix-pack/src/index/encode.rs b/gix-pack/src/index/encode.rs index 91f03ab3bcb..2c8c4748239 100644 --- a/gix-pack/src/index/encode.rs +++ b/gix-pack/src/index/encode.rs @@ -74,7 +74,7 @@ mod function { pack_hash: &gix_hash::ObjectId, kind: crate::index::Version, progress: &mut dyn DynNestedProgress, - ) -> io::Result { + ) -> Result { use io::Write; assert_eq!(kind, crate::index::Version::V2, "Can only write V2 packs right now"); assert!( @@ -136,7 +136,7 @@ mod function { out.write_all(pack_hash.as_slice())?; let bytes_written_without_trailer = out.bytes; - let out = out.inner.into_inner()?; + let out = out.inner.into_inner().map_err(io::Error::from)?; let index_hash = out.hash.finalize(); out.inner.write_all(index_hash.as_slice())?; out.inner.flush()?; diff --git a/gix-pack/src/index/write/error.rs b/gix-pack/src/index/write/error.rs index a5ef6ad67b6..2ac0f710225 100644 --- a/gix-pack/src/index/write/error.rs +++ b/gix-pack/src/index/write/error.rs @@ -1,11 +1,11 @@ -use std::io; +use gix_hash::hasher; /// Returned by [`crate::index::File::write_data_iter_to_stream()`] #[derive(thiserror::Error, Debug)] #[allow(missing_docs)] pub enum Error { - #[error("An IO error occurred when reading the pack or creating a temporary file")] - Io(#[from] io::Error), + #[error("An error occurred when writing the pack index file")] + Io(#[from] hasher::io::Error), #[error("A pack entry could not be extracted")] PackEntryDecode(#[from] crate::data::input::Error), #[error("Indices of type {} cannot be written, only {} are supported", *.0 as usize, crate::index::Version::default() as usize)] diff --git a/gix-pack/src/index/write/mod.rs b/gix-pack/src/index/write/mod.rs index 49ffe14184a..9b69252e112 100644 --- a/gix-pack/src/index/write/mod.rs +++ b/gix-pack/src/index/write/mod.rs @@ -2,6 +2,7 @@ use std::{io, sync::atomic::AtomicBool}; pub use error::Error; use gix_features::progress::{self, prodash::DynNestedProgress, Count, Progress}; +use gix_hash::hasher; use crate::cache::delta::{traverse, Tree}; @@ -180,7 +181,7 @@ impl crate::index::File { root_progress.inc(); - let (resolver, pack) = make_resolver()?; + let (resolver, pack) = make_resolver().map_err(hasher::io::Error::from)?; let sorted_pack_offsets_by_oid = { let traverse::Outcome { roots, children } = tree.traverse( resolver, diff --git a/gix-pack/src/multi_index/write.rs b/gix-pack/src/multi_index/write.rs index 887ac14aa0c..59bc21a392d 100644 --- a/gix-pack/src/multi_index/write.rs +++ b/gix-pack/src/multi_index/write.rs @@ -15,7 +15,7 @@ mod error { #[allow(missing_docs)] pub enum Error { #[error(transparent)] - Io(#[from] std::io::Error), + Io(#[from] gix_hash::hasher::io::Error), #[error("Interrupted")] Interrupted, #[error(transparent)] @@ -182,30 +182,34 @@ impl multi_index::File { cf.num_chunks().try_into().expect("BUG: wrote more than 256 chunks"), index_paths_sorted.len() as u32, object_hash, - )?; + ) + .map_err(hasher::io::Error::from)?; { progress.set_name("Writing chunks".into()); progress.init(Some(cf.num_chunks()), gix_features::progress::count("chunks")); - let mut chunk_write = cf.into_write(&mut out, bytes_written)?; + let mut chunk_write = cf + .into_write(&mut out, bytes_written) + .map_err(hasher::io::Error::from)?; while let Some(chunk_to_write) = chunk_write.next_chunk() { match chunk_to_write { multi_index::chunk::index_names::ID => { - multi_index::chunk::index_names::write(&index_filenames_sorted, &mut chunk_write)?; + multi_index::chunk::index_names::write(&index_filenames_sorted, &mut chunk_write) } - multi_index::chunk::fanout::ID => multi_index::chunk::fanout::write(&entries, &mut chunk_write)?, - multi_index::chunk::lookup::ID => multi_index::chunk::lookup::write(&entries, &mut chunk_write)?, + multi_index::chunk::fanout::ID => multi_index::chunk::fanout::write(&entries, &mut chunk_write), + multi_index::chunk::lookup::ID => multi_index::chunk::lookup::write(&entries, &mut chunk_write), multi_index::chunk::offsets::ID => { - multi_index::chunk::offsets::write(&entries, num_large_offsets.is_some(), &mut chunk_write)?; + multi_index::chunk::offsets::write(&entries, num_large_offsets.is_some(), &mut chunk_write) } multi_index::chunk::large_offsets::ID => multi_index::chunk::large_offsets::write( &entries, num_large_offsets.expect("available if planned"), &mut chunk_write, - )?, + ), unknown => unreachable!("BUG: forgot to implement chunk {:?}", std::str::from_utf8(&unknown)), } + .map_err(hasher::io::Error::from)?; progress.inc(); if should_interrupt.load(Ordering::Relaxed) { return Err(Error::Interrupted); @@ -215,7 +219,10 @@ impl multi_index::File { // write trailing checksum let multi_index_checksum = out.inner.hash.finalize(); - out.inner.inner.write_all(multi_index_checksum.as_slice())?; + out.inner + .inner + .write_all(multi_index_checksum.as_slice()) + .map_err(hasher::io::Error::from)?; out.progress.show_throughput(write_start); Ok(Outcome { multi_index_checksum }) diff --git a/gix-pack/src/verify.rs b/gix-pack/src/verify.rs index 75704c68746..9f8ed7da297 100644 --- a/gix-pack/src/verify.rs +++ b/gix-pack/src/verify.rs @@ -1,6 +1,7 @@ use std::{path::Path, sync::atomic::AtomicBool}; use gix_features::progress::Progress; +use gix_hash::hasher; /// pub mod checksum { @@ -42,8 +43,10 @@ pub fn checksum_on_disk_or_mmap( should_interrupt, ) { Ok(id) => id, - Err(err) if err.kind() == std::io::ErrorKind::Interrupted => return Err(checksum::Error::Interrupted), - Err(_io_err) => { + Err(hasher::io::Error::Io(err)) if err.kind() == std::io::ErrorKind::Interrupted => { + return Err(checksum::Error::Interrupted); + } + Err(hasher::io::Error::Io(_io_err)) => { let start = std::time::Instant::now(); let mut hasher = gix_hash::hasher(object_hash); hasher.update(&data[..data_len_without_trailer]); diff --git a/gix-status/src/index_as_worktree/function.rs b/gix-status/src/index_as_worktree/function.rs index c6edc032d69..d7c8c3031ce 100644 --- a/gix-status/src/index_as_worktree/function.rs +++ b/gix-status/src/index_as_worktree/function.rs @@ -9,6 +9,7 @@ use bstr::BStr; use filetime::FileTime; use gix_features::parallel::{in_parallel_if, Reduce}; use gix_filter::pipeline::convert::ToGitOutcome; +use gix_hash::hasher; use gix_object::FindExt; use crate::index_as_worktree::Context; @@ -359,7 +360,7 @@ impl<'index> State<'_, 'index> { Err(err) if gix_fs::io_err::is_not_found(err.kind(), err.raw_os_error()) => { return Ok(Some(Change::Removed.into())) } - Err(err) => return Err(err.into()), + Err(err) => return Err(Error::Io(err.into())), }; self.symlink_metadata_calls.fetch_add(1, Ordering::Relaxed); let metadata = match gix_index::fs::Metadata::from_path_no_follow(worktree_path) { @@ -385,7 +386,7 @@ impl<'index> State<'_, 'index> { return Ok(Some(Change::Removed.into())) } Err(err) => { - return Err(err.into()); + return Err(Error::Io(err.into())); } }; if entry.flags.contains(gix_index::entry::Flags::INTENT_TO_ADD) { @@ -562,8 +563,9 @@ where self.buf.clear(); let platform = self .attr_stack - .at_entry(self.rela_path, Some(self.entry.mode), &self.objects)?; - let file = std::fs::File::open(self.path)?; + .at_entry(self.rela_path, Some(self.entry.mode), &self.objects) + .map_err(hasher::io::Error::from)?; + let file = std::fs::File::open(self.path).map_err(hasher::io::Error::from)?; let out = self .filter .convert_to_git( @@ -574,7 +576,7 @@ where }, &mut |buf| Ok(self.objects.find_blob(self.id, buf).map(|_| Some(()))?), ) - .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; + .map_err(|err| Error::Io(io::Error::new(io::ErrorKind::Other, err).into()))?; let len = match out { ToGitOutcome::Unchanged(_) => Some(self.file_len), ToGitOutcome::Process(_) | ToGitOutcome::Buffer(_) => None, diff --git a/gix-status/src/index_as_worktree/traits.rs b/gix-status/src/index_as_worktree/traits.rs index e5224f38609..ba23be03d86 100644 --- a/gix-status/src/index_as_worktree/traits.rs +++ b/gix-status/src/index_as_worktree/traits.rs @@ -1,7 +1,7 @@ use std::{io::Read, sync::atomic::AtomicBool}; use bstr::BStr; -use gix_hash::ObjectId; +use gix_hash::{hasher, ObjectId}; use gix_index as index; use index::Entry; @@ -149,7 +149,7 @@ impl CompareBlobs for HashEq { None => { let file_hash = match stream.size() { None => { - stream.read_to_end(buf)?; + stream.read_to_end(buf).map_err(hasher::io::Error::from)?; gix_object::compute_hash(entry.id.kind(), gix_object::Kind::Blob, buf) } Some(len) => gix_object::compute_stream_hash( diff --git a/gix-status/src/index_as_worktree/types.rs b/gix-status/src/index_as_worktree/types.rs index f38f131e74d..d3d526cb1aa 100644 --- a/gix-status/src/index_as_worktree/types.rs +++ b/gix-status/src/index_as_worktree/types.rs @@ -10,7 +10,7 @@ pub enum Error { #[error("The clock was off when reading file related metadata after updating a file on disk")] Time(#[from] std::time::SystemTimeError), #[error("IO error while writing blob or reading file metadata or changing filetype")] - Io(#[from] std::io::Error), + Io(#[from] gix_hash::hasher::io::Error), #[error("Failed to obtain blob from object database")] Find(#[from] gix_object::find::existing_object::Error), #[error("Could not determine status for submodule at '{rela_path}'")] diff --git a/gix-status/src/index_as_worktree_with_renames/mod.rs b/gix-status/src/index_as_worktree_with_renames/mod.rs index 8368102c246..58474af95ba 100644 --- a/gix-status/src/index_as_worktree_with_renames/mod.rs +++ b/gix-status/src/index_as_worktree_with_renames/mod.rs @@ -539,7 +539,7 @@ pub(super) mod function { ToGitOutcome::Buffer(buf) => gix_object::compute_hash(object_hash, gix_object::Kind::Blob, buf), ToGitOutcome::Process(mut stream) => { buf.clear(); - stream.read_to_end(buf).map_err(Error::HashFile)?; + stream.read_to_end(buf).map_err(|err| Error::HashFile(err.into()))?; gix_object::compute_hash(object_hash, gix_object::Kind::Blob, buf) } } diff --git a/gix-status/src/index_as_worktree_with_renames/types.rs b/gix-status/src/index_as_worktree_with_renames/types.rs index add7717897e..8a4cdb614dd 100644 --- a/gix-status/src/index_as_worktree_with_renames/types.rs +++ b/gix-status/src/index_as_worktree_with_renames/types.rs @@ -17,7 +17,7 @@ pub enum Error { #[error("Could not open worktree file for reading")] OpenWorktreeFile(std::io::Error), #[error(transparent)] - HashFile(std::io::Error), + HashFile(gix_hash::hasher::io::Error), #[error("Could not read worktree link content")] ReadLink(std::io::Error), #[error(transparent)] From 5095f44db58014f4a35ea8996a90d56d2ac19d45 Mon Sep 17 00:00:00 2001 From: Emily Date: Sat, 22 Mar 2025 19:09:23 +0000 Subject: [PATCH 15/22] change!: adjust error return types to handle collision detection This does mean a lot of churn across the tree, but the change is usually just an adjustment to variants of an existing error type, so I expect that most downstream users will require little to no adaption for this change. --- gix-commitgraph/src/file/verify.rs | 2 ++ gix-filter/src/ident.rs | 2 ++ gix-hash/src/hasher/io.rs | 2 ++ gix-index/src/decode/mod.rs | 4 +++- .../src/extension/end_of_index_entry/decode.rs | 16 ++++++++-------- .../src/extension/end_of_index_entry/write.rs | 2 +- gix-index/src/file/init.rs | 1 + gix-object/src/data.rs | 2 ++ gix-odb/src/store_impls/loose/verify.rs | 7 +++++++ gix-pack/src/verify.rs | 2 ++ 10 files changed, 30 insertions(+), 10 deletions(-) diff --git a/gix-commitgraph/src/file/verify.rs b/gix-commitgraph/src/file/verify.rs index 79e93f88b6f..8d3a0f731ea 100644 --- a/gix-commitgraph/src/file/verify.rs +++ b/gix-commitgraph/src/file/verify.rs @@ -45,6 +45,8 @@ pub mod checksum { #[derive(thiserror::Error, Debug)] #[allow(missing_docs)] pub enum Error { + #[error("failed to hash commit graph file")] + Hasher(#[from] gix_hash::hasher::Error), #[error(transparent)] Verify(#[from] gix_hash::verify::Error), } diff --git a/gix-filter/src/ident.rs b/gix-filter/src/ident.rs index e915ace3c46..f050922d606 100644 --- a/gix-filter/src/ident.rs +++ b/gix-filter/src/ident.rs @@ -48,6 +48,8 @@ pub mod apply { pub enum Error { #[error("Could not allocate buffer")] OutOfMemory(#[from] std::collections::TryReserveError), + #[error("Could not hash blob")] + Hasher(#[from] gix_hash::hasher::Error), } } diff --git a/gix-hash/src/hasher/io.rs b/gix-hash/src/hasher/io.rs index 2561382ffaa..0c1883d3fed 100644 --- a/gix-hash/src/hasher/io.rs +++ b/gix-hash/src/hasher/io.rs @@ -6,6 +6,8 @@ use crate::{hasher, Hasher}; pub enum Error { #[error(transparent)] Io(#[from] std::io::Error), + #[error("Failed to hash data")] + Hasher(#[from] hasher::Error), } /// Compute the hash of `kind` for the bytes in the file at `path`, hashing only the first `num_bytes_from_start` diff --git a/gix-index/src/decode/mod.rs b/gix-index/src/decode/mod.rs index 708df929ad1..4872c7e23fc 100644 --- a/gix-index/src/decode/mod.rs +++ b/gix-index/src/decode/mod.rs @@ -16,6 +16,8 @@ mod error { pub enum Error { #[error(transparent)] Header(#[from] decode::header::Error), + #[error("Could not hash index data")] + Hasher(#[from] gix_hash::hasher::Error), #[error("Could not parse entry at index {index}")] Entry { index: u32 }, #[error("Mandatory extension wasn't implemented or malformed.")] @@ -64,7 +66,7 @@ impl State { ) -> Result<(Self, Option), Error> { let _span = gix_features::trace::detail!("gix_index::State::from_bytes()", options = ?_options); let (version, num_entries, post_header_data) = header::decode(data, object_hash)?; - let start_of_extensions = extension::end_of_index_entry::decode(data, object_hash); + let start_of_extensions = extension::end_of_index_entry::decode(data, object_hash)?; let mut num_threads = gix_features::parallel::num_threads(thread_limit); let path_backing_buffer_size = entries::estimate_path_storage_requirements_in_bytes( diff --git a/gix-index/src/extension/end_of_index_entry/decode.rs b/gix-index/src/extension/end_of_index_entry/decode.rs index 89b1c341a22..a3bb30cb6b8 100644 --- a/gix-index/src/extension/end_of_index_entry/decode.rs +++ b/gix-index/src/extension/end_of_index_entry/decode.rs @@ -13,10 +13,10 @@ use crate::{ /// stored prior to this one to assure they are correct. /// /// If the checksum wasn't matched, we will ignore this extension entirely. -pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option { +pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Result, gix_hash::hasher::Error> { let hash_len = object_hash.len_in_bytes(); if data.len() < MIN_SIZE_WITH_HEADER + hash_len { - return None; + return Ok(None); } let start_of_eoie = data.len() - MIN_SIZE_WITH_HEADER - hash_len; @@ -24,16 +24,16 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option { let (signature, ext_size, ext_data) = extension::decode::header(ext_data); if signature != SIGNATURE || ext_size as usize != MIN_SIZE { - return None; + return Ok(None); } let (offset, checksum) = ext_data.split_at(4); let offset = from_be_u32(offset) as usize; let Ok(checksum) = gix_hash::oid::try_from_bytes(checksum) else { - return None; + return Ok(None); }; if offset < header::SIZE || offset > start_of_eoie || checksum.kind() != gix_hash::Kind::Sha1 { - return None; + return Ok(None); } let mut hasher = gix_hash::hasher(gix_hash::Kind::Sha1); @@ -45,12 +45,12 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option { } if hasher.finalize().verify(checksum).is_err() { - return None; + return Ok(None); } // The last-to-this chunk ends where ours starts if last_chunk.map_or(true, |s| s.as_ptr_range().end != (&data[start_of_eoie]) as *const _) { - return None; + return Ok(None); } - Some(offset) + Ok(Some(offset)) } diff --git a/gix-index/src/extension/end_of_index_entry/write.rs b/gix-index/src/extension/end_of_index_entry/write.rs index ad432cc3ee4..acf3561352a 100644 --- a/gix-index/src/extension/end_of_index_entry/write.rs +++ b/gix-index/src/extension/end_of_index_entry/write.rs @@ -11,7 +11,7 @@ pub fn write_to( hash_kind: gix_hash::Kind, offset_to_extensions: u32, prior_extensions: impl IntoIterator, -) -> Result<(), std::io::Error> { +) -> Result<(), gix_hash::hasher::io::Error> { out.write_all(&SIGNATURE)?; let extension_size: u32 = 4 + hash_kind.len_in_bytes() as u32; out.write_all(&extension_size.to_be_bytes())?; diff --git a/gix-index/src/file/init.rs b/gix-index/src/file/init.rs index 6957fc77a37..8590f91d7e7 100644 --- a/gix-index/src/file/init.rs +++ b/gix-index/src/file/init.rs @@ -86,6 +86,7 @@ impl File { ) .map_err(|err| match err { hasher::io::Error::Io(err) => Error::Io(err), + hasher::io::Error::Hasher(err) => Error::Decode(err.into()), })? .verify(&expected) .map_err(decode::Error::from)?; diff --git a/gix-object/src/data.rs b/gix-object/src/data.rs index 8251ed6eaa5..5dd12906a19 100644 --- a/gix-object/src/data.rs +++ b/gix-object/src/data.rs @@ -55,6 +55,8 @@ pub mod verify { #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { + #[error("Failed to hash object")] + Hasher(#[from] gix_hash::hasher::Error), #[error(transparent)] Verify(#[from] gix_hash::verify::Error), } diff --git a/gix-odb/src/store_impls/loose/verify.rs b/gix-odb/src/store_impls/loose/verify.rs index 0568a38db9e..607b57aef13 100644 --- a/gix-odb/src/store_impls/loose/verify.rs +++ b/gix-odb/src/store_impls/loose/verify.rs @@ -19,6 +19,13 @@ pub mod integrity { kind: gix_object::Kind, id: gix_hash::ObjectId, }, + #[error("{kind} object {expected} could not be hashed")] + ObjectHasher { + #[source] + source: gix_hash::hasher::Error, + kind: gix_object::Kind, + expected: gix_hash::ObjectId, + }, #[error("{kind} object wasn't re-encoded without change")] ObjectEncodeMismatch { #[source] diff --git a/gix-pack/src/verify.rs b/gix-pack/src/verify.rs index 9f8ed7da297..108552dbb6a 100644 --- a/gix-pack/src/verify.rs +++ b/gix-pack/src/verify.rs @@ -11,6 +11,8 @@ pub mod checksum { pub enum Error { #[error("Interrupted by user")] Interrupted, + #[error("Failed to hash data")] + Hasher(#[from] gix_hash::hasher::Error), #[error(transparent)] Verify(#[from] gix_hash::verify::Error), } From f2b07c0783ac98a08f648836e743cb0e6b4fdc7c Mon Sep 17 00:00:00 2001 From: Emily Date: Sat, 22 Mar 2025 19:09:23 +0000 Subject: [PATCH 16/22] add fallible `gix_object::try_compute_hash` for migration --- gix-object/src/lib.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index 96843309902..d35f7c71f61 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -412,6 +412,17 @@ pub fn compute_hash(hash_kind: gix_hash::Kind, object_kind: Kind, data: &[u8]) - hasher.finalize() } +/// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its `data`. +pub fn try_compute_hash( + hash_kind: gix_hash::Kind, + object_kind: Kind, + data: &[u8], +) -> Result { + let mut hasher = object_hasher(hash_kind, object_kind, data.len() as u64); + hasher.update(data); + hasher.try_finalize() +} + /// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its data read from `stream` /// which has to yield exactly `stream_len` bytes. /// Use `progress` to learn about progress in bytes processed and `should_interrupt` to be able to abort the operation From fbf6cc897cfeff5ed2a2d5946c060e0cebbd1afd Mon Sep 17 00:00:00 2001 From: Emily Date: Sat, 22 Mar 2025 19:09:23 +0000 Subject: [PATCH 17/22] migrate hashing API users to fallible versions --- gix-commitgraph/src/file/verify.rs | 2 +- gix-diff/tests/diff/blob/pipeline.rs | 28 +++++++++---------- gix-diff/tests/diff/blob/platform.rs | 8 +++--- gix-diff/tests/diff/main.rs | 6 ++-- gix-diff/tests/diff/rewrites/tracker.rs | 22 +++++++-------- gix-filter/src/ident.rs | 2 +- gix-hash/src/hasher/io.rs | 2 +- .../extension/end_of_index_entry/decode.rs | 2 +- .../src/extension/end_of_index_entry/write.rs | 2 +- gix-index/src/file/write.rs | 2 +- gix-merge/tests/merge/blob/mod.rs | 6 ++-- gix-merge/tests/merge/blob/pipeline.rs | 12 ++++---- gix-merge/tests/merge/blob/platform.rs | 10 +++---- gix-object/benches/edit_tree.rs | 2 +- gix-object/src/data.rs | 2 +- gix-object/tests/object/main.rs | 4 +-- gix-object/tests/object/tree/editor.rs | 2 +- gix-odb/src/memory.rs | 2 +- gix-odb/src/sink.rs | 2 +- gix-odb/src/store_impls/loose/verify.rs | 6 +++- gix-odb/src/store_impls/loose/write.rs | 6 +++- gix-pack/src/data/input/bytes_to_entries.rs | 4 +-- gix-pack/src/data/output/bytes.rs | 7 ++++- gix-pack/src/index/encode.rs | 2 +- gix-pack/src/index/write/mod.rs | 17 ++++++----- gix-pack/src/multi_index/write.rs | 2 +- gix-pack/src/verify.rs | 3 +- gix-status/src/index_as_worktree/traits.rs | 6 ++-- .../src/index_as_worktree_with_renames/mod.rs | 11 ++++++-- gix/src/repository/impls.rs | 2 +- gix/src/repository/object.rs | 9 ++++-- tests/it/src/commands/git_to_sh.rs | 6 +++- 32 files changed, 115 insertions(+), 84 deletions(-) diff --git a/gix-commitgraph/src/file/verify.rs b/gix-commitgraph/src/file/verify.rs index 8d3a0f731ea..cbcde73f743 100644 --- a/gix-commitgraph/src/file/verify.rs +++ b/gix-commitgraph/src/file/verify.rs @@ -156,7 +156,7 @@ impl File { let data_len_without_trailer = self.data.len() - self.hash_len; let mut hasher = gix_hash::hasher(self.object_hash()); hasher.update(&self.data[..data_len_without_trailer]); - let actual = hasher.finalize(); + let actual = hasher.try_finalize()?; actual.verify(self.checksum())?; Ok(actual) } diff --git a/gix-diff/tests/diff/blob/pipeline.rs b/gix-diff/tests/diff/blob/pipeline.rs index 664e080bf3d..d042e8fda51 100644 --- a/gix-diff/tests/diff/blob/pipeline.rs +++ b/gix-diff/tests/diff/blob/pipeline.rs @@ -72,7 +72,7 @@ pub(crate) mod convert_to_diffable { let mut db = ObjectDb::default(); let b_content = "b-content"; - let id = db.insert(b_content); + let id = db.insert(b_content)?; let out = filter.convert_to_diffable( &id, @@ -129,7 +129,7 @@ pub(crate) mod convert_to_diffable { assert_eq!(buf.len(), 0, "it should avoid querying that data in the first place"); let mut db = ObjectDb::default(); - let id = db.insert(large_content); + let id = db.insert(large_content)?; let out = filter.convert_to_diffable( &id, EntryKind::Blob, @@ -211,7 +211,7 @@ pub(crate) mod convert_to_diffable { drop(tmp); let mut db = ObjectDb::default(); - let id = db.insert(large_content); + let id = db.insert(large_content)?; let out = filter.convert_to_diffable( &id, @@ -397,7 +397,7 @@ pub(crate) mod convert_to_diffable { let mut db = ObjectDb::default(); let b_content = "b-content\n"; - let id = db.insert(b_content); + let id = db.insert(b_content)?; let out = filter.convert_to_diffable( &id, @@ -416,7 +416,7 @@ pub(crate) mod convert_to_diffable { let mut db = ObjectDb::default(); let b_content = "b\n"; - let id = db.insert(b_content); + let id = db.insert(b_content)?; let out = filter.convert_to_diffable( &id, EntryKind::Blob, @@ -490,7 +490,7 @@ pub(crate) mod convert_to_diffable { let mut db = ObjectDb::default(); let b_content = "b-co\0ntent\n"; - let id = db.insert(b_content); + let id = db.insert(b_content)?; let out = filter.convert_to_diffable( &id, @@ -509,7 +509,7 @@ pub(crate) mod convert_to_diffable { let platform = attributes.at_entry("c", None, &gix_object::find::Never)?; - let id = db.insert("b"); + let id = db.insert("b")?; let out = filter.convert_to_diffable( &id, EntryKind::Blob, @@ -638,7 +638,7 @@ pub(crate) mod convert_to_diffable { assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false })); assert_eq!(buf.as_bstr(), "a\n", "unconditionally use git according to mode"); - let id = db.insert("a\n"); + let id = db.insert("a\n")?; for mode in worktree_modes { let out = filter.convert_to_diffable( &id, @@ -714,7 +714,7 @@ pub(crate) mod convert_to_diffable { assert_eq!(buf.len(), 0, "always cleared"); buf.push(1); - let id = db.insert("link-target"); + let id = db.insert("link-target")?; let out = filter.convert_to_diffable( &id, EntryKind::Link, @@ -761,7 +761,7 @@ pub(crate) mod convert_to_diffable { assert_eq!(buf.len(), 0, "it's always cleared before any potential use"); } - let id = db.insert("b\n"); + let id = db.insert("b\n")?; for mode in all_modes { buf.push(1); let out = filter.convert_to_diffable( @@ -814,7 +814,7 @@ pub(crate) mod convert_to_diffable { ); } - let id = db.insert("c\n"); + let id = db.insert("c\n")?; for mode in worktree_modes { let out = filter.convert_to_diffable( &id, @@ -863,7 +863,7 @@ pub(crate) mod convert_to_diffable { assert_eq!(buf.len(), 0); } - let id = db.insert("unset\n"); + let id = db.insert("unset\n")?; for mode in all_modes { let out = filter.convert_to_diffable( &id, @@ -890,7 +890,7 @@ pub(crate) mod convert_to_diffable { } let platform = attributes.at_entry("d", None, &gix_object::find::Never)?; - let id = db.insert("d-in-db"); + let id = db.insert("d-in-db")?; for mode in worktree_modes { let out = filter.convert_to_diffable( &null, @@ -959,7 +959,7 @@ pub(crate) mod convert_to_diffable { "no text filter, so git conversion was applied for worktree source" ); - let id = db.insert("e-in-db"); + let id = db.insert("e-in-db")?; let out = filter.convert_to_diffable( &id, EntryKind::Blob, diff --git a/gix-diff/tests/diff/blob/platform.rs b/gix-diff/tests/diff/blob/platform.rs index 6ab3a3dbc68..6976d805270 100644 --- a/gix-diff/tests/diff/blob/platform.rs +++ b/gix-diff/tests/diff/blob/platform.rs @@ -30,7 +30,7 @@ fn resources_of_worktree_and_odb_and_check_link() -> crate::Result { let mut db = ObjectDb::default(); let a_content = "a-content"; - let id = db.insert(a_content); + let id = db.insert(a_content)?; platform.set_resource( id, EntryKind::BlobExecutable, @@ -194,7 +194,7 @@ fn diff_binary() -> crate::Result { let mut db = ObjectDb::default(); let a_content = "b"; - let id = db.insert(a_content); + let id = db.insert(a_content)?; platform.set_resource(id, EntryKind::Blob, "b".into(), ResourceKind::NewOrDestination, &db)?; let out = platform.prepare_diff()?; @@ -235,7 +235,7 @@ fn diff_performed_despite_external_command() -> crate::Result { let mut db = ObjectDb::default(); let a_content = "b"; - let id = db.insert(a_content); + let id = db.insert(a_content)?; platform.set_resource(id, EntryKind::Blob, "b".into(), ResourceKind::NewOrDestination, &db)?; let out = platform.prepare_diff()?; @@ -277,7 +277,7 @@ fn diff_skipped_due_to_external_command_and_enabled_option() -> crate::Result { let mut db = ObjectDb::default(); let a_content = "b"; - let id = db.insert(a_content); + let id = db.insert(a_content)?; platform.set_resource(id, EntryKind::Blob, "b".into(), ResourceKind::NewOrDestination, &db)?; let out = platform.prepare_diff()?; diff --git a/gix-diff/tests/diff/main.rs b/gix-diff/tests/diff/main.rs index a6be530ffdb..1c736ec2e3c 100644 --- a/gix-diff/tests/diff/main.rs +++ b/gix-diff/tests/diff/main.rs @@ -51,10 +51,10 @@ mod util { impl ObjectDb { /// Insert `data` and return its hash. That can be used to find it again. - pub fn insert(&mut self, data: &str) -> gix_hash::ObjectId { - let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Blob, data.as_bytes()); + pub fn insert(&mut self, data: &str) -> Result { + let id = gix_object::try_compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Blob, data.as_bytes())?; self.data_by_id.insert(id, data.into()); - id + Ok(id) } } } diff --git a/gix-diff/tests/diff/rewrites/tracker.rs b/gix-diff/tests/diff/rewrites/tracker.rs index e010a9c81f8..9469494e89a 100644 --- a/gix-diff/tests/diff/rewrites/tracker.rs +++ b/gix-diff/tests/diff/rewrites/tracker.rs @@ -92,7 +92,7 @@ fn copy_by_similarity_reports_limit_if_encountered() -> crate::Result { (Change::addition(), "a-cpy-2", "a"), (Change::modification(), "d", "ab"), ], - ); + )?; let mut calls = 0; let out = util::assert_emit_with_objects( @@ -145,7 +145,7 @@ fn copy_by_id() -> crate::Result { (Change::addition(), "a-cpy-2", "a"), (Change::modification(), "d", "a"), ], - ); + )?; let mut calls = 0; let out = util::assert_emit_with_objects( @@ -218,7 +218,7 @@ fn copy_by_id_search_in_all_sources() -> crate::Result { (Change::addition(), "a-cpy-1", "a"), (Change::addition(), "a-cpy-2", "a"), ], - ); + )?; let mut calls = 0; let content_id = hex_to_id("2e65efe2a145dda7ee51d1741299f848e5bf752e"); @@ -299,7 +299,7 @@ fn copy_by_50_percent_similarity() -> crate::Result { (Change::addition(), "a-cpy-2", "a\nc"), (Change::modification(), "d", "a"), ], - ); + )?; let mut calls = 0; let out = util::assert_emit_with_objects( @@ -377,7 +377,7 @@ fn copy_by_id_in_additions_only() -> crate::Result { (Change::modification(), "a", "a"), (Change::modification(), "a-cpy-1", "a"), ], - ); + )?; let mut calls = 0; let out = util::assert_emit_with_objects( @@ -429,7 +429,7 @@ fn rename_by_similarity_reports_limit_if_encountered() -> crate::Result { (Change::addition(), "b", "firt\nsecond\n"), (Change::addition(), "c", "second\nunrelated\n"), ], - ); + )?; let mut calls = 0; let out = util::assert_emit_with_objects( @@ -475,7 +475,7 @@ fn rename_by_50_percent_similarity() -> crate::Result { (Change::addition(), "b", "firt\nsecond\n"), (Change::addition(), "c", "second\nunrelated\n"), ], - ); + )?; let mut calls = 0; let out = util::assert_emit_with_objects( @@ -596,7 +596,7 @@ fn directory_renames_by_id_can_fail_gracefully() -> crate::Result { (Change::deletion(), "a", "first\nsecond\n"), (Change::addition(), "b", "firt\nsecond\n"), ], - ); + )?; let mut calls = 0; let out = util::assert_emit_with_objects( @@ -803,16 +803,16 @@ mod util { pub fn add_retained_blobs<'a>( tracker: &mut rewrites::Tracker, blobs: impl IntoIterator, - ) -> ObjectDb { + ) -> crate::Result { let mut db = ObjectDb::default(); for (mut change, location, data) in blobs { - change.id = db.insert(data); + change.id = db.insert(data)?; assert!( tracker.try_push_change(change, location.into()).is_none(), "input changes must be tracked" ); } - db + Ok(db) } pub fn assert_emit( diff --git a/gix-filter/src/ident.rs b/gix-filter/src/ident.rs index f050922d606..3faee19b1a7 100644 --- a/gix-filter/src/ident.rs +++ b/gix-filter/src/ident.rs @@ -68,7 +68,7 @@ pub fn apply(src: &[u8], object_hash: gix_hash::Kind, buf: &mut Vec) -> Resu while let Some(pos) = src[ofs..].find(b"$Id$") { let id = match id { None => { - let new_id = gix_object::compute_hash(object_hash, gix_object::Kind::Blob, src); + let new_id = gix_object::try_compute_hash(object_hash, gix_object::Kind::Blob, src)?; id = new_id.into(); clear_and_set_capacity(buf, src.len() + HASH_LEN)?; // pre-allocate for one ID new_id diff --git a/gix-hash/src/hasher/io.rs b/gix-hash/src/hasher/io.rs index 0c1883d3fed..0ec51b2308b 100644 --- a/gix-hash/src/hasher/io.rs +++ b/gix-hash/src/hasher/io.rs @@ -76,7 +76,7 @@ pub fn bytes_with_hasher( } } - let id = hasher.finalize(); + let id = hasher.try_finalize()?; progress.show_throughput(start); Ok(id) } diff --git a/gix-index/src/extension/end_of_index_entry/decode.rs b/gix-index/src/extension/end_of_index_entry/decode.rs index a3bb30cb6b8..51fb1702bfa 100644 --- a/gix-index/src/extension/end_of_index_entry/decode.rs +++ b/gix-index/src/extension/end_of_index_entry/decode.rs @@ -44,7 +44,7 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Result, last_chunk = Some(chunk); } - if hasher.finalize().verify(checksum).is_err() { + if hasher.try_finalize()?.verify(checksum).is_err() { return Ok(None); } // The last-to-this chunk ends where ours starts diff --git a/gix-index/src/extension/end_of_index_entry/write.rs b/gix-index/src/extension/end_of_index_entry/write.rs index acf3561352a..9c7a3afa015 100644 --- a/gix-index/src/extension/end_of_index_entry/write.rs +++ b/gix-index/src/extension/end_of_index_entry/write.rs @@ -23,7 +23,7 @@ pub fn write_to( hasher.update(&signature); hasher.update(&size.to_be_bytes()); } - out.write_all(hasher.finalize().as_slice())?; + out.write_all(hasher.try_finalize()?.as_slice())?; Ok(()) } diff --git a/gix-index/src/file/write.rs b/gix-index/src/file/write.rs index a942ece5586..78f7b13ecd9 100644 --- a/gix-index/src/file/write.rs +++ b/gix-index/src/file/write.rs @@ -31,7 +31,7 @@ impl File { let mut hasher = hasher::io::Write::new(&mut out, self.state.object_hash); let out: &mut dyn std::io::Write = &mut hasher; let version = self.state.write_to(out, options)?; - (version, hasher.hash.finalize()) + (version, hasher.hash.try_finalize()?) }; out.write_all(hash.as_slice())?; Ok((version, hash)) diff --git a/gix-merge/tests/merge/blob/mod.rs b/gix-merge/tests/merge/blob/mod.rs index 57d9205d79a..c7695b81547 100644 --- a/gix-merge/tests/merge/blob/mod.rs +++ b/gix-merge/tests/merge/blob/mod.rs @@ -43,10 +43,10 @@ mod util { impl ObjectDb { /// Insert `data` and return its hash. That can be used to find it again. - pub fn insert(&mut self, data: &str) -> gix_hash::ObjectId { - let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Blob, data.as_bytes()); + pub fn insert(&mut self, data: &str) -> Result { + let id = gix_object::try_compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Blob, data.as_bytes())?; self.data_by_id.insert(id, data.into()); - id + Ok(id) } } } diff --git a/gix-merge/tests/merge/blob/pipeline.rs b/gix-merge/tests/merge/blob/pipeline.rs index 080a9d601f6..86c856808db 100644 --- a/gix-merge/tests/merge/blob/pipeline.rs +++ b/gix-merge/tests/merge/blob/pipeline.rs @@ -67,7 +67,7 @@ fn without_transformation() -> crate::Result { let mut db = ObjectDb::default(); let b_content = "b-content"; - let id = db.insert(b_content); + let id = db.insert(b_content)?; let out = filter.convert_to_mergeable( &id, @@ -148,7 +148,7 @@ fn binary_below_large_file_threshold() -> crate::Result { assert_eq!(buf.as_bstr(), binary_content); let mut db = ObjectDb::default(); - let id = db.insert(binary_content); + let id = db.insert(binary_content)?; let out = filter.convert_to_mergeable( &id, EntryKind::Blob, @@ -203,7 +203,7 @@ fn above_large_file_threshold() -> crate::Result { drop(tmp); let mut db = ObjectDb::default(); - let id = db.insert(large_content); + let id = db.insert(large_content)?; let out = filter.convert_to_mergeable( &id, @@ -352,7 +352,7 @@ fn worktree_filter() -> crate::Result { "worktree files need to be converted back to what's stored in Git" ); - let id = db.insert(a_content); + let id = db.insert(a_content)?; let out = filter.convert_to_mergeable( &id, EntryKind::Blob, @@ -385,7 +385,7 @@ fn worktree_filter() -> crate::Result { drop(tmp); let b_content = "b-content\n"; - let id = db.insert(b_content); + let id = db.insert(b_content)?; let out = filter.convert_to_mergeable( &id, @@ -403,7 +403,7 @@ fn worktree_filter() -> crate::Result { let mut db = ObjectDb::default(); let b_content = "b-content\r\n"; - let id = db.insert(b_content); + let id = db.insert(b_content)?; let out = filter.convert_to_mergeable( &id, EntryKind::Blob, diff --git a/gix-merge/tests/merge/blob/platform.rs b/gix-merge/tests/merge/blob/platform.rs index 75aa716e83c..7bb38ed305e 100644 --- a/gix-merge/tests/merge/blob/platform.rs +++ b/gix-merge/tests/merge/blob/platform.rs @@ -31,7 +31,7 @@ mod merge { ("ours", ResourceKind::CurrentOrOurs), ("theirs\0", ResourceKind::OtherOrTheirs), ] { - let id = db.insert(content); + let id = db.insert(content)?; platform.set_resource( id, EntryKind::Blob, @@ -81,7 +81,7 @@ mod merge { ("any\0", ResourceKind::CurrentOrOurs), ("any\0", ResourceKind::OtherOrTheirs), ] { - let id = db.insert(content); + let id = db.insert(content)?; platform.set_resource( id, EntryKind::Blob, @@ -131,7 +131,7 @@ mod merge { ("ours", ResourceKind::CurrentOrOurs), ("theirs", ResourceKind::OtherOrTheirs), ] { - let id = db.insert(content); + let id = db.insert(content)?; platform.set_resource(id, EntryKind::Blob, "b".into(), kind, &db)?; } @@ -284,7 +284,7 @@ theirs ("ours", ResourceKind::CurrentOrOurs), ("theirs", ResourceKind::OtherOrTheirs), ] { - let id = db.insert(content); + let id = db.insert(content)?; platform.set_resource(id, EntryKind::Blob, "b".into(), kind, &db)?; } @@ -313,7 +313,7 @@ theirs "we handle word-splitting and definitely pick-up what's written into the %A buffer" ); - let id = db.insert("binary\0"); + let id = db.insert("binary\0")?; platform.set_resource(id, EntryKind::Blob, "b".into(), ResourceKind::OtherOrTheirs, &db)?; let platform_ref = platform.prepare_merge(&db, Default::default())?; let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?; diff --git a/gix-object/benches/edit_tree.rs b/gix-object/benches/edit_tree.rs index a083a3ff4be..13f9397e0e3 100644 --- a/gix-object/benches/edit_tree.rs +++ b/gix-object/benches/edit_tree.rs @@ -147,7 +147,7 @@ fn new_inmemory_writes() -> ( move |tree: &Tree| { buf.clear(); tree.write_to(&mut buf)?; - let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf); + let id = gix_object::try_compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf)?; let mut borrowed = store.borrow_mut(); match borrowed.entry(id) { Entry::Occupied(_) => {} diff --git a/gix-object/src/data.rs b/gix-object/src/data.rs index 5dd12906a19..f30447d1527 100644 --- a/gix-object/src/data.rs +++ b/gix-object/src/data.rs @@ -66,7 +66,7 @@ pub mod verify { /// If the hashes do not match, an [`Error`] is returned, containing the actual /// hash of `self`. pub fn verify_checksum(&self, expected: &gix_hash::oid) -> Result { - let actual = crate::compute_hash(expected.kind(), self.kind, self.data); + let actual = crate::try_compute_hash(expected.kind(), self.kind, self.data)?; actual.verify(expected)?; Ok(actual) } diff --git a/gix-object/tests/object/main.rs b/gix-object/tests/object/main.rs index 2ff45a19081..6a96f25949e 100644 --- a/gix-object/tests/object/main.rs +++ b/gix-object/tests/object/main.rs @@ -12,11 +12,11 @@ mod tree; fn compute_hash() { let hk = gix_hash::Kind::Sha1; assert_eq!( - gix_object::compute_hash(hk, gix_object::Kind::Blob, &[]), + gix_object::try_compute_hash(hk, gix_object::Kind::Blob, &[]).expect("empty hash doesn’t collide"), gix_hash::ObjectId::empty_blob(hk) ); assert_eq!( - gix_object::compute_hash(hk, gix_object::Kind::Tree, &[]), + gix_object::try_compute_hash(hk, gix_object::Kind::Tree, &[]).expect("empty hash doesn’t collide"), gix_hash::ObjectId::empty_tree(hk) ); } diff --git a/gix-object/tests/object/tree/editor.rs b/gix-object/tests/object/tree/editor.rs index 9065c5aad1c..e32015e9a08 100644 --- a/gix-object/tests/object/tree/editor.rs +++ b/gix-object/tests/object/tree/editor.rs @@ -805,7 +805,7 @@ mod utils { move |tree: &Tree| { buf.clear(); tree.write_to(&mut buf)?; - let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf); + let id = gix_object::try_compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf)?; store.borrow_mut().insert(id, tree.clone()); let old = num_writes.get(); num_writes.set(old + 1); diff --git a/gix-odb/src/memory.rs b/gix-odb/src/memory.rs index 6d5f405e7cb..8c426c3324a 100644 --- a/gix-odb/src/memory.rs +++ b/gix-odb/src/memory.rs @@ -219,7 +219,7 @@ where let mut buf = Vec::new(); from.read_to_end(&mut buf)?; - let id = gix_object::compute_hash(self.object_hash, kind, &buf); + let id = gix_object::try_compute_hash(self.object_hash, kind, &buf)?; map.borrow_mut().insert(id, (kind, buf)); Ok(id) } diff --git a/gix-odb/src/sink.rs b/gix-odb/src/sink.rs index d0952d1df8a..c06df83631c 100644 --- a/gix-odb/src/sink.rs +++ b/gix-odb/src/sink.rs @@ -53,6 +53,6 @@ impl gix_object::Write for Sink { c.reset(); } - Ok(hasher.finalize()) + Ok(hasher.try_finalize()?) } } diff --git a/gix-odb/src/store_impls/loose/verify.rs b/gix-odb/src/store_impls/loose/verify.rs index 607b57aef13..afe3aa27887 100644 --- a/gix-odb/src/store_impls/loose/verify.rs +++ b/gix-odb/src/store_impls/loose/verify.rs @@ -85,7 +85,11 @@ impl Store { .map_err(|_| integrity::Error::Retry)? .ok_or(integrity::Error::Retry)?; sink.write_buf(object.kind, object.data) - .expect("sink never fails") + .map_err(|err| integrity::Error::ObjectHasher { + source: *err.downcast().expect("sink can only fail in hasher"), + kind: object.kind, + expected: id, + })? .verify(&id) .map_err(|err| integrity::Error::ObjectEncodeMismatch { source: err, diff --git a/gix-odb/src/store_impls/loose/write.rs b/gix-odb/src/store_impls/loose/write.rs index 8dbd3fa1e3d..a222f743dd2 100644 --- a/gix-odb/src/store_impls/loose/write.rs +++ b/gix-odb/src/store_impls/loose/write.rs @@ -130,7 +130,11 @@ impl Store { &self, hasher::io::Write { hash, inner: file }: hasher::io::Write, ) -> Result { - let id = hash.finalize(); + let id = hash.try_finalize().map_err(|err| Error::Io { + source: err.into(), + message: "hash tempfile in", + path: self.path.to_owned(), + })?; let object_path = loose::hash_path(&id, self.path.clone()); let object_dir = object_path .parent() diff --git a/gix-pack/src/data/input/bytes_to_entries.rs b/gix-pack/src/data/input/bytes_to_entries.rs index 42924808c1f..92f48d9de69 100644 --- a/gix-pack/src/data/input/bytes_to_entries.rs +++ b/gix-pack/src/data/input/bytes_to_entries.rs @@ -180,7 +180,7 @@ where } if let Some(hash) = self.hash.take() { - let actual_id = hash.finalize(); + let actual_id = hash.try_finalize().map_err(hasher::io::Error::from)?; if self.mode == input::Mode::Restore { id = actual_id; } else { @@ -190,7 +190,7 @@ where Some(id) } else if self.mode == input::Mode::Restore { let hash = self.hash.clone().expect("in restore mode a hash is set"); - Some(hash.finalize()) + Some(hash.try_finalize().map_err(hasher::io::Error::from)?) } else { None }) diff --git a/gix-pack/src/data/output/bytes.rs b/gix-pack/src/data/output/bytes.rs index 4c2eb1c60bf..be501af2d41 100644 --- a/gix-pack/src/data/output/bytes.rs +++ b/gix-pack/src/data/output/bytes.rs @@ -126,7 +126,12 @@ where } } None => { - let digest = self.output.hash.clone().finalize(); + let digest = self + .output + .hash + .clone() + .try_finalize() + .map_err(hasher::io::Error::from)?; self.output .inner .write_all(digest.as_slice()) diff --git a/gix-pack/src/index/encode.rs b/gix-pack/src/index/encode.rs index 2c8c4748239..5d9450a5a3e 100644 --- a/gix-pack/src/index/encode.rs +++ b/gix-pack/src/index/encode.rs @@ -137,7 +137,7 @@ mod function { let bytes_written_without_trailer = out.bytes; let out = out.inner.into_inner().map_err(io::Error::from)?; - let index_hash = out.hash.finalize(); + let index_hash = out.hash.try_finalize()?; out.inner.write_all(index_hash.as_slice())?; out.inner.flush()?; diff --git a/gix-pack/src/index/write/mod.rs b/gix-pack/src/index/write/mod.rs index 9b69252e112..24c4a34559a 100644 --- a/gix-pack/src/index/write/mod.rs +++ b/gix-pack/src/index/write/mod.rs @@ -193,10 +193,7 @@ impl crate::index::File { entry, decompressed: bytes, .. - }| { - modify_base(data, entry, bytes, version.hash()); - Ok::<_, Error>(()) - }, + }| { modify_base(data, entry, bytes, version.hash()) }, traverse::Options { object_progress: Box::new( root_progress.add_child_with_id("Resolving".into(), ProgressId::ResolveObjects.into()), @@ -228,7 +225,7 @@ impl crate::index::File { let header = crate::data::header::encode(pack_version, 0); let mut hasher = gix_hash::hasher(object_hash); hasher.update(&header); - hasher.finalize() + hasher.try_finalize().map_err(hasher::io::Error::from)? } None => return Err(Error::IteratorInvariantTrailer), }; @@ -254,8 +251,14 @@ impl crate::index::File { } } -fn modify_base(entry: &mut TreeEntry, pack_entry: &crate::data::Entry, decompressed: &[u8], hash: gix_hash::Kind) { +fn modify_base( + entry: &mut TreeEntry, + pack_entry: &crate::data::Entry, + decompressed: &[u8], + hash: gix_hash::Kind, +) -> Result<(), hasher::Error> { let object_kind = pack_entry.header.as_kind().expect("base object as source of iteration"); - let id = gix_object::compute_hash(hash, object_kind, decompressed); + let id = gix_object::try_compute_hash(hash, object_kind, decompressed)?; entry.id = id; + Ok(()) } diff --git a/gix-pack/src/multi_index/write.rs b/gix-pack/src/multi_index/write.rs index 59bc21a392d..f0fafac8baa 100644 --- a/gix-pack/src/multi_index/write.rs +++ b/gix-pack/src/multi_index/write.rs @@ -218,7 +218,7 @@ impl multi_index::File { } // write trailing checksum - let multi_index_checksum = out.inner.hash.finalize(); + let multi_index_checksum = out.inner.hash.try_finalize().map_err(hasher::io::Error::from)?; out.inner .inner .write_all(multi_index_checksum.as_slice()) diff --git a/gix-pack/src/verify.rs b/gix-pack/src/verify.rs index 108552dbb6a..77c693278b5 100644 --- a/gix-pack/src/verify.rs +++ b/gix-pack/src/verify.rs @@ -54,8 +54,9 @@ pub fn checksum_on_disk_or_mmap( hasher.update(&data[..data_len_without_trailer]); progress.inc_by(data_len_without_trailer); progress.show_throughput(start); - hasher.finalize() + hasher.try_finalize()? } + Err(hasher::io::Error::Hasher(err)) => return Err(checksum::Error::Hasher(err)), }; actual.verify(&expected)?; diff --git a/gix-status/src/index_as_worktree/traits.rs b/gix-status/src/index_as_worktree/traits.rs index ba23be03d86..e7a86574950 100644 --- a/gix-status/src/index_as_worktree/traits.rs +++ b/gix-status/src/index_as_worktree/traits.rs @@ -143,14 +143,16 @@ impl CompareBlobs for HashEq { let mut stream = data.stream_worktree_file()?; match stream.as_bytes() { Some(buffer) => { - let file_hash = gix_object::compute_hash(entry.id.kind(), gix_object::Kind::Blob, buffer); + let file_hash = gix_object::try_compute_hash(entry.id.kind(), gix_object::Kind::Blob, buffer) + .map_err(hasher::io::Error::from)?; Ok((entry.id != file_hash).then_some(file_hash)) } None => { let file_hash = match stream.size() { None => { stream.read_to_end(buf).map_err(hasher::io::Error::from)?; - gix_object::compute_hash(entry.id.kind(), gix_object::Kind::Blob, buf) + gix_object::try_compute_hash(entry.id.kind(), gix_object::Kind::Blob, buf) + .map_err(hasher::io::Error::from)? } Some(len) => gix_object::compute_stream_hash( entry.id.kind(), diff --git a/gix-status/src/index_as_worktree_with_renames/mod.rs b/gix-status/src/index_as_worktree_with_renames/mod.rs index 58474af95ba..6f066260fc0 100644 --- a/gix-status/src/index_as_worktree_with_renames/mod.rs +++ b/gix-status/src/index_as_worktree_with_renames/mod.rs @@ -536,18 +536,23 @@ pub(super) mod function { should_interrupt, ) .map_err(Error::HashFile)?, - ToGitOutcome::Buffer(buf) => gix_object::compute_hash(object_hash, gix_object::Kind::Blob, buf), + ToGitOutcome::Buffer(buf) => { + gix_object::try_compute_hash(object_hash, gix_object::Kind::Blob, buf) + .map_err(|err| Error::HashFile(err.into()))? + } ToGitOutcome::Process(mut stream) => { buf.clear(); stream.read_to_end(buf).map_err(|err| Error::HashFile(err.into()))?; - gix_object::compute_hash(object_hash, gix_object::Kind::Blob, buf) + gix_object::try_compute_hash(object_hash, gix_object::Kind::Blob, buf) + .map_err(|err| Error::HashFile(err.into()))? } } } Kind::Symlink => { let path = worktree_root.join(gix_path::from_bstr(rela_path)); let target = gix_path::into_bstr(std::fs::read_link(path).map_err(Error::ReadLink)?); - gix_object::compute_hash(object_hash, gix_object::Kind::Blob, &target) + gix_object::try_compute_hash(object_hash, gix_object::Kind::Blob, &target) + .map_err(|err| Error::HashFile(err.into()))? } Kind::Directory | Kind::Repository => object_hash.null(), }) diff --git a/gix/src/repository/impls.rs b/gix/src/repository/impls.rs index 31c149c9494..76629905382 100644 --- a/gix/src/repository/impls.rs +++ b/gix/src/repository/impls.rs @@ -106,7 +106,7 @@ impl gix_object::Write for crate::Repository { } fn write_buf(&self, object: gix_object::Kind, from: &[u8]) -> Result { - let oid = gix_object::compute_hash(self.object_hash(), object, from); + let oid = gix_object::try_compute_hash(self.object_hash(), object, from)?; if self.objects.exists(&oid) { return Ok(oid); } diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs index 873e143792b..99e79cf9331 100644 --- a/gix/src/repository/object.rs +++ b/gix/src/repository/object.rs @@ -172,7 +172,8 @@ impl crate::Repository { } fn write_object_inner(&self, buf: &[u8], kind: gix_object::Kind) -> Result, object::write::Error> { - let oid = gix_object::compute_hash(self.object_hash(), kind, buf); + let oid = gix_object::try_compute_hash(self.object_hash(), kind, buf) + .map_err(|err| Box::new(err) as Box)?; if self.objects.exists(&oid) { return Ok(oid.attach(self)); } @@ -189,7 +190,8 @@ impl crate::Repository { /// pre-hashing the data, and checking if the object is already present. pub fn write_blob(&self, bytes: impl AsRef<[u8]>) -> Result, object::write::Error> { let bytes = bytes.as_ref(); - let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, bytes); + let oid = gix_object::try_compute_hash(self.object_hash(), gix_object::Kind::Blob, bytes) + .map_err(|err| Box::new(err) as Box)?; if self.objects.exists(&oid) { return Ok(oid.attach(self)); } @@ -214,7 +216,8 @@ impl crate::Repository { } fn write_blob_stream_inner(&self, buf: &[u8]) -> Result, object::write::Error> { - let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, buf); + let oid = gix_object::try_compute_hash(self.object_hash(), gix_object::Kind::Blob, buf) + .map_err(|err| Box::new(err) as Box)?; if self.objects.exists(&oid) { return Ok(oid.attach(self)); } diff --git a/tests/it/src/commands/git_to_sh.rs b/tests/it/src/commands/git_to_sh.rs index 499a9a5d953..249b441691e 100644 --- a/tests/it/src/commands/git_to_sh.rs +++ b/tests/it/src/commands/git_to_sh.rs @@ -115,7 +115,11 @@ pub(super) mod function { })?; let mapped = crate::commands::copy_royal::remapped(data); ( - gix::objs::compute_hash(repo.object_hash(), gix::object::Kind::Blob, mapped.as_bytes()), + gix::objs::try_compute_hash( + repo.object_hash(), + gix::object::Kind::Blob, + mapped.as_bytes(), + )?, Cow::Owned(mapped.into()), ) } From b5ac93a9a02c97bd611d455091c60bfbd5f49efa Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 2 Apr 2025 17:02:38 +0100 Subject: [PATCH 18/22] change!: make `gix_object::compute_hash` fallible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `compute_stream_hash` is already fallible, so we don’t want to keep the `try_*` prefix on the fallible API. --- gix-object/src/lib.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index d35f7c71f61..519bd398528 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -406,10 +406,14 @@ fn object_hasher(hash_kind: gix_hash::Kind, object_kind: Kind, object_size: u64) /// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its `data`. #[doc(alias = "hash_object", alias = "git2")] -pub fn compute_hash(hash_kind: gix_hash::Kind, object_kind: Kind, data: &[u8]) -> gix_hash::ObjectId { +pub fn compute_hash( + hash_kind: gix_hash::Kind, + object_kind: Kind, + data: &[u8], +) -> Result { let mut hasher = object_hasher(hash_kind, object_kind, data.len() as u64); hasher.update(data); - hasher.finalize() + hasher.try_finalize() } /// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its `data`. @@ -418,9 +422,7 @@ pub fn try_compute_hash( object_kind: Kind, data: &[u8], ) -> Result { - let mut hasher = object_hasher(hash_kind, object_kind, data.len() as u64); - hasher.update(data); - hasher.try_finalize() + compute_hash(hash_kind, object_kind, data) } /// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its data read from `stream` From 3d7e379f26cbe53ddb430427b8e88ce0966be456 Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 2 Apr 2025 17:03:30 +0100 Subject: [PATCH 19/22] migrate `gix_object::{try_ =>}compute_hash` users Trivial rename. --- gix-diff/tests/diff/main.rs | 2 +- gix-filter/src/ident.rs | 2 +- gix-merge/tests/merge/blob/mod.rs | 2 +- gix-object/benches/edit_tree.rs | 2 +- gix-object/src/data.rs | 2 +- gix-object/tests/object/main.rs | 4 ++-- gix-object/tests/object/tree/editor.rs | 2 +- gix-odb/src/memory.rs | 2 +- gix-pack/src/index/write/mod.rs | 2 +- gix-status/src/index_as_worktree/traits.rs | 4 ++-- gix-status/src/index_as_worktree_with_renames/mod.rs | 10 ++++------ gix/src/repository/impls.rs | 2 +- gix/src/repository/object.rs | 6 +++--- tests/it/src/commands/git_to_sh.rs | 2 +- 14 files changed, 21 insertions(+), 23 deletions(-) diff --git a/gix-diff/tests/diff/main.rs b/gix-diff/tests/diff/main.rs index 1c736ec2e3c..667f1daf8d3 100644 --- a/gix-diff/tests/diff/main.rs +++ b/gix-diff/tests/diff/main.rs @@ -52,7 +52,7 @@ mod util { impl ObjectDb { /// Insert `data` and return its hash. That can be used to find it again. pub fn insert(&mut self, data: &str) -> Result { - let id = gix_object::try_compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Blob, data.as_bytes())?; + let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Blob, data.as_bytes())?; self.data_by_id.insert(id, data.into()); Ok(id) } diff --git a/gix-filter/src/ident.rs b/gix-filter/src/ident.rs index 3faee19b1a7..3a5ee814db2 100644 --- a/gix-filter/src/ident.rs +++ b/gix-filter/src/ident.rs @@ -68,7 +68,7 @@ pub fn apply(src: &[u8], object_hash: gix_hash::Kind, buf: &mut Vec) -> Resu while let Some(pos) = src[ofs..].find(b"$Id$") { let id = match id { None => { - let new_id = gix_object::try_compute_hash(object_hash, gix_object::Kind::Blob, src)?; + let new_id = gix_object::compute_hash(object_hash, gix_object::Kind::Blob, src)?; id = new_id.into(); clear_and_set_capacity(buf, src.len() + HASH_LEN)?; // pre-allocate for one ID new_id diff --git a/gix-merge/tests/merge/blob/mod.rs b/gix-merge/tests/merge/blob/mod.rs index c7695b81547..580ef7dfeb4 100644 --- a/gix-merge/tests/merge/blob/mod.rs +++ b/gix-merge/tests/merge/blob/mod.rs @@ -44,7 +44,7 @@ mod util { impl ObjectDb { /// Insert `data` and return its hash. That can be used to find it again. pub fn insert(&mut self, data: &str) -> Result { - let id = gix_object::try_compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Blob, data.as_bytes())?; + let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Blob, data.as_bytes())?; self.data_by_id.insert(id, data.into()); Ok(id) } diff --git a/gix-object/benches/edit_tree.rs b/gix-object/benches/edit_tree.rs index 13f9397e0e3..d5e9e4e5f35 100644 --- a/gix-object/benches/edit_tree.rs +++ b/gix-object/benches/edit_tree.rs @@ -147,7 +147,7 @@ fn new_inmemory_writes() -> ( move |tree: &Tree| { buf.clear(); tree.write_to(&mut buf)?; - let id = gix_object::try_compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf)?; + let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf)?; let mut borrowed = store.borrow_mut(); match borrowed.entry(id) { Entry::Occupied(_) => {} diff --git a/gix-object/src/data.rs b/gix-object/src/data.rs index f30447d1527..1f733263811 100644 --- a/gix-object/src/data.rs +++ b/gix-object/src/data.rs @@ -66,7 +66,7 @@ pub mod verify { /// If the hashes do not match, an [`Error`] is returned, containing the actual /// hash of `self`. pub fn verify_checksum(&self, expected: &gix_hash::oid) -> Result { - let actual = crate::try_compute_hash(expected.kind(), self.kind, self.data)?; + let actual = crate::compute_hash(expected.kind(), self.kind, self.data)?; actual.verify(expected)?; Ok(actual) } diff --git a/gix-object/tests/object/main.rs b/gix-object/tests/object/main.rs index 6a96f25949e..b58d6118c18 100644 --- a/gix-object/tests/object/main.rs +++ b/gix-object/tests/object/main.rs @@ -12,11 +12,11 @@ mod tree; fn compute_hash() { let hk = gix_hash::Kind::Sha1; assert_eq!( - gix_object::try_compute_hash(hk, gix_object::Kind::Blob, &[]).expect("empty hash doesn’t collide"), + gix_object::compute_hash(hk, gix_object::Kind::Blob, &[]).expect("empty hash doesn’t collide"), gix_hash::ObjectId::empty_blob(hk) ); assert_eq!( - gix_object::try_compute_hash(hk, gix_object::Kind::Tree, &[]).expect("empty hash doesn’t collide"), + gix_object::compute_hash(hk, gix_object::Kind::Tree, &[]).expect("empty hash doesn’t collide"), gix_hash::ObjectId::empty_tree(hk) ); } diff --git a/gix-object/tests/object/tree/editor.rs b/gix-object/tests/object/tree/editor.rs index e32015e9a08..7dc14c901d5 100644 --- a/gix-object/tests/object/tree/editor.rs +++ b/gix-object/tests/object/tree/editor.rs @@ -805,7 +805,7 @@ mod utils { move |tree: &Tree| { buf.clear(); tree.write_to(&mut buf)?; - let id = gix_object::try_compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf)?; + let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf)?; store.borrow_mut().insert(id, tree.clone()); let old = num_writes.get(); num_writes.set(old + 1); diff --git a/gix-odb/src/memory.rs b/gix-odb/src/memory.rs index 8c426c3324a..4058b47e609 100644 --- a/gix-odb/src/memory.rs +++ b/gix-odb/src/memory.rs @@ -219,7 +219,7 @@ where let mut buf = Vec::new(); from.read_to_end(&mut buf)?; - let id = gix_object::try_compute_hash(self.object_hash, kind, &buf)?; + let id = gix_object::compute_hash(self.object_hash, kind, &buf)?; map.borrow_mut().insert(id, (kind, buf)); Ok(id) } diff --git a/gix-pack/src/index/write/mod.rs b/gix-pack/src/index/write/mod.rs index 24c4a34559a..adb0f90d16e 100644 --- a/gix-pack/src/index/write/mod.rs +++ b/gix-pack/src/index/write/mod.rs @@ -258,7 +258,7 @@ fn modify_base( hash: gix_hash::Kind, ) -> Result<(), hasher::Error> { let object_kind = pack_entry.header.as_kind().expect("base object as source of iteration"); - let id = gix_object::try_compute_hash(hash, object_kind, decompressed)?; + let id = gix_object::compute_hash(hash, object_kind, decompressed)?; entry.id = id; Ok(()) } diff --git a/gix-status/src/index_as_worktree/traits.rs b/gix-status/src/index_as_worktree/traits.rs index e7a86574950..a75b60beb77 100644 --- a/gix-status/src/index_as_worktree/traits.rs +++ b/gix-status/src/index_as_worktree/traits.rs @@ -143,7 +143,7 @@ impl CompareBlobs for HashEq { let mut stream = data.stream_worktree_file()?; match stream.as_bytes() { Some(buffer) => { - let file_hash = gix_object::try_compute_hash(entry.id.kind(), gix_object::Kind::Blob, buffer) + let file_hash = gix_object::compute_hash(entry.id.kind(), gix_object::Kind::Blob, buffer) .map_err(hasher::io::Error::from)?; Ok((entry.id != file_hash).then_some(file_hash)) } @@ -151,7 +151,7 @@ impl CompareBlobs for HashEq { let file_hash = match stream.size() { None => { stream.read_to_end(buf).map_err(hasher::io::Error::from)?; - gix_object::try_compute_hash(entry.id.kind(), gix_object::Kind::Blob, buf) + gix_object::compute_hash(entry.id.kind(), gix_object::Kind::Blob, buf) .map_err(hasher::io::Error::from)? } Some(len) => gix_object::compute_stream_hash( diff --git a/gix-status/src/index_as_worktree_with_renames/mod.rs b/gix-status/src/index_as_worktree_with_renames/mod.rs index 6f066260fc0..4eaa8f09014 100644 --- a/gix-status/src/index_as_worktree_with_renames/mod.rs +++ b/gix-status/src/index_as_worktree_with_renames/mod.rs @@ -536,14 +536,12 @@ pub(super) mod function { should_interrupt, ) .map_err(Error::HashFile)?, - ToGitOutcome::Buffer(buf) => { - gix_object::try_compute_hash(object_hash, gix_object::Kind::Blob, buf) - .map_err(|err| Error::HashFile(err.into()))? - } + ToGitOutcome::Buffer(buf) => gix_object::compute_hash(object_hash, gix_object::Kind::Blob, buf) + .map_err(|err| Error::HashFile(err.into()))?, ToGitOutcome::Process(mut stream) => { buf.clear(); stream.read_to_end(buf).map_err(|err| Error::HashFile(err.into()))?; - gix_object::try_compute_hash(object_hash, gix_object::Kind::Blob, buf) + gix_object::compute_hash(object_hash, gix_object::Kind::Blob, buf) .map_err(|err| Error::HashFile(err.into()))? } } @@ -551,7 +549,7 @@ pub(super) mod function { Kind::Symlink => { let path = worktree_root.join(gix_path::from_bstr(rela_path)); let target = gix_path::into_bstr(std::fs::read_link(path).map_err(Error::ReadLink)?); - gix_object::try_compute_hash(object_hash, gix_object::Kind::Blob, &target) + gix_object::compute_hash(object_hash, gix_object::Kind::Blob, &target) .map_err(|err| Error::HashFile(err.into()))? } Kind::Directory | Kind::Repository => object_hash.null(), diff --git a/gix/src/repository/impls.rs b/gix/src/repository/impls.rs index 76629905382..32f319c8199 100644 --- a/gix/src/repository/impls.rs +++ b/gix/src/repository/impls.rs @@ -106,7 +106,7 @@ impl gix_object::Write for crate::Repository { } fn write_buf(&self, object: gix_object::Kind, from: &[u8]) -> Result { - let oid = gix_object::try_compute_hash(self.object_hash(), object, from)?; + let oid = gix_object::compute_hash(self.object_hash(), object, from)?; if self.objects.exists(&oid) { return Ok(oid); } diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs index 99e79cf9331..2528e1c51cd 100644 --- a/gix/src/repository/object.rs +++ b/gix/src/repository/object.rs @@ -172,7 +172,7 @@ impl crate::Repository { } fn write_object_inner(&self, buf: &[u8], kind: gix_object::Kind) -> Result, object::write::Error> { - let oid = gix_object::try_compute_hash(self.object_hash(), kind, buf) + let oid = gix_object::compute_hash(self.object_hash(), kind, buf) .map_err(|err| Box::new(err) as Box)?; if self.objects.exists(&oid) { return Ok(oid.attach(self)); @@ -190,7 +190,7 @@ impl crate::Repository { /// pre-hashing the data, and checking if the object is already present. pub fn write_blob(&self, bytes: impl AsRef<[u8]>) -> Result, object::write::Error> { let bytes = bytes.as_ref(); - let oid = gix_object::try_compute_hash(self.object_hash(), gix_object::Kind::Blob, bytes) + let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, bytes) .map_err(|err| Box::new(err) as Box)?; if self.objects.exists(&oid) { return Ok(oid.attach(self)); @@ -216,7 +216,7 @@ impl crate::Repository { } fn write_blob_stream_inner(&self, buf: &[u8]) -> Result, object::write::Error> { - let oid = gix_object::try_compute_hash(self.object_hash(), gix_object::Kind::Blob, buf) + let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, buf) .map_err(|err| Box::new(err) as Box)?; if self.objects.exists(&oid) { return Ok(oid.attach(self)); diff --git a/tests/it/src/commands/git_to_sh.rs b/tests/it/src/commands/git_to_sh.rs index 249b441691e..85ef05eec9c 100644 --- a/tests/it/src/commands/git_to_sh.rs +++ b/tests/it/src/commands/git_to_sh.rs @@ -115,7 +115,7 @@ pub(super) mod function { })?; let mapped = crate::commands::copy_royal::remapped(data); ( - gix::objs::try_compute_hash( + gix::objs::compute_hash( repo.object_hash(), gix::object::Kind::Blob, mapped.as_bytes(), From a68f1156b4a24e49ceb165bdaa9f4fa8e4c70cd9 Mon Sep 17 00:00:00 2001 From: Emily Date: Sat, 22 Mar 2025 19:26:35 +0000 Subject: [PATCH 20/22] change!: drop migration shims for fallible hashing Since the APIs were already adjusted and all callers migrated, we only need to drop the migration shims. --- gix-hash/src/hasher/mod.rs | 6 ------ gix-object/src/lib.rs | 9 --------- 2 files changed, 15 deletions(-) diff --git a/gix-hash/src/hasher/mod.rs b/gix-hash/src/hasher/mod.rs index 6a226f65d49..1837ed58530 100644 --- a/gix-hash/src/hasher/mod.rs +++ b/gix-hash/src/hasher/mod.rs @@ -57,12 +57,6 @@ impl Hasher { }), } } - - /// Finalize the hash and produce an object ID. - #[inline] - pub fn finalize(self) -> crate::ObjectId { - self.try_finalize().expect("Detected SHA-1 collision attack") - } } /// Produce a hasher suitable for the given kind of hash. diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index 519bd398528..a7399a7bd0b 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -416,15 +416,6 @@ pub fn compute_hash( hasher.try_finalize() } -/// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its `data`. -pub fn try_compute_hash( - hash_kind: gix_hash::Kind, - object_kind: Kind, - data: &[u8], -) -> Result { - compute_hash(hash_kind, object_kind, data) -} - /// A function to compute a hash of kind `hash_kind` for an object of `object_kind` and its data read from `stream` /// which has to yield exactly `stream_len` bytes. /// Use `progress` to learn about progress in bytes processed and `should_interrupt` to be able to abort the operation From f8796547213c5fe15efa35ee947223ee0ebe6c12 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 3 Apr 2025 13:00:21 +0800 Subject: [PATCH 21/22] Prepare `gix-index` end-of-index extension parsing for SHA256. Previously SHA1 would be hardcoded. --- gix-index/src/extension/end_of_index_entry/decode.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gix-index/src/extension/end_of_index_entry/decode.rs b/gix-index/src/extension/end_of_index_entry/decode.rs index 51fb1702bfa..a9666bd64ce 100644 --- a/gix-index/src/extension/end_of_index_entry/decode.rs +++ b/gix-index/src/extension/end_of_index_entry/decode.rs @@ -28,15 +28,15 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Result, } let (offset, checksum) = ext_data.split_at(4); - let offset = from_be_u32(offset) as usize; let Ok(checksum) = gix_hash::oid::try_from_bytes(checksum) else { return Ok(None); }; - if offset < header::SIZE || offset > start_of_eoie || checksum.kind() != gix_hash::Kind::Sha1 { + let offset = from_be_u32(offset) as usize; + if offset < header::SIZE || offset > start_of_eoie || checksum.kind() != object_hash { return Ok(None); } - let mut hasher = gix_hash::hasher(gix_hash::Kind::Sha1); + let mut hasher = gix_hash::hasher(object_hash); let mut last_chunk = None; for (signature, chunk) in extension::Iter::new(&data[offset..data.len() - MIN_SIZE_WITH_HEADER - hash_len]) { hasher.update(&signature); From 4501086adc544e675b3043c4c23b78a6c6711d8b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 3 Apr 2025 11:36:21 +0800 Subject: [PATCH 22/22] refactor - align `gix-hash` integration tests with 'new' style. - reorganize `hasher` module to avoid duplicate paths to the same types/functions. - use shorter paths to `gix_hash::hasher|io` everywhere. --- gix-features/src/hash.rs | 6 +- gix-hash/src/hasher.rs | 71 ++++++++++ gix-hash/src/hasher/io.rs | 120 ----------------- gix-hash/src/hasher/mod.rs | 71 ---------- gix-hash/src/io.rs | 126 ++++++++++++++++++ gix-hash/src/lib.rs | 7 +- gix-hash/src/verify.rs | 4 +- gix-hash/tests/hash/hasher.rs | 20 +++ gix-hash/tests/{kind/mod.rs => hash/kind.rs} | 0 gix-hash/tests/{hash.rs => hash/main.rs} | 0 .../{object_id/mod.rs => hash/object_id.rs} | 0 gix-hash/tests/{oid/mod.rs => hash/oid.rs} | 0 .../tests/{prefix/mod.rs => hash/prefix.rs} | 0 gix-hash/tests/hasher/mod.rs | 9 -- .../src/extension/end_of_index_entry/write.rs | 2 +- gix-index/src/file/init.rs | 6 +- gix-index/src/file/verify.rs | 6 +- gix-index/src/file/write.rs | 8 +- gix-index/src/write.rs | 2 +- gix-object/benches/edit_tree.rs | 5 +- gix-object/src/lib.rs | 2 +- gix-object/tests/object/tree/editor.rs | 2 +- gix-odb/src/store_impls/loose/write.rs | 9 +- gix-pack/src/data/input/bytes_to_entries.rs | 16 +-- gix-pack/src/data/input/entries_to_bytes.rs | 6 +- gix-pack/src/data/input/types.rs | 4 +- gix-pack/src/data/output/bytes.rs | 20 ++- gix-pack/src/index/encode.rs | 8 +- gix-pack/src/index/write/error.rs | 4 +- gix-pack/src/index/write/mod.rs | 7 +- gix-pack/src/multi_index/write.rs | 15 +-- gix-pack/src/verify.rs | 7 +- gix-status/src/index_as_worktree/function.rs | 5 +- gix-status/src/index_as_worktree/traits.rs | 8 +- gix-status/src/index_as_worktree/types.rs | 2 +- .../index_as_worktree_with_renames/types.rs | 2 +- 36 files changed, 288 insertions(+), 292 deletions(-) create mode 100644 gix-hash/src/hasher.rs delete mode 100644 gix-hash/src/hasher/io.rs delete mode 100644 gix-hash/src/hasher/mod.rs create mode 100644 gix-hash/src/io.rs create mode 100644 gix-hash/tests/hash/hasher.rs rename gix-hash/tests/{kind/mod.rs => hash/kind.rs} (100%) rename gix-hash/tests/{hash.rs => hash/main.rs} (100%) rename gix-hash/tests/{object_id/mod.rs => hash/object_id.rs} (100%) rename gix-hash/tests/{oid/mod.rs => hash/oid.rs} (100%) rename gix-hash/tests/{prefix/mod.rs => hash/prefix.rs} (100%) delete mode 100644 gix-hash/tests/hasher/mod.rs diff --git a/gix-features/src/hash.rs b/gix-features/src/hash.rs index 24e7b36881b..27db01384fe 100644 --- a/gix-features/src/hash.rs +++ b/gix-features/src/hash.rs @@ -2,9 +2,9 @@ /// Compute a CRC32 hash from the given `bytes`, returning the CRC32 hash. /// -/// When calling this function for the first time, `previous_value` should be `0`. Otherwise it -/// should be the previous return value of this function to provide a hash of multiple sequential -/// chunks of `bytes`. +/// When calling this function for the first time, `previous_value` should be `0`. +/// Otherwise, it should be the previous return value of this function to provide a hash +/// of multiple sequential chunks of `bytes`. #[cfg(feature = "crc32")] pub fn crc32_update(previous_value: u32, bytes: &[u8]) -> u32 { let mut h = crc32fast::Hasher::new_with_initial(previous_value); diff --git a/gix-hash/src/hasher.rs b/gix-hash/src/hasher.rs new file mode 100644 index 00000000000..2a03c3d219a --- /dev/null +++ b/gix-hash/src/hasher.rs @@ -0,0 +1,71 @@ +/// The error returned by [`Hasher::try_finalize()`](crate::Hasher::try_finalize()). +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error("Detected SHA-1 collision attack with digest {digest}")] + CollisionAttack { digest: crate::ObjectId }, +} + +pub(super) mod _impl { + use crate::hasher::Error; + use sha1_checked::{CollisionResult, Digest}; + + /// An implementation of the Sha1 hash, which can be used once. + /// + /// We use [`sha1_checked`] to implement the same collision detection + /// algorithm as Git. + #[derive(Clone)] + pub struct Hasher(sha1_checked::Sha1); + + impl Hasher { + /// Let's not provide a public default implementation to force people to go through [`hasher()`]. + fn default() -> Self { + // This matches the configuration used by Git, which only uses + // the collision detection to bail out, rather than computing + // alternate “safe hashes” for inputs where a collision attack + // was detected. + Self(sha1_checked::Builder::default().safe_hash(false).build()) + } + } + + impl Hasher { + /// Digest the given `bytes`. + pub fn update(&mut self, bytes: &[u8]) { + self.0.update(bytes); + } + + /// Finalize the hash and produce an object ID. + /// + /// Returns [`Error`] if a collision attack is detected. + #[inline] + pub fn try_finalize(self) -> Result { + match self.0.try_finalize() { + CollisionResult::Ok(digest) => Ok(crate::ObjectId::Sha1(digest.into())), + CollisionResult::Mitigated(_) => { + // SAFETY: `CollisionResult::Mitigated` is only + // returned when `safe_hash()` is on. `Hasher`’s field + // is private, and we only construct it in the + // `Default` instance, which turns `safe_hash()` off. + // + // As of Rust 1.84.1, the compiler can’t figure out + // this function cannot panic without this. + #[allow(unsafe_code)] + unsafe { + std::hint::unreachable_unchecked() + } + } + CollisionResult::Collision(digest) => Err(Error::CollisionAttack { + digest: crate::ObjectId::Sha1(digest.into()), + }), + } + } + } + + /// Produce a hasher suitable for the given `kind` of hash. + #[inline] + pub fn hasher(kind: crate::Kind) -> Hasher { + match kind { + crate::Kind::Sha1 => Hasher::default(), + } + } +} diff --git a/gix-hash/src/hasher/io.rs b/gix-hash/src/hasher/io.rs deleted file mode 100644 index 0ec51b2308b..00000000000 --- a/gix-hash/src/hasher/io.rs +++ /dev/null @@ -1,120 +0,0 @@ -use crate::{hasher, Hasher}; - -/// The error type for I/O operations that compute hashes. -#[derive(Debug, thiserror::Error)] -#[allow(missing_docs)] -pub enum Error { - #[error(transparent)] - Io(#[from] std::io::Error), - #[error("Failed to hash data")] - Hasher(#[from] hasher::Error), -} - -/// Compute the hash of `kind` for the bytes in the file at `path`, hashing only the first `num_bytes_from_start` -/// while initializing and calling `progress`. -/// -/// `num_bytes_from_start` is useful to avoid reading trailing hashes, which are never part of the hash itself, -/// denoting the amount of bytes to hash starting from the beginning of the file. -/// -/// # Note -/// -/// * [Interrupts][gix_features::interrupt] are supported. -pub fn bytes_of_file( - path: &std::path::Path, - num_bytes_from_start: u64, - kind: crate::Kind, - progress: &mut dyn gix_features::progress::Progress, - should_interrupt: &std::sync::atomic::AtomicBool, -) -> Result { - bytes( - &mut std::fs::File::open(path)?, - num_bytes_from_start, - kind, - progress, - should_interrupt, - ) -} - -/// Similar to [`bytes_of_file`], but operates on a stream of bytes. -pub fn bytes( - read: &mut dyn std::io::Read, - num_bytes_from_start: u64, - kind: crate::Kind, - progress: &mut dyn gix_features::progress::Progress, - should_interrupt: &std::sync::atomic::AtomicBool, -) -> Result { - bytes_with_hasher(read, num_bytes_from_start, hasher(kind), progress, should_interrupt) -} - -/// Similar to [`bytes()`], but takes a `hasher` instead of a hash kind. -pub fn bytes_with_hasher( - read: &mut dyn std::io::Read, - num_bytes_from_start: u64, - mut hasher: Hasher, - progress: &mut dyn gix_features::progress::Progress, - should_interrupt: &std::sync::atomic::AtomicBool, -) -> Result { - let start = std::time::Instant::now(); - // init progress before the possibility for failure, as convenience in case people want to recover - progress.init( - Some(num_bytes_from_start as gix_features::progress::prodash::progress::Step), - gix_features::progress::bytes(), - ); - - const BUF_SIZE: usize = u16::MAX as usize; - let mut buf = [0u8; BUF_SIZE]; - let mut bytes_left = num_bytes_from_start; - - while bytes_left > 0 { - let out = &mut buf[..BUF_SIZE.min(bytes_left as usize)]; - read.read_exact(out)?; - bytes_left -= out.len() as u64; - progress.inc_by(out.len()); - hasher.update(out); - if should_interrupt.load(std::sync::atomic::Ordering::SeqCst) { - return Err(std::io::Error::new(std::io::ErrorKind::Other, "Interrupted").into()); - } - } - - let id = hasher.try_finalize()?; - progress.show_throughput(start); - Ok(id) -} - -/// A utility to automatically generate a hash while writing into an inner writer. -pub struct Write { - /// The hash implementation. - pub hash: Hasher, - /// The inner writer. - pub inner: T, -} - -impl std::io::Write for Write -where - T: std::io::Write, -{ - fn write(&mut self, buf: &[u8]) -> std::io::Result { - let written = self.inner.write(buf)?; - self.hash.update(&buf[..written]); - Ok(written) - } - - fn flush(&mut self) -> std::io::Result<()> { - self.inner.flush() - } -} - -impl Write -where - T: std::io::Write, -{ - /// Create a new hash writer which hashes all bytes written to `inner` with a hash of `kind`. - pub fn new(inner: T, object_hash: crate::Kind) -> Self { - match object_hash { - crate::Kind::Sha1 => Write { - inner, - hash: Hasher::default(), - }, - } - } -} diff --git a/gix-hash/src/hasher/mod.rs b/gix-hash/src/hasher/mod.rs deleted file mode 100644 index 1837ed58530..00000000000 --- a/gix-hash/src/hasher/mod.rs +++ /dev/null @@ -1,71 +0,0 @@ -use sha1_checked::{CollisionResult, Digest}; - -/// The error returned by [`Hasher::try_finalize()`]. -#[derive(Debug, thiserror::Error)] -#[allow(missing_docs)] -pub enum Error { - #[error("Detected SHA-1 collision attack with digest {digest}")] - CollisionAttack { digest: crate::ObjectId }, -} - -/// A implementation of the Sha1 hash, which can be used once. -/// -/// We use [`sha1_checked`] to implement the same collision detection -/// algorithm as Git. -#[derive(Clone)] -pub struct Hasher(sha1_checked::Sha1); - -impl Default for Hasher { - #[inline] - fn default() -> Self { - // This matches the configuration used by Git, which only uses - // the collision detection to bail out, rather than computing - // alternate “safe hashes” for inputs where a collision attack - // was detected. - Self(sha1_checked::Builder::default().safe_hash(false).build()) - } -} - -impl Hasher { - /// Digest the given `bytes`. - pub fn update(&mut self, bytes: &[u8]) { - self.0.update(bytes); - } - - /// Finalize the hash and produce an object ID. - /// - /// Returns [`Error`] if a collision attack is detected. - #[inline] - pub fn try_finalize(self) -> Result { - match self.0.try_finalize() { - CollisionResult::Ok(digest) => Ok(crate::ObjectId::Sha1(digest.into())), - CollisionResult::Mitigated(_) => { - // SAFETY: `CollisionResult::Mitigated` is only - // returned when `safe_hash()` is on. `Hasher`’s field - // is private, and we only construct it in the - // `Default` instance, which turns `safe_hash()` off. - // - // As of Rust 1.84.1, the compiler can’t figure out - // this function cannot panic without this. - #[allow(unsafe_code)] - unsafe { - std::hint::unreachable_unchecked() - } - } - CollisionResult::Collision(digest) => Err(Error::CollisionAttack { - digest: crate::ObjectId::Sha1(digest.into()), - }), - } - } -} - -/// Produce a hasher suitable for the given kind of hash. -#[inline] -pub fn hasher(kind: crate::Kind) -> Hasher { - match kind { - crate::Kind::Sha1 => Hasher::default(), - } -} - -/// Hashing utilities for I/O operations. -pub mod io; diff --git a/gix-hash/src/io.rs b/gix-hash/src/io.rs new file mode 100644 index 00000000000..f764d7c6b81 --- /dev/null +++ b/gix-hash/src/io.rs @@ -0,0 +1,126 @@ +use crate::hasher; + +/// The error type for I/O operations that compute hashes. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error(transparent)] + Io(#[from] std::io::Error), + #[error("Failed to hash data")] + Hasher(#[from] hasher::Error), +} + +pub(super) mod _impl { + use crate::io::Error; + use crate::{hasher, Hasher}; + + /// Compute the hash of `kind` for the bytes in the file at `path`, hashing only the first `num_bytes_from_start` + /// while initializing and calling `progress`. + /// + /// `num_bytes_from_start` is useful to avoid reading trailing hashes, which are never part of the hash itself, + /// denoting the amount of bytes to hash starting from the beginning of the file. + /// + /// # Note + /// + /// * [Interrupts][gix_features::interrupt] are supported. + pub fn bytes_of_file( + path: &std::path::Path, + num_bytes_from_start: u64, + kind: crate::Kind, + progress: &mut dyn gix_features::progress::Progress, + should_interrupt: &std::sync::atomic::AtomicBool, + ) -> Result { + bytes( + &mut std::fs::File::open(path)?, + num_bytes_from_start, + kind, + progress, + should_interrupt, + ) + } + + /// Similar to [`bytes_of_file`], but operates on a stream of bytes. + pub fn bytes( + read: &mut dyn std::io::Read, + num_bytes_from_start: u64, + kind: crate::Kind, + progress: &mut dyn gix_features::progress::Progress, + should_interrupt: &std::sync::atomic::AtomicBool, + ) -> Result { + bytes_with_hasher(read, num_bytes_from_start, hasher(kind), progress, should_interrupt) + } + + /// Similar to [`bytes()`], but takes a `hasher` instead of a hash kind. + pub fn bytes_with_hasher( + read: &mut dyn std::io::Read, + num_bytes_from_start: u64, + mut hasher: Hasher, + progress: &mut dyn gix_features::progress::Progress, + should_interrupt: &std::sync::atomic::AtomicBool, + ) -> Result { + let start = std::time::Instant::now(); + // init progress before the possibility for failure, as convenience in case people want to recover + progress.init( + Some(num_bytes_from_start as gix_features::progress::prodash::progress::Step), + gix_features::progress::bytes(), + ); + + const BUF_SIZE: usize = u16::MAX as usize; + let mut buf = [0u8; BUF_SIZE]; + let mut bytes_left = num_bytes_from_start; + + while bytes_left > 0 { + let out = &mut buf[..BUF_SIZE.min(bytes_left as usize)]; + read.read_exact(out)?; + bytes_left -= out.len() as u64; + progress.inc_by(out.len()); + hasher.update(out); + if should_interrupt.load(std::sync::atomic::Ordering::SeqCst) { + return Err(std::io::Error::new(std::io::ErrorKind::Other, "Interrupted").into()); + } + } + + let id = hasher.try_finalize()?; + progress.show_throughput(start); + Ok(id) + } + + /// A utility to automatically generate a hash while writing into an inner writer. + pub struct Write { + /// The hash implementation. + pub hash: Hasher, + /// The inner writer. + pub inner: T, + } + + impl std::io::Write for Write + where + T: std::io::Write, + { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + let written = self.inner.write(buf)?; + self.hash.update(&buf[..written]); + Ok(written) + } + + fn flush(&mut self) -> std::io::Result<()> { + self.inner.flush() + } + } + + impl Write + where + T: std::io::Write, + { + /// Create a new hash writer which hashes all bytes written to `inner` with a hash of `kind`. + pub fn new(inner: T, object_hash: crate::Kind) -> Self { + match object_hash { + crate::Kind::Sha1 => Write { + inner, + hash: crate::hasher(object_hash), + }, + } + } + } +} +pub use _impl::Write; diff --git a/gix-hash/src/lib.rs b/gix-hash/src/lib.rs index 3c94ee8d6df..785fbe28c1a 100644 --- a/gix-hash/src/lib.rs +++ b/gix-hash/src/lib.rs @@ -15,8 +15,11 @@ pub use borrowed::{oid, Error}; /// Hash functions and hash utilities pub mod hasher; -pub use hasher::io::{bytes, bytes_of_file, bytes_with_hasher}; -pub use hasher::{hasher, Hasher}; +pub use hasher::_impl::{hasher, Hasher}; + +/// Error types for utility hash functions +pub mod io; +pub use io::_impl::{bytes, bytes_of_file, bytes_with_hasher}; mod object_id; pub use object_id::{decode, ObjectId}; diff --git a/gix-hash/src/verify.rs b/gix-hash/src/verify.rs index 9cb7f6b19ef..7fba80549b0 100644 --- a/gix-hash/src/verify.rs +++ b/gix-hash/src/verify.rs @@ -5,8 +5,8 @@ use crate::{oid, ObjectId}; #[allow(missing_docs)] #[error("Hash was {actual}, but should have been {expected}")] pub struct Error { - actual: ObjectId, - expected: ObjectId, + pub actual: ObjectId, + pub expected: ObjectId, } impl oid { diff --git a/gix-hash/tests/hash/hasher.rs b/gix-hash/tests/hash/hasher.rs new file mode 100644 index 00000000000..282b7c8666f --- /dev/null +++ b/gix-hash/tests/hash/hasher.rs @@ -0,0 +1,20 @@ +use gix_hash::{Hasher, ObjectId}; + +#[test] +fn size_of_hasher() { + assert_eq!( + std::mem::size_of::(), + if cfg!(target_arch = "x86") { 820 } else { 824 }, + "The size of this type may be relevant when hashing millions of objects,\ + and shouldn't change unnoticed. The DetectionState alone clocks in at 724 bytes." + ); +} + +#[test] +fn size_of_try_finalize_return_type() { + assert_eq!( + std::mem::size_of::>(), + 21, + "The size of the return value is just 1 byte larger than just returning the object hash itself" + ); +} diff --git a/gix-hash/tests/kind/mod.rs b/gix-hash/tests/hash/kind.rs similarity index 100% rename from gix-hash/tests/kind/mod.rs rename to gix-hash/tests/hash/kind.rs diff --git a/gix-hash/tests/hash.rs b/gix-hash/tests/hash/main.rs similarity index 100% rename from gix-hash/tests/hash.rs rename to gix-hash/tests/hash/main.rs diff --git a/gix-hash/tests/object_id/mod.rs b/gix-hash/tests/hash/object_id.rs similarity index 100% rename from gix-hash/tests/object_id/mod.rs rename to gix-hash/tests/hash/object_id.rs diff --git a/gix-hash/tests/oid/mod.rs b/gix-hash/tests/hash/oid.rs similarity index 100% rename from gix-hash/tests/oid/mod.rs rename to gix-hash/tests/hash/oid.rs diff --git a/gix-hash/tests/prefix/mod.rs b/gix-hash/tests/hash/prefix.rs similarity index 100% rename from gix-hash/tests/prefix/mod.rs rename to gix-hash/tests/hash/prefix.rs diff --git a/gix-hash/tests/hasher/mod.rs b/gix-hash/tests/hasher/mod.rs deleted file mode 100644 index 1dd100ffa74..00000000000 --- a/gix-hash/tests/hasher/mod.rs +++ /dev/null @@ -1,9 +0,0 @@ -use gix_hash::Hasher; - -#[test] -fn size_of_sha1() { - assert_eq!( - std::mem::size_of::(), - if cfg!(target_arch = "x86") { 820 } else { 824 }, - ); -} diff --git a/gix-index/src/extension/end_of_index_entry/write.rs b/gix-index/src/extension/end_of_index_entry/write.rs index 9c7a3afa015..649edae4c2d 100644 --- a/gix-index/src/extension/end_of_index_entry/write.rs +++ b/gix-index/src/extension/end_of_index_entry/write.rs @@ -11,7 +11,7 @@ pub fn write_to( hash_kind: gix_hash::Kind, offset_to_extensions: u32, prior_extensions: impl IntoIterator, -) -> Result<(), gix_hash::hasher::io::Error> { +) -> Result<(), gix_hash::io::Error> { out.write_all(&SIGNATURE)?; let extension_size: u32 = 4 + hash_kind.len_in_bytes() as u32; out.write_all(&extension_size.to_be_bytes())?; diff --git a/gix-index/src/file/init.rs b/gix-index/src/file/init.rs index 8590f91d7e7..29419e241fc 100644 --- a/gix-index/src/file/init.rs +++ b/gix-index/src/file/init.rs @@ -2,8 +2,6 @@ use std::path::{Path, PathBuf}; -use gix_hash::hasher; - use crate::{decode, extension, File, State}; mod error { @@ -85,8 +83,8 @@ impl File { &Default::default(), ) .map_err(|err| match err { - hasher::io::Error::Io(err) => Error::Io(err), - hasher::io::Error::Hasher(err) => Error::Decode(err.into()), + gix_hash::io::Error::Io(err) => Error::Io(err), + gix_hash::io::Error::Hasher(err) => Error::Decode(err.into()), })? .verify(&expected) .map_err(decode::Error::from)?; diff --git a/gix-index/src/file/verify.rs b/gix-index/src/file/verify.rs index d4b31800350..ee3be86cc93 100644 --- a/gix-index/src/file/verify.rs +++ b/gix-index/src/file/verify.rs @@ -1,7 +1,5 @@ use std::sync::atomic::AtomicBool; -use gix_hash::hasher; - use crate::File; mod error { @@ -10,7 +8,7 @@ mod error { #[allow(missing_docs)] pub enum Error { #[error("Could not read index file to generate hash")] - Io(#[from] gix_hash::hasher::io::Error), + Io(#[from] gix_hash::io::Error), #[error("Index checksum mismatch")] Verify(#[from] gix_hash::verify::Error), } @@ -23,7 +21,7 @@ impl File { let _span = gix_features::trace::coarse!("gix_index::File::verify_integrity()"); if let Some(checksum) = self.checksum { let num_bytes_to_hash = - self.path.metadata().map_err(hasher::io::Error::from)?.len() - checksum.as_bytes().len() as u64; + self.path.metadata().map_err(gix_hash::io::Error::from)?.len() - checksum.as_bytes().len() as u64; let should_interrupt = AtomicBool::new(false); gix_hash::bytes_of_file( &self.path, diff --git a/gix-index/src/file/write.rs b/gix-index/src/file/write.rs index 78f7b13ecd9..87fd2e2d1b4 100644 --- a/gix-index/src/file/write.rs +++ b/gix-index/src/file/write.rs @@ -1,5 +1,3 @@ -use gix_hash::hasher; - use crate::{write, File, Version}; /// The error produced by [`File::write()`]. @@ -7,7 +5,7 @@ use crate::{write, File, Version}; #[allow(missing_docs)] pub enum Error { #[error(transparent)] - Io(#[from] hasher::io::Error), + Io(#[from] gix_hash::io::Error), #[error("Could not acquire lock for index file")] AcquireLock(#[from] gix_lock::acquire::Error), #[error("Could not commit lock for index file")] @@ -21,14 +19,14 @@ impl File { &self, mut out: impl std::io::Write, options: write::Options, - ) -> Result<(Version, gix_hash::ObjectId), hasher::io::Error> { + ) -> Result<(Version, gix_hash::ObjectId), gix_hash::io::Error> { let _span = gix_features::trace::detail!("gix_index::File::write_to()", skip_hash = options.skip_hash); let (version, hash) = if options.skip_hash { let out: &mut dyn std::io::Write = &mut out; let version = self.state.write_to(out, options)?; (version, self.state.object_hash.null()) } else { - let mut hasher = hasher::io::Write::new(&mut out, self.state.object_hash); + let mut hasher = gix_hash::io::Write::new(&mut out, self.state.object_hash); let out: &mut dyn std::io::Write = &mut hasher; let version = self.state.write_to(out, options)?; (version, hasher.hash.try_finalize()?) diff --git a/gix-index/src/write.rs b/gix-index/src/write.rs index 436b8c5898a..e0edbb0eb89 100644 --- a/gix-index/src/write.rs +++ b/gix-index/src/write.rs @@ -67,7 +67,7 @@ impl State { extensions, skip_hash: _, }: Options, - ) -> Result { + ) -> Result { let _span = gix_features::trace::detail!("gix_index::State::write()"); let version = self.detect_required_version(); diff --git a/gix-object/benches/edit_tree.rs b/gix-object/benches/edit_tree.rs index d5e9e4e5f35..8154a7c1c76 100644 --- a/gix-object/benches/edit_tree.rs +++ b/gix-object/benches/edit_tree.rs @@ -136,10 +136,7 @@ criterion_main!(benches); type TreeStore = Rc>>; -fn new_inmemory_writes() -> ( - TreeStore, - impl FnMut(&Tree) -> Result, -) { +fn new_inmemory_writes() -> (TreeStore, impl FnMut(&Tree) -> Result) { let store = TreeStore::default(); let write_tree = { let store = store.clone(); diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index a7399a7bd0b..7e0f52d28fa 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -428,7 +428,7 @@ pub fn compute_stream_hash( stream_len: u64, progress: &mut dyn gix_features::progress::Progress, should_interrupt: &std::sync::atomic::AtomicBool, -) -> Result { +) -> Result { let hasher = object_hasher(hash_kind, object_kind, stream_len); gix_hash::bytes_with_hasher(stream, stream_len, hasher, progress, should_interrupt) } diff --git a/gix-object/tests/object/tree/editor.rs b/gix-object/tests/object/tree/editor.rs index 7dc14c901d5..7f7bc6ccc11 100644 --- a/gix-object/tests/object/tree/editor.rs +++ b/gix-object/tests/object/tree/editor.rs @@ -793,7 +793,7 @@ mod utils { pub(super) fn new_inmemory_writes() -> ( TreeStore, - impl FnMut(&Tree) -> Result, + impl FnMut(&Tree) -> Result, impl Fn() -> usize, ) { let store = TreeStore::default(); diff --git a/gix-odb/src/store_impls/loose/write.rs b/gix-odb/src/store_impls/loose/write.rs index a222f743dd2..d94c16556bd 100644 --- a/gix-odb/src/store_impls/loose/write.rs +++ b/gix-odb/src/store_impls/loose/write.rs @@ -1,7 +1,6 @@ use std::{fs, io, io::Write, path::PathBuf}; use gix_features::zlib::stream::deflate; -use gix_hash::hasher; use gix_object::WriteTo; use tempfile::NamedTempFile; @@ -14,7 +13,7 @@ use crate::store_impls::loose; pub enum Error { #[error("Could not {message} '{path}'")] Io { - source: hasher::io::Error, + source: gix_hash::io::Error, message: &'static str, path: PathBuf, }, @@ -107,7 +106,7 @@ impl Store { } impl Store { - fn dest(&self) -> Result, Error> { + fn dest(&self) -> Result, Error> { #[cfg_attr(not(unix), allow(unused_mut))] let mut builder = tempfile::Builder::new(); #[cfg(unix)] @@ -116,7 +115,7 @@ impl Store { let perms = std::fs::Permissions::from_mode(0o444); builder.permissions(perms); } - Ok(hasher::io::Write::new( + Ok(gix_hash::io::Write::new( deflate::Write::new(builder.tempfile_in(&self.path).map_err(|err| Error::Io { source: err.into(), message: "create named temp file in", @@ -128,7 +127,7 @@ impl Store { fn finalize_object( &self, - hasher::io::Write { hash, inner: file }: hasher::io::Write, + gix_hash::io::Write { hash, inner: file }: gix_hash::io::Write, ) -> Result { let id = hash.try_finalize().map_err(|err| Error::Io { source: err.into(), diff --git a/gix-pack/src/data/input/bytes_to_entries.rs b/gix-pack/src/data/input/bytes_to_entries.rs index 92f48d9de69..7f54977504c 100644 --- a/gix-pack/src/data/input/bytes_to_entries.rs +++ b/gix-pack/src/data/input/bytes_to_entries.rs @@ -1,7 +1,7 @@ use std::{fs, io}; use gix_features::zlib::Decompress; -use gix_hash::{hasher, Hasher, ObjectId}; +use gix_hash::{Hasher, ObjectId}; use crate::data::input; @@ -52,7 +52,7 @@ where object_hash: gix_hash::Kind, ) -> Result, input::Error> { let mut header_data = [0u8; 12]; - read.read_exact(&mut header_data).map_err(hasher::io::Error::from)?; + read.read_exact(&mut header_data).map_err(gix_hash::io::Error::from)?; let (version, num_objects) = crate::data::header::decode(&header_data)?; assert_eq!( @@ -97,7 +97,7 @@ where } None => crate::data::Entry::from_read(&mut self.read, self.offset, self.hash_len), } - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; // Decompress object to learn its compressed bytes let compressed_buf = self.compressed_buf.take().unwrap_or_else(|| Vec::with_capacity(4096)); @@ -114,7 +114,7 @@ where decompressor: &mut self.decompressor, }; - let bytes_copied = io::copy(&mut decompressed_reader, &mut io::sink()).map_err(hasher::io::Error::from)?; + let bytes_copied = io::copy(&mut decompressed_reader, &mut io::sink()).map_err(gix_hash::io::Error::from)?; if bytes_copied != entry.decompressed_size { return Err(input::Error::IncompletePack { actual: bytes_copied, @@ -141,7 +141,7 @@ where let header_len = entry .header .write_to(bytes_copied, &mut header_buf.as_mut()) - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; let state = gix_features::hash::crc32_update(0, &header_buf[..header_len]); Some(gix_features::hash::crc32_update(state, &compressed)) } else { @@ -180,7 +180,7 @@ where } if let Some(hash) = self.hash.take() { - let actual_id = hash.try_finalize().map_err(hasher::io::Error::from)?; + let actual_id = hash.try_finalize().map_err(gix_hash::io::Error::from)?; if self.mode == input::Mode::Restore { id = actual_id; } else { @@ -190,7 +190,7 @@ where Some(id) } else if self.mode == input::Mode::Restore { let hash = self.hash.clone().expect("in restore mode a hash is set"); - Some(hash.try_finalize().map_err(hasher::io::Error::from)?) + Some(hash.try_finalize().map_err(gix_hash::io::Error::from)?) } else { None }) @@ -273,7 +273,7 @@ impl crate::data::File { /// Returns an iterator over [`Entries`][crate::data::input::Entry], without making use of the memory mapping. pub fn streaming_iter(&self) -> Result, input::Error> { let reader = - io::BufReader::with_capacity(4096 * 8, fs::File::open(&self.path).map_err(hasher::io::Error::from)?); + io::BufReader::with_capacity(4096 * 8, fs::File::open(&self.path).map_err(gix_hash::io::Error::from)?); BytesToEntriesIter::new_from_header( reader, input::Mode::Verify, diff --git a/gix-pack/src/data/input/entries_to_bytes.rs b/gix-pack/src/data/input/entries_to_bytes.rs index 2a428349010..1cf308a6d89 100644 --- a/gix-pack/src/data/input/entries_to_bytes.rs +++ b/gix-pack/src/data/input/entries_to_bytes.rs @@ -1,7 +1,5 @@ use std::iter::Peekable; -use gix_hash::hasher; - use crate::data::input; /// An implementation of [`Iterator`] to write [encoded entries][input::Entry] to an inner implementation each time @@ -66,7 +64,7 @@ where self.trailer } - fn next_inner(&mut self, entry: input::Entry) -> Result { + fn next_inner(&mut self, entry: input::Entry) -> Result { if self.num_entries == 0 { let header_bytes = crate::data::header::encode(self.data_version, 0); self.output.write_all(&header_bytes[..])?; @@ -82,7 +80,7 @@ where Ok(entry) } - fn write_header_and_digest(&mut self, last_entry: Option<&mut input::Entry>) -> Result<(), hasher::io::Error> { + fn write_header_and_digest(&mut self, last_entry: Option<&mut input::Entry>) -> Result<(), gix_hash::io::Error> { let header_bytes = crate::data::header::encode(self.data_version, self.num_entries); let num_bytes_written = if last_entry.is_some() { self.output.stream_position()? diff --git a/gix-pack/src/data/input/types.rs b/gix-pack/src/data/input/types.rs index 6d066855028..46a694a217b 100644 --- a/gix-pack/src/data/input/types.rs +++ b/gix-pack/src/data/input/types.rs @@ -1,12 +1,10 @@ -use gix_hash::hasher; - /// Returned by [`BytesToEntriesIter::new_from_header()`][crate::data::input::BytesToEntriesIter::new_from_header()] and as part /// of `Item` of [`BytesToEntriesIter`][crate::data::input::BytesToEntriesIter]. #[derive(thiserror::Error, Debug)] #[allow(missing_docs)] pub enum Error { #[error("An IO operation failed while streaming an entry")] - Io(#[from] hasher::io::Error), + Io(#[from] gix_hash::io::Error), #[error(transparent)] PackParse(#[from] crate::data::header::decode::Error), #[error("Failed to verify pack checksum in trailer")] diff --git a/gix-pack/src/data/output/bytes.rs b/gix-pack/src/data/output/bytes.rs index be501af2d41..39f7a1d8a05 100644 --- a/gix-pack/src/data/output/bytes.rs +++ b/gix-pack/src/data/output/bytes.rs @@ -1,7 +1,5 @@ use std::io::Write; -use gix_hash::hasher; - use crate::data::output; use crate::exact_vec; @@ -13,7 +11,7 @@ where E: std::error::Error + 'static, { #[error(transparent)] - Io(#[from] hasher::io::Error), + Io(#[from] gix_hash::io::Error), #[error(transparent)] Input(E), } @@ -24,7 +22,7 @@ pub struct FromEntriesIter { /// An iterator for input [`output::Entry`] instances pub input: I, /// A way of writing encoded bytes. - output: hasher::io::Write, + output: gix_hash::io::Write, /// Our trailing hash when done writing all input entries trailer: Option, /// The amount of objects in the iteration and the version of the packfile to be written. @@ -71,7 +69,7 @@ where ); FromEntriesIter { input, - output: hasher::io::Write::new(output, object_hash), + output: gix_hash::io::Write::new(output, object_hash), trailer: None, entry_version: version, pack_offsets_and_validity: exact_vec(num_entries as usize), @@ -100,7 +98,7 @@ where let header_bytes = crate::data::header::encode(version, num_entries); self.output .write_all(&header_bytes[..]) - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; self.written += header_bytes.len() as u64; } match self.input.next() { @@ -120,9 +118,9 @@ where }); self.written += header .write_to(entry.decompressed_size as u64, &mut self.output) - .map_err(hasher::io::Error::from)? as u64; + .map_err(gix_hash::io::Error::from)? as u64; self.written += std::io::copy(&mut &*entry.compressed_data, &mut self.output) - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; } } None => { @@ -131,13 +129,13 @@ where .hash .clone() .try_finalize() - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; self.output .inner .write_all(digest.as_slice()) - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; self.written += digest.as_slice().len() as u64; - self.output.inner.flush().map_err(hasher::io::Error::from)?; + self.output.inner.flush().map_err(gix_hash::io::Error::from)?; self.is_done = true; self.trailer = Some(digest); } diff --git a/gix-pack/src/index/encode.rs b/gix-pack/src/index/encode.rs index 5d9450a5a3e..58f63cea954 100644 --- a/gix-pack/src/index/encode.rs +++ b/gix-pack/src/index/encode.rs @@ -36,11 +36,9 @@ pub(crate) fn fanout(iter: &mut dyn ExactSizeIterator) -> [u32; 256] mod function { use std::io; - use gix_features::progress::{self, DynNestedProgress}; - use gix_hash::hasher; - use super::{fanout, HIGH_BIT, LARGE_OFFSET_THRESHOLD}; use crate::index::V2_SIGNATURE; + use gix_features::progress::{self, DynNestedProgress}; struct Count { bytes: u64, @@ -74,7 +72,7 @@ mod function { pack_hash: &gix_hash::ObjectId, kind: crate::index::Version, progress: &mut dyn DynNestedProgress, - ) -> Result { + ) -> Result { use io::Write; assert_eq!(kind, crate::index::Version::V2, "Can only write V2 packs right now"); assert!( @@ -85,7 +83,7 @@ mod function { // Write header let mut out = Count::new(std::io::BufWriter::with_capacity( 8 * 4096, - hasher::io::Write::new(out, kind.hash()), + gix_hash::io::Write::new(out, kind.hash()), )); out.write_all(V2_SIGNATURE)?; out.write_all(&(kind as u32).to_be_bytes())?; diff --git a/gix-pack/src/index/write/error.rs b/gix-pack/src/index/write/error.rs index 2ac0f710225..dbdbef717f4 100644 --- a/gix-pack/src/index/write/error.rs +++ b/gix-pack/src/index/write/error.rs @@ -1,11 +1,9 @@ -use gix_hash::hasher; - /// Returned by [`crate::index::File::write_data_iter_to_stream()`] #[derive(thiserror::Error, Debug)] #[allow(missing_docs)] pub enum Error { #[error("An error occurred when writing the pack index file")] - Io(#[from] hasher::io::Error), + Io(#[from] gix_hash::io::Error), #[error("A pack entry could not be extracted")] PackEntryDecode(#[from] crate::data::input::Error), #[error("Indices of type {} cannot be written, only {} are supported", *.0 as usize, crate::index::Version::default() as usize)] diff --git a/gix-pack/src/index/write/mod.rs b/gix-pack/src/index/write/mod.rs index adb0f90d16e..2c1c0330c2e 100644 --- a/gix-pack/src/index/write/mod.rs +++ b/gix-pack/src/index/write/mod.rs @@ -2,7 +2,6 @@ use std::{io, sync::atomic::AtomicBool}; pub use error::Error; use gix_features::progress::{self, prodash::DynNestedProgress, Count, Progress}; -use gix_hash::hasher; use crate::cache::delta::{traverse, Tree}; @@ -181,7 +180,7 @@ impl crate::index::File { root_progress.inc(); - let (resolver, pack) = make_resolver().map_err(hasher::io::Error::from)?; + let (resolver, pack) = make_resolver().map_err(gix_hash::io::Error::from)?; let sorted_pack_offsets_by_oid = { let traverse::Outcome { roots, children } = tree.traverse( resolver, @@ -225,7 +224,7 @@ impl crate::index::File { let header = crate::data::header::encode(pack_version, 0); let mut hasher = gix_hash::hasher(object_hash); hasher.update(&header); - hasher.try_finalize().map_err(hasher::io::Error::from)? + hasher.try_finalize().map_err(gix_hash::io::Error::from)? } None => return Err(Error::IteratorInvariantTrailer), }; @@ -256,7 +255,7 @@ fn modify_base( pack_entry: &crate::data::Entry, decompressed: &[u8], hash: gix_hash::Kind, -) -> Result<(), hasher::Error> { +) -> Result<(), gix_hash::hasher::Error> { let object_kind = pack_entry.header.as_kind().expect("base object as source of iteration"); let id = gix_object::compute_hash(hash, object_kind, decompressed)?; entry.id = id; diff --git a/gix-pack/src/multi_index/write.rs b/gix-pack/src/multi_index/write.rs index f0fafac8baa..cfcc8bb7d18 100644 --- a/gix-pack/src/multi_index/write.rs +++ b/gix-pack/src/multi_index/write.rs @@ -5,7 +5,6 @@ use std::{ }; use gix_features::progress::{Count, DynNestedProgress, Progress}; -use gix_hash::hasher; use crate::multi_index; @@ -15,7 +14,7 @@ mod error { #[allow(missing_docs)] pub enum Error { #[error(transparent)] - Io(#[from] gix_hash::hasher::io::Error), + Io(#[from] gix_hash::io::Error), #[error("Interrupted")] Interrupted, #[error(transparent)] @@ -84,7 +83,7 @@ impl multi_index::File { should_interrupt: &AtomicBool, Options { object_hash }: Options, ) -> Result { - let out = hasher::io::Write::new(out, object_hash); + let out = gix_hash::io::Write::new(out, object_hash); let (index_paths_sorted, index_filenames_sorted) = { index_paths.sort(); let file_names = index_paths @@ -183,7 +182,7 @@ impl multi_index::File { index_paths_sorted.len() as u32, object_hash, ) - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; { progress.set_name("Writing chunks".into()); @@ -191,7 +190,7 @@ impl multi_index::File { let mut chunk_write = cf .into_write(&mut out, bytes_written) - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; while let Some(chunk_to_write) = chunk_write.next_chunk() { match chunk_to_write { multi_index::chunk::index_names::ID => { @@ -209,7 +208,7 @@ impl multi_index::File { ), unknown => unreachable!("BUG: forgot to implement chunk {:?}", std::str::from_utf8(&unknown)), } - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; progress.inc(); if should_interrupt.load(Ordering::Relaxed) { return Err(Error::Interrupted); @@ -218,11 +217,11 @@ impl multi_index::File { } // write trailing checksum - let multi_index_checksum = out.inner.hash.try_finalize().map_err(hasher::io::Error::from)?; + let multi_index_checksum = out.inner.hash.try_finalize().map_err(gix_hash::io::Error::from)?; out.inner .inner .write_all(multi_index_checksum.as_slice()) - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; out.progress.show_throughput(write_start); Ok(Outcome { multi_index_checksum }) diff --git a/gix-pack/src/verify.rs b/gix-pack/src/verify.rs index 77c693278b5..8bb6ce6f55a 100644 --- a/gix-pack/src/verify.rs +++ b/gix-pack/src/verify.rs @@ -1,7 +1,6 @@ use std::{path::Path, sync::atomic::AtomicBool}; use gix_features::progress::Progress; -use gix_hash::hasher; /// pub mod checksum { @@ -45,10 +44,10 @@ pub fn checksum_on_disk_or_mmap( should_interrupt, ) { Ok(id) => id, - Err(hasher::io::Error::Io(err)) if err.kind() == std::io::ErrorKind::Interrupted => { + Err(gix_hash::io::Error::Io(err)) if err.kind() == std::io::ErrorKind::Interrupted => { return Err(checksum::Error::Interrupted); } - Err(hasher::io::Error::Io(_io_err)) => { + Err(gix_hash::io::Error::Io(_io_err)) => { let start = std::time::Instant::now(); let mut hasher = gix_hash::hasher(object_hash); hasher.update(&data[..data_len_without_trailer]); @@ -56,7 +55,7 @@ pub fn checksum_on_disk_or_mmap( progress.show_throughput(start); hasher.try_finalize()? } - Err(hasher::io::Error::Hasher(err)) => return Err(checksum::Error::Hasher(err)), + Err(gix_hash::io::Error::Hasher(err)) => return Err(checksum::Error::Hasher(err)), }; actual.verify(&expected)?; diff --git a/gix-status/src/index_as_worktree/function.rs b/gix-status/src/index_as_worktree/function.rs index d7c8c3031ce..57981937ee9 100644 --- a/gix-status/src/index_as_worktree/function.rs +++ b/gix-status/src/index_as_worktree/function.rs @@ -9,7 +9,6 @@ use bstr::BStr; use filetime::FileTime; use gix_features::parallel::{in_parallel_if, Reduce}; use gix_filter::pipeline::convert::ToGitOutcome; -use gix_hash::hasher; use gix_object::FindExt; use crate::index_as_worktree::Context; @@ -564,8 +563,8 @@ where let platform = self .attr_stack .at_entry(self.rela_path, Some(self.entry.mode), &self.objects) - .map_err(hasher::io::Error::from)?; - let file = std::fs::File::open(self.path).map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; + let file = std::fs::File::open(self.path).map_err(gix_hash::io::Error::from)?; let out = self .filter .convert_to_git( diff --git a/gix-status/src/index_as_worktree/traits.rs b/gix-status/src/index_as_worktree/traits.rs index a75b60beb77..854cedf31f1 100644 --- a/gix-status/src/index_as_worktree/traits.rs +++ b/gix-status/src/index_as_worktree/traits.rs @@ -1,7 +1,7 @@ use std::{io::Read, sync::atomic::AtomicBool}; use bstr::BStr; -use gix_hash::{hasher, ObjectId}; +use gix_hash::ObjectId; use gix_index as index; use index::Entry; @@ -144,15 +144,15 @@ impl CompareBlobs for HashEq { match stream.as_bytes() { Some(buffer) => { let file_hash = gix_object::compute_hash(entry.id.kind(), gix_object::Kind::Blob, buffer) - .map_err(hasher::io::Error::from)?; + .map_err(gix_hash::io::Error::from)?; Ok((entry.id != file_hash).then_some(file_hash)) } None => { let file_hash = match stream.size() { None => { - stream.read_to_end(buf).map_err(hasher::io::Error::from)?; + stream.read_to_end(buf).map_err(gix_hash::io::Error::from)?; gix_object::compute_hash(entry.id.kind(), gix_object::Kind::Blob, buf) - .map_err(hasher::io::Error::from)? + .map_err(gix_hash::io::Error::from)? } Some(len) => gix_object::compute_stream_hash( entry.id.kind(), diff --git a/gix-status/src/index_as_worktree/types.rs b/gix-status/src/index_as_worktree/types.rs index d3d526cb1aa..c4ad3d16cb1 100644 --- a/gix-status/src/index_as_worktree/types.rs +++ b/gix-status/src/index_as_worktree/types.rs @@ -10,7 +10,7 @@ pub enum Error { #[error("The clock was off when reading file related metadata after updating a file on disk")] Time(#[from] std::time::SystemTimeError), #[error("IO error while writing blob or reading file metadata or changing filetype")] - Io(#[from] gix_hash::hasher::io::Error), + Io(#[from] gix_hash::io::Error), #[error("Failed to obtain blob from object database")] Find(#[from] gix_object::find::existing_object::Error), #[error("Could not determine status for submodule at '{rela_path}'")] diff --git a/gix-status/src/index_as_worktree_with_renames/types.rs b/gix-status/src/index_as_worktree_with_renames/types.rs index 8a4cdb614dd..a6fff8980bd 100644 --- a/gix-status/src/index_as_worktree_with_renames/types.rs +++ b/gix-status/src/index_as_worktree_with_renames/types.rs @@ -17,7 +17,7 @@ pub enum Error { #[error("Could not open worktree file for reading")] OpenWorktreeFile(std::io::Error), #[error(transparent)] - HashFile(gix_hash::hasher::io::Error), + HashFile(gix_hash::io::Error), #[error("Could not read worktree link content")] ReadLink(std::io::Error), #[error(transparent)]