this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Fix parser advancement in header functions

After parsing a valid header, the current line of the parser should be
the last line of the header. If the header is invalid (the parse
function returns an error), the state of the parser is undefined.

Also add a test since this is easy to mess up.

+70 -9
+16 -9
gitdiff/file_header.go
··· 28 28 29 29 f := &File{} 30 30 for { 31 + end, err := parseGitHeaderData(f, p.Line(1), defaultName) 32 + if err != nil { 33 + return nil, p.Errorf(1, "git file header: %v", err) 34 + } 35 + if end { 36 + break 37 + } 38 + 31 39 if err := p.Next(); err != nil { 32 40 if err == io.EOF { 33 41 break 34 42 } 35 43 return nil, err 36 44 } 37 - 38 - end, err := parseGitHeaderData(f, p.Line(0), defaultName) 39 - if err != nil { 40 - return nil, p.Errorf(1, "git file header: %v", err) 41 - } 42 - if end { 43 - break 44 - } 45 45 } 46 46 47 47 if f.OldName == "" && f.NewName == "" { ··· 76 76 return nil, nil 77 77 } 78 78 79 + // advance past the first line so parser is at end of header 80 + // no EOF check needed because we know there are >=3 valid lines 81 + if err := p.Next(); err != nil { 82 + return nil, err 83 + } 84 + 79 85 oldName, _, err := parseName(oldLine[len(oldPrefix):], '\t', 0) 80 86 if err != nil { 81 87 return nil, p.Errorf(0, "file header: %v", err) ··· 105 111 f.NewName = newName 106 112 } 107 113 } 114 + 108 115 return f, nil 109 116 } 110 117 ··· 138 145 // It returns true when header parsing is complete; in that case, line was the 139 146 // first line of non-header content. 140 147 func parseGitHeaderData(f *File, line, defaultName string) (end bool, err error) { 141 - if line[len(line)-1] == '\n' { 148 + if len(line) > 0 && line[len(line)-1] == '\n' { 142 149 line = line[:len(line)-1] 143 150 } 144 151
+54
gitdiff/file_header_test.go
··· 277 277 } 278 278 } 279 279 280 + func TestParserAdvancment(t *testing.T) { 281 + tests := map[string]struct { 282 + Input string 283 + Parse func(p *parser) error 284 + NextLine string 285 + }{ 286 + "ParseGitFileHeader": { 287 + Input: `diff --git a/dir/file.txt b/dir/file.txt 288 + index 9540595..30e6333 100644 289 + --- a/dir/file.txt 290 + +++ b/dir/file.txt 291 + @@ -1,2 +1,3 @@ 292 + context line 293 + `, 294 + Parse: func(p *parser) error { 295 + _, err := p.ParseGitFileHeader() 296 + return err 297 + }, 298 + NextLine: "@@ -1,2 +1,3 @@\n", 299 + }, 300 + "ParseTraditionalFileHeader": { 301 + Input: `--- dir/file.txt 302 + +++ dir/file.txt 303 + @@ -1,2 +1,3 @@ 304 + context line 305 + `, 306 + Parse: func(p *parser) error { 307 + _, err := p.ParseTraditionalFileHeader() 308 + return err 309 + }, 310 + NextLine: "@@ -1,2 +1,3 @@\n", 311 + }, 312 + } 313 + 314 + for name, test := range tests { 315 + t.Run(name, func(t *testing.T) { 316 + p := &parser{r: bufio.NewReader(strings.NewReader(test.Input))} 317 + p.Next() 318 + 319 + if err := test.Parse(p); err != nil { 320 + t.Fatalf("unexpected error while parsing: %v", err) 321 + } 322 + 323 + if err := p.Next(); err != nil { 324 + t.Fatalf("advancing the parser after parsing returned an error: %v", err) 325 + } 326 + 327 + if test.NextLine != p.Line(0) { 328 + t.Errorf("incorrect next line after parsing\nexpected: %q\nactual: %q", test.NextLine, p.Line(0)) 329 + } 330 + }) 331 + } 332 + } 333 + 280 334 func TestCleanName(t *testing.T) { 281 335 tests := map[string]struct { 282 336 Input string