Merge pull request #2105 from stevvooe/algorithm-own-file

digest: cleanup digester and verifier creation

digest/algorithm.go

@@ -0,0 +1,145 @@
+package digest
+
+import (
+	"crypto"
+	"fmt"
+	"hash"
+	"io"
+)
+
+// Algorithm identifies and implementation of a digester by an identifier.
+// Note the that this defines both the hash algorithm used and the string
+// encoding.
+type Algorithm string
+
+// supported digest types
+const (
+	SHA256 Algorithm = "sha256" // sha256 with hex encoding
+	SHA384 Algorithm = "sha384" // sha384 with hex encoding
+	SHA512 Algorithm = "sha512" // sha512 with hex encoding
+
+	// Canonical is the primary digest algorithm used with the distribution
+	// project. Other digests may be used but this one is the primary storage
+	// digest.
+	Canonical = SHA256
+)
+
+var (
+	// TODO(stevvooe): Follow the pattern of the standard crypto package for
+	// registration of digests. Effectively, we are a registerable set and
+	// common symbol access.
+
+	// algorithms maps values to hash.Hash implementations. Other algorithms
+	// may be available but they cannot be calculated by the digest package.
+	algorithms = map[Algorithm]crypto.Hash{
+		SHA256: crypto.SHA256,
+		SHA384: crypto.SHA384,
+		SHA512: crypto.SHA512,
+	}
+)
+
+// Available returns true if the digest type is available for use. If this
+// returns false, New and Hash will return nil.
+func (a Algorithm) Available() bool {
+	h, ok := algorithms[a]
+	if !ok {
+		return false
+	}
+
+	// check availability of the hash, as well
+	return h.Available()
+}
+
+func (a Algorithm) String() string {
+	return string(a)
+}
+
+// Size returns number of bytes returned by the hash.
+func (a Algorithm) Size() int {
+	h, ok := algorithms[a]
+	if !ok {
+		return 0
+	}
+	return h.Size()
+}
+
+// Set implemented to allow use of Algorithm as a command line flag.
+func (a *Algorithm) Set(value string) error {
+	if value == "" {
+		*a = Canonical
+	} else {
+		// just do a type conversion, support is queried with Available.
+		*a = Algorithm(value)
+	}
+
+	return nil
+}
+
+// New returns a new digester for the specified algorithm. If the algorithm
+// does not have a digester implementation, nil will be returned. This can be
+// checked by calling Available before calling New.
+func (a Algorithm) Digester() Digester {
+	return &digester{
+		alg:  a,
+		hash: a.Hash(),
+	}
+}
+
+// New is deprecated. Use Algorithm.Digester.
+func (a Algorithm) New() Digester {
+	return a.Digester()
+}
+
+// Hash returns a new hash as used by the algorithm. If not available, the
+// 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
+		// when using stevvooe/resumable or a hardware accelerated package).
+		//
+		// Applications that may want to resolve the hash at runtime should
+		// call Algorithm.Available before call Algorithm.Hash().
+		panic(fmt.Sprintf("%v not available (make sure it is imported)", a))
+	}
+
+	return algorithms[a].New()
+}
+
+// FromReader returns the digest of the reader using the algorithm.
+func (a Algorithm) FromReader(rd io.Reader) (Digest, error) {
+	digester := a.New()
+
+	if _, err := io.Copy(digester.Hash(), rd); err != nil {
+		return "", err
+	}
+
+	return digester.Digest(), nil
+}
+
+// FromBytes digests the input and returns a Digest.
+func (a Algorithm) FromBytes(p []byte) Digest {
+	digester := a.New()
+
+	if _, err := digester.Hash().Write(p); err != nil {
+		// Writes to a Hash should never fail. None of the existing
+		// hash implementations in the stdlib or hashes vendored
+		// here can return errors from Write. Having a panic in this
+		// condition instead of having FromBytes return an error value
+		// avoids unnecessary error handling paths in all callers.
+		panic("write to hash function returned error: " + err.Error())
+	}
+
+	return digester.Digest()
+}
+
+// FromString digests the string input and returns a Digest.
+func (a Algorithm) FromString(s string) Digest {
+	return a.FromBytes([]byte(s))
+}

digest/digest.go

@@ -108,16 +108,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
 }
 
@@ -127,6 +128,15 @@ func (d Digest) Algorithm() Algorithm {
 	return Algorithm(d[:d.sepIndex()])
 }
 
+// Verifier returns a writer object that can be used to verify a stream of
+// 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,
+	}
+}
+
 // Hex returns the hex digest portion of the digest. This will panic if the
 // underlying digest is not in a valid format.
 func (d Digest) Hex() string {
@@ -141,7 +151,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/digester.go

@@ -1,141 +1,6 @@
 package digest
 
-import (
+import "hash"
-	"crypto"
-	"fmt"
-	"hash"
-	"io"
-)
-
-// Algorithm identifies and implementation of a digester by an identifier.
-// Note the that this defines both the hash algorithm used and the string
-// encoding.
-type Algorithm string
-
-// supported digest types
-const (
-	SHA256 Algorithm = "sha256" // sha256 with hex encoding
-	SHA384 Algorithm = "sha384" // sha384 with hex encoding
-	SHA512 Algorithm = "sha512" // sha512 with hex encoding
-
-	// Canonical is the primary digest algorithm used with the distribution
-	// project. Other digests may be used but this one is the primary storage
-	// digest.
-	Canonical = SHA256
-)
-
-var (
-	// TODO(stevvooe): Follow the pattern of the standard crypto package for
-	// registration of digests. Effectively, we are a registerable set and
-	// common symbol access.
-
-	// algorithms maps values to hash.Hash implementations. Other algorithms
-	// may be available but they cannot be calculated by the digest package.
-	algorithms = map[Algorithm]crypto.Hash{
-		SHA256: crypto.SHA256,
-		SHA384: crypto.SHA384,
-		SHA512: crypto.SHA512,
-	}
-)
-
-// Available returns true if the digest type is available for use. If this
-// returns false, New and Hash will return nil.
-func (a Algorithm) Available() bool {
-	h, ok := algorithms[a]
-	if !ok {
-		return false
-	}
-
-	// check availability of the hash, as well
-	return h.Available()
-}
-
-func (a Algorithm) String() string {
-	return string(a)
-}
-
-// Size returns number of bytes returned by the hash.
-func (a Algorithm) Size() int {
-	h, ok := algorithms[a]
-	if !ok {
-		return 0
-	}
-	return h.Size()
-}
-
-// Set implemented to allow use of Algorithm as a command line flag.
-func (a *Algorithm) Set(value string) error {
-	if value == "" {
-		*a = Canonical
-	} else {
-		// just do a type conversion, support is queried with Available.
-		*a = Algorithm(value)
-	}
-
-	return nil
-}
-
-// New returns a new digester for the specified algorithm. If the algorithm
-// does not have a digester implementation, nil will be returned. This can be
-// checked by calling Available before calling New.
-func (a Algorithm) New() Digester {
-	return &digester{
-		alg:  a,
-		hash: a.Hash(),
-	}
-}
-
-// Hash returns a new hash as used by the algorithm. If not available, the
-// method will panic. Check Algorithm.Available() before calling.
-func (a Algorithm) Hash() hash.Hash {
-	if !a.Available() {
-		// 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
-		// when using stevvooe/resumable or a hardware accelerated package).
-		//
-		// Applications that may want to resolve the hash at runtime should
-		// call Algorithm.Available before call Algorithm.Hash().
-		panic(fmt.Sprintf("%v not available (make sure it is imported)", a))
-	}
-
-	return algorithms[a].New()
-}
-
-// FromReader returns the digest of the reader using the algorithm.
-func (a Algorithm) FromReader(rd io.Reader) (Digest, error) {
-	digester := a.New()
-
-	if _, err := io.Copy(digester.Hash(), rd); err != nil {
-		return "", err
-	}
-
-	return digester.Digest(), nil
-}
-
-// FromBytes digests the input and returns a Digest.
-func (a Algorithm) FromBytes(p []byte) Digest {
-	digester := a.New()
-
-	if _, err := digester.Hash().Write(p); err != nil {
-		// Writes to a Hash should never fail. None of the existing
-		// hash implementations in the stdlib or hashes vendored
-		// here can return errors from Write. Having a panic in this
-		// condition instead of having FromBytes return an error value
-		// avoids unnecessary error handling paths in all callers.
-		panic("write to hash function returned error: " + err.Error())
-	}
-
-	return digester.Digest()
-}
-
-// FromString digests the string input and returns a Digest.
-func (a Algorithm) FromString(s string) Digest {
-	return a.FromBytes([]byte(s))
-}
-
-// TODO(stevvooe): Allow resolution of verifiers using the digest type and
-// this registration system.
 
 // Digester calculates the digest of written data. Writes should go directly
 // to the return value of Hash, while calling Digest will return the current

digest/verifiers.go

@@ -17,17 +17,9 @@ type Verifier interface {
 	Verified() bool
 }
 
-// NewDigestVerifier returns a verifier that compares the written bytes
+// NewDigestVerifier is deprecated. Please use Digest.Verifier.
-// against a passed in digest.
 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/handlers/api_test.go

@@ -552,7 +552,7 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv {
 	})
 
 	// Verify the body
-	verifier, err := digest.NewDigestVerifier(layerDigest)
+	verifier, err := layerDigest.Verifier()
 	if err != nil {
 		t.Fatalf("unexpected error getting digest verifier: %s", err)
 	}

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 := digest.NewDigestVerifier(desc.Digest)
-			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 := digest.NewDigestVerifier(dgst)
+	verifier := dgst.Verifier()
-	if err != nil {
-		t.Fatalf("error getting digest verifier: %s", err)
-	}
-
 	io.Copy(verifier, fr)
 
 	if !verifier.Verified() {