From 380524ae2757ccbacf83c77984157a9e9622a220 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Tue, 14 Aug 2018 18:35:30 -0700
Subject: [PATCH] Unlock read lock on uploadID upon errors (#6283)

---
 cmd/object-api-errors.go         |  9 +++++++--
 cmd/object-api-multipart_test.go |  8 +++++---
 cmd/xl-v1-multipart.go           | 18 ++++++++++++++----
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/cmd/object-api-errors.go b/cmd/object-api-errors.go
index b8fc53fc5..327dd7c3a 100644
--- a/cmd/object-api-errors.go
+++ b/cmd/object-api-errors.go
@@ -314,10 +314,15 @@ func (e InvalidUploadID) Error() string {
 }
 
 // InvalidPart One or more of the specified parts could not be found
-type InvalidPart struct{}
+type InvalidPart struct {
+	PartNumber int
+	ExpETag    string
+	GotETag    string
+}
 
 func (e InvalidPart) Error() string {
-	return "One or more of the specified parts could not be found. The part may not have been uploaded, or the specified entity tag may not match the part's entity tag."
+	return fmt.Sprintf("Specified part could not be found. PartNumber %d, Expected %s, got %s",
+		e.PartNumber, e.ExpETag, e.GotETag)
 }
 
 // PartTooSmall - error if part size is less than 5MB.
diff --git a/cmd/object-api-multipart_test.go b/cmd/object-api-multipart_test.go
index f01b6c7b4..bcc233d3a 100644
--- a/cmd/object-api-multipart_test.go
+++ b/cmd/object-api-multipart_test.go
@@ -19,8 +19,10 @@ package cmd
 import (
 	"bytes"
 	"context"
+	"encoding/hex"
 	"fmt"
 	"os"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -1877,8 +1879,8 @@ func testObjectCompleteMultipartUpload(obj ObjectLayer, instanceType string, t T
 		// Asserting for Invalid UploadID (Test number 9).
 		{bucketNames[0], objectNames[0], "abc", []CompletePart{}, "", InvalidUploadID{UploadID: "abc"}, false},
 		// Test case with invalid Part Etag (Test number 10-11).
-		{bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abc"}}, "", fmt.Errorf("encoding/hex: odd length hex string"), false},
-		{bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abcz"}}, "", fmt.Errorf("encoding/hex: invalid byte: U+007A 'z'"), false},
+		{bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abc"}}, "", hex.ErrLength, false},
+		{bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abcz"}}, "", hex.InvalidByteError(00), false},
 		// Part number 0 doesn't exist, expecting InvalidPart error (Test number 12).
 		{bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abcd", PartNumber: 0}}, "", InvalidPart{}, false},
 		// // Upload and PartNumber exists, But a deliberate ETag mismatch is introduced (Test number 13).
@@ -1909,7 +1911,7 @@ func testObjectCompleteMultipartUpload(obj ObjectLayer, instanceType string, t T
 		}
 		// Failed as expected, but does it fail for the expected reason.
 		if actualErr != nil && !testCase.shouldPass {
-			if !strings.Contains(actualErr.Error(), testCase.expectedErr.Error()) {
+			if reflect.TypeOf(actualErr) != reflect.TypeOf(testCase.expectedErr) {
 				t.Errorf("Test %d: %s: Expected to fail with error \"%s\", but instead failed with error \"%s\"", i+1, instanceType, testCase.expectedErr, actualErr)
 			}
 		}
diff --git a/cmd/xl-v1-multipart.go b/cmd/xl-v1-multipart.go
index 8e4b8d7f4..3e1701a1a 100644
--- a/cmd/xl-v1-multipart.go
+++ b/cmd/xl-v1-multipart.go
@@ -331,6 +331,7 @@ func (xl xlObjects) PutObjectPart(ctx context.Context, bucket, object, uploadID
 	// get Quorum for this object
 	_, writeQuorum, err := objectQuorumFromMeta(ctx, xl, partsMetadata, errs)
 	if err != nil {
+		preUploadIDLock.RUnlock()
 		return pi, toObjectErr(err, bucket, object)
 	}
 
@@ -665,14 +666,23 @@ func (xl xlObjects) CompleteMultipartUpload(ctx context.Context, bucket string,
 		partIdx := objectPartIndex(currentXLMeta.Parts, part.PartNumber)
 		// All parts should have same part number.
 		if partIdx == -1 {
-			logger.LogIf(ctx, InvalidPart{})
-			return oi, InvalidPart{}
+			invp := InvalidPart{
+				PartNumber: part.PartNumber,
+				GotETag:    part.ETag,
+			}
+			logger.LogIf(ctx, invp)
+			return oi, invp
 		}
 
 		// All parts should have same ETag as previously generated.
 		if currentXLMeta.Parts[partIdx].ETag != part.ETag {
-			logger.LogIf(ctx, InvalidPart{})
-			return oi, InvalidPart{}
+			invp := InvalidPart{
+				PartNumber: part.PartNumber,
+				ExpETag:    currentXLMeta.Parts[partIdx].ETag,
+				GotETag:    part.ETag,
+			}
+			logger.LogIf(ctx, invp)
+			return oi, invp
 		}
 
 		// All parts except the last part has to be atleast 5MB.