Fix CopyObject regression calculating md5sum (#6868)

CopyObject() failed to calculate proper md5sum
when without encryption headers. This is a regression
fix perhaps introduced in commit 5f6d717b7a

Fixes https://github.com/minio/minio-go/issues/1044

cmd/object-api-utils.go

@@ -635,12 +635,13 @@ func (p *PutObjReader) MD5CurrentHexString() string {
 func NewPutObjReader(rawReader *hash.Reader, encReader *hash.Reader, encKey []byte) *PutObjReader {
 	p := PutObjReader{Reader: rawReader, rawReader: rawReader}
 
-	var objKey crypto.ObjectKey
-	copy(objKey[:], encKey)
-	p.sealMD5Fn = sealETagFn(objKey)
-	if encReader != nil {
+	if len(encKey) != 0 && encReader != nil {
+		var objKey crypto.ObjectKey
+		copy(objKey[:], encKey)
+		p.sealMD5Fn = sealETagFn(objKey)
 		p.Reader = encReader
 	}
+
 	return &p
 }
 
@@ -651,9 +652,10 @@ func sealETag(encKey crypto.ObjectKey, md5CurrSum []byte) []byte {
 	}
 	return encKey.SealETag(md5CurrSum)
 }
+
 func sealETagFn(key crypto.ObjectKey) SealMD5CurrFn {
-	fn1 := func(md5sumcurr []byte) []byte {
+	fn := func(md5sumcurr []byte) []byte {
 		return sealETag(key, md5sumcurr)
 	}
-	return fn1
+	return fn
 }

cmd/object-handlers.go

@@ -774,6 +774,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 			writeErrorResponse(w, toAPIErrorCode(ctx, err), r.URL, guessIsBrowserReq(r))
 			return
 		}
+		length = actualSize
 	}
 
 	// No need to compress for remote etcd calls
@@ -813,8 +814,10 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 		writeErrorResponse(w, toAPIErrorCode(ctx, err), r.URL, guessIsBrowserReq(r))
 		return
 	}
+
 	rawReader := srcInfo.Reader
-	pReader := NewPutObjReader(srcInfo.Reader, srcInfo.Reader, nil)
+	pReader := NewPutObjReader(srcInfo.Reader, nil, nil)
+
 	var encMetadata = make(map[string]string)
 	if objectAPI.IsEncryptionSupported() && !isCompressed {
 		// Encryption parameters not applicable for this object.
@@ -882,15 +885,15 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 
 			switch {
 			case !isSourceEncrypted && !isTargetEncrypted:
-				fallthrough
+				targetSize = srcInfo.Size
 			case isSourceEncrypted && isTargetEncrypted:
 				targetSize = srcInfo.Size
-			// Source not encrypted and target encrypted
 			case !isSourceEncrypted && isTargetEncrypted:
 				targetSize = srcInfo.EncryptedSize()
 			case isSourceEncrypted && !isTargetEncrypted:
 				targetSize, _ = srcInfo.DecryptedSize()
 			}
+
 			if isTargetEncrypted {
 				reader, objEncKey, err = newEncryptReader(srcInfo.Reader, newKey, dstBucket, dstObject, encMetadata, sseS3)
 				if err != nil {
@@ -910,6 +913,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 				writeErrorResponse(w, toAPIErrorCode(ctx, err), r.URL, guessIsBrowserReq(r))
 				return
 			}
+
 			pReader = NewPutObjReader(rawReader, srcInfo.Reader, objEncKey)
 		}
 	}

cmd/object-handlers_test.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"crypto/md5"
 	"encoding/base64"
+	"encoding/hex"
 	"encoding/xml"
 	"fmt"
 	"io"
@@ -1811,9 +1812,13 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
 	// this is required even to assert the copied object,
 	bytesData := []struct {
 		byteData []byte
+		md5sum   string
 	}{
-		{generateBytesData(6 * humanize.KiByte)},
+		{byteData: generateBytesData(6 * humanize.KiByte)},
 	}
+	h := md5.New()
+	h.Write(bytesData[0].byteData)
+	bytesData[0].md5sum = hex.EncodeToString(h.Sum(nil))
 
 	buffers := []*bytes.Buffer{
 		new(bytes.Buffer),
@@ -1826,23 +1831,29 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
 		objectName    string
 		contentLength int64
 		textData      []byte
+		md5sum        string
 		metaData      map[string]string
 	}{
 		// case - 1.
-		{bucketName, objectName, int64(len(bytesData[0].byteData)), bytesData[0].byteData, make(map[string]string)},
+		{bucketName, objectName, int64(len(bytesData[0].byteData)), bytesData[0].byteData, bytesData[0].md5sum, make(map[string]string)},
 
 		// case - 2.
 		// used for anonymous HTTP request test.
-		{bucketName, anonObject, int64(len(bytesData[0].byteData)), bytesData[0].byteData, make(map[string]string)},
+		{bucketName, anonObject, int64(len(bytesData[0].byteData)), bytesData[0].byteData, bytesData[0].md5sum, make(map[string]string)},
 	}
+
 	// iterate through the above set of inputs and upload the object.
 	for i, input := range putObjectInputs {
 		// uploading the object.
-		_, err = obj.PutObject(context.Background(), input.bucketName, input.objectName, mustGetPutObjReader(t, bytes.NewBuffer(input.textData), input.contentLength, input.metaData[""], ""), input.metaData, opts)
+		var objInfo ObjectInfo
+		objInfo, err = obj.PutObject(context.Background(), input.bucketName, input.objectName, mustGetPutObjReader(t, bytes.NewBuffer(input.textData), input.contentLength, input.metaData[""], ""), input.metaData, opts)
 		// if object upload fails stop the test.
 		if err != nil {
 			t.Fatalf("Put Object case %d:  Error uploading object: <ERROR> %v", i+1, err)
 		}
+		if objInfo.ETag != input.md5sum {
+			t.Fatalf("Put Object case %d:  Checksum mismatched: <ERROR> got %s, expected %s", i+1, input.md5sum, objInfo.ETag)
+		}
 	}
 
 	// test cases with inputs and expected result for Copy Object.
@@ -2105,6 +2116,16 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
 			t.Fatalf("Test %d: %s:  Expected the response status to be `%d`, but instead found `%d`", i+1, instanceType, testCase.expectedRespStatus, rec.Code)
 		}
 		if rec.Code == http.StatusOK {
+			var cpObjResp CopyObjectResponse
+			if err = xml.Unmarshal(rec.Body.Bytes(), &cpObjResp); err != nil {
+				t.Fatalf("Test %d: %s: Failed to parse the CopyObjectResult response: <ERROR> %s", i+1, instanceType, err)
+			}
+
+			retag := canonicalizeETag(cpObjResp.ETag)
+			if retag != bytesData[0].md5sum {
+				t.Fatalf("Test %d: %s: Failed to CopyObject: <ERROR> got %s, expected %s", i+1, instanceType, retag, bytesData[0].md5sum)
+			}
+
 			// See if the new object is formed.
 			// testing whether the copy was successful.
 			err = obj.GetObject(context.Background(), testCase.bucketName, testCase.newObjectName, 0, int64(len(bytesData[0].byteData)), buffers[0], "", opts)