diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 65919cbe5..c52b5bcef 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -653,12 +653,8 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re // TODO: Reject requests where body/payload is present, for now we don't even read it. - // Copy source path. - cpSrcPath, err := url.QueryUnescape(r.Header.Get("X-Amz-Copy-Source")) - if err != nil { - // Save unescaped string as is. - cpSrcPath = r.Header.Get("X-Amz-Copy-Source") - } + // Read escaped copy source path to check for parameters. + cpSrcPath := r.Header.Get("X-Amz-Copy-Source") // Check https://docs.aws.amazon.com/AmazonS3/latest/dev/ObjectVersioning.html // Regardless of whether you have enabled versioning, each object in your bucket @@ -673,8 +669,18 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re writeErrorResponse(w, ErrNoSuchVersion, r.URL, guessIsBrowserReq(r)) return } + // Note that url.Parse does the unescaping cpSrcPath = u.Path } + if vid := r.Header.Get("X-Amz-Copy-Source-Version-Id"); vid != "" { + // Check if versionId header was added, if yes then check if + // its non "null" value, we should error out since we do not support + // any versions other than "null". + if vid != "null" { + writeErrorResponse(w, ErrNoSuchVersion, r.URL, guessIsBrowserReq(r)) + return + } + } srcBucket, srcObject := path2BucketAndObject(cpSrcPath) // If source object is empty or bucket is empty, reply back invalid copy source. @@ -699,7 +705,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re } var srcOpts, dstOpts ObjectOptions - srcOpts, err = copySrcEncryptionOpts(ctx, r, srcBucket, srcObject) + srcOpts, err := copySrcEncryptionOpts(ctx, r, srcBucket, srcObject) if err != nil { logger.LogIf(ctx, err) writeErrorResponse(w, toAPIErrorCode(ctx, err), r.URL, guessIsBrowserReq(r)) @@ -1466,12 +1472,8 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt return } - // Copy source path. - cpSrcPath, err := url.QueryUnescape(r.Header.Get("X-Amz-Copy-Source")) - if err != nil { - // Save unescaped string as is. - cpSrcPath = r.Header.Get("X-Amz-Copy-Source") - } + // Read escaped copy source path to check for parameters. + cpSrcPath := r.Header.Get("X-Amz-Copy-Source") // Check https://docs.aws.amazon.com/AmazonS3/latest/dev/ObjectVersioning.html // Regardless of whether you have enabled versioning, each object in your bucket @@ -1486,8 +1488,18 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt writeErrorResponse(w, ErrNoSuchVersion, r.URL, guessIsBrowserReq(r)) return } + // Note that url.Parse does the unescaping cpSrcPath = u.Path } + if vid := r.Header.Get("X-Amz-Copy-Source-Version-Id"); vid != "" { + // Check if X-Amz-Copy-Source-Version-Id header was added, if yes then check if + // its non "null" value, we should error out since we do not support + // any versions other than "null". + if vid != "null" { + writeErrorResponse(w, ErrNoSuchVersion, r.URL, guessIsBrowserReq(r)) + return + } + } srcBucket, srcObject := path2BucketAndObject(cpSrcPath) // If source object is empty or bucket is empty, reply back invalid copy source. diff --git a/cmd/object-handlers_test.go b/cmd/object-handlers_test.go index c2cbcffc2..85443b384 100644 --- a/cmd/object-handlers_test.go +++ b/cmd/object-handlers_test.go @@ -25,6 +25,7 @@ import ( "encoding/xml" "fmt" "io" + "runtime" "strings" "io/ioutil" @@ -1523,14 +1524,15 @@ 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. - 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. + 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 // expected output. expectedRespStatus int }{ @@ -1694,6 +1696,44 @@ func testAPICopyObjectPartHandler(obj ObjectLayer, instanceType, bucketName stri secretKey: credentials.SecretKey, expectedRespStatus: http.StatusOK, }, + // Test case - 14, copy part 1 from from newObject1 with null versionId + { + bucketName: bucketName, + uploadID: uploadID, + copySourceHeader: url.QueryEscape("/"+bucketName+"/"+objectName) + "?versionId=null", + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + expectedRespStatus: http.StatusOK, + }, + // Test case - 15, copy part 1 from from newObject1 with non null versionId + { + bucketName: bucketName, + uploadID: uploadID, + copySourceHeader: url.QueryEscape("/"+bucketName+"/"+objectName) + "?versionId=17", + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + expectedRespStatus: http.StatusNotFound, + }, + // Test case - 16, copy part 1 from from newObject1 with null X-Amz-Copy-Source-Version-Id + { + bucketName: bucketName, + uploadID: uploadID, + copySourceHeader: url.QueryEscape("/" + bucketName + "/" + 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("/" + bucketName + "/" + objectName), + copySourceVersionId: "17", + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + expectedRespStatus: http.StatusNotFound, + }, } for i, testCase := range testCases { @@ -1717,6 +1757,9 @@ 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) } @@ -1752,6 +1795,9 @@ func testAPICopyObjectPartHandler(obj ObjectLayer, instanceType, bucketName stri 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.copySourceRange != "" { reqV2.Header.Set("X-Amz-Copy-Source-Range", testCase.copySourceRange) } @@ -1802,7 +1848,10 @@ func TestAPICopyObjectHandler(t *testing.T) { func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, credentials auth.Credentials, t *testing.T) { - objectName := "test-object" + objectName := "test?object" // use file with ? to test URL parsing... + if runtime.GOOS == "windows" { + objectName = "test-object" // ...except on Windows + } // object used for anonymous HTTP request test. anonObject := "anon-object" var err error @@ -1861,6 +1910,7 @@ 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 @@ -2071,6 +2121,44 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string, secretKey: credentials.SecretKey, expectedRespStatus: http.StatusOK, }, + // Test case - 17, copy metadata from newObject1 with null versionId + { + bucketName: bucketName, + newObjectName: "newObject1", + copySourceHeader: url.QueryEscape("/"+bucketName+"/"+objectName) + "?versionId=null", + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + expectedRespStatus: http.StatusOK, + }, + // Test case - 18, copy metadata from newObject1 with non null versionId + { + bucketName: bucketName, + newObjectName: "newObject1", + copySourceHeader: url.QueryEscape("/"+bucketName+"/"+objectName) + "?versionId=17", + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + expectedRespStatus: http.StatusNotFound, + }, + // Test case - 19, copy metadata from newObject1 with null X-Amz-Copy-Source-Version-Id + { + bucketName: bucketName, + newObjectName: "newObject1", + copySourceHeader: url.QueryEscape("/" + bucketName + "/" + 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("/" + bucketName + "/" + objectName), + copySourceVersionId: "17", + accessKey: credentials.AccessKey, + secretKey: credentials.SecretKey, + expectedRespStatus: http.StatusNotFound, + }, } for i, testCase := range testCases { @@ -2089,6 +2177,9 @@ 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) } @@ -2150,6 +2241,9 @@ 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) }