digest: remove error return from Digest.Verifier

Signed-off-by: Stephen J Day <stephen.day@docker.com>

digest/algorithm.go

@@ -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

digest/digest.go

@@ -104,16 +104,17 @@ func (d Digest) Validate() error {
 		return ErrDigestInvalidFormat
 	}
 
-	switch algorithm := Algorithm(s[:i]); algorithm {
+	algorithm := Algorithm(s[:i])
-	case SHA256, SHA384, SHA512:
+	if !algorithm.Available() {
-		if algorithm.Size()*2 != len(s[i+1:]) {
-			return ErrDigestInvalidLength
-		}
-		break
-	default:
 		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
 }
 
@@ -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
+// content against the digest. If the digest is invalid, the method will panic.
-// returned.
+func (d Digest) Verifier() Verifier {
-func (d Digest) Verifier() (Verifier, error) {
-	if err := d.Validate(); err != nil {
-		return nil, err
-	}
-
 	return hashVerifier{
 		hash:   d.Algorithm().Hash(),
 		digest: d,
-	}, nil
+	}
 }
 
 // Hex returns the hex digest portion of the digest. This will panic if the
@@ -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

digest/digest_test.go

@@ -1,6 +1,8 @@
 package digest
 
 import (
+	_ "crypto/sha256"
+	_ "crypto/sha512"
 	"testing"
 )
 

digest/verifiers.go

@@ -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 d.Verifier(), nil
-		return nil, err
-	}
-
-	return hashVerifier{
-		hash:   d.Algorithm().Hash(),
-		digest: d,
-	}, nil
 }
 
 type hashVerifier struct {

digest/verifiers_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"crypto/rand"
 	"io"
+	"reflect"
 	"testing"
 )
 
@@ -12,10 +13,7 @@ func TestDigestVerifier(t *testing.T) {
 	rand.Read(p)
 	digest := FromBytes(p)
 
-	verifier, err := NewDigestVerifier(digest)
+	verifier := digest.Verifier()
-	if err != nil {
-		t.Fatalf("unexpected error getting digest verifier: %s", err)
-	}
 
 	io.Copy(verifier, bytes.NewReader(p))
 
@@ -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")
+	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)
+				}
+			}()
 
-	_, err := NewDigestVerifier(unsupported)
+			_ = testcase.Digest.Verifier()
-	if err == nil {
+		})
-		t.Fatalf("expected error when creating verifier")
-	}
-
-	if err != ErrDigestUnsupported {
-		t.Fatalf("incorrect error for unsupported digest: %v", err)
 	}
 }
-
-// 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
-//

registry/storage/blobwriter.go

@@ -235,11 +235,7 @@ func (bw *blobWriter) validateBlob(ctx context.Context, desc distribution.Descri
 		// guarantee, so this may be defensive.
 		if !verified {
 			digester := digest.Canonical.New()
-
+			verifier := desc.Digest.Verifier()
-			digestVerifier, err := desc.Digest.Verifier()
-			if err != nil {
-				return distribution.Descriptor{}, err
-			}
 
 			// Read the file from the backend driver and validate it.
 			fr, err := newFileReader(ctx, bw.driver, bw.path, desc.Size)
@@ -250,12 +246,12 @@ func (bw *blobWriter) validateBlob(ctx context.Context, desc distribution.Descri
 
 			tr := io.TeeReader(fr, digester.Hash())
 
-			if _, err := io.Copy(digestVerifier, tr); err != nil {
+			if _, err := io.Copy(verifier, tr); err != nil {
 				return distribution.Descriptor{}, err
 			}
 
 			canonical = digester.Digest()
-			verified = digestVerifier.Verified()
+			verified = verifier.Verified()
 		}
 	}
 

registry/storage/filereader_test.go

@@ -41,11 +41,7 @@ func TestSimpleRead(t *testing.T) {
 		t.Fatalf("error allocating file reader: %v", err)
 	}
 
-	verifier, err := dgst.Verifier()
+	verifier := dgst.Verifier()
-	if err != nil {
-		t.Fatalf("error getting digest verifier: %s", err)
-	}
-
 	io.Copy(verifier, fr)
 
 	if !verifier.Verified() {