Skip to content

Commit b7ad2d4

Browse files
6543zeripath
andauthored
* 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 <[email protected]> Co-authored-by: Andrew Thornton <[email protected]>
1 parent 6edd6d5 commit b7ad2d4

File tree

2 files changed

+96
-6
lines changed

2 files changed

+96
-6
lines changed

services/gitdiff/gitdiff.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,15 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio
613613
leftLine, rightLine := 1, 1
614614

615615
for {
616+
for isFragment {
617+
curFile.IsIncomplete = true
618+
_, isFragment, err = input.ReadLine()
619+
if err != nil {
620+
// Now by the definition of ReadLine this cannot be io.EOF
621+
err = fmt.Errorf("Unable to ReadLine: %v", err)
622+
return
623+
}
624+
}
616625
sb.Reset()
617626
lineBytes, isFragment, err = input.ReadLine()
618627
if err != nil {
@@ -726,6 +735,10 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio
726735
}
727736
}
728737
}
738+
if len(line) > maxLineCharacters {
739+
curFile.IsIncomplete = true
740+
line = line[:maxLineCharacters]
741+
}
729742
curSection.Lines[len(curSection.Lines)-1].Content = line
730743

731744
// handle LFS

services/gitdiff/gitdiff_test.go

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"encoding/json"
1010
"fmt"
1111
"html/template"
12+
"strconv"
1213
"strings"
1314
"testing"
1415

@@ -95,11 +96,11 @@ func TestParsePatch_singlefile(t *testing.T) {
9596
name: "really weird filename",
9697
gitdiff: `diff --git "\\a/a b/file b/a a/file" "\\b/a b/file b/a a/file"
9798
index d2186f1..f5c8ed2 100644
98-
--- "\\a/a b/file b/a a/file"
99-
+++ "\\b/a b/file b/a a/file"
99+
--- "\\a/a b/file b/a a/file" ` + `
100+
+++ "\\b/a b/file b/a a/file" ` + `
100101
@@ -1,3 +1,2 @@
101102
Create a weird file.
102-
103+
` + `
103104
-and what does diff do here?
104105
\ No newline at end of file`,
105106
addition: 0,
@@ -112,7 +113,7 @@ index d2186f1..f5c8ed2 100644
112113
gitdiff: `diff --git "\\a/file with blanks" "\\b/file with blanks"
113114
deleted file mode 100644
114115
index 898651a..0000000
115-
--- "\\a/file with blanks"
116+
--- "\\a/file with blanks" ` + `
116117
+++ /dev/null
117118
@@ -1,5 +0,0 @@
118119
-a blank file
@@ -205,7 +206,83 @@ index 6961180..9ba1a00 100644
205206
})
206207
}
207208

208-
var diff = `diff --git "a/README.md" "b/README.md"
209+
// Test max lines
210+
diffBuilder := &strings.Builder{}
211+
212+
var diff = `diff --git a/newfile2 b/newfile2
213+
new file mode 100644
214+
index 0000000..6bb8f39
215+
--- /dev/null
216+
+++ b/newfile2
217+
@@ -0,0 +1,35 @@
218+
`
219+
diffBuilder.WriteString(diff)
220+
221+
for i := 0; i < 35; i++ {
222+
diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n")
223+
}
224+
diff = diffBuilder.String()
225+
result, err := ParsePatch(20, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
226+
if err != nil {
227+
t.Errorf("There should not be an error: %v", err)
228+
}
229+
if !result.Files[0].IsIncomplete {
230+
t.Errorf("Files should be incomplete! %v", result.Files[0])
231+
}
232+
result, err = ParsePatch(40, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
233+
if err != nil {
234+
t.Errorf("There should not be an error: %v", err)
235+
}
236+
if result.Files[0].IsIncomplete {
237+
t.Errorf("Files should not be incomplete! %v", result.Files[0])
238+
}
239+
result, err = ParsePatch(40, 5, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
240+
if err != nil {
241+
t.Errorf("There should not be an error: %v", err)
242+
}
243+
if !result.Files[0].IsIncomplete {
244+
t.Errorf("Files should be incomplete! %v", result.Files[0])
245+
}
246+
247+
// Test max characters
248+
diff = `diff --git a/newfile2 b/newfile2
249+
new file mode 100644
250+
index 0000000..6bb8f39
251+
--- /dev/null
252+
+++ b/newfile2
253+
@@ -0,0 +1,35 @@
254+
`
255+
diffBuilder.Reset()
256+
diffBuilder.WriteString(diff)
257+
258+
for i := 0; i < 33; i++ {
259+
diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n")
260+
}
261+
diffBuilder.WriteString("+line33")
262+
for i := 0; i < 512; i++ {
263+
diffBuilder.WriteString("0123456789ABCDEF")
264+
}
265+
diffBuilder.WriteByte('\n')
266+
diffBuilder.WriteString("+line" + strconv.Itoa(34) + "\n")
267+
diffBuilder.WriteString("+line" + strconv.Itoa(35) + "\n")
268+
diff = diffBuilder.String()
269+
270+
result, err = ParsePatch(20, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
271+
if err != nil {
272+
t.Errorf("There should not be an error: %v", err)
273+
}
274+
if !result.Files[0].IsIncomplete {
275+
t.Errorf("Files should be incomplete! %v", result.Files[0])
276+
}
277+
result, err = ParsePatch(40, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
278+
if err != nil {
279+
t.Errorf("There should not be an error: %v", err)
280+
}
281+
if !result.Files[0].IsIncomplete {
282+
t.Errorf("Files should be incomplete! %v", result.Files[0])
283+
}
284+
285+
diff = `diff --git "a/README.md" "b/README.md"
209286
--- a/README.md
210287
+++ b/README.md
211288
@@ -1,3 +1,6 @@
@@ -216,7 +293,7 @@ index 6961180..9ba1a00 100644
216293
Docker Pulls
217294
+ cut off
218295
+ cut off`
219-
result, err := ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
296+
result, err = ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
220297
if err != nil {
221298
t.Errorf("ParsePatch failed: %s", err)
222299
}

0 commit comments

Comments
 (0)