this repo has no description
0
fork

Configure Feed

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

Split up binary parsing to match text parsing

Parse the fragment header separately from the fragment chunk, which
makes each function a bit more understandable.

+79 -54
+1
gitdiff/gitdiff.go
··· 98 98 // BinaryFragment describes changes to a binary file. 99 99 type BinaryFragment struct { 100 100 Method BinaryPatchMethod 101 + Size int64 101 102 Data []byte 102 103 } 103 104
+78 -54
gitdiff/parser.go
··· 7 7 "encoding/ascii85" 8 8 "fmt" 9 9 "io" 10 + "io/ioutil" 10 11 "strconv" 11 12 "strings" 12 13 ) ··· 326 327 } 327 328 328 329 f.IsBinary = true 329 - if hasData { 330 - forward, err := p.ParseBinaryFragment() 331 - if err != nil { 332 - return n, err 333 - } 334 - if forward == nil { 335 - return n, p.Errorf(0, "missing data for binary patch") 336 - } 337 - f.BinaryFragment = forward 338 - n++ 330 + if !hasData { 331 + return 0, nil 332 + } 339 333 340 - // valid for reverse to not exist, but it must be valid if present 341 - reverse, err := p.ParseBinaryFragment() 342 - if err != nil { 343 - return n, err 344 - } 345 - f.ReverseBinaryFragment = reverse 334 + forward, err := p.ParseBinaryFragmentHeader() 335 + if err != nil { 336 + return 0, err 337 + } 338 + if forward == nil { 339 + return 0, p.Errorf(0, "missing data for binary patch") 346 340 } 341 + if err := p.ParseBinaryChunk(forward); err != nil { 342 + return 0, err 343 + } 344 + f.BinaryFragment = forward 347 345 348 - return n, nil 346 + // valid for reverse to not exist, but it must be valid if present 347 + reverse, err := p.ParseBinaryFragmentHeader() 348 + if err != nil { 349 + return 1, err 350 + } 351 + if reverse == nil { 352 + return 1, nil 353 + } 354 + if err := p.ParseBinaryChunk(reverse); err != nil { 355 + return 1, err 356 + } 357 + f.ReverseBinaryFragment = reverse 358 + 359 + return 1, nil 349 360 } 350 361 351 362 func (p *parser) ParseBinaryMarker() (isBinary bool, hasData bool, err error) { ··· 364 375 return true, hasData, nil 365 376 } 366 377 367 - func (p *parser) ParseBinaryFragment() (*BinaryFragment, error) { 368 - // TODO(bkeyes): split this function into small parts 369 - // TODO(bkeyes): add summary of data format so this is less mysterious 370 - const ( 371 - shortestValidLine = "A00000\n" 372 - maxBytesPerLine = 52 373 - ) 374 - 378 + func (p *parser) ParseBinaryFragmentHeader() (*BinaryFragment, error) { 375 379 parts := strings.SplitN(p.Line(0), " ", 2) 376 380 if len(parts) < 2 { 377 381 return nil, nil ··· 387 391 return nil, nil 388 392 } 389 393 390 - totalBytes, err := strconv.ParseInt(parts[1], 10, 64) 391 - if err != nil { 394 + var err error 395 + if frag.Size, err = strconv.ParseInt(parts[1], 10, 64); err != nil { 392 396 nerr := err.(*strconv.NumError) 393 - return nil, p.Errorf(0, "binary patch: invalid data length: %v", nerr.Err) 397 + return nil, p.Errorf(0, "binary patch: invalid size: %v", nerr.Err) 398 + } 399 + 400 + if err := p.Next(); err != nil && err != io.EOF { 401 + return nil, err 394 402 } 403 + return frag, nil 404 + } 405 + 406 + func (p *parser) ParseBinaryChunk(frag *BinaryFragment) error { 407 + // Binary fragments are encoded as a series of base85 encoded lines. Each 408 + // line starts with a character in [A-Za-z] giving the number of bytes on 409 + // the line, where A = 1 and z = 52, and ends with a newline character. 410 + // 411 + // The base85 encoding means each line is a multiple of 5 characters + 2 412 + // additional characters for the length byte and the newline. The fragment 413 + // ends with a blank line. 414 + const ( 415 + shortestValidLine = "A00000\n" 416 + maxBytesPerLine = 52 417 + ) 395 418 396 419 var data bytes.Buffer 397 420 buf := make([]byte, maxBytesPerLine) 398 - 399 421 for { 400 - if err := p.Next(); err != nil { 401 - if err == io.EOF { 402 - break 403 - } 404 - return nil, err 405 - } 406 422 line := p.Line(0) 407 - 408 423 if line == "\n" { 409 - // blank line indicates the end of the fragment 410 424 break 411 425 } 412 426 413 - // base85 encoding means each line is a multiple of 5 + first char and newline 414 427 if len(line) < len(shortestValidLine) || (len(line)-2)%5 != 0 { 415 - return nil, p.Errorf(0, "binary patch: corrupt data line") 428 + return p.Errorf(0, "binary patch: corrupt data line") 416 429 } 417 430 418 431 byteCount := int(line[0]) ··· 422 435 case 'a' <= byteCount && byteCount <= 'z': 423 436 byteCount = byteCount - 'a' + 27 424 437 default: 425 - return nil, p.Errorf(0, "binary patch: invalid length byte: %q", line[0]) 438 + return p.Errorf(0, "binary patch: invalid length byte: %q", line[0]) 426 439 } 427 440 428 441 // base85 encodes every 4 bytes into 5 characters, with up to 3 bytes of end padding 429 442 maxByteCount := (len(line) - 2) / 5 * 4 430 443 if byteCount >= maxByteCount || byteCount < maxByteCount-3 { 431 - return nil, p.Errorf(0, "binary patch: incorrect byte count: %d", byteCount) 444 + return p.Errorf(0, "binary patch: incorrect byte count: %d", byteCount) 432 445 } 433 446 434 447 ndst, _, err := ascii85.Decode(buf, []byte(line[1:]), byteCount < maxBytesPerLine) 435 448 if err != nil { 436 - return nil, p.Errorf(0, "binary patch: %v", err) 449 + return p.Errorf(0, "binary patch: %v", err) 437 450 } 438 451 if ndst != byteCount { 439 - return nil, p.Errorf(0, "binary patch: expected %d bytes, but decoded %d", byteCount, ndst) 452 + return p.Errorf(0, "binary patch: %d byte line decoded as %d", byteCount, ndst) 440 453 } 441 454 data.Write(buf[:ndst]) 455 + 456 + if err := p.Next(); err != nil { 457 + if err == io.EOF { 458 + return p.Errorf(0, "binary patch: unexpected EOF") 459 + } 460 + return err 461 + } 442 462 } 443 463 444 - if err := inflateBinaryChunk(frag, &data, totalBytes); err != nil { 445 - return nil, p.Errorf(0, "binary patch: %v", err) 464 + if err := inflateBinaryChunk(frag, &data); err != nil { 465 + return p.Errorf(0, "binary patch: %v", err) 446 466 } 447 467 448 468 // consume the empty line that ended the fragment 449 469 if err := p.Next(); err != nil && err != io.EOF { 450 - return nil, err 470 + return err 451 471 } 452 - return frag, nil 472 + return nil 453 473 } 454 474 455 - func inflateBinaryChunk(frag *BinaryFragment, r io.Reader, length int64) error { 456 - data := make([]byte, length) 457 - 475 + func inflateBinaryChunk(frag *BinaryFragment, r io.Reader) (err error) { 458 476 inflater := flate.NewReader(r) 459 - if _, err := io.ReadFull(inflater, frag.Data); err != nil { 477 + defer func() { 478 + if cerr := inflater.Close(); cerr != nil && err == nil { 479 + err = cerr 480 + } 481 + }() 482 + 483 + data, err := ioutil.ReadAll(inflater) 484 + if err != nil { 460 485 return err 461 486 } 462 - if err := inflater.Close(); err != nil { 463 - return err 487 + if int64(len(data)) != frag.Size { 488 + return fmt.Errorf("%d byte fragment inflated to %d", frag.Size, len(data)) 464 489 } 465 - 466 490 frag.Data = data 467 491 return nil 468 492 }