From 96c7b79d1c65ef34c65a1c1d7beaee898312e2c1 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Mon, 29 Aug 2022 16:08:05 +0300 Subject: [PATCH] [#683] Forbid copy to itself in unversioned bucket Signed-off-by: Denis Kirillov --- api/handler/copy.go | 29 +++++++++++++++++++++++------ api/handler/copy_test.go | 28 ++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/api/handler/copy.go b/api/handler/copy.go index 50b11b8e6..bfe6eb8f0 100644 --- a/api/handler/copy.go +++ b/api/handler/copy.go @@ -84,6 +84,12 @@ func (h *handler) CopyObjectHandler(w http.ResponseWriter, r *http.Request) { return } + settings, err := h.obj.GetBucketSettings(r.Context(), dstBktInfo) + if err != nil { + h.logAndSendError(w, "could not get bucket settings", reqInfo, err) + return + } + if containsACL { if sessionTokenEACL, err = getSessionTokenSetEACL(r.Context()); err != nil { h.logAndSendError(w, "could not get eacl session token from a box", reqInfo, err) @@ -103,6 +109,11 @@ func (h *handler) CopyObjectHandler(w http.ResponseWriter, r *http.Request) { return } + if isCopyingToItselfForbidden(reqInfo, srcBucket, srcObject, settings, args) { + h.logAndSendError(w, "copying to itself without changing anything", reqInfo, errors.GetAPIError(errors.ErrInvalidCopyDest)) + return + } + if args.MetadataDirective == replaceDirective { metadata = parseMetadata(r) } @@ -169,12 +180,6 @@ func (h *handler) CopyObjectHandler(w http.ResponseWriter, r *http.Request) { CopiesNuber: copiesNumber, } - settings, err := h.obj.GetBucketSettings(r.Context(), dstBktInfo) - if err != nil { - h.logAndSendError(w, "could not get bucket settings", reqInfo, err) - return - } - params.Lock, err = formObjectLock(dstBktInfo, settings.LockConfiguration, r.Header) if err != nil { h.logAndSendError(w, "could not form object lock", reqInfo, err) @@ -241,6 +246,18 @@ func (h *handler) CopyObjectHandler(w http.ResponseWriter, r *http.Request) { } } +func isCopyingToItselfForbidden(reqInfo *api.ReqInfo, srcBucket string, srcObject string, settings *data.BucketSettings, args *copyObjectArgs) bool { + if reqInfo.BucketName != srcBucket || reqInfo.ObjectName != srcObject { + return false + } + + if !settings.Unversioned() { + return false + } + + return args.MetadataDirective != replaceDirective +} + func parseCopyObjectArgs(headers http.Header) (*copyObjectArgs, error) { var err error args := &conditionalArgs{ diff --git a/api/handler/copy_test.go b/api/handler/copy_test.go index 01b412a58..316d360fc 100644 --- a/api/handler/copy_test.go +++ b/api/handler/copy_test.go @@ -29,21 +29,41 @@ func TestCopyWithTaggingDirective(t *testing.T) { copyMeta := CopyMeta{ Tags: map[string]string{"key2": "val"}, } - copyObject(t, tc, bktName, objName, objToCopy, copyMeta) + copyObject(t, tc, bktName, objName, objToCopy, copyMeta, http.StatusOK) tagging := getObjectTagging(t, tc, bktName, objToCopy, emptyVersion) require.Len(t, tagging.TagSet, 1) require.Equal(t, "key", tagging.TagSet[0].Key) require.Equal(t, "val", tagging.TagSet[0].Value) copyMeta.TaggingDirective = replaceDirective - copyObject(t, tc, bktName, objName, objToCopy2, copyMeta) + copyObject(t, tc, bktName, objName, objToCopy2, copyMeta, http.StatusOK) tagging = getObjectTagging(t, tc, bktName, objToCopy2, emptyVersion) require.Len(t, tagging.TagSet, 1) require.Equal(t, "key2", tagging.TagSet[0].Key) require.Equal(t, "val", tagging.TagSet[0].Value) } -func copyObject(t *testing.T, tc *handlerContext, bktName, fromObject, toObject string, copyMeta CopyMeta) { +func TestCopyToItself(t *testing.T) { + tc := prepareHandlerContext(t) + + bktName, objName := "bucket-for-copy", "object-for-copy" + createBucketAndObject(t, tc, bktName, objName) + + copyMeta := CopyMeta{MetadataDirective: replaceDirective} + + copyObject(t, tc, bktName, objName, objName, CopyMeta{}, http.StatusBadRequest) + copyObject(t, tc, bktName, objName, objName, copyMeta, http.StatusOK) + + putBucketVersioning(t, tc, bktName, true) + copyObject(t, tc, bktName, objName, objName, CopyMeta{}, http.StatusOK) + copyObject(t, tc, bktName, objName, objName, copyMeta, http.StatusOK) + + putBucketVersioning(t, tc, bktName, false) + copyObject(t, tc, bktName, objName, objName, CopyMeta{}, http.StatusOK) + copyObject(t, tc, bktName, objName, objName, copyMeta, http.StatusOK) +} + +func copyObject(t *testing.T, tc *handlerContext, bktName, fromObject, toObject string, copyMeta CopyMeta, statusCode int) { w, r := prepareTestRequest(t, bktName, toObject, nil) r.Header.Set(api.AmzCopySource, bktName+"/"+fromObject) @@ -60,7 +80,7 @@ func copyObject(t *testing.T, tc *handlerContext, bktName, fromObject, toObject r.Header.Set(api.AmzTagging, tagsQuery.Encode()) tc.Handler().CopyObjectHandler(w, r) - assertStatus(t, w, http.StatusOK) + assertStatus(t, w, statusCode) } func putObjectTagging(t *testing.T, tc *handlerContext, bktName, objName string, tags map[string]string) {