aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author65432020-11-22 14:57:28 +0100
committerGitHub2020-11-22 15:57:28 +0200
commitb7ad2d45576d27b4934ed1860ad545d48e106391 (patch)
treee6ff4d41a0522ae1a33b852f94bd3b04b4aa76e8
parent6edd6d5a2485bb22a4cdb8e01015ce6b2f1df4d3 (diff)
* Handle incomplete diff files properly (#13669)
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 * Also allow git max line length <4096 * Add test case Fix #13602 Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Andrew Thornton <art27@cantab.net>
-rw-r--r--services/gitdiff/gitdiff.go13
-rw-r--r--services/gitdiff/gitdiff_test.go89
2 files changed, 96 insertions, 6 deletions
diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go
index 4efe9e93a..90c5795ba 100644
--- a/services/gitdiff/gitdiff.go
+++ b/services/gitdiff/gitdiff.go
@@ -613,6 +613,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 {
@@ -726,6 +735,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 3a8246633..f2f84bc44 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"
@@ -95,11 +96,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,
@@ -112,7 +113,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
@@ -205,7 +206,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 @@
@@ -216,7 +293,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)
}