From 45ca2e460538943b0db5421770297e2929e4273a Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 21 Nov 2020 22:41:24 +0000 Subject: [PATCH] Handle incomplete diff files properly (#13662) * Handle incomplete diff files properly The code for parsing diff hunks has a bug whereby a very long line in a very long diff would not be completely read leading to an unexpected character. This PR ensures that the line is completely cleared Fix #13602 Signed-off-by: Andrew Thornton * Also allow git max line length <4096 Signed-off-by: Andrew Thornton * Add test case Signed-off-by: Andrew Thornton Co-authored-by: techknowlogick --- services/gitdiff/gitdiff.go | 13 +++++ services/gitdiff/gitdiff_test.go | 89 +++++++++++++++++++++++++++++--- 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 4f223fdc031..79cd16e193d 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -676,6 +676,15 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio leftLine, rightLine := 1, 1 for { + for isFragment { + curFile.IsIncomplete = true + _, isFragment, err = input.ReadLine() + if err != nil { + // Now by the definition of ReadLine this cannot be io.EOF + err = fmt.Errorf("Unable to ReadLine: %v", err) + return + } + } sb.Reset() lineBytes, isFragment, err = input.ReadLine() if err != nil { @@ -790,6 +799,10 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio } } } + if len(line) > maxLineCharacters { + curFile.IsIncomplete = true + line = line[:maxLineCharacters] + } curSection.Lines[len(curSection.Lines)-1].Content = line // handle LFS diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index 6e3b5b09946..cd7b2273cc3 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -9,6 +9,7 @@ import ( "encoding/json" "fmt" "html/template" + "strconv" "strings" "testing" @@ -153,11 +154,11 @@ func TestParsePatch_singlefile(t *testing.T) { name: "really weird filename", gitdiff: `diff --git "\\a/a b/file b/a a/file" "\\b/a b/file b/a a/file" index d2186f1..f5c8ed2 100644 ---- "\\a/a b/file b/a a/file" -+++ "\\b/a b/file b/a a/file" +--- "\\a/a b/file b/a a/file" ` + ` ++++ "\\b/a b/file b/a a/file" ` + ` @@ -1,3 +1,2 @@ Create a weird file. - + ` + ` -and what does diff do here? \ No newline at end of file`, addition: 0, @@ -170,7 +171,7 @@ index d2186f1..f5c8ed2 100644 gitdiff: `diff --git "\\a/file with blanks" "\\b/file with blanks" deleted file mode 100644 index 898651a..0000000 ---- "\\a/file with blanks" +--- "\\a/file with blanks" ` + ` +++ /dev/null @@ -1,5 +0,0 @@ -a blank file @@ -263,7 +264,83 @@ index 6961180..9ba1a00 100644 }) } - var diff = `diff --git "a/README.md" "b/README.md" + // Test max lines + diffBuilder := &strings.Builder{} + + var diff = `diff --git a/newfile2 b/newfile2 +new file mode 100644 +index 0000000..6bb8f39 +--- /dev/null ++++ b/newfile2 +@@ -0,0 +1,35 @@ +` + diffBuilder.WriteString(diff) + + for i := 0; i < 35; i++ { + diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n") + } + diff = diffBuilder.String() + result, err := ParsePatch(20, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff)) + if err != nil { + t.Errorf("There should not be an error: %v", err) + } + if !result.Files[0].IsIncomplete { + t.Errorf("Files should be incomplete! %v", result.Files[0]) + } + result, err = ParsePatch(40, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff)) + if err != nil { + t.Errorf("There should not be an error: %v", err) + } + if result.Files[0].IsIncomplete { + t.Errorf("Files should not be incomplete! %v", result.Files[0]) + } + result, err = ParsePatch(40, 5, setting.Git.MaxGitDiffFiles, strings.NewReader(diff)) + if err != nil { + t.Errorf("There should not be an error: %v", err) + } + if !result.Files[0].IsIncomplete { + t.Errorf("Files should be incomplete! %v", result.Files[0]) + } + + // Test max characters + diff = `diff --git a/newfile2 b/newfile2 +new file mode 100644 +index 0000000..6bb8f39 +--- /dev/null ++++ b/newfile2 +@@ -0,0 +1,35 @@ +` + diffBuilder.Reset() + diffBuilder.WriteString(diff) + + for i := 0; i < 33; i++ { + diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n") + } + diffBuilder.WriteString("+line33") + for i := 0; i < 512; i++ { + diffBuilder.WriteString("0123456789ABCDEF") + } + diffBuilder.WriteByte('\n') + diffBuilder.WriteString("+line" + strconv.Itoa(34) + "\n") + diffBuilder.WriteString("+line" + strconv.Itoa(35) + "\n") + diff = diffBuilder.String() + + result, err = ParsePatch(20, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff)) + if err != nil { + t.Errorf("There should not be an error: %v", err) + } + if !result.Files[0].IsIncomplete { + t.Errorf("Files should be incomplete! %v", result.Files[0]) + } + result, err = ParsePatch(40, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff)) + if err != nil { + t.Errorf("There should not be an error: %v", err) + } + if !result.Files[0].IsIncomplete { + t.Errorf("Files should be incomplete! %v", result.Files[0]) + } + + diff = `diff --git "a/README.md" "b/README.md" --- a/README.md +++ b/README.md @@ -1,3 +1,6 @@ @@ -274,7 +351,7 @@ index 6961180..9ba1a00 100644 Docker Pulls + cut off + cut off` - result, err := ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff)) + result, err = ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff)) if err != nil { t.Errorf("ParsePatch failed: %s", err) }