Skip to content

Commit

Permalink
digest: remove error return from Digest.Verifier
Browse files Browse the repository at this point in the history
Signed-off-by: Stephen J Day <stephen.day@docker.com>
  • Loading branch information
stevvooe committed Dec 16, 2016
1 parent 1bb0bb7 commit 2e6b5ec
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 45 deletions.
5 changes: 5 additions & 0 deletions algorithm.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ func (a Algorithm) New() Digester {
// method will panic. Check Algorithm.Available() before calling.
func (a Algorithm) Hash() hash.Hash {
if !a.Available() {
// Empty algorithm string is invalid
if a == "" {
panic(fmt.Sprintf("empty digest algorithm, validate before calling Algorithm.Hash()"))
}

// NOTE(stevvooe): A missing hash is usually a programming error that
// must be resolved at compile time. We don't import in the digest
// package to allow users to choose their hash implementation (such as
Expand Down
28 changes: 12 additions & 16 deletions digest.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,17 @@ func (d Digest) Validate() error {
return ErrDigestInvalidFormat
}

switch algorithm := Algorithm(s[:i]); algorithm {
case SHA256, SHA384, SHA512:
if algorithm.Size()*2 != len(s[i+1:]) {
return ErrDigestInvalidLength
}
break
default:
algorithm := Algorithm(s[:i])
if !algorithm.Available() {
return ErrDigestUnsupported
}

// Digests much always be hex-encoded, ensuring that their hex portion will
// always be size*2
if algorithm.Size()*2 != len(s[i+1:]) {
return ErrDigestInvalidLength
}

return nil
}

Expand All @@ -124,17 +125,12 @@ func (d Digest) Algorithm() Algorithm {
}

// Verifier returns a writer object that can be used to verify a stream of
// content against the digest. If the digest is invalid, an error will be
// returned.
func (d Digest) Verifier() (Verifier, error) {
if err := d.Validate(); err != nil {
return nil, err
}

// content against the digest. If the digest is invalid, the method will panic.
func (d Digest) Verifier() Verifier {
return hashVerifier{
hash: d.Algorithm().Hash(),
digest: d,
}, nil
}
}

// Hex returns the hex digest portion of the digest. This will panic if the
Expand All @@ -151,7 +147,7 @@ func (d Digest) sepIndex() int {
i := strings.Index(string(d), ":")

if i < 0 {
panic("could not find ':' in digest: " + d)
panic(fmt.Sprintf("no ':' separator in digest %q", d))
}

return i
Expand Down
2 changes: 2 additions & 0 deletions digest_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package digest

import (
_ "crypto/sha256"
_ "crypto/sha512"
"testing"
)

Expand Down
9 changes: 1 addition & 8 deletions verifiers.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,7 @@ type Verifier interface {

// NewDigestVerifier is deprecated. Please use Digest.Verifier.
func NewDigestVerifier(d Digest) (Verifier, error) {
if err := d.Validate(); err != nil {
return nil, err
}

return hashVerifier{
hash: d.Algorithm().Hash(),
digest: d,
}, nil
return d.Verifier(), nil
}

type hashVerifier struct {
Expand Down
59 changes: 38 additions & 21 deletions verifiers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"crypto/rand"
"io"
"reflect"
"testing"
)

Expand All @@ -12,10 +13,7 @@ func TestDigestVerifier(t *testing.T) {
rand.Read(p)
digest := FromBytes(p)

verifier, err := NewDigestVerifier(digest)
if err != nil {
t.Fatalf("unexpected error getting digest verifier: %s", err)
}
verifier := digest.Verifier()

io.Copy(verifier, bytes.NewReader(p))

Expand All @@ -27,23 +25,42 @@ func TestDigestVerifier(t *testing.T) {
// TestVerifierUnsupportedDigest ensures that unsupported digest validation is
// flowing through verifier creation.
func TestVerifierUnsupportedDigest(t *testing.T) {
unsupported := Digest("bean:0123456789abcdef")

_, err := NewDigestVerifier(unsupported)
if err == nil {
t.Fatalf("expected error when creating verifier")
}
for _, testcase := range []struct {
Name string
Digest Digest
Expected interface{} // expected panic target
}{
{
Name: "Empty",
Digest: "",
Expected: "no ':' separator in digest \"\"",
},
{
Name: "EmptyAlg",
Digest: ":",
Expected: "empty digest algorithm, validate before calling Algorithm.Hash()",
},
{
Name: "Unsupported",
Digest: Digest("bean:0123456789abcdef"),
Expected: "bean not available (make sure it is imported)",
},
{
Name: "Garbage",
Digest: Digest("sha256-garbage:pure"),
Expected: "sha256-garbage not available (make sure it is imported)",
},
} {
t.Run(testcase.Name, func(t *testing.T) {
expected := testcase.Expected
defer func() {
recovered := recover()
if !reflect.DeepEqual(recovered, expected) {
t.Fatalf("unexpected recover: %v != %v", recovered, expected)
}
}()

if err != ErrDigestUnsupported {
t.Fatalf("incorrect error for unsupported digest: %v", err)
_ = testcase.Digest.Verifier()
})
}
}

// TODO(stevvooe): Add benchmarks to measure bytes/second throughput for
// DigestVerifier.
//
// The relevant benchmark for comparison can be run with the following
// commands:
//
// go test -bench . crypto/sha1
//

0 comments on commit 2e6b5ec

Please sign in to comment.