From a0b4e056f2bce32f77f26fb87ce671a99f0a1124 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:18:53 +0800 Subject: [PATCH] test(git): fuzz the status parser, and drop the pathless entry it found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `parse_porcelain_v2` had tests for every record type, non-UTF8 paths, newlines in paths and both caps — and none for a stream that stops in the middle. Truncating a realistic sample at every byte, plus 2,000 seeded corruptions, found no panic but did find 27 cuts that produce a `StatusEntry` naming no file at all, one for each record type: a record severed between its last field and its path parses down to an entry with an empty path. Not reachable today — `status_of` parses only when git exited 0, and a git that exits 0 has written all of it. But this module's stated bargain is that a record it cannot read is dropped, "far better for the panel than no status at all", and a pathless entry is one it passed on half-built instead. It would draw as a blank row whose own Stage and Discard buttons hand git an empty pathspec. So `push` drops it, and does not count it either. The fuzz stays as the guard: every truncation rather than a sampled few, because the interesting cuts are exactly the ones between a field and its separator, and the corruptions are seeded so a failure reproduces from its message. --- crates/tty7-core/src/core/git/status.rs | 99 +++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/crates/tty7-core/src/core/git/status.rs b/crates/tty7-core/src/core/git/status.rs index 54cd0d4d..4bebd7de 100644 --- a/crates/tty7-core/src/core/git/status.rs +++ b/crates/tty7-core/src/core/git/status.rs @@ -691,7 +691,18 @@ impl Parser { /// Past the cap we keep counting but stop keeping, so the panel can say /// "10000 of 42311" rather than quietly showing a short list. + /// + /// A record with no path is dropped outright, not counted. Git does not + /// emit one — a file cannot be named "" — but a record cut off between its + /// last field and its path parses down to exactly that, and this module's + /// bargain is that a record it cannot read is dropped rather than passed + /// on half-built. A pathless entry is worse than a missing one: it draws + /// as a blank row, and the row's own Stage and Discard buttons would hand + /// git an empty pathspec. fn push(&mut self, entry: StatusEntry) { + if entry.path.as_str().is_empty() { + return; + } self.total += 1; if self.entries.len() < MAX_STATUS_ENTRIES { self.entries.push(entry); @@ -1071,6 +1082,94 @@ mod tests { ) } + /// Nothing `git status` can hand back makes the parser panic. + /// + /// It reads a pipe. `git` killed mid-write, a disk that filled, or a + /// daemon torn down under a slow repository all end the stream between + /// bytes, and the parser sees a record that stops in the middle of a + /// field. A panic here used to cost the whole thread pool (see + /// `ui::host_ops`); it now costs one operation, which is still an SCM + /// panel that never fills in. + /// + /// Every truncation, not a sampled few — the interesting cuts are exactly + /// the ones between a field and its separator, and there is no guessing + /// which those are. The bit flips and splices then cover what a truncation + /// cannot reach: a length that disagrees with its payload, a NUL where a + /// path was, a record type that does not exist. + #[test] + fn no_truncation_or_corruption_of_a_status_stream_panics() { + let sample = [ + head_records(), + rec(&["# branch.upstream origin/main"]), + rec(&["# branch.ab +2 -3"]), + ordinary("M.", "src/main.rs"), + ordinary(".M", "a file with spaces.txt"), + rec(&[ + "2 R. N... 100644 100644 100644 ", + SHA, + " ", + SHA, + " R92 new/path.rs\0old/path.rs", + ]), + rec(&[ + "u UU N... 100644 100644 100644 100644 ", + SHA, + " ", + SHA, + " ", + SHA, + " both.rs", + ]), + rec(&["? untracked.rs"]), + rec(&["! ignored.rs"]), + ] + .concat(); + + // A parse that returns is a parse that did not panic; the invariants + // are what keep a returned-but-nonsense answer from passing as one. + let check = |bytes: &[u8], what: &str| { + let parsed = parse_porcelain_v2(bytes); + assert!( + parsed.entries.len() <= parsed.total_entries, + "{what}: {} entries reported out of a total of {}", + parsed.entries.len(), + parsed.total_entries + ); + for entry in &parsed.entries { + assert!( + !entry.path.as_str().is_empty(), + "{what}: an entry naming no file, whose Stage and Discard \ + buttons would hand git an empty pathspec" + ); + } + }; + + for cut in 0..=sample.len() { + check(&sample[..cut], &format!("truncated to {cut} bytes")); + } + + // Deterministic, so a failure is reproducible from the message alone. + let mut seed = 0x2545_F491_4F6C_DD1Du64; + let mut next = move || { + seed ^= seed << 13; + seed ^= seed >> 7; + seed ^= seed << 17; + seed + }; + for round in 0..2_000 { + let mut bytes = sample.clone(); + for _ in 0..(next() % 8 + 1) { + let at = (next() as usize) % bytes.len(); + match next() % 3 { + 0 => bytes[at] ^= 1 << (next() % 8), + 1 => bytes[at] = 0, + _ => bytes[at] = b'\n', + } + } + check(&bytes, &format!("corruption round {round}")); + } + } + #[test] fn a_full_branch_header_lands_in_every_field() { let parsed = parse_porcelain_v2(