From 023866642ce31deeaf0ff789e1486729cb7647d9 Mon Sep 17 00:00:00 2001 From: poornas Date: Mon, 1 Apr 2019 12:19:52 -0700 Subject: [PATCH] canonicalize ETag correctly (#7442) Fixes #7441 Trim extra quotes prefixing/suffixing ETag in CompleteMultipartUpload request. --- cmd/fs-v1-multipart.go | 5 +++ cmd/object-api-multipart_test.go | 3 +- cmd/object-api-utils.go | 2 +- cmd/object-handlers-common.go | 9 +++-- cmd/object-handlers-common_test.go | 53 ++++++++++++++++++++++++++++++ cmd/xl-v1-multipart.go | 2 ++ 6 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 cmd/object-handlers-common_test.go diff --git a/cmd/fs-v1-multipart.go b/cmd/fs-v1-multipart.go index 257b33fa5..c2f4c3d90 100644 --- a/cmd/fs-v1-multipart.go +++ b/cmd/fs-v1-multipart.go @@ -524,6 +524,11 @@ func (fs *FSObjects) CompleteMultipartUpload(ctx context.Context, bucket string, return oi, err } + // ensure that part ETag is canonicalized to strip off extraneous quotes + for i := range parts { + parts[i].ETag = canonicalizeETag(parts[i].ETag) + } + // Save consolidated actual size. var objectActualSize int64 // Validate all parts and then commit to disk. diff --git a/cmd/object-api-multipart_test.go b/cmd/object-api-multipart_test.go index 8fcd550bb..3cfe56ba9 100644 --- a/cmd/object-api-multipart_test.go +++ b/cmd/object-api-multipart_test.go @@ -1796,6 +1796,7 @@ func testObjectCompleteMultipartUpload(obj ObjectLayer, instanceType string, t T // Part with size larger than 5Mb. {bucketNames[0], objectNames[0], uploadIDs[0], 5, string(validPart), validPartMD5, int64(len(string(validPart)))}, {bucketNames[0], objectNames[0], uploadIDs[0], 6, string(validPart), validPartMD5, int64(len(string(validPart)))}, + {bucketNames[0], objectNames[0], uploadIDs[0], 7, string(validPart), validPartMD5, int64(len(string(validPart)))}, } sha256sum := "" var opts ObjectOptions @@ -1837,7 +1838,7 @@ func testObjectCompleteMultipartUpload(obj ObjectLayer, instanceType string, t T // Part size greater than 5MB. { []CompletePart{ - {ETag: validPartMD5, PartNumber: 5}, + {ETag: fmt.Sprintf("\"\"\"\"\"%s\"\"\"", validPartMD5), PartNumber: 5}, }, }, // inputParts - 4. diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go index f2af7b2b4..be510e366 100644 --- a/cmd/object-api-utils.go +++ b/cmd/object-api-utils.go @@ -193,7 +193,7 @@ func mustGetUUID() string { func getCompleteMultipartMD5(ctx context.Context, parts []CompletePart) (string, error) { var finalMD5Bytes []byte for _, part := range parts { - md5Bytes, err := hex.DecodeString(part.ETag) + md5Bytes, err := hex.DecodeString(canonicalizeETag(part.ETag)) if err != nil { logger.LogIf(ctx, err) return "", err diff --git a/cmd/object-handlers-common.go b/cmd/object-handlers-common.go index 979047d8d..c99d9fe26 100644 --- a/cmd/object-handlers-common.go +++ b/cmd/object-handlers-common.go @@ -19,7 +19,7 @@ package cmd import ( "context" "net/http" - "strings" + "regexp" "time" "github.com/minio/minio/cmd/crypto" @@ -27,6 +27,10 @@ import ( "github.com/minio/minio/pkg/handlers" ) +var ( + etagRegex = regexp.MustCompile("\"*?([^\"]*?)\"*?$") +) + // Validates the preconditions for CopyObjectPart, returns true if CopyObjectPart // operation should not proceed. Preconditions supported are: // x-amz-copy-source-if-modified-since @@ -230,8 +234,7 @@ func ifModifiedSince(objTime time.Time, givenTime time.Time) bool { // canonicalizeETag returns ETag with leading and trailing double-quotes removed, // if any present func canonicalizeETag(etag string) string { - canonicalETag := strings.TrimPrefix(etag, "\"") - return strings.TrimSuffix(canonicalETag, "\"") + return etagRegex.ReplaceAllString(etag, "$1") } // isETagEqual return true if the canonical representations of two ETag strings diff --git a/cmd/object-handlers-common_test.go b/cmd/object-handlers-common_test.go new file mode 100644 index 000000000..c6a4d1922 --- /dev/null +++ b/cmd/object-handlers-common_test.go @@ -0,0 +1,53 @@ +/* + * Minio Cloud Storage, (C) 2019 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "testing" +) + +// Tests - canonicalizeETag() +func TestCanonicalizeETag(t *testing.T) { + testCases := []struct { + etag string + canonicalizedETag string + }{ + { + etag: "\"\"\"", + canonicalizedETag: "", + }, + { + etag: "\"\"\"abc\"", + canonicalizedETag: "abc", + }, + { + etag: "abcd", + canonicalizedETag: "abcd", + }, + { + etag: "abcd\"\"", + canonicalizedETag: "abcd", + }, + } + for _, test := range testCases { + etag := canonicalizeETag(test.etag) + if test.canonicalizedETag != etag { + t.Fatalf("Expected %s , got %s", test.canonicalizedETag, etag) + + } + } +} diff --git a/cmd/xl-v1-multipart.go b/cmd/xl-v1-multipart.go index 27c4a5a13..7dde53868 100644 --- a/cmd/xl-v1-multipart.go +++ b/cmd/xl-v1-multipart.go @@ -675,6 +675,8 @@ func (xl xlObjects) CompleteMultipartUpload(ctx context.Context, bucket string, // Validate each part and then commit to disk. for i, part := range parts { + // ensure that part ETag is canonicalized to strip off extraneous quotes + part.ETag = canonicalizeETag(part.ETag) partIdx := objectPartIndex(currentXLMeta.Parts, part.PartNumber) // All parts should have same part number. if partIdx == -1 {