From 87959abe8f3e634936382b29dee08b995ea75414 Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Tue, 10 Mar 2015 14:40:58 -0700 Subject: [PATCH] digest: Minor refactoring Docker-DCO-1.1-Signed-off-by: Josh Hawn (github: jlhawn) --- digest/digest.go | 4 ++-- digest/tarsum.go | 8 +++----- digest/tarsum_test.go | 4 ++-- digest/verifiers.go | 18 +++++++++--------- digest/verifiers_test.go | 12 ++++++++++-- registry/handlers/api_test.go | 5 ++++- registry/storage/filereader_test.go | 6 +++++- registry/storage/filewriter_test.go | 18 +++++++++++++++--- registry/storage/layerupload.go | 5 ++++- 9 files changed, 54 insertions(+), 26 deletions(-) diff --git a/digest/digest.go b/digest/digest.go index da9a6276..d465c217 100644 --- a/digest/digest.go +++ b/digest/digest.go @@ -49,7 +49,7 @@ func NewDigestFromHex(alg, hex string) Digest { } // DigestRegexp matches valid digest types. -var DigestRegexp = regexp.MustCompile(`[a-zA-Z0-9-_+.]+:[a-zA-Z0-9-_+.=]+`) +var DigestRegexp = regexp.MustCompile(`[a-zA-Z0-9-_+.]+:[a-fA-F0-9]+`) var ( // ErrDigestInvalidFormat returned when digest format invalid. @@ -158,7 +158,7 @@ func (d Digest) sepIndex() int { i := strings.Index(string(d), ":") if i < 0 { - panic("invalid digest: " + d) + panic("could not find ':' in digest: " + d) } return i diff --git a/digest/tarsum.go b/digest/tarsum.go index 6c32bc59..acf878b6 100644 --- a/digest/tarsum.go +++ b/digest/tarsum.go @@ -27,12 +27,10 @@ type TarSumInfo struct { // InvalidTarSumError provides informations about a TarSum that cannot be parsed // by ParseTarSum. -type InvalidTarSumError struct { - TarSum string -} +type InvalidTarSumError string func (e InvalidTarSumError) Error() string { - return fmt.Sprintf("invalid tarsum: %q", e.TarSum) + return fmt.Sprintf("invalid tarsum: %q", string(e)) } // ParseTarSum parses a tarsum string into its components of interest. For @@ -52,7 +50,7 @@ func ParseTarSum(tarSum string) (tsi TarSumInfo, err error) { components := TarsumRegexpCapturing.FindStringSubmatch(tarSum) if len(components) != 1+TarsumRegexpCapturing.NumSubexp() { - return TarSumInfo{}, InvalidTarSumError{TarSum: tarSum} + return TarSumInfo{}, InvalidTarSumError(tarSum) } return TarSumInfo{ diff --git a/digest/tarsum_test.go b/digest/tarsum_test.go index 5a10b746..894c25ab 100644 --- a/digest/tarsum_test.go +++ b/digest/tarsum_test.go @@ -21,11 +21,11 @@ func TestParseTarSumComponents(t *testing.T) { }, { input: "", - err: InvalidTarSumError{}, + err: InvalidTarSumError(""), }, { input: "purejunk", - err: InvalidTarSumError{TarSum: "purejunk"}, + err: InvalidTarSumError("purejunk"), }, { input: "tarsum.v23+test:12341234123412341effefefe", diff --git a/digest/verifiers.go b/digest/verifiers.go index 59b16446..5180a588 100644 --- a/digest/verifiers.go +++ b/digest/verifiers.go @@ -20,27 +20,27 @@ type Verifier interface { // Verified will return true if the content written to Verifier matches // the digest. Verified() bool - - // Planned methods: - // Err() error - // Reset() } // NewDigestVerifier returns a verifier that compares the written bytes // against a passed in digest. -func NewDigestVerifier(d Digest) Verifier { +func NewDigestVerifier(d Digest) (Verifier, error) { + if err := d.Validate(); err != nil { + return nil, err + } + alg := d.Algorithm() switch alg { case "sha256", "sha384", "sha512": return hashVerifier{ hash: newHash(alg), digest: d, - } + }, nil default: // Assume we have a tarsum. version, err := tarsum.GetVersionFromTarsum(string(d)) if err != nil { - panic(err) // Always assume valid tarsum at this point. + return nil, err } pr, pw := io.Pipe() @@ -50,7 +50,7 @@ func NewDigestVerifier(d Digest) Verifier { ts, err := tarsum.NewTarSum(pr, true, version) if err != nil { - panic(err) + return nil, err } // TODO(sday): Ick! A goroutine per digest verification? We'll have to @@ -65,7 +65,7 @@ func NewDigestVerifier(d Digest) Verifier { ts: ts, pr: pr, pw: pw, - } + }, nil } } diff --git a/digest/verifiers_test.go b/digest/verifiers_test.go index 4f2ae5c0..32599c94 100644 --- a/digest/verifiers_test.go +++ b/digest/verifiers_test.go @@ -18,7 +18,11 @@ func TestDigestVerifier(t *testing.T) { t.Fatalf("unexpected error digesting bytes: %#v", err) } - verifier := NewDigestVerifier(digest) + verifier, err := NewDigestVerifier(digest) + if err != nil { + t.Fatalf("unexpected error getting digest verifier: %s", err) + } + io.Copy(verifier, bytes.NewReader(p)) if !verifier.Verified() { @@ -45,7 +49,11 @@ func TestDigestVerifier(t *testing.T) { // This is the most relevant example for the registry application. It's // effectively a read through pipeline, where the final sink is the digest // verifier. - verifier = NewDigestVerifier(digest) + verifier, err = NewDigestVerifier(digest) + if err != nil { + t.Fatalf("unexpected error getting digest verifier: %s", err) + } + lengthVerifier := NewLengthVerifier(expectedSize) rd := io.TeeReader(tf, lengthVerifier) io.Copy(verifier, rd) diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 22f2d9ca..ab8187c1 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -236,7 +236,10 @@ func TestLayerAPI(t *testing.T) { }) // Verify the body - verifier := digest.NewDigestVerifier(layerDigest) + verifier, err := digest.NewDigestVerifier(layerDigest) + if err != nil { + t.Fatalf("unexpected error getting digest verifier: %s", err) + } io.Copy(verifier, resp.Body) if !verifier.Verified() { diff --git a/registry/storage/filereader_test.go b/registry/storage/filereader_test.go index 7c554e8b..8a077603 100644 --- a/registry/storage/filereader_test.go +++ b/registry/storage/filereader_test.go @@ -41,7 +41,11 @@ func TestSimpleRead(t *testing.T) { t.Fatalf("error allocating file reader: %v", err) } - verifier := digest.NewDigestVerifier(dgst) + verifier, err := digest.NewDigestVerifier(dgst) + if err != nil { + t.Fatalf("error getting digest verifier: %s", err) + } + io.Copy(verifier, fr) if !verifier.Verified() { diff --git a/registry/storage/filewriter_test.go b/registry/storage/filewriter_test.go index 06db31f3..a8ea6241 100644 --- a/registry/storage/filewriter_test.go +++ b/registry/storage/filewriter_test.go @@ -55,7 +55,11 @@ func TestSimpleWrite(t *testing.T) { } defer fr.Close() - verifier := digest.NewDigestVerifier(dgst) + verifier, err := digest.NewDigestVerifier(dgst) + if err != nil { + t.Fatalf("unexpected error getting digest verifier: %s", err) + } + io.Copy(verifier, fr) if !verifier.Verified() { @@ -94,7 +98,11 @@ func TestSimpleWrite(t *testing.T) { } defer fr.Close() - verifier = digest.NewDigestVerifier(doubledgst) + verifier, err = digest.NewDigestVerifier(doubledgst) + if err != nil { + t.Fatalf("unexpected error getting digest verifier: %s", err) + } + io.Copy(verifier, fr) if !verifier.Verified() { @@ -141,7 +149,11 @@ func TestSimpleWrite(t *testing.T) { } defer fr.Close() - verifier = digest.NewDigestVerifier(doubledgst) + verifier, err = digest.NewDigestVerifier(doubledgst) + if err != nil { + t.Fatalf("unexpected error getting digest verifier: %s", err) + } + io.Copy(verifier, fr) if !verifier.Verified() { diff --git a/registry/storage/layerupload.go b/registry/storage/layerupload.go index 69b547f5..fdb00e93 100644 --- a/registry/storage/layerupload.go +++ b/registry/storage/layerupload.go @@ -85,7 +85,10 @@ func (luc *layerUploadController) Cancel() error { // validateLayer checks the layer data against the digest, returning an error // if it does not match. The canonical digest is returned. func (luc *layerUploadController) validateLayer(dgst digest.Digest) (digest.Digest, error) { - digestVerifier := digest.NewDigestVerifier(dgst) + digestVerifier, err := digest.NewDigestVerifier(dgst) + if err != nil { + return "", err + } // TODO(stevvooe): Store resumable hash calculations in upload directory // in driver. Something like a file at path /resumablehash/