Merge pull request #4176 from stevvooe/valdiate-digest-before-disk

content/local: validate digest before calculating path

content/local/store.go

@@ -92,7 +92,11 @@ func NewLabeledStore(root string, ls LabelStore) (content.Store, error) {
 }
 
 func (s *store) Info(ctx context.Context, dgst digest.Digest) (content.Info, error) {
-	p := s.blobPath(dgst)
+	p, err := s.blobPath(dgst)
+	if err != nil {
+		return content.Info{}, errors.Wrapf(err, "calculating blob info path")
+	}
+
 	fi, err := os.Stat(p)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -123,7 +127,10 @@ func (s *store) info(dgst digest.Digest, fi os.FileInfo, labels map[string]strin
 
 // ReaderAt returns an io.ReaderAt for the blob.
 func (s *store) ReaderAt(ctx context.Context, desc ocispec.Descriptor) (content.ReaderAt, error) {
-	p := s.blobPath(desc.Digest)
+	p, err := s.blobPath(desc.Digest)
+	if err != nil {
+		return nil, errors.Wrapf(err, "calculating blob path for ReaderAt")
+	}
 	fi, err := os.Stat(p)
 	if err != nil {
 		if !os.IsNotExist(err) {
@@ -150,7 +157,12 @@ func (s *store) ReaderAt(ctx context.Context, desc ocispec.Descriptor) (content.
 // While this is safe to do concurrently, safe exist-removal logic must hold
 // some global lock on the store.
 func (s *store) Delete(ctx context.Context, dgst digest.Digest) error {
-	if err := os.RemoveAll(s.blobPath(dgst)); err != nil {
+	bp, err := s.blobPath(dgst)
+	if err != nil {
+		return errors.Wrapf(err, "calculating blob path for delete")
+	}
+
+	if err := os.RemoveAll(bp); err != nil {
 		if !os.IsNotExist(err) {
 			return err
 		}
@@ -166,7 +178,11 @@ func (s *store) Update(ctx context.Context, info content.Info, fieldpaths ...str
 		return content.Info{}, errors.Wrapf(errdefs.ErrFailedPrecondition, "update not supported on immutable content store")
 	}
 
-	p := s.blobPath(info.Digest)
+	p, err := s.blobPath(info.Digest)
+	if err != nil {
+		return content.Info{}, errors.Wrapf(err, "calculating blob path for update")
+	}
+
 	fi, err := os.Stat(p)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -512,7 +528,10 @@ func (s *store) writer(ctx context.Context, ref string, total int64, expected di
 	// TODO(stevvooe): Need to actually store expected here. We have
 	// code in the service that shouldn't be dealing with this.
 	if expected != "" {
-		p := s.blobPath(expected)
+		p, err := s.blobPath(expected)
+		if err != nil {
+			return nil, errors.Wrap(err, "calculating expected blob path for writer")
+		}
 		if _, err := os.Stat(p); err == nil {
 			return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", expected)
 		}
@@ -607,11 +626,17 @@ func (s *store) Abort(ctx context.Context, ref string) error {
 	return nil
 }
 
-func (s *store) blobPath(dgst digest.Digest) string {
+func (s *store) blobPath(dgst digest.Digest) (string, error) {
-	return filepath.Join(s.root, "blobs", dgst.Algorithm().String(), dgst.Hex())
+	if err := dgst.Validate(); err != nil {
+		return "", errors.Wrapf(errdefs.ErrInvalidArgument, "cannot calculate blob path from invalid digest: %v", err)
+	}
+
+	return filepath.Join(s.root, "blobs", dgst.Algorithm().String(), dgst.Hex()), nil
 }
 
 func (s *store) ingestRoot(ref string) string {
+	// we take a digest of the ref to keep the ingest paths constant length.
+	// Note that this is not the current or potential digest of incoming content.
 	dgst := digest.FromString(ref)
 	return filepath.Join(s.root, "ingest", dgst.Hex())
 }

content/local/store_test.go

@@ -329,7 +329,10 @@ func checkCopy(t checker, size int64, dst io.Writer, src io.Reader) {
 }
 
 func checkBlobPath(t *testing.T, cs content.Store, dgst digest.Digest) string {
-	path := cs.(*store).blobPath(dgst)
+	path, err := cs.(*store).blobPath(dgst)
+	if err != nil {
+		t.Fatalf("failed to calculate blob path: %v", err)
+	}
 
 	if path != filepath.Join(cs.(*store).root, "blobs", dgst.Algorithm().String(), dgst.Hex()) {
 		t.Fatalf("unexpected path: %q", path)

content/local/writer.go

@@ -115,8 +115,8 @@ func (w *writer) Commit(ctx context.Context, size int64, expected digest.Digest,
 	}
 
 	var (
-		ingest = filepath.Join(w.path, "data")
+		ingest    = filepath.Join(w.path, "data")
-		target = w.s.blobPath(dgst)
+		target, _ = w.s.blobPath(dgst) // ignore error because we calculated this dgst
 	)
 
 	// make sure parent directories of blob exist