Simplify digest.FromBytes calling convention

The current implementation of digest.FromBytes returns an error. This
error can never be non-nil, but its presence in the function signature
means each call site needs error handling code for an error that is
always nil.

I verified that none of the hash.Hash implementations in the standard
library can return an error on Write. Nor can any of the hash.Hash
implementations vendored in distribution.

This commit changes digest.FromBytes not to return an error. If Write
returns an error, it will panic, but as discussed above, this should
never happen.

This commit also avoids using a bytes.Reader to feed data into the hash
function in FromBytes. This makes the hypothetical case that would panic
a bit more explicit, and should also be more performant.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

digest/digest.go

@@ -1,7 +1,6 @@
 package digest
 
 import (
-	"bytes"
 	"fmt"
 	"hash"
 	"io"
@@ -99,8 +98,19 @@ func FromTarArchive(rd io.Reader) (Digest, error) {
 }
 
 // FromBytes digests the input and returns a Digest.
-func FromBytes(p []byte) (Digest, error) {
+func FromBytes(p []byte) Digest {
-	return FromReader(bytes.NewReader(p))
+	digester := Canonical.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()
 }
 
 // Validate checks that the contents of d is a valid digest, returning an

digest/verifiers_test.go

@@ -15,10 +15,7 @@ import (
 func TestDigestVerifier(t *testing.T) {
 	p := make([]byte, 1<<20)
 	rand.Read(p)
-	digest, err := FromBytes(p)
+	digest := FromBytes(p)
-	if err != nil {
-		t.Fatalf("unexpected error digesting bytes: %#v", err)
-	}
 
 	verifier, err := NewDigestVerifier(digest)
 	if err != nil {

notifications/bridge.go

@@ -98,10 +98,7 @@ func (b *bridge) createManifestEvent(action string, repo string, sm *schema1.Sig
 
 	event.Target.Length = int64(len(p))
 	event.Target.Size = int64(len(p))
-	event.Target.Digest, err = digest.FromBytes(p)
+	event.Target.Digest = digest.FromBytes(p)
-	if err != nil {
-		return nil, err
-	}
 
 	event.Target.URL, err = b.ub.BuildManifestURL(sm.Name, event.Target.Digest.String())
 	if err != nil {

notifications/bridge_test.go

@@ -90,10 +90,7 @@ func createTestEnv(t *testing.T, fn testSinkFn) Listener {
 		t.Fatalf("error getting manifest payload: %v", err)
 	}
 
-	dgst, err = digest.FromBytes(payload)
+	dgst = digest.FromBytes(payload)
-	if err != nil {
-		t.Fatalf("error digesting manifest payload: %v", err)
-	}
 
 	return NewBridge(ub, source, actor, request, fn)
 }

notifications/listener_test.go

@@ -167,11 +167,7 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) {
 		t.Fatalf("unexpected error getting manifest payload: %v", err)
 	}
 
-	dgst, err := digest.FromBytes(p)
+	dgst := digest.FromBytes(p)
-	if err != nil {
-		t.Fatalf("unexpected error digesting manifest payload: %v", err)
-	}
-
 	fetchedByManifest, err := manifests.Get(dgst)
 	if err != nil {
 		t.Fatalf("unexpected error fetching manifest: %v", err)

registry/client/repository_test.go

@@ -38,12 +38,7 @@ func newRandomBlob(size int) (digest.Digest, []byte) {
 		panic("unable to read enough bytes")
 	}
 
-	dgst, err := digest.FromBytes(b)
+	return digest.FromBytes(b), b
-	if err != nil {
-		panic(err)
-	}
-
-	return dgst, b
 }
 
 func addTestFetch(repo string, dgst digest.Digest, content []byte, m *testutil.RequestResponseMap) {
@@ -509,16 +504,11 @@ func newRandomSchemaV1Manifest(name, tag string, blobCount int) (*schema1.Signed
 		panic(err)
 	}
 
-	dgst, err := digest.FromBytes(p)
+	return sm, digest.FromBytes(p), p
-	if err != nil {
-		panic(err)
-	}
-
-	return sm, dgst, p
 }
 
 func addTestManifestWithEtag(repo, reference string, content []byte, m *testutil.RequestResponseMap, dgst string) {
-	actualDigest, _ := digest.FromBytes(content)
+	actualDigest := digest.FromBytes(content)
 	getReqWithEtag := testutil.Request{
 		Method: "GET",
 		Route:  "/v2/" + repo + "/manifests/" + reference,

registry/handlers/api_test.go

@@ -880,8 +880,7 @@ func testManifestAPI(t *testing.T, env *testEnv, args manifestArgs) (*testEnv, m
 	payload, err := signedManifest.Payload()
 	checkErr(t, err, "getting manifest payload")
 
-	dgst, err := digest.FromBytes(payload)
+	dgst := digest.FromBytes(payload)
-	checkErr(t, err, "digesting manifest")
 
 	args.signedManifest = signedManifest
 	args.dgst = dgst
@@ -1487,8 +1486,7 @@ func createRepository(env *testEnv, t *testing.T, imageName string, tag string)
 	payload, err := signedManifest.Payload()
 	checkErr(t, err, "getting manifest payload")
 
-	dgst, err := digest.FromBytes(payload)
+	dgst := digest.FromBytes(payload)
-	checkErr(t, err, "digesting manifest")
 
 	manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String())
 	checkErr(t, err, "building manifest url")

registry/handlers/images.go

@@ -250,11 +250,5 @@ func digestManifest(ctx context.Context, sm *schema1.SignedManifest) (digest.Dig
 		p = sm.Raw
 	}
 
-	dgst, err := digest.FromBytes(p)
+	return digest.FromBytes(p), nil
-	if err != nil {
-		ctxu.GetLogger(ctx).Errorf("error digesting manifest: %v", err)
-		return "", err
-	}
-
-	return dgst, err
 }

registry/proxy/proxyblobstore_test.go

@@ -298,10 +298,7 @@ func testProxyStoreServe(t *testing.T, te *testEnv, numClients int) {
 				}
 
 				bodyBytes := w.Body.Bytes()
-				localDigest, err := digest.FromBytes(bodyBytes)
+				localDigest := digest.FromBytes(bodyBytes)
-				if err != nil {
-					t.Fatalf("Error making digest from blob")
-				}
 				if localDigest != remoteBlob.Digest {
 					t.Fatalf("Mismatching blob fetch from proxy")
 				}
@@ -335,10 +332,7 @@ func testProxyStoreServe(t *testing.T, te *testEnv, numClients int) {
 			t.Fatalf(err.Error())
 		}
 
-		dl, err := digest.FromBytes(w.Body.Bytes())
+		dl := digest.FromBytes(w.Body.Bytes())
-		if err != nil {
-			t.Fatalf("Error making digest from blob")
-		}
 		if dl != dr.Digest {
 			t.Errorf("Mismatching blob fetch from proxy")
 		}

registry/proxy/proxymanifeststore.go

@@ -137,12 +137,7 @@ func manifestDigest(sm *schema1.SignedManifest) (digest.Digest, error) {
 
 	}
 
-	dgst, err := digest.FromBytes(payload)
+	return digest.FromBytes(payload), nil
-	if err != nil {
-		return "", err
-	}
-
-	return dgst, nil
 }
 
 func (pms proxyManifestStore) Put(manifest *schema1.SignedManifest) error {

registry/proxy/proxymanifeststore_test.go

@@ -177,7 +177,7 @@ func populateRepo(t *testing.T, ctx context.Context, repository distribution.Rep
 	if err != nil {
 		t.Fatal(err)
 	}
-	return digest.FromBytes(pl)
+	return digest.FromBytes(pl), nil
 }
 
 // TestProxyManifests contains basic acceptance tests

registry/storage/blob_test.go

@@ -176,10 +176,7 @@ func TestSimpleBlobUpload(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error reading all of blob %s", err.Error())
 	}
-	expectedDigest, err := digest.FromBytes(randomBlob)
+	expectedDigest := digest.FromBytes(randomBlob)
-	if err != nil {
-		t.Fatalf("Error getting digest from bytes: %s", err)
-	}
 	simpleUpload(t, bs, randomBlob, expectedDigest)
 
 	d, err = bs.Stat(ctx, expectedDigest)

registry/storage/blobstore.go

@@ -56,12 +56,7 @@ func (bs *blobStore) Open(ctx context.Context, dgst digest.Digest) (distribution
 // content is already present, only the digest will be returned. This should
 // only be used for small objects, such as manifests. This implemented as a convenience for other Put implementations
 func (bs *blobStore) Put(ctx context.Context, mediaType string, p []byte) (distribution.Descriptor, error) {
-	dgst, err := digest.FromBytes(p)
+	dgst := digest.FromBytes(p)
-	if err != nil {
-		context.GetLogger(ctx).Errorf("blobStore: error digesting content: %v, %s", err, string(p))
-		return distribution.Descriptor{}, err
-	}
-
 	desc, err := bs.statter.Stat(ctx, dgst)
 	if err == nil {
 		// content already present

registry/storage/linkedblobstore.go

@@ -75,10 +75,7 @@ func (lbs *linkedBlobStore) ServeBlob(ctx context.Context, w http.ResponseWriter
 }
 
 func (lbs *linkedBlobStore) Put(ctx context.Context, mediaType string, p []byte) (distribution.Descriptor, error) {
-	dgst, err := digest.FromBytes(p)
+	dgst := digest.FromBytes(p)
-	if err != nil {
-		return distribution.Descriptor{}, err
-	}
 	// Place the data in the blob store first.
 	desc, err := lbs.blobStore.Put(ctx, mediaType, p)
 	if err != nil {

registry/storage/manifeststore_test.go

@@ -185,11 +185,7 @@ func TestManifestStorage(t *testing.T) {
 
 	// Now that we have a payload, take a moment to check that the manifest is
 	// return by the payload digest.
-	dgst, err := digest.FromBytes(payload)
+	dgst := digest.FromBytes(payload)
-	if err != nil {
-		t.Fatalf("error getting manifest digest: %v", err)
-	}
-
 	exists, err = ms.Exists(dgst)
 	if err != nil {
 		t.Fatalf("error checking manifest existence by digest: %v", err)