this repo has no description
Return errors for empty text fragments (#29)
This fixes two issues:
1. Fragments with only context lines were accepted
2. Fragments with only a "no newline" marker caused a panic
The panic was found by go-fuzz. While fixing that issue, I
discovered the first problem with other types of empty fragments while
comparing behavior against 'git apply'.
Also extract some helper functions and modify the loop conditions to
clean things up while making changes in this area.
authored by
Billy Keyes
and committed by
GitHub
4f540371
b6a90110
+48
-18
+30
-18
gitdiff/text.go
+30
-18
gitdiff/text.go
···
79
79
return p.Errorf(0, "no content following fragment header")
80
80
}
81
81
82
-
isNoNewlineLine := func(s string) bool {
83
-
// test for "\ No newline at end of file" by prefix because the text
84
-
// changes by locale (git claims all versions are at least 12 chars)
85
-
return len(s) >= 12 && s[:2] == "\\ "
86
-
}
87
-
88
82
oldLines, newLines := frag.OldLines, frag.NewLines
89
-
for {
83
+
for oldLines > 0 || newLines > 0 {
90
84
line := p.Line(0)
91
85
op, data := line[0], line[1:]
92
86
···
113
107
frag.LinesAdded++
114
108
frag.TrailingContext = 0
115
109
frag.Lines = append(frag.Lines, Line{OpAdd, data})
116
-
default:
110
+
case '\\':
117
111
// this may appear in middle of fragment if it's for a deleted line
118
-
if isNoNewlineLine(line) {
119
-
last := &frag.Lines[len(frag.Lines)-1]
120
-
last.Line = strings.TrimSuffix(last.Line, "\n")
112
+
if isNoNewlineMarker(line) {
113
+
removeLastNewline(frag)
121
114
break
122
115
}
116
+
fallthrough
117
+
default:
123
118
// TODO(bkeyes): if this is because we hit the next header, it
124
119
// would be helpful to return the miscounts line error. We could
125
120
// either test for the common headers ("@@ -", "diff --git") or
126
121
// assume any invalid op ends the fragment; git returns the same
127
122
// generic error in all cases so either is compatible
128
123
return p.Errorf(0, "invalid line operation: %q", op)
129
-
}
130
-
131
-
next := p.Line(1)
132
-
if oldLines <= 0 && newLines <= 0 && !isNoNewlineLine(next) {
133
-
break
134
124
}
135
125
136
126
if err := p.Next(); err != nil {
···
145
135
hdr := max(frag.OldLines-oldLines, frag.NewLines-newLines) + 1
146
136
return p.Errorf(-hdr, "fragment header miscounts lines: %+d old, %+d new", -oldLines, -newLines)
147
137
}
138
+
if frag.LinesAdded == 0 && frag.LinesDeleted == 0 {
139
+
return p.Errorf(0, "fragment contains no changes")
140
+
}
148
141
149
-
if err := p.Next(); err != nil && err != io.EOF {
150
-
return err
142
+
// check for a final "no newline" marker since it is not included in the
143
+
// counters used to stop the loop above
144
+
if isNoNewlineMarker(p.Line(0)) {
145
+
removeLastNewline(frag)
146
+
if err := p.Next(); err != nil && err != io.EOF {
147
+
return err
148
+
}
151
149
}
150
+
152
151
return nil
152
+
}
153
+
154
+
func isNoNewlineMarker(s string) bool {
155
+
// test for "\ No newline at end of file" by prefix because the text
156
+
// changes by locale (git claims all versions are at least 12 chars)
157
+
return len(s) >= 12 && s[:2] == "\\ "
158
+
}
159
+
160
+
func removeLastNewline(frag *TextFragment) {
161
+
if len(frag.Lines) > 0 {
162
+
last := &frag.Lines[len(frag.Lines)-1]
163
+
last.Line = strings.TrimSuffix(last.Line, "\n")
164
+
}
153
165
}
154
166
155
167
func parseRange(s string) (start int64, end int64, err error) {
+18
gitdiff/text_test.go
+18
gitdiff/text_test.go
···
317
317
},
318
318
Err: true,
319
319
},
320
+
"onlyContext": {
321
+
Input: ` context line
322
+
context line
323
+
`,
324
+
Fragment: TextFragment{
325
+
OldLines: 2,
326
+
NewLines: 2,
327
+
},
328
+
Err: true,
329
+
},
330
+
"unexpectedNoNewlineMarker": {
331
+
Input: `\ No newline at end of file`,
332
+
Fragment: TextFragment{
333
+
OldLines: 1,
334
+
NewLines: 1,
335
+
},
336
+
Err: true,
337
+
},
320
338
}
321
339
322
340
for name, test := range tests {