From ba672e8b69f994179109bd53c1cb2a70db60e0da Mon Sep 17 00:00:00 2001
From: Richard Scothern <richard.scothern@docker.com>
Date: Tue, 19 Apr 2016 16:31:25 -0700
Subject: [PATCH] When a blob upload is committed prevent writing out hashstate
 in the subsequent close.

When a blob upload is cancelled close the blobwriter before removing
upload state to ensure old hashstates don't persist.

Signed-off-by: Richard Scothern <richard.scothern@docker.com>
---
 registry/storage/blob_test.go  | 17 +++++++++++++++++
 registry/storage/blobwriter.go | 15 ++++++++++++---
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/registry/storage/blob_test.go b/registry/storage/blob_test.go
index 3698a415d..7e1a7cd44 100644
--- a/registry/storage/blob_test.go
+++ b/registry/storage/blob_test.go
@@ -16,6 +16,7 @@ import (
 	"github.com/docker/distribution/registry/storage/cache/memory"
 	"github.com/docker/distribution/registry/storage/driver/inmemory"
 	"github.com/docker/distribution/testutil"
+	"path"
 )
 
 // TestWriteSeek tests that the current file size can be
@@ -83,6 +84,15 @@ func TestSimpleBlobUpload(t *testing.T) {
 		t.Fatalf("unexpected error during upload cancellation: %v", err)
 	}
 
+	// get the enclosing directory
+	uploadPath := path.Dir(blobUpload.(*blobWriter).path)
+
+	// ensure state was cleaned up
+	_, err = driver.List(ctx, uploadPath)
+	if err == nil {
+		t.Fatal("files in upload path after cleanup")
+	}
+
 	// Do a resume, get unknown upload
 	blobUpload, err = bs.Resume(ctx, blobUpload.ID())
 	if err != distribution.ErrBlobUploadUnknown {
@@ -128,6 +138,13 @@ func TestSimpleBlobUpload(t *testing.T) {
 		t.Fatalf("unexpected error finishing layer upload: %v", err)
 	}
 
+	// ensure state was cleaned up
+	uploadPath = path.Dir(blobUpload.(*blobWriter).path)
+	_, err = driver.List(ctx, uploadPath)
+	if err == nil {
+		t.Fatal("files in upload path after commit")
+	}
+
 	// After finishing an upload, it should no longer exist.
 	if _, err := bs.Resume(ctx, blobUpload.ID()); err != distribution.ErrBlobUploadUnknown {
 		t.Fatalf("expected layer upload to be unknown, got %v", err)
diff --git a/registry/storage/blobwriter.go b/registry/storage/blobwriter.go
index 418df8188..3387bafb1 100644
--- a/registry/storage/blobwriter.go
+++ b/registry/storage/blobwriter.go
@@ -18,8 +18,8 @@ var (
 	errResumableDigestNotAvailable = errors.New("resumable digest not available")
 )
 
-// layerWriter is used to control the various aspects of resumable
-// layer upload. It implements the LayerUpload interface.
+// blobWriter is used to control the various aspects of resumable
+// blob upload.
 type blobWriter struct {
 	ctx       context.Context
 	blobStore *linkedBlobStore
@@ -34,6 +34,7 @@ type blobWriter struct {
 	path       string
 
 	resumableDigestEnabled bool
+	committed              bool
 }
 
 var _ distribution.BlobWriter = &blobWriter{}
@@ -80,6 +81,7 @@ func (bw *blobWriter) Commit(ctx context.Context, desc distribution.Descriptor)
 		return distribution.Descriptor{}, err
 	}
 
+	bw.committed = true
 	return canonical, nil
 }
 
@@ -91,11 +93,14 @@ func (bw *blobWriter) Cancel(ctx context.Context) error {
 		return err
 	}
 
+	if err := bw.Close(); err != nil {
+		context.GetLogger(ctx).Errorf("error closing blobwriter: %s", err)
+	}
+
 	if err := bw.removeResources(ctx); err != nil {
 		return err
 	}
 
-	bw.Close()
 	return nil
 }
 
@@ -132,6 +137,10 @@ func (bw *blobWriter) ReadFrom(r io.Reader) (n int64, err error) {
 }
 
 func (bw *blobWriter) Close() error {
+	if bw.committed {
+		return errors.New("blobwriter close after commit")
+	}
+
 	if err := bw.storeHashState(bw.blobStore.ctx); err != nil {
 		return err
 	}