diff --git a/diff/git_extended_headers.go b/diff/git_extended_headers.go new file mode 100644 index 0000000..ace3b7b --- /dev/null +++ b/diff/git_extended_headers.go @@ -0,0 +1,94 @@ +package diff + +import "strings" + +const ( + gitExtendedHeaderDiff = "diff --git " + gitExtendedHeaderOldMode = "old mode " + gitExtendedHeaderNewMode = "new mode " + gitExtendedHeaderNewFileMode = "new file mode " + gitExtendedHeaderDeletedFileMode = "deleted file mode " + gitExtendedHeaderRenameFrom = "rename from " + gitExtendedHeaderRenameTo = "rename to " + gitExtendedHeaderCopyFrom = "copy from " + gitExtendedHeaderCopyTo = "copy to " + gitExtendedHeaderIndex = "index " + gitExtendedHeaderBinaryFiles = "Binary files " + gitExtendedHeaderBinaryPatch = "GIT binary patch" +) + +var knownGitExtendedHeaderKinds = [...]string{ + gitExtendedHeaderDiff, + gitExtendedHeaderOldMode, + gitExtendedHeaderNewMode, + gitExtendedHeaderNewFileMode, + gitExtendedHeaderDeletedFileMode, + gitExtendedHeaderRenameFrom, + gitExtendedHeaderRenameTo, + gitExtendedHeaderCopyFrom, + gitExtendedHeaderCopyTo, + gitExtendedHeaderIndex, + gitExtendedHeaderBinaryFiles, + gitExtendedHeaderBinaryPatch, +} + +type gitExtendedHeader struct { + raw string + kind string +} + +func (h gitExtendedHeader) value() string { + return h.raw[len(h.kind):] +} + +type gitExtendedHeaders []gitExtendedHeader + +func parseGitExtendedHeaders(raw []string) (gitExtendedHeaders, bool) { + if len(raw) == 0 || !strings.HasPrefix(raw[0], gitExtendedHeaderDiff) { + return nil, false + } + + headers := make(gitExtendedHeaders, len(raw)) + for i, line := range raw { + headers[i].raw = line + for _, kind := range knownGitExtendedHeaderKinds { + if strings.HasPrefix(line, kind) { + headers[i].kind = kind + break + } + } + } + return headers, true +} + +type gitExtendedHeaderPair struct { + from string + to string +} + +var ( + gitModeHeaderPair = gitExtendedHeaderPair{gitExtendedHeaderOldMode, gitExtendedHeaderNewMode} + gitRenameHeaderPair = gitExtendedHeaderPair{gitExtendedHeaderRenameFrom, gitExtendedHeaderRenameTo} + gitCopyHeaderPair = gitExtendedHeaderPair{gitExtendedHeaderCopyFrom, gitExtendedHeaderCopyTo} +) + +func (h gitExtendedHeaders) hasKind(index int, kind string) bool { + return index >= 0 && index < len(h) && h[index].kind == kind +} + +func (h gitExtendedHeaders) hasPairAt(index int, pair gitExtendedHeaderPair) bool { + return h.hasKind(index, pair.from) && h.hasKind(index+1, pair.to) +} + +func (h gitExtendedHeaders) pairIndices(pair gitExtendedHeaderPair) (from, to int, ok bool) { + from, to = -1, -1 + for i, header := range h { + if from < 0 && header.kind == pair.from { + from = i + } + if to < 0 && header.kind == pair.to { + to = i + } + } + return from, to, from >= 0 && to >= 0 +} diff --git a/diff/git_extended_headers_test.go b/diff/git_extended_headers_test.go new file mode 100644 index 0000000..8deb19f --- /dev/null +++ b/diff/git_extended_headers_test.go @@ -0,0 +1,70 @@ +package diff + +import ( + "bytes" + "errors" + "reflect" + "testing" +) + +func TestParseFileDiffPreservesCRLFGitExtendedHeaders(t *testing.T) { + input := []byte("diff --git a/old name b/new name\r\nsimilarity index 100%\r\nrename from old name\r\nrename to new name\r\n") + wantExtended := []string{ + "diff --git a/old name b/new name\r", + "similarity index 100%\r", + "rename from old name\r", + "rename to new name\r", + } + + fd, err := ParseFileDiffOptions(input, ParseOptions{KeepCR: true}) + if err != nil { + t.Fatal(err) + } + // Keep the existing edge behavior: with KeepCR, reconstruction + // finds the first ambiguous path but not the second. + if fd.OrigName != "a/old name" || fd.NewName != "" { + t.Errorf("got names %q -> %q, want %q -> empty", fd.OrigName, fd.NewName, "a/old name") + } + if !reflect.DeepEqual(fd.Extended, wantExtended) { + t.Errorf("extended headers changed:\nwant: %q\n got: %q", wantExtended, fd.Extended) + } +} + +func TestHandleEmptyRejectsMalformedGitExtendedHeaders(t *testing.T) { + tests := []struct { + name string + extended []string + }{ + {name: "empty"}, + {name: "non-git", extended: []string{"diff --ruN a/f b/f", "old mode 100644", "new mode 100755"}}, + {name: "reversed mode pair", extended: []string{"diff --git a/f b/f", "new mode 100755", "old mode 100644"}}, + {name: "copy prefixes missing spaces", extended: []string{"diff --git a/f b/g", "similarity index 100%", "copy from", "copy to"}}, + {name: "unknown extra header", extended: []string{"diff --git a/f b/f", "old mode 100644", "new mode 100755", "x-header value"}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fd := &FileDiff{Extended: test.extended} + if handleEmpty(fd) { + t.Fatal("handleEmpty accepted malformed headers") + } + if fd.OrigName != "" || fd.NewName != "" { + t.Errorf("handleEmpty changed names to %q -> %q", fd.OrigName, fd.NewName) + } + }) + } +} + +func TestMalformedGitExtendedHeadersStillReturnOriginalParseError(t *testing.T) { + input := []byte("diff --git a/f b/f\nold mode 100644\nunknown header\n") + r := NewFileDiffReader(bytes.NewReader(input)) + fd, err := r.ReadAllHeaders() + var parseErr *ParseError + if !errors.As(err, &parseErr) || parseErr.Err != ErrExtendedHeadersEOF { + t.Fatalf("got error %v, want ErrExtendedHeadersEOF", err) + } + wantExtended := []string{"diff --git a/f b/f", "old mode 100644", "unknown header"} + if !reflect.DeepEqual(fd.Extended, wantExtended) { + t.Errorf("extended headers changed:\nwant: %q\n got: %q", wantExtended, fd.Extended) + } +} diff --git a/diff/parse.go b/diff/parse.go index ca947cf..9bbaccb 100644 --- a/diff/parse.go +++ b/diff/parse.go @@ -373,7 +373,7 @@ func (r *FileDiffReader) ReadExtendedHeaders() ([]string, error) { r.fileHeaderLine = nil } - if bytes.HasPrefix(line, []byte("diff --git ")) { + if bytes.HasPrefix(line, []byte(gitExtendedHeaderDiff)) { if firstLine { firstLine = false } else { @@ -508,43 +508,36 @@ func parseDiffGitArgs(diffArgs string) (string, string, bool) { // that follow. It updates fd fields from the parsed extended headers. func handleEmpty(fd *FileDiff) (wasEmpty bool) { lineCount := len(fd.Extended) - if lineCount > 0 && !strings.HasPrefix(fd.Extended[0], "diff --git ") { + headers, ok := parseGitExtendedHeaders(fd.Extended) + if !ok { return false } - lineHasPrefix := func(idx int, prefix string) bool { - return strings.HasPrefix(fd.Extended[idx], prefix) - } - - linesHavePrefixes := func(idx1 int, prefix1 string, idx2 int, prefix2 string) bool { - return lineHasPrefix(idx1, prefix1) && lineHasPrefix(idx2, prefix2) - } - - isCopy := (lineCount == 4 && linesHavePrefixes(2, "copy from ", 3, "copy to ")) || - (lineCount == 6 && linesHavePrefixes(2, "copy from ", 3, "copy to ") && lineHasPrefix(5, "Binary files ")) || - (lineCount == 6 && linesHavePrefixes(1, "old mode ", 2, "new mode ") && linesHavePrefixes(4, "copy from ", 5, "copy to ")) + isCopy := (lineCount == 4 && headers.hasPairAt(2, gitCopyHeaderPair)) || + (lineCount == 6 && headers.hasPairAt(2, gitCopyHeaderPair) && headers.hasKind(5, gitExtendedHeaderBinaryFiles)) || + (lineCount == 6 && headers.hasPairAt(1, gitModeHeaderPair) && headers.hasPairAt(4, gitCopyHeaderPair)) - isRename := (lineCount == 4 && linesHavePrefixes(2, "rename from ", 3, "rename to ")) || - (lineCount == 5 && linesHavePrefixes(2, "rename from ", 3, "rename to ") && lineHasPrefix(4, "Binary files ")) || - (lineCount == 6 && linesHavePrefixes(2, "rename from ", 3, "rename to ") && lineHasPrefix(5, "Binary files ")) || - (lineCount == 6 && linesHavePrefixes(1, "old mode ", 2, "new mode ") && linesHavePrefixes(4, "rename from ", 5, "rename to ")) + isRename := (lineCount == 4 && headers.hasPairAt(2, gitRenameHeaderPair)) || + (lineCount == 5 && headers.hasPairAt(2, gitRenameHeaderPair) && headers.hasKind(4, gitExtendedHeaderBinaryFiles)) || + (lineCount == 6 && headers.hasPairAt(2, gitRenameHeaderPair) && headers.hasKind(5, gitExtendedHeaderBinaryFiles)) || + (lineCount == 6 && headers.hasPairAt(1, gitModeHeaderPair) && headers.hasPairAt(4, gitRenameHeaderPair)) - isDeletedFile := (lineCount == 3 || lineCount == 4 && lineHasPrefix(3, "Binary files ") || lineCount > 4 && lineHasPrefix(3, "GIT binary patch")) && - lineHasPrefix(1, "deleted file mode ") + isDeletedFile := (lineCount == 3 || lineCount == 4 && headers.hasKind(3, gitExtendedHeaderBinaryFiles) || lineCount > 4 && headers.hasKind(3, gitExtendedHeaderBinaryPatch)) && + headers.hasKind(1, gitExtendedHeaderDeletedFileMode) - isNewFile := (lineCount == 3 || lineCount == 4 && lineHasPrefix(3, "Binary files ") || lineCount > 4 && lineHasPrefix(3, "GIT binary patch")) && - lineHasPrefix(1, "new file mode ") + isNewFile := (lineCount == 3 || lineCount == 4 && headers.hasKind(3, gitExtendedHeaderBinaryFiles) || lineCount > 4 && headers.hasKind(3, gitExtendedHeaderBinaryPatch)) && + headers.hasKind(1, gitExtendedHeaderNewFileMode) - isModeChange := lineCount == 3 && linesHavePrefixes(1, "old mode ", 2, "new mode ") + isModeChange := lineCount == 3 && headers.hasPairAt(1, gitModeHeaderPair) - isBinaryPatch := lineCount == 3 && lineHasPrefix(2, "Binary files ") || lineCount > 3 && lineHasPrefix(2, "GIT binary patch") + isBinaryPatch := lineCount == 3 && headers.hasKind(2, gitExtendedHeaderBinaryFiles) || lineCount > 3 && headers.hasKind(2, gitExtendedHeaderBinaryPatch) if !isModeChange && !isCopy && !isRename && !isBinaryPatch && !isNewFile && !isDeletedFile { return false } var success bool - fd.OrigName, fd.NewName, success = parseDiffGitArgs(fd.Extended[0][len("diff --git "):]) + fd.OrigName, fd.NewName, success = parseDiffGitArgs(headers[0].value()) if isNewFile { fd.OrigName = "/dev/null" } @@ -555,13 +548,10 @@ func handleEmpty(fd *FileDiff) (wasEmpty bool) { // For ambiguous 'diff --git' lines, try to reconstruct filenames using extended headers. if success && (isCopy || isRename) && fd.OrigName == "" && fd.NewName == "" { - diffArgs := fd.Extended[0][len("diff --git "):] + diffArgs := headers[0].value() - tryReconstruct := func(header string, prefix string, whichFile int, result *string) { - if !strings.HasPrefix(header, prefix) { - return - } - rawFilename := header[len(prefix):] + tryReconstruct := func(header gitExtendedHeader, whichFile int, result *string) { + rawFilename := header.value() rawFilename = strings.TrimSuffix(rawFilename, "\r") // extract the filename prefix (e.g. "a/") from the 'diff --git' line. @@ -578,11 +568,13 @@ func handleEmpty(fd *FileDiff) (wasEmpty bool) { *result = diffArgs[prefixLetterIndex:prefixLetterIndex+2] + rawFilename } - for _, header := range fd.Extended { - tryReconstruct(header, "copy from ", 1, &fd.OrigName) - tryReconstruct(header, "copy to ", 2, &fd.NewName) - tryReconstruct(header, "rename from ", 1, &fd.OrigName) - tryReconstruct(header, "rename to ", 2, &fd.NewName) + for _, header := range headers { + switch header.kind { + case gitExtendedHeaderCopyFrom, gitExtendedHeaderRenameFrom: + tryReconstruct(header, 1, &fd.OrigName) + case gitExtendedHeaderCopyTo, gitExtendedHeaderRenameTo: + tryReconstruct(header, 2, &fd.NewName) + } } } return success diff --git a/diff/reverse.go b/diff/reverse.go index f9045d8..0c4d1b6 100644 --- a/diff/reverse.go +++ b/diff/reverse.go @@ -38,27 +38,27 @@ func ReverseFileDiff(fd *FileDiff) (*FileDiff, error) { // reverseExtendedHeaders reverses the direction encoded in git's extended headers. func reverseExtendedHeaders(headers []string, origName, newName string) ([]string, error) { - // handleEmpty gates on the same prefix when it reads the direction back out. - if len(headers) == 0 || !strings.HasPrefix(headers[0], "diff --git ") { + parsed, ok := parseGitExtendedHeaders(headers) + if !ok { return headers, nil } reversed := make([]string, len(headers)) copy(reversed, headers) reversed[0] = reverseDiffGitHeader(reversed[0], origName, newName) - for i, header := range reversed { - switch { - case strings.HasPrefix(header, "new file mode "): - reversed[i] = "deleted file mode " + header[len("new file mode "):] - case strings.HasPrefix(header, "deleted file mode "): - reversed[i] = "new file mode " + header[len("deleted file mode "):] - case strings.HasPrefix(header, "index "): + for i, header := range parsed { + switch header.kind { + case gitExtendedHeaderNewFileMode: + reversed[i] = gitExtendedHeaderDeletedFileMode + header.value() + case gitExtendedHeaderDeletedFileMode: + reversed[i] = gitExtendedHeaderNewFileMode + header.value() + case gitExtendedHeaderIndex: reversed[i] = reverseIndexHeader(header) - case strings.HasPrefix(header, "copy from "), strings.HasPrefix(header, "copy to "): + case gitExtendedHeaderCopyFrom, gitExtendedHeaderCopyTo: return nil, ErrCannotReverseCopy } } - swapHeaderValues(reversed, "old mode ", "new mode ") - swapHeaderValues(reversed, "rename from ", "rename to ") + swapHeaderValues(reversed, parsed, gitModeHeaderPair) + swapHeaderValues(reversed, parsed, gitRenameHeaderPair) return reversed, nil } @@ -67,7 +67,7 @@ func reverseExtendedHeaders(headers []string, origName, newName string) ([]strin // ambiguous input; names recovered from other headers can disambiguate Git's // unquoted paths containing spaces. func reverseDiffGitHeader(header, origName, newName string) string { - const prefix = "diff --git " + const prefix = gitExtendedHeaderDiff args := header[len(prefix):] lineEnding := "" if strings.HasSuffix(args, "\r") { @@ -122,30 +122,23 @@ func splitDiffGitArgs(args, first, second string) (string, string, bool) { // swapHeaderValues exchanges the values of the first "from" header and the // first "to" header, leaving both prefixes where they are. -func swapHeaderValues(headers []string, fromPrefix, toPrefix string) { - from, to := -1, -1 - for i, header := range headers { - if from < 0 && strings.HasPrefix(header, fromPrefix) { - from = i - } - if to < 0 && strings.HasPrefix(header, toPrefix) { - to = i - } - } - if from < 0 || to < 0 { +func swapHeaderValues(headers []string, parsed gitExtendedHeaders, pair gitExtendedHeaderPair) { + from, to, ok := parsed.pairIndices(pair) + if !ok { return } - headers[from], headers[to] = fromPrefix+headers[to][len(toPrefix):], toPrefix+headers[from][len(fromPrefix):] + headers[from] = pair.from + parsed[to].value() + headers[to] = pair.to + parsed[from].value() } // reverseIndexHeader swaps the two blob hashes in an "index ..[ ]" // header, leaving the trailing mode (if any) alone. -func reverseIndexHeader(header string) string { - const prefix = "index " - oldHash, newHash, ok := strings.Cut(header[len(prefix):], "..") +func reverseIndexHeader(header gitExtendedHeader) string { + oldHash, newHash, ok := strings.Cut(header.value(), "..") if !ok || strings.ContainsAny(oldHash, " \r") { - return header + return header.raw } + prefix := gitExtendedHeaderIndex if i := strings.IndexAny(newHash, " \r"); i >= 0 { return prefix + newHash[:i] + ".." + oldHash + newHash[i:] } diff --git a/diff/reverse_test.go b/diff/reverse_test.go index bcda325..d37c1aa 100644 --- a/diff/reverse_test.go +++ b/diff/reverse_test.go @@ -255,6 +255,16 @@ func TestReverseFileDiffExtendedHeaders(t *testing.T) { input: []string{"diff --git a/f b/f\r", "index 94954ab..8b14c4f 100644\r"}, want: []string{"diff --git b/f a/f\r", "index 8b14c4f..94954ab 100644\r"}, }, + { + name: "CRLF mode and rename", + input: []string{"diff --git a/old b/new\r", "old mode 100644\r", "new mode 100755\r", "rename from old\r", "rename to new\r"}, + want: []string{"diff --git b/new a/old\r", "old mode 100755\r", "new mode 100644\r", "rename from new\r", "rename to old\r"}, + }, + { + name: "unknown and malformed headers", + input: []string{"diff --git a/f b/f", "x-header value", "old mode 100644", "copy from", "index missing-separator"}, + want: []string{"diff --git b/f a/f", "x-header value", "old mode 100644", "copy from", "index missing-separator"}, + }, { name: "no extended headers", input: nil, @@ -343,13 +353,15 @@ func TestReverseFileDiffGitHeader(t *testing.T) { } func TestReverseFileDiffRejectsCopy(t *testing.T) { - input := []byte("diff --git a/old b/new\nsimilarity index 100%\ncopy from old\ncopy to new\n") - fd, err := ParseFileDiff(input) - if err != nil { - t.Fatal(err) + tests := [][]string{ + {"diff --git a/old b/new", "similarity index 100%", "copy from old", "copy to new"}, + {"diff --git a/old b/new", "copy from old"}, } - if _, err := ReverseFileDiff(fd); err != ErrCannotReverseCopy { - t.Fatalf("ReverseFileDiff error = %v, want ErrCannotReverseCopy", err) + for _, extended := range tests { + fd := &FileDiff{Extended: extended} + if _, err := ReverseFileDiff(fd); err != ErrCannotReverseCopy { + t.Errorf("ReverseFileDiff error = %v, want ErrCannotReverseCopy for headers %q", err, extended) + } } }