From 9389a55e5d45e2085f0eac54831f74e6626e2a62 Mon Sep 17 00:00:00 2001
From: Daryl Finlay <djfinlay@users.noreply.github.com>
Date: Tue, 23 Jul 2019 05:36:15 +0000
Subject: [PATCH] Cancel PutObjectPart on upload abort (#7940)

Calling ListMultipartUploads fails if an upload is aborted while a
part is being uploaded because the directory for the upload exists
(since fsRenameFile ends up calling os.MkdirAll) but the meta JSON file
doesn't. To fix this we make sure an upload hasn't been aborted during
PutObjectPart by checking the existence of the directory for the upload
while moving the temporary part file into it.
---
 cmd/fs-v1-helpers.go   | 22 +++++++++++++++++++++-
 cmd/fs-v1-multipart.go |  8 ++++++--
 cmd/os-reliable.go     |  6 +++---
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/cmd/fs-v1-helpers.go b/cmd/fs-v1-helpers.go
index f00d732e4..e06fcf554 100644
--- a/cmd/fs-v1-helpers.go
+++ b/cmd/fs-v1-helpers.go
@@ -280,7 +280,7 @@ func fsOpenFile(ctx context.Context, readPath string, offset int64) (io.ReadClos
 	}
 
 	// Stat to get the size of the file at path.
-	st, err := os.Stat(readPath)
+	st, err := fr.Stat()
 	if err != nil {
 		err = osErrToFSFileErr(err)
 		if err != errFileNotFound {
@@ -386,6 +386,26 @@ func fsFAllocate(fd int, offset int64, len int64) (err error) {
 	return nil
 }
 
+// Renames source path to destination path, fails if the destination path
+// parents are not already created.
+func fsSimpleRenameFile(ctx context.Context, sourcePath, destPath string) error {
+	if err := checkPathLength(sourcePath); err != nil {
+		logger.LogIf(ctx, err)
+		return err
+	}
+	if err := checkPathLength(destPath); err != nil {
+		logger.LogIf(ctx, err)
+		return err
+	}
+
+	if err := os.Rename(sourcePath, destPath); err != nil {
+		logger.LogIf(ctx, err)
+		return osErrToFSFileErr(err)
+	}
+
+	return nil
+}
+
 // Renames source path to destination path, creates all the
 // missing parents if they don't exist.
 func fsRenameFile(ctx context.Context, sourcePath, destPath string) error {
diff --git a/cmd/fs-v1-multipart.go b/cmd/fs-v1-multipart.go
index 0f66be404..a137aaa10 100644
--- a/cmd/fs-v1-multipart.go
+++ b/cmd/fs-v1-multipart.go
@@ -152,7 +152,7 @@ func (fs *FSObjects) ListMultipartUploads(ctx context.Context, bucket, object, k
 		return result, toObjectErr(err)
 	}
 
-	// S3 spec says uploaIDs should be sorted based on initiated time. ModTime of fs.json
+	// S3 spec says uploadIDs should be sorted based on initiated time. ModTime of fs.json
 	// is the creation time of the uploadID, hence we will use that.
 	var uploads []MultipartInfo
 	for _, uploadID := range uploadIDs {
@@ -326,7 +326,11 @@ func (fs *FSObjects) PutObjectPart(ctx context.Context, bucket, object, uploadID
 
 	partPath := pathJoin(uploadIDDir, fs.encodePartFile(partID, etag, data.ActualSize()))
 
-	if err = fsRenameFile(ctx, tmpPartPath, partPath); err != nil {
+	// Make sure not to create parent directories if they don't exist - the upload might have been aborted.
+	if err = fsSimpleRenameFile(ctx, tmpPartPath, partPath); err != nil {
+		if err == errFileNotFound || err == errFileAccessDenied {
+			return pi, InvalidUploadID{UploadID: uploadID}
+		}
 		return pi, toObjectErr(err, minioMetaMultipartBucket, partPath)
 	}
 
diff --git a/cmd/os-reliable.go b/cmd/os-reliable.go
index 669bce6ca..410bcc2d3 100644
--- a/cmd/os-reliable.go
+++ b/cmd/os-reliable.go
@@ -156,11 +156,11 @@ func renameAll(srcFilePath, dstFilePath string) (err error) {
 // Reliably retries os.RenameAll if for some reason os.RenameAll returns
 // syscall.ENOENT (parent does not exist).
 func reliableRename(srcFilePath, dstFilePath string) (err error) {
+	if err = reliableMkdirAll(path.Dir(dstFilePath), 0777); err != nil {
+		return err
+	}
 	i := 0
 	for {
-		if err = reliableMkdirAll(path.Dir(dstFilePath), 0777); err != nil {
-			return err
-		}
 		// After a successful parent directory create attempt a renameAll.
 		if err = os.Rename(srcFilePath, dstFilePath); err != nil {
 			// Retry only for the first retryable error.