registry/storage: do not write hash state when closing a cancelled upload

Cancel removes the upload directory, but the handler then closes the
writer, and Close stores the hash state again. That recreates the
upload directory without a startedat file, so the upload purger can
never remove it, and Close reports "already closed". Skip storing the
hash state once the upload has been cancelled.

Signed-off-by: YoungjuKim <fjvbn2003@gmail.com>

registry/storage/blob_test.go

@@ -50,6 +50,44 @@ func TestWriteSeek(t *testing.T) {
 	}
 }
 
+// TestBlobUploadCloseAfterCancel checks that closing a cancelled upload, as
+// the HTTP handler does with a deferred Close, does not write the hash state
+// back into the upload directory that Cancel removed.
+func TestBlobUploadCloseAfterCancel(t *testing.T) {
+	ctx := context.Background()
+	imageName, _ := reference.WithName("foo/bar")
+	driver := inmemory.New()
+	registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize)), EnableDelete, EnableRedirect)
+	if err != nil {
+		t.Fatalf("error creating registry: %v", err)
+	}
+	repository, err := registry.Repository(ctx, imageName)
+	if err != nil {
+		t.Fatalf("unexpected error getting repo: %v", err)
+	}
+	bs := repository.Blobs(ctx)
+
+	blobUpload, err := bs.Create(ctx)
+	if err != nil {
+		t.Fatalf("unexpected error starting layer upload: %s", err)
+	}
+	if _, err := blobUpload.Write([]byte{1, 2, 3}); err != nil {
+		t.Fatalf("unexpected error writing contents: %v", err)
+	}
+	uploadPath := path.Dir(blobUpload.(*blobWriter).path)
+
+	if err := blobUpload.Cancel(ctx); err != nil {
+		t.Fatalf("unexpected error during upload cancellation: %v", err)
+	}
+	if err := blobUpload.Close(); err != nil {
+		t.Fatalf("unexpected error closing cancelled upload: %v", err)
+	}
+
+	if paths, err := driver.List(ctx, uploadPath); err == nil {
+		t.Fatalf("upload path recreated after cancel and close: %v", paths)
+	}
+}
+
 // TestSimpleBlobUpload covers the blob upload process, exercising common
 // error paths that might be seen during an upload.
 func TestSimpleBlobUpload(t *testing.T) {

registry/storage/blobwriter.go

@@ -40,6 +40,7 @@ type blobWriter struct {
 
 	resumableDigestEnabled bool
 	committed              bool
+	cancelled              bool
 }
 
 var _ distribution.BlobWriter = &blobWriter{}
@@ -98,6 +99,7 @@ func (bw *blobWriter) Cancel(ctx context.Context) error {
 	if err := bw.fileWriter.Cancel(ctx); err != nil {
 		return err
 	}
+	bw.cancelled = true
 
 	if err := bw.Close(); err != nil {
 		dcontext.GetLogger(ctx).Errorf("error closing blobwriter: %s", err)
@@ -152,6 +154,16 @@ func (bw *blobWriter) Close() error {
 		return errors.New("blobwriter close after commit")
 	}
 
+	if bw.cancelled {
+		// The upload is being discarded, so there is no hash state worth
+		// saving. Writing it would recreate the upload directory after
+		// removeResources deleted it, without a startedat file, and
+		// PurgeUploads never removes such a directory. The file writer has
+		// already been cancelled, so an error from closing it is expected.
+		_ = bw.fileWriter.Close()
+		return nil
+	}
+
 	if err := bw.storeHashState(bw.blobStore.ctx); err != nil && err != errResumableDigestNotAvailable {
 		return err
 	}