From d2e29acce05144bb8ec8e07ff60246ebc7164745 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 3698a415..7e1a7cd4 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 7f280d36..2ae944a4 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{}
@@ -78,6 +79,7 @@ func (bw *blobWriter) Commit(ctx context.Context, desc distribution.Descriptor)
return distribution.Descriptor{}, err
}
+ bw.committed = true
return canonical, nil
}
@@ -89,11 +91,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
}
@@ -130,6 +135,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
}