* 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>

diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go
index 4efe9e93ae2aee663f88649aa49b0ebf4d6c2e67..90c5795ba54cba3134a3e4459ea6609f5a6e4a4c 100644 (file)
--- 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 3a82466330ef3d4d358299cf2a184a5db4f12800..f2f84bc449caf1939a0fc19c58eb2ac0716bc8ab 100644 (file)
--- 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)
        }