From 67062840c1f13342e4442e108be732a12bc3b2b0 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 19 Jun 2020 13:53:45 -0700 Subject: [PATCH] fix: perform CopyObject under more conditions (#9879) - x-amz-storage-class specified CopyObject should proceed regardless, its not a precondition - sourceVersionID is specified CopyObject should proceed regardless, its not a precondition --- cmd/object-handlers.go | 18 +++++----- cmd/object-handlers_test.go | 67 +++++-------------------------------- 2 files changed, 17 insertions(+), 68 deletions(-) diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 069c76cc7..2a1b83257 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -768,9 +768,6 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re // Note that url.Parse does the unescaping cpSrcPath = u.Path } - if vid == "" { - vid = strings.TrimSpace(r.Header.Get(xhttp.AmzCopySourceVersionID)) - } srcBucket, srcObject := path2BucketObject(cpSrcPath) // If source object is empty or bucket is empty, reply back invalid copy source. @@ -884,11 +881,8 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re var chStorageClass bool if dstSc != "" { - sc, ok := srcInfo.UserDefined[xhttp.AmzStorageClass] - if (ok && dstSc != sc) || (srcInfo.StorageClass != dstSc) { - chStorageClass = true - srcInfo.metadataOnly = false - } + chStorageClass = true + srcInfo.metadataOnly = false } var reader io.Reader @@ -1121,7 +1115,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re // if encryption is enabled we do not need explicit "REPLACE" metadata to // be enabled as well - this is to allow for key-rotation. if !isDirectiveReplace(r.Header.Get(xhttp.AmzMetadataDirective)) && !isDirectiveReplace(r.Header.Get(xhttp.AmzTagDirective)) && - srcInfo.metadataOnly && !crypto.IsEncrypted(srcInfo.UserDefined) { + srcInfo.metadataOnly && !crypto.IsEncrypted(srcInfo.UserDefined) && srcOpts.VersionID == "" { // If x-amz-metadata-directive is not set to REPLACE then we need // to error out if source and destination are same. writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidCopyDest), r.URL, guessIsBrowserReq(r)) @@ -1181,6 +1175,12 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re encodedSuccessResponse := encodeResponse(response) setPutObjHeaders(w, objInfo, false) + // We must not use the http.Header().Set method here because some (broken) + // clients expect the x-amz-copy-source-version-id header key to be literally + // "x-amz-copy-source-version-id"- not in canonicalized form, preserve it. + if srcOpts.VersionID != "" { + w.Header()[strings.ToLower(xhttp.AmzCopySourceVersionID)] = []string{srcOpts.VersionID} + } // Write success response. writeSuccessResponseXML(w, encodedSuccessResponse) diff --git a/cmd/object-handlers_test.go b/cmd/object-handlers_test.go index 3b7c2d71d..f953657f9 100644 --- a/cmd/object-handlers_test.go +++ b/cmd/object-handlers_test.go @@ -1578,15 +1578,14 @@ func testAPICopyObjectPartHandler(obj ObjectLayer, instanceType, bucketName stri // test cases with inputs and expected result for Copy Object. testCases := []struct { - bucketName string - copySourceHeader string // data for "X-Amz-Copy-Source" header. Contains the object to be copied in the URL. - copySourceVersionID string // data for "X-Amz-Copy-Source-Version-Id" header. - copySourceRange string // data for "X-Amz-Copy-Source-Range" header, contains the byte range offsets of data to be copied. - uploadID string // uploadID of the transaction. - invalidPartNumber bool // Sets an invalid multipart. - maximumPartNumber bool // Sets a maximum parts. - accessKey string - secretKey string + bucketName string + copySourceHeader string // data for "X-Amz-Copy-Source" header. Contains the object to be copied in the URL. + copySourceRange string // data for "X-Amz-Copy-Source-Range" header, contains the byte range offsets of data to be copied. + uploadID string // uploadID of the transaction. + invalidPartNumber bool // Sets an invalid multipart. + maximumPartNumber bool // Sets a maximum parts. + accessKey string + secretKey string // expected output. expectedRespStatus int }{ @@ -1768,26 +1767,6 @@ func testAPICopyObjectPartHandler(obj ObjectLayer, instanceType, bucketName stri secretKey: credentials.SecretKey, expectedRespStatus: http.StatusBadRequest, }, - // Test case - 16, copy part 1 from from newObject1 with null X-Amz-Copy-Source-Version-Id - { - bucketName: bucketName, - uploadID: uploadID, - copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName), - copySourceVersionID: "null", - accessKey: credentials.AccessKey, - secretKey: credentials.SecretKey, - expectedRespStatus: http.StatusOK, - }, - // Test case - 16, copy part 1 from from newObject1 with non null X-Amz-Copy-Source-Version-Id - { - bucketName: bucketName, - uploadID: uploadID, - copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName), - copySourceVersionID: "17", // invalid id - accessKey: credentials.AccessKey, - secretKey: credentials.SecretKey, - expectedRespStatus: http.StatusBadRequest, - }, } for i, testCase := range testCases { @@ -1810,9 +1789,6 @@ func testAPICopyObjectPartHandler(obj ObjectLayer, instanceType, bucketName stri if testCase.copySourceHeader != "" { req.Header.Set("X-Amz-Copy-Source", testCase.copySourceHeader) } - if testCase.copySourceVersionID != "" { - req.Header.Set("X-Amz-Copy-Source-Version-Id", testCase.copySourceVersionID) - } if testCase.copySourceRange != "" { req.Header.Set("X-Amz-Copy-Source-Range", testCase.copySourceRange) } @@ -1933,7 +1909,6 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string, bucketName string newObjectName string // name of the newly copied object. copySourceHeader string // data for "X-Amz-Copy-Source" header. Contains the object to be copied in the URL. - copySourceVersionID string // data for "X-Amz-Copy-Source-Version-Id" header. copyModifiedHeader string // data for "X-Amz-Copy-Source-If-Modified-Since" header copyUnmodifiedHeader string // data for "X-Amz-Copy-Source-If-Unmodified-Since" header metadataGarbage bool @@ -2162,26 +2137,6 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string, secretKey: credentials.SecretKey, expectedRespStatus: http.StatusBadRequest, }, - // Test case - 19, copy metadata from newObject1 with null X-Amz-Copy-Source-Version-Id - { - bucketName: bucketName, - newObjectName: "newObject1", - copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName), - copySourceVersionID: "null", - accessKey: credentials.AccessKey, - secretKey: credentials.SecretKey, - expectedRespStatus: http.StatusOK, - }, - // Test case - 20, copy metadata from newObject1 with non null X-Amz-Copy-Source-Version-Id - { - bucketName: bucketName, - newObjectName: "newObject1", - copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName), - copySourceVersionID: "17", - accessKey: credentials.AccessKey, - secretKey: credentials.SecretKey, - expectedRespStatus: http.StatusBadRequest, - }, } for i, testCase := range testCases { @@ -2200,9 +2155,6 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string, if testCase.copySourceHeader != "" { req.Header.Set("X-Amz-Copy-Source", testCase.copySourceHeader) } - if testCase.copySourceVersionID != "" { - req.Header.Set("X-Amz-Copy-Source-Version-Id", testCase.copySourceVersionID) - } if testCase.copyModifiedHeader != "" { req.Header.Set("X-Amz-Copy-Source-If-Modified-Since", testCase.copyModifiedHeader) } @@ -2259,9 +2211,6 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string, if testCase.copySourceHeader != "" { reqV2.Header.Set("X-Amz-Copy-Source", testCase.copySourceHeader) } - if testCase.copySourceVersionID != "" { - reqV2.Header.Set("X-Amz-Copy-Source-Version-Id", testCase.copySourceVersionID) - } if testCase.copyModifiedHeader != "" { reqV2.Header.Set("X-Amz-Copy-Source-If-Modified-Since", testCase.copyModifiedHeader) }