Skip to content

Commit 1e5e25b

Browse files
committed
fix(pack): infer trailer hash kind from checksum length, release 0.8.4
PackEncoder finalizes its running checksum on whichever async worker thread runs the task, but ObjectHash::from_bytes re-read the thread-local HashKind at finalize time. On threads where set_hash_kind was never called (default SHA-1), finalizing a SHA-256 pack panicked with "Invalid byte length: got 32, expected 20", surfacing as flaky SHA-256 pack failures (e.g. 'libra bundle create' on SHA-256 repositories). Add ObjectHash::from_bytes_infer_kind, which derives the kind from the checksum byte length (20 -> SHA-1, 32 -> SHA-256) instead of the thread-local kind, and use it in both the delta (encode/mod.rs) and non-delta parallel (encode/parallel.rs) trailer paths, replacing the unwrap() with a propagated GitError. Includes a regression test that drives a SHA-256 encoder on a fresh thread holding the default SHA-1 thread-local kind. Also: bump version to 0.8.4, point repository metadata at https://github.com/libra-tools/git-internal, add CHANGELOG.md, and resolve pre-existing clippy -D warnings debt (delta::decode match -> ?, internal::index iter() -> values()) with no behavior change. Signed-off-by: Quanyi Ma <eli@patch.sh>
1 parent fd0bacb commit 1e5e25b

9 files changed

Lines changed: 158 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Changelog
2+
3+
All notable changes to this project are documented in this file.
4+
5+
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
6+
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
7+
8+
## [0.8.4] - 2026-07-25
9+
10+
### Fixed
11+
12+
- Pack encoding no longer panics with `Invalid byte length: got 32, expected 20`
13+
when finalizing a SHA-256 pack trailer on a thread whose thread-local
14+
`HashKind` was never set. `PackEncoder` finalizes its running checksum on
15+
whichever async worker thread happens to run the task; the previous
16+
`ObjectHash::from_bytes` call re-read the thread-local kind at finalize
17+
time and could disagree with the hasher chosen at encoder construction,
18+
which surfaced as flaky SHA-256 pack failures (affecting, for example,
19+
`libra bundle create` on SHA-256 repositories). Both the delta path
20+
(`encode/mod.rs`) and the non-delta parallel path (`encode/parallel.rs`)
21+
now infer the hash kind from the checksum byte length via the new
22+
`ObjectHash::from_bytes_infer_kind`, and propagate a `GitError` instead of
23+
unwrapping.
24+
25+
### Added
26+
27+
- `ObjectHash::from_bytes_infer_kind` — constructs an `ObjectHash` from raw
28+
bytes by inferring the hash kind from the byte length (20 → SHA-1,
29+
32 → SHA-256) instead of consulting the thread-local `HashKind`.
30+
- Regression test
31+
`internal::pack::encode::tests::test_parallel_encode_trailer_ignores_thread_local_kind`,
32+
which drives a SHA-256 encoder on a fresh thread holding the default SHA-1
33+
thread-local kind.
34+
35+
### Changed
36+
37+
- Package `repository` metadata now points to
38+
<https://github.com/libra-tools/git-internal>.
39+
- Resolved pre-existing clippy `-D warnings` debt so the lint gate passes
40+
again: `match``?` in `delta::decode`, and `iter()``values()` map
41+
iteration in `internal::index` (no behavior change).
42+
43+
## [0.8.3] - 2026-07-24
44+
45+
- Previous release; changes predate this changelog.

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "git-internal"
3-
version = "0.8.3"
3+
version = "0.8.4"
44
edition = "2024"
55
authors = ["Eli Ma <genedna@gmail.com>"]
66
description = "High-performance Rust library for Git internal objects, Pack files, and AI-assisted development objects (Intent, Plan, Task, Run, Evidence, Decision) with delta compression, streaming I/O, and smart protocol support."
@@ -9,7 +9,7 @@ categories = ["development-tools", "encoding", "parser-implementations"]
99
documentation = "https://libra.tools/docs/internal"
1010
readme = "README.md"
1111
homepage = "https://libra.tools"
12-
repository = "https://github.com/web3infra-foundation/libra"
12+
repository = "https://github.com/libra-tools/git-internal"
1313
license = "MIT"
1414
exclude = ["tests", ".github", ".claude", ".devcontainer", "examples"]
1515

src/delta/decode/mod.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,7 @@ pub fn delta_decode(
7171
GitDeltaError::DeltaDecoderError("Invalid copy instruction".to_string())
7272
});
7373

74-
match base_data {
75-
Ok(data) => buffer.extend_from_slice(data),
76-
Err(e) => return Err(e),
77-
}
74+
buffer.extend_from_slice(base_data?);
7875
}
7976
}
8077
assert!(buffer.len() == result_size);

src/hash.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,33 @@ impl ObjectHash {
211211
}
212212
}
213213
}
214+
/// Create `ObjectHash` from raw bytes, inferring the hash kind from the
215+
/// byte length (20 → SHA-1, 32 → SHA-256).
216+
///
217+
/// Unlike [`ObjectHash::from_bytes`], this does not consult the
218+
/// thread-local [`HashKind`], so it is safe on threads where
219+
/// [`set_hash_kind`] was never called. The pack encoder finalizes its
220+
/// running checksum on whichever async worker thread happens to run the
221+
/// task; the checksum bytes already carry the correct length, while the
222+
/// worker's thread-local may still hold the default SHA-1 kind — the
223+
/// mismatch used to panic with "Invalid byte length: got 32, expected 20".
224+
pub fn from_bytes_infer_kind(bytes: &[u8]) -> Result<ObjectHash, String> {
225+
match bytes.len() {
226+
20 => {
227+
let mut h = [0u8; 20];
228+
h.copy_from_slice(bytes);
229+
Ok(ObjectHash::Sha1(h))
230+
}
231+
32 => {
232+
let mut h = [0u8; 32];
233+
h.copy_from_slice(bytes);
234+
Ok(ObjectHash::Sha256(h))
235+
}
236+
other => Err(format!(
237+
"Invalid byte length: got {other}, expected 20 (SHA-1) or 32 (SHA-256)"
238+
)),
239+
}
240+
}
214241
/// Read hash bytes from a stream according to current hash size.
215242
pub fn from_stream(data: &mut impl io::Read) -> io::Result<ObjectHash> {
216243
match get_hash_kind() {

src/internal/index.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ impl Index {
383383
file.write_all(&header)?;
384384
hash.update(&header);
385385

386-
for (_, entry) in self.entries.iter() {
386+
for entry in self.entries.values() {
387387
let mut entry_bytes = Vec::new();
388388
entry_bytes.write_u32::<BigEndian>(entry.ctime.seconds)?;
389389
entry_bytes.write_u32::<BigEndian>(entry.ctime.nanos)?;
@@ -607,7 +607,7 @@ mod tests {
607607

608608
let index = Index::from_file(source).unwrap();
609609
assert_eq!(index.size(), 760);
610-
for (_, entry) in index.entries.iter() {
610+
for entry in index.entries.values() {
611611
println!("{entry}");
612612
}
613613
}
@@ -621,7 +621,7 @@ mod tests {
621621

622622
let index = Index::from_file(source).unwrap();
623623
assert_eq!(index.size(), 9);
624-
for (_, entry) in index.entries.iter() {
624+
for entry in index.entries.values() {
625625
println!("{entry}");
626626
}
627627
}

src/internal/pack/encode/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,8 +404,13 @@ impl PackEncoder {
404404
self.idx_entries = Some(idx_entries);
405405

406406
// The checksum is both the pack trailer and the identifier used in pack-<hash>.pack.
407+
// Infer the kind from the checksum length: this task may run on an async worker
408+
// thread whose thread-local `HashKind` was never set, so `ObjectHash::from_bytes`
409+
// could disagree with the hasher chosen at encoder construction.
407410
let hash_result = self.inner_hash.clone().finalize();
408-
self.final_hash = Some(ObjectHash::from_bytes(&hash_result).unwrap());
411+
self.final_hash = Some(
412+
ObjectHash::from_bytes_infer_kind(&hash_result).map_err(GitError::PackEncodeError)?,
413+
);
409414
self.send_data(hash_result).await;
410415

411416
self.drop_sender();

src/internal/pack/encode/parallel.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,13 @@ impl super::PackEncoder {
110110
}
111111

112112
// Append the checksum trailer only after every encoded entry has updated the running hash.
113+
// Infer the kind from the checksum length: this task may run on an async worker
114+
// thread whose thread-local `HashKind` was never set, so `ObjectHash::from_bytes`
115+
// could disagree with the hasher chosen at encoder construction.
113116
let hash_result = self.inner_hash.clone().finalize();
114-
self.final_hash = Some(ObjectHash::from_bytes(&hash_result).unwrap());
117+
self.final_hash = Some(
118+
ObjectHash::from_bytes_infer_kind(&hash_result).map_err(GitError::PackEncodeError)?,
119+
);
115120
self.send_data(hash_result).await;
116121
self.drop_sender();
117122

src/internal/pack/encode/tests.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,3 +1071,70 @@ fn test_multi_point_similar_no_match() {
10711071
fn test_multi_point_similar_too_small() {
10721072
assert!(!multi_point_similar(&[1u8, 2, 3], &[4u8, 5, 6]));
10731073
}
1074+
1075+
/// Regression: the pack trailer must be built from the checksum's own length,
1076+
/// not from the thread-local `HashKind`. The encoder is constructed under
1077+
/// SHA-256 and then driven on a fresh thread whose thread-local still holds
1078+
/// the default SHA-1 kind — the previous `ObjectHash::from_bytes` call
1079+
/// panicked there ("Invalid byte length: got 32, expected 20"), which
1080+
/// surfaced as flaky SHA-256 pack failures in async runtimes that migrate
1081+
/// tasks across worker threads.
1082+
#[test]
1083+
fn test_parallel_encode_trailer_ignores_thread_local_kind() {
1084+
use crate::hash::get_hash_kind;
1085+
1086+
let _guard = set_hash_kind_for_test(HashKind::Sha256);
1087+
1088+
let entries: Vec<Entry> = (0..8)
1089+
.map(|i| Entry::from(Blob::from_content(&format!("thread-local-kind-{i}"))))
1090+
.collect();
1091+
let (tx, mut rx) = mpsc::channel(16);
1092+
let (entry_tx, entry_rx) = mpsc::channel::<MetaAttached<Entry, EntryMeta>>(16);
1093+
let mut encoder = PackEncoder::new(entries.len(), 0, tx);
1094+
1095+
std::thread::spawn(move || {
1096+
// A fresh thread never saw `set_hash_kind`, so it holds the default
1097+
// SHA-1 kind — the exact condition that used to panic on finalize.
1098+
assert_eq!(get_hash_kind(), HashKind::Sha1);
1099+
let rt = tokio::runtime::Builder::new_current_thread()
1100+
.enable_all()
1101+
.build()
1102+
.expect("build current-thread runtime");
1103+
rt.block_on(async move {
1104+
for entry in entries {
1105+
entry_tx
1106+
.send(MetaAttached {
1107+
inner: entry,
1108+
meta: EntryMeta::new(),
1109+
})
1110+
.await
1111+
.expect("send entry");
1112+
}
1113+
drop(entry_tx);
1114+
1115+
encoder
1116+
.parallel_encode(entry_rx)
1117+
.await
1118+
.expect("parallel encode must succeed off the origin thread");
1119+
1120+
let mut pack = Vec::new();
1121+
while let Some(chunk) = rx.recv().await {
1122+
pack.extend(chunk);
1123+
}
1124+
let trailer = encoder
1125+
.get_hash()
1126+
.expect("final hash must be recorded after encode");
1127+
assert!(
1128+
matches!(trailer, ObjectHash::Sha256(_)),
1129+
"trailer must stay SHA-256 regardless of the worker thread's kind"
1130+
);
1131+
assert_eq!(
1132+
&pack[pack.len() - 32..],
1133+
trailer.to_data().as_slice(),
1134+
"pack trailer bytes must be the 32-byte SHA-256 checksum"
1135+
);
1136+
});
1137+
})
1138+
.join()
1139+
.expect("encoder thread must not panic");
1140+
}

0 commit comments

Comments
 (0)