pack: instantiate all packs and indexes with a hash

bk2204 · bk2204 · commit 6324dd6e8708 · 2020-07-10T18:47:50.000Z
Whenever we instantiate or parse a pack or an index, pass or include the
relevant hash parameter.  Update the tests accordingly.  Use SHA-256 in
one case where the hash makes no actual difference to demonstrate that
we parse both algorithms correctly.
diff --git a/pack/index_decode.go b/pack/index_decode.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
+	"hash"
 	"io"
 )
 
@@ -69,8 +70,8 @@ var (
 // parse index entries.
 //
 // If there was an error parsing, it will be returned immediately.
-func DecodeIndex(r io.ReaderAt) (*Index, error) {
-	version, err := decodeIndexHeader(r)
+func DecodeIndex(r io.ReaderAt, hash hash.Hash) (*Index, error) {
+	version, err := decodeIndexHeader(r, hash)
 	if err != nil {
 		return nil, err
 	}
@@ -89,7 +90,7 @@ func DecodeIndex(r io.ReaderAt) (*Index, error) {
 }
 
 // decodeIndexHeader determines which version the index given by "r" is.
-func decodeIndexHeader(r io.ReaderAt) (IndexVersion, error) {
+func decodeIndexHeader(r io.ReaderAt, hash hash.Hash) (IndexVersion, error) {
 	hdr := make([]byte, 4)
 	if _, err := r.ReadAt(hdr, 0); err != nil {
 		return nil, err
@@ -104,13 +105,13 @@ func decodeIndexHeader(r io.ReaderAt) (IndexVersion, error) {
 		version := binary.BigEndian.Uint32(vb)
 		switch version {
 		case 1:
-			return new(V1), nil
+			return &V1{hash: hash}, nil
 		case 2:
-			return new(V2), nil
+			return &V2{hash: hash}, nil
 		}
 		return nil, &UnsupportedVersionErr{uint32(version)}
 	}
-	return new(V1), nil
+	return &V1{hash: hash}, nil
 }
 
 // decodeIndexFanout decodes the fanout table given by "r" and beginning at the
diff --git a/pack/index_decode_test.go b/pack/index_decode_test.go
@@ -2,6 +2,7 @@ package pack
 
 import (
 	"bytes"
+	"crypto/sha1"
 	"encoding/binary"
 	"io"
 	"testing"
@@ -21,7 +22,7 @@ func TestDecodeIndexV2(t *testing.T) {
 		buf = append(buf, x...)
 	}
 
-	idx, err := DecodeIndex(bytes.NewReader(buf))
+	idx, err := DecodeIndex(bytes.NewReader(buf), sha1.New())
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 3, idx.Count())
@@ -33,21 +34,21 @@ func TestDecodeIndexV2InvalidFanout(t *testing.T) {
 	buf = append(buf, 0x0, 0x0, 0x0, 0x2)
 	buf = append(buf, make([]byte, indexFanoutWidth-1)...)
 
-	idx, err := DecodeIndex(bytes.NewReader(buf))
+	idx, err := DecodeIndex(bytes.NewReader(buf), sha1.New())
 
 	assert.Equal(t, ErrShortFanout, err)
 	assert.Nil(t, idx)
 }
 
 func TestDecodeIndexV1(t *testing.T) {
-	idx, err := DecodeIndex(bytes.NewReader(make([]byte, indexFanoutWidth)))
+	idx, err := DecodeIndex(bytes.NewReader(make([]byte, indexFanoutWidth)), sha1.New())
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 0, idx.Count())
 }
 
 func TestDecodeIndexV1InvalidFanout(t *testing.T) {
-	idx, err := DecodeIndex(bytes.NewReader(make([]byte, indexFanoutWidth-1)))
+	idx, err := DecodeIndex(bytes.NewReader(make([]byte, indexFanoutWidth-1)), sha1.New())
 
 	assert.Equal(t, ErrShortFanout, err)
 	assert.Nil(t, idx)
@@ -58,14 +59,14 @@ func TestDecodeIndexUnsupportedVersion(t *testing.T) {
 	buf = append(buf, 0xff, 0x74, 0x4f, 0x63)
 	buf = append(buf, 0x0, 0x0, 0x0, 0x3)
 
-	idx, err := DecodeIndex(bytes.NewReader(buf))
+	idx, err := DecodeIndex(bytes.NewReader(buf), sha1.New())
 
 	assert.EqualError(t, err, "gitobj/pack: unsupported version: 3")
 	assert.Nil(t, idx)
 }
 
 func TestDecodeIndexEmptyContents(t *testing.T) {
-	idx, err := DecodeIndex(bytes.NewReader(make([]byte, 0)))
+	idx, err := DecodeIndex(bytes.NewReader(make([]byte, 0)), sha1.New())
 
 	assert.Equal(t, io.EOF, err)
 	assert.Nil(t, idx)
diff --git a/pack/index_test.go b/pack/index_test.go
@@ -2,6 +2,7 @@ package pack
 
 import (
 	"bytes"
+	"crypto/sha1"
 	"encoding/binary"
 	"fmt"
 	"testing"
@@ -165,7 +166,7 @@ func init() {
 		fanout: fanout,
 		// version is unimportant here, use V2 since it's more common in
 		// the wild.
-		version: new(V2),
+		version: &V2{hash: sha1.New()},
 
 		// *bytes.Buffer does not implement io.ReaderAt, but
 		// *bytes.Reader does.
diff --git a/pack/index_v1_test.go b/pack/index_v1_test.go
@@ -2,6 +2,7 @@ package pack
 
 import (
 	"bytes"
+	"crypto/sha1"
 	"encoding/binary"
 	"testing"
 
@@ -37,19 +38,21 @@ var (
 
 	V1Index = &Index{
 		fanout:  V1IndexFanout,
-		version: new(V1),
+		version: &V1{hash: sha1.New()},
 	}
 )
 
 func TestIndexV1SearchExact(t *testing.T) {
-	e, err := new(V1).Entry(V1Index, 1)
+	v := &V1{hash: sha1.New()}
+	e, err := v.Entry(V1Index, 1)
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 2, e.PackOffset)
 }
 
 func TestIndexVersionWidthV1(t *testing.T) {
-	assert.EqualValues(t, 0, new(V1).Width())
+	v := &V1{hash: sha1.New()}
+	assert.EqualValues(t, 0, v.Width())
 }
 
 func init() {
diff --git a/pack/index_v2_test.go b/pack/index_v2_test.go
@@ -2,6 +2,7 @@ package pack
 
 import (
 	"bytes"
+	"crypto/sha1"
 	"encoding/binary"
 	"testing"
 
@@ -46,26 +47,29 @@ var (
 
 	V2Index = &Index{
 		fanout:  V2IndexFanout,
-		version: new(V2),
+		version: &V2{hash: sha1.New()},
 	}
 )
 
 func TestIndexV2EntryExact(t *testing.T) {
-	e, err := new(V2).Entry(V2Index, 1)
+	v := &V2{hash: sha1.New()}
+	e, err := v.Entry(V2Index, 1)
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 2, e.PackOffset)
 }
 
 func TestIndexV2EntryExtendedOffset(t *testing.T) {
-	e, err := new(V2).Entry(V2Index, 2)
+	v := &V2{hash: sha1.New()}
+	e, err := v.Entry(V2Index, 2)
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 3, e.PackOffset)
 }
 
 func TestIndexVersionWidthV2(t *testing.T) {
-	assert.EqualValues(t, 8, new(V2).Width())
+	v := &V2{hash: sha1.New()}
+	assert.EqualValues(t, 8, v.Width())
 }
 
 func init() {
diff --git a/pack/packfile_decode.go b/pack/packfile_decode.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"hash"
 	"io"
 )
 
@@ -22,7 +23,7 @@ var (
 //
 // If the header is malformed, or otherwise cannot be read, an error will be
 // returned without a corresponding packfile.
-func DecodePackfile(r io.ReaderAt) (*Packfile, error) {
+func DecodePackfile(r io.ReaderAt, hash hash.Hash) (*Packfile, error) {
 	header := make([]byte, 12)
 	if _, err := r.ReadAt(header[:], 0); err != nil {
 		return nil, err
@@ -40,5 +41,6 @@ func DecodePackfile(r io.ReaderAt) (*Packfile, error) {
 		Objects: objects,
 
 		r: r,
+		hash: hash,
 	}, nil
 }
diff --git a/pack/packfile_decode_test.go b/pack/packfile_decode_test.go
@@ -2,6 +2,8 @@ package pack
 
 import (
 	"bytes"
+	"crypto/sha1"
+	"crypto/sha256"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -12,7 +14,7 @@ func TestDecodePackfileDecodesIntegerVersion(t *testing.T) {
 		'P', 'A', 'C', 'K', // Pack header.
 		0x0, 0x0, 0x0, 0x2, // Pack version.
 		0x0, 0x0, 0x0, 0x0, // Number of packed objects.
-	}))
+	}), sha1.New())
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 2, p.Version)
@@ -23,7 +25,7 @@ func TestDecodePackfileDecodesIntegerCount(t *testing.T) {
 		'P', 'A', 'C', 'K', // Pack header.
 		0x0, 0x0, 0x0, 0x2, // Pack version.
 		0x0, 0x0, 0x1, 0x2, // Number of packed objects.
-	}))
+	}), sha256.New())
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 258, p.Objects)
@@ -34,7 +36,7 @@ func TestDecodePackfileReportsBadHeaders(t *testing.T) {
 		'W', 'R', 'O', 'N', 'G', // Malformed pack header.
 		0x0, 0x0, 0x0, 0x0, // Pack version.
 		0x0, 0x0, 0x0, 0x0, // Number of packed objects.
-	}))
+	}), sha1.New())
 
 	assert.Equal(t, errBadPackHeader, err)
 	assert.Nil(t, p)
diff --git a/pack/packfile_test.go b/pack/packfile_test.go
@@ -2,6 +2,7 @@ package pack
 
 import (
 	"bytes"
+	"crypto/sha1"
 	"encoding/binary"
 	"encoding/hex"
 	"fmt"
@@ -30,6 +31,7 @@ func TestPackObjectReturnsObjectWithSingleBaseAtLowOffset(t *testing.T) {
 			// (0001 1000) (msb=0, type=commit, size=14)
 			0x1e}, compressed...),
 		),
+		hash: sha1.New(),
 	}
 
 	o, err := p.Object(DecodeHex(t, "cccccccccccccccccccccccccccccccccccccccc"))
@@ -63,6 +65,7 @@ func TestPackObjectReturnsObjectWithSingleBaseAtHighOffset(t *testing.T) {
 
 			compressed...,
 		)),
+		hash: sha1.New(),
 	}
 
 	o, err := p.Object(DecodeHex(t, "cccccccccccccccccccccccccccccccccccccccc"))
@@ -108,6 +111,7 @@ func TestPackObjectReturnsObjectWithDeltaBaseOffset(t *testing.T) {
 			0x6e, // (0110 1010) (msb=0, type=obj_ofs_delta, size=10)
 			0x12, // (0001 0001) (ofs_delta=-17, len(compressed))
 		}, delta...)...)),
+		hash: sha1.New(),
 	}
 
 	o, err := p.Object(DecodeHex(t, "cccccccccccccccccccccccccccccccccccccccc"))
@@ -160,6 +164,7 @@ func TestPackfileObjectReturnsObjectWithDeltaBaseReference(t *testing.T) {
 			0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc,
 			0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc,
 		}, delta...)...)),
+		hash: sha1.New(),
 	}
 
 	o, err := p.Object(DecodeHex(t, "dddddddddddddddddddddddddddddddddddddddd"))
@@ -261,7 +266,7 @@ func IndexWith(offsets map[string]uint32) *Index {
 		fanout: fanout,
 		r:      bytes.NewReader(buf),
 
-		version: new(V2),
+		version: &V2{hash: sha1.New()},
 	}
 }
 
diff --git a/pack/set.go b/pack/set.go
@@ -76,12 +76,12 @@ func NewSet(db string, algo hash.Hash) (*Set, error) {
 			return nil, err
 		}
 
-		pack, err := DecodePackfile(packf)
+		pack, err := DecodePackfile(packf, algo)
 		if err != nil {
 			return nil, err
 		}
 
-		idx, err := DecodeIndex(idxf)
+		idx, err := DecodeIndex(idxf, algo)
 		if err != nil {
 			return nil, err
 		}
@@ -139,7 +139,7 @@ func NewSetPacks(packs ...*Packfile) *Set {
 	}
 
 	return &Set{
-		m: m,
+		m:    m,
 		closeFn: func() error {
 			for _, pack := range packs {
 				if err := pack.Close(); err != nil {

Original file line number	Diff line number	Diff line change
`@@ -76,12 +76,12 @@ func NewSet(db string, algo hash.Hash) (*Set, error) {`
`76`	`76`	`return nil, err`
`77`	`77`	`}`
`78`	`78`
`79`		`- pack, err := DecodePackfile(packf)`
	`79`	`+ pack, err := DecodePackfile(packf, algo)`
`80`	`80`	`if err != nil {`
`81`	`81`	`return nil, err`
`82`	`82`	`}`
`83`	`83`
`84`		`- idx, err := DecodeIndex(idxf)`
	`84`	`+ idx, err := DecodeIndex(idxf, algo)`
`85`	`85`	`if err != nil {`
`86`	`86`	`return nil, err`
`87`	`87`	`}`
`@@ -139,7 +139,7 @@ func NewSetPacks(packs ...Packfile) Set {`
`139`	`139`	`}`
`140`	`140`
`141`	`141`	`return &Set{`
`142`		`- m: m,`
	`142`	`+ m: m,`
`143`	`143`	`closeFn: func() error {`
`144`	`144`	`for _, pack := range packs {`
`145`	`145`	`if err := pack.Close(); err != nil {`