diff --git a/internal/git/diff.go b/internal/git/diff.go index 95c74c0..045862d 100644 --- a/internal/git/diff.go +++ b/internal/git/diff.go @@ -1,7 +1,6 @@ package git import ( - "bufio" "bytes" "crypto/sha256" "fmt" @@ -229,29 +228,34 @@ func getGitDiff(data DiffContext, executor gitCommandExecutor) ([]*diff.FileDiff return gitDiff, nil } -func hunkHash(hunk *diff.Hunk) [32]byte { - // Generate a hash for a hunk based on its added and removed lines. - var lines []byte - data := hunk.Body +const ( + addedLine = '+' + removedLine = '-' + noNewlineAtEOF = '\\' +) - if len(data) == 0 { - return sha256.Sum256(nil) +func splitLine(data []byte) (line, rest []byte) { + i := bytes.IndexByte(data, '\n') + if i < 0 { + return data, nil } + return bytes.TrimSuffix(data[:i], []byte("\r")), data[i+1:] +} - scanner := bufio.NewScanner(bytes.NewReader(data)) +func hunkHash(hunk *diff.Hunk) [32]byte { + sum := sha256.New() + var line []byte - for scanner.Scan() { - line := scanner.Text() + for data := hunk.Body; len(data) > 0; { + line, data = splitLine(data) if len(line) == 0 { continue } switch line[0] { - case '+', '-': - // Include the line type and content - lines = append(lines, line...) - default: - // Skip context lines + case addedLine, removedLine, noNewlineAtEOF: + _, _ = sum.Write(line) + _, _ = sum.Write([]byte{'\n'}) } } - return sha256.Sum256(lines) + return [32]byte(sum.Sum(nil)) } diff --git a/internal/git/diff_test.go b/internal/git/diff_test.go index 59a2244..4b7d711 100644 --- a/internal/git/diff_test.go +++ b/internal/git/diff_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "strings" "testing" "github.com/multimediallc/codeowners-plus/pkg/codeowners" @@ -129,7 +130,7 @@ Binary files a/assets/img/offline.png and b/assets/img/offline.png differ`, expectedErr: false, expectedFiles: 2, expectedHunks: map[string]int{ - "file1.go": 1, + "file1.go": 1, "assets/img/offline.png": 0, }, }, @@ -387,6 +388,48 @@ func TestHunkHash(t *testing.T) { hunk2Body: []byte(``), expectedSame: true, }, + { + name: "two lines vs their concatenation", + hunkBody: []byte("+total = x\n+y\n"), + hunk2Body: []byte("+total = x+y\n"), + expectedSame: false, + }, + { + name: "added then removed vs one joined line", + hunkBody: []byte("+foo\n-bar\n"), + hunk2Body: []byte("+foo-bar\n"), + expectedSame: false, + }, + { + name: "a trailing newline is a change", + hunkBody: []byte("+value\n\\ No newline at end of file\n"), + hunk2Body: []byte("+value\n"), + expectedSame: false, + }, + { + name: "differ only past the old 64KB scanner limit", + hunkBody: []byte("+keep()\n+" + strings.Repeat("x", 70*1024) + "A"), + hunk2Body: []byte("+keep()\n+" + strings.Repeat("x", 70*1024) + "B"), + expectedSame: false, + }, + { + name: "identical over-long hunks", + hunkBody: []byte("+keep()\n+" + strings.Repeat("x", 70*1024) + "A"), + hunk2Body: []byte("+keep()\n+" + strings.Repeat("x", 70*1024) + "A"), + expectedSame: true, + }, + { + name: "an unterminated CR is content", + hunkBody: []byte("+keep()\n+value\r"), + hunk2Body: []byte("+keep()\n+value"), + expectedSame: false, + }, + { + name: "a CRLF file hashes like its LF twin", + hunkBody: []byte("+keep()\r\n+value\r\n"), + hunk2Body: []byte("+keep()\n+value\n"), + expectedSame: true, + }, } for _, tc := range tt { @@ -806,3 +849,6 @@ func TestDiffOfDiffs(t *testing.T) { } } } + +// A hunk matching the approval-time diff is dropped as already reviewed, so a +// collision past the old 64KB read limit retained an approval over unseen change.