From 5e6dacf7a95fcac988684713b9c125401dd37374 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Sun, 13 Sep 2026 07:23:14 +0000 Subject: [PATCH 1/2] fix: bound the numstat split so a tab in a path does not throw [patch] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The numstat record carries its path inside the same NUL-delimited token as the counts, so splitting on every tab turns a legally tab-named path into a fourth field and throws GitParseException — taking out the entire listing rather than one entry. Bound the split at three fields and relax the length check to `< 3`, matching what GitLogParser and GitTagParser already do. Both existing shapes still read the same: a rename's "0\t0\t" yields an empty third field and consumes its two following path tokens, and a malformed record with too few fields still throws. Fixes #102 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Vh7BAYZXphEn2dcrQddn1X --- .../Parsing/GitDiffParserTests.cs | 42 +++++++++++++++++++ GitIntegration/Parsing/GitDiffParser.cs | 11 +++-- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/GitIntegration.Test/Parsing/GitDiffParserTests.cs b/GitIntegration.Test/Parsing/GitDiffParserTests.cs index 6fe83df..0c10b71 100644 --- a/GitIntegration.Test/Parsing/GitDiffParserTests.cs +++ b/GitIntegration.Test/Parsing/GitDiffParserTests.cs @@ -201,6 +201,48 @@ public void ReadsAPathBeginningWithAColonWithoutMistakingItForARawHeader() Assert.AreEqual(2, entries[0].Insertions); } + [TestMethod] + public void ReadsANumstatPathContainingATabWithoutMistakingItForAnExtraField() + { + // A tab is a legal byte in a git path, and the numstat record carries its path inside the same + // NUL-delimited token as the counts — so an unbounded split on '\t' sees four fields and throws, + // taking out the whole listing over one oddly-named file. The raw section is unaffected, because + // there the path is its own token; only this section needs the split bounded. + // + // Shape captured from git 2.43 for a file named "tabname.txt". + string output = + ":000000 100644 0000000 bca70f3 A\0tab\tname.txt\0" + + "1\t0\ttab\tname.txt\0"; + + IReadOnlyList entries = GitDiffParser.ParseWithLineCounts(output); + + Assert.AreEqual(1, entries.Count); + Assert.AreEqual(GitChangeKind.Added, entries[0].Kind); + Assert.AreEqual("tab\tname.txt", entries[0].Path.WeakString); + Assert.AreEqual(1, entries[0].Insertions); + Assert.AreEqual(0, entries[0].Deletions); + } + + [TestMethod] + public void ReadsARenameWhoseNumstatPathsContainTabs() + { + // The rename form leaves the path field empty and follows the record with two path tokens, so + // the bound must not disturb it: "1\t0\t" still has to read as an empty third field and consume + // the two tokens after it, however many tabs those paths themselves carry. + string output = + ":100644 100644 de98044 d68dd40 R075\0old\tname.txt\0new\tname.txt\0" + + "1\t0\t\0old\tname.txt\0new\tname.txt\0"; + + IReadOnlyList entries = GitDiffParser.ParseWithLineCounts(output); + + Assert.AreEqual(1, entries.Count); + Assert.AreEqual(GitChangeKind.Renamed, entries[0].Kind); + Assert.AreEqual("new\tname.txt", entries[0].Path.WeakString); + Assert.AreEqual("old\tname.txt", entries[0].OriginalPath?.WeakString); + Assert.AreEqual(1, entries[0].Insertions); + Assert.AreEqual(0, entries[0].Deletions); + } + [TestMethod] public void ReportsAnEmptyListForAnEmptyDiff() { diff --git a/GitIntegration/Parsing/GitDiffParser.cs b/GitIntegration/Parsing/GitDiffParser.cs index 328d0c0..21ecab5 100644 --- a/GitIntegration/Parsing/GitDiffParser.cs +++ b/GitIntegration/Parsing/GitDiffParser.cs @@ -180,9 +180,14 @@ private static List ReadRawSection(string[] tokens, ref int index) // "\t\t", or "\t\t" followed by two // path tokens for a rename or a copy. The path is not read here at all: correlation is // positional, so only the counts and the number of tokens each record consumes matter. - string[] fields = record.Split('\t'); - - if (fields.Length != 3) + // + // Bounded so a tab embedded in the trailing path — a legal byte in a git path — is absorbed + // by that field rather than shifting the count, matching how GitLogParser and GitTagParser + // bound their own splits. Unlike the raw section, where the path is its own NUL-delimited + // token, here it shares a token with the counts, so only the bound keeps the two apart. + string[] fields = record.Split('\t', 3); + + if (fields.Length < 3) { throw new GitParseException($"Malformed numstat diff record: '{record}'."); } From 6f40677d61f0108a59667abdfdb46e37199aef67 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Sun, 13 Sep 2026 07:28:32 +0000 Subject: [PATCH 2/2] test: assert the platform-correct outcome for a tab-named path [patch] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two new parser tests read a tab-named path end to end, which Windows CI refused: RelativeFilePath rejects a control character there, and the raw section converts its path before any numstat record is read, so the entry is turned away before the bounded split is reached. That refusal is deliberate — GitParseValues documents it as reporting the path rather than dropping the entry — so branch on OperatingSystem.IsWindows() and assert what each platform can observe: the full read on POSIX, and on Windows that the failure is the representability one rather than a malformed numstat record. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Vh7BAYZXphEn2dcrQddn1X --- .../Parsing/GitDiffParserTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/GitIntegration.Test/Parsing/GitDiffParserTests.cs b/GitIntegration.Test/Parsing/GitDiffParserTests.cs index 0c10b71..b60955b 100644 --- a/GitIntegration.Test/Parsing/GitDiffParserTests.cs +++ b/GitIntegration.Test/Parsing/GitDiffParserTests.cs @@ -214,6 +214,21 @@ public void ReadsANumstatPathContainingATabWithoutMistakingItForAnExtraField() ":000000 100644 0000000 bca70f3 A\0tab\tname.txt\0" + "1\t0\ttab\tname.txt\0"; + if (OperatingSystem.IsWindows()) + { + // RelativeFilePath refuses a control character on Windows, which GitParseValues documents + // as deliberate: reporting the path beats dropping the entry. That refusal happens in the + // raw section, where the path is converted, before the numstat section is read at all — so + // a tab-named file is unreadable end to end on this platform whatever the split does, and + // what is worth pinning here is that it is turned away for that stated reason rather than + // as a malformed numstat record. + GitParseException refused = Assert.ThrowsExactly( + () => _ = GitDiffParser.ParseWithLineCounts(output)); + + StringAssert.Contains(refused.Message, "cannot be represented as a relative file path"); + return; + } + IReadOnlyList entries = GitDiffParser.ParseWithLineCounts(output); Assert.AreEqual(1, entries.Count); @@ -233,6 +248,17 @@ public void ReadsARenameWhoseNumstatPathsContainTabs() ":100644 100644 de98044 d68dd40 R075\0old\tname.txt\0new\tname.txt\0" + "1\t0\t\0old\tname.txt\0new\tname.txt\0"; + if (OperatingSystem.IsWindows()) + { + // Same platform limit as the case above, and reached the same way: the raw section converts + // both of a rename's paths before any numstat record is read. + GitParseException refused = Assert.ThrowsExactly( + () => _ = GitDiffParser.ParseWithLineCounts(output)); + + StringAssert.Contains(refused.Message, "cannot be represented as a relative file path"); + return; + } + IReadOnlyList entries = GitDiffParser.ParseWithLineCounts(output); Assert.AreEqual(1, entries.Count);