diff --git a/gitdiff/apply_binary.go b/gitdiff/apply_binary.go index b34772d..1668aca 100644 --- a/gitdiff/apply_binary.go +++ b/gitdiff/apply_binary.go @@ -49,11 +49,15 @@ func (a *BinaryApplier) ApplyFragment(f *BinaryFragment) error { switch f.Method { case BinaryPatchLiteral: - if _, err := a.dst.Write(f.Data); err != nil { + if _, err := io.Copy(a.dst, f.Data()); err != nil { return applyError(err) } case BinaryPatchDelta: - if err := applyBinaryDeltaFragment(a.dst, a.src, f.Data); err != nil { + data, err := io.ReadAll(f.Data()) + if err != nil { + return applyError(err) + } + if err := applyBinaryDeltaFragment(a.dst, a.src, data); err != nil { return applyError(err) } default: diff --git a/gitdiff/binary.go b/gitdiff/binary.go index 282e323..cb1e058 100644 --- a/gitdiff/binary.go +++ b/gitdiff/binary.go @@ -2,10 +2,7 @@ package gitdiff import ( "bytes" - "compress/zlib" - "fmt" "io" - "io/ioutil" "strconv" "strings" ) @@ -93,6 +90,9 @@ func (p *parser) ParseBinaryFragmentHeader() (*BinaryFragment, error) { nerr := err.(*strconv.NumError) return nil, p.Errorf(0, "binary patch: invalid size: %v", nerr.Err) } + if frag.Size < 0 { + return nil, p.Errorf(0, "binary patch: invalid size: %d", frag.Size) + } if err := p.Next(); err != nil && err != io.EOF { return nil, err @@ -152,10 +152,7 @@ func (p *parser) ParseBinaryChunk(frag *BinaryFragment) error { return err } } - - if err := inflateBinaryChunk(frag, &data); err != nil { - return p.Errorf(0, "binary patch: %v", err) - } + frag.RawData = data.Bytes() // consume the empty line that ended the fragment if err := p.Next(); err != nil && err != io.EOF { @@ -163,24 +160,3 @@ func (p *parser) ParseBinaryChunk(frag *BinaryFragment) error { } return nil } - -func inflateBinaryChunk(frag *BinaryFragment, r io.Reader) error { - zr, err := zlib.NewReader(r) - if err != nil { - return err - } - - data, err := ioutil.ReadAll(zr) - if err != nil { - return err - } - if err := zr.Close(); err != nil { - return err - } - - if int64(len(data)) != frag.Size { - return fmt.Errorf("%d byte fragment inflated to %d", frag.Size, len(data)) - } - frag.Data = data - return nil -} diff --git a/gitdiff/binary_reader.go b/gitdiff/binary_reader.go new file mode 100644 index 0000000..b57df40 --- /dev/null +++ b/gitdiff/binary_reader.go @@ -0,0 +1,56 @@ +package gitdiff + +import ( + "bytes" + "compress/zlib" + "fmt" + "io" +) + +// binaryReader reads the compressed raw data in a binary fragment, +// decompressing it and checking it against the expected size. Clients should +// read until it returns an error or io.EOF. +type binaryReader struct { + r *io.LimitedReader + c io.Closer + size int64 + raw []byte +} + +func newBinaryReader(f *BinaryFragment) io.Reader { + return &binaryReader{ + raw: f.RawData, + size: f.Size, + } +} + +func (r *binaryReader) Read(p []byte) (int, error) { + // Defer initialization of the zlib reader so that we report any header + // errors from Read() instead of when creating the binaryReader. This + // allows clients to use io.ReadAll() directly with BinaryFragment.Data(). + if r.r == nil { + zr, err := zlib.NewReader(bytes.NewReader(r.raw)) + if err != nil { + return 0, err + } + r.r = &io.LimitedReader{R: zr, N: r.size + 1} + r.c = zr + } + + n, err := r.r.Read(p) + if err == io.EOF { + // If we reached the "end", first check that we read the correct amount + // of data. On an exact read, the limit reader has one byte remaining. + switch { + case r.r.N > 1: + return n, fmt.Errorf("incorrect inflated size: expected %d bytes, received %d", r.size, r.size-(r.r.N-1)) + case r.r.N < 1: + return n, fmt.Errorf("incorrect inflated size: expected %d bytes, received more", r.size) + } + // Then check that the zlib checksum is valid by closing the reader + if err := r.c.Close(); err != nil { + return n, err + } + } + return n, err +} diff --git a/gitdiff/binary_reader_test.go b/gitdiff/binary_reader_test.go new file mode 100644 index 0000000..137db7f --- /dev/null +++ b/gitdiff/binary_reader_test.go @@ -0,0 +1,88 @@ +package gitdiff + +import ( + "bytes" + "compress/zlib" + "encoding/binary" + "io" + "strings" + "testing" +) + +func TestBinaryReader(t *testing.T) { + tests := map[string]struct { + Fragment BinaryFragment + Data []byte + Err string + }{ + "fromCompressed": { + Fragment: BinaryFragment{ + Size: 40, + RawData: deflateBinaryData(fib(10, binary.BigEndian)), + }, + Data: fib(10, binary.BigEndian), + }, + "fromCompressedGit": { + Fragment: BinaryFragment{ + Size: 40, + RawData: []byte{ + // Literal data as compressed and base85 encoded by Git + 0x78, 0x01, 0x63, 0x60, 0x60, 0x60, 0x64, 0x80, 0x60, 0x26, 0x20, + 0xcd, 0x0c, 0xc4, 0xac, 0x40, 0xcc, 0x01, 0xc4, 0xbc, 0x40, 0x2c, + 0x0a, 0xc4, 0x4a, 0x40, 0x6c, 0x0e, 0x00, 0x04, 0x2b, 0x00, 0x90, + }, + }, + Data: fib(10, binary.BigEndian), + }, + "invalidCompression": { + Fragment: BinaryFragment{ + Size: 40, + RawData: fib(10, binary.BigEndian), + }, + Err: "zlib", + }, + "incorrectSizeOver": { + Fragment: BinaryFragment{ + Size: 16, + RawData: deflateBinaryData(fib(10, binary.BigEndian)), + }, + Err: "expected 16 bytes, received more", + }, + "incorrectSizeUnder": { + Fragment: BinaryFragment{ + Size: 16, + RawData: deflateBinaryData(fib(2, binary.BigEndian)), + }, + Err: "expected 16 bytes, received 8", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + data, err := io.ReadAll(test.Fragment.Data()) + if test.Err == "" && err != nil { + t.Fatalf("unexpected data error: %v", err) + } + if test.Err != "" { + if err == nil { + t.Fatal("expected data error, but got nil") + } + if !strings.Contains(err.Error(), test.Err) { + t.Fatalf("incorrect data error: %q is not in %q", test.Err, err.Error()) + } + } else if !bytes.Equal(test.Data, data) { + t.Errorf("incorrect data\nexpected: %+v (len=%d)\n actual: %+v (len=%d)", test.Data, len(test.Data), data, len(data)) + } + }) + } +} + +func deflateBinaryData(data []byte) []byte { + var b bytes.Buffer + + zw := zlib.NewWriter(&b) + _, _ = zw.Write(data) + _ = zw.Close() + + return b.Bytes() +} diff --git a/gitdiff/binary_test.go b/gitdiff/binary_test.go index 64db243..5faa04e 100644 --- a/gitdiff/binary_test.go +++ b/gitdiff/binary_test.go @@ -1,6 +1,7 @@ package gitdiff import ( + "bytes" "encoding/binary" "io" "reflect" @@ -122,6 +123,11 @@ func TestParseBinaryFragmentHeader(t *testing.T) { } func TestParseBinaryChunk(t *testing.T) { + // NOTE: input data in this test is encoded correctly but is not valid + // binary patch content because it is not compressed with zlib. This is not + // relevant for the logic under test, but means tests must not call Data() + // on the parsed fragments. + tests := map[string]struct { Input string Fragment BinaryFragment @@ -129,16 +135,17 @@ func TestParseBinaryChunk(t *testing.T) { Err string }{ "singleline": { - Input: "TcmZQzU|?i`U?w2V48*Je09XJG\n\n", + Input: "T0000100001000020000300005\n\n", Fragment: BinaryFragment{ Size: 20, }, Output: fib(5, binary.BigEndian), }, "multiline": { - Input: "zcmZQzU|?i`U?w2V48*KJ%mKu_Kr9NxNf->s?WfX|B-=Vs{#X~svra7Ekg#T|4s}nH;WnAZ)|1Y*`&cB\n" + - "s(sh?X(Uz6L^!Ou&aF*u`J!eibJifSrv0z>$Q%Hd(^HIJ