From a845e7f7983531df86fc801e6c3e75b7ddaf73c5 Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Thu, 12 Sep 2024 18:48:00 +0300 Subject: [PATCH] [#486] Fix PUT object with negative Content-Length (cherry picked from commit f187141ae56dd3a51f00eeb43f734e668f7b5a0c) Signed-off-by: Marina Biryukova --- api/handler/handlers_test.go | 6 +++++- api/handler/multipart_upload_test.go | 32 ++++++++++++++++++++++++++++ api/handler/put.go | 7 ++++-- api/handler/put_test.go | 4 ++++ api/layer/layer.go | 4 ++-- api/layer/multipart_upload.go | 2 +- api/layer/object.go | 22 ++++++++++++++----- api/layer/versioning_test.go | 6 +++++- 8 files changed, 71 insertions(+), 12 deletions(-) diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index eb06207..2a6b59f 100644 --- a/api/handler/handlers_test.go +++ b/api/handler/handlers_test.go @@ -416,7 +416,7 @@ func createTestObject(hc *handlerContext, bktInfo *data.BucketInfo, objName stri extObjInfo, err := hc.Layer().PutObject(hc.Context(), &layer.PutObjectParams{ BktInfo: bktInfo, Object: objName, - Size: uint64(len(content)), + Size: ptr(uint64(len(content))), Reader: bytes.NewReader(content), Header: header, Encryption: encryption, @@ -502,3 +502,7 @@ func readResponse(t *testing.T, w *httptest.ResponseRecorder, status int, model require.NoError(t, err) } } + +func ptr[T any](t T) *T { + return &t +} diff --git a/api/handler/multipart_upload_test.go b/api/handler/multipart_upload_test.go index aef7347..b0fba6d 100644 --- a/api/handler/multipart_upload_test.go +++ b/api/handler/multipart_upload_test.go @@ -2,6 +2,7 @@ package handler import ( "crypto/md5" + "crypto/rand" "crypto/tls" "encoding/base64" "encoding/hex" @@ -435,6 +436,37 @@ func TestUploadPartCheckContentSHA256(t *testing.T) { } } +func TestUploadPartWithNegativeContentLength(t *testing.T) { + hc := prepareHandlerContext(t) + + bktName, objName := "bucket-to-upload-part", "object-multipart" + createTestBucket(hc, bktName) + partSize := 5 * 1024 * 1024 + + multipartUpload := createMultipartUpload(hc, bktName, objName, map[string]string{}) + + partBody := make([]byte, partSize) + _, err := rand.Read(partBody) + require.NoError(hc.t, err) + + query := make(url.Values) + query.Set(uploadIDQuery, multipartUpload.UploadID) + query.Set(partNumberQuery, "1") + + w, r := prepareTestRequestWithQuery(hc, bktName, objName, query, partBody) + r.ContentLength = -1 + hc.Handler().UploadPartHandler(w, r) + assertStatus(hc.t, w, http.StatusOK) + + completeMultipartUpload(hc, bktName, objName, multipartUpload.UploadID, []string{w.Header().Get(api.ETag)}) + res, _ := getObject(hc, bktName, objName) + equalDataSlices(t, partBody, res) + + resp := getObjectAttributes(hc, bktName, objName, objectParts) + require.Len(t, resp.ObjectParts.Parts, 1) + require.Equal(t, partSize, resp.ObjectParts.Parts[0].Size) +} + func uploadPartCopy(hc *handlerContext, bktName, objName, uploadID string, num int, srcObj string, start, end int) *UploadPartCopyResponse { return uploadPartCopyBase(hc, bktName, objName, false, uploadID, num, srcObj, start, end) } diff --git a/api/handler/put.go b/api/handler/put.go index 4000f53..3759de0 100644 --- a/api/handler/put.go +++ b/api/handler/put.go @@ -247,13 +247,16 @@ func (h *handler) PutObjectHandler(w http.ResponseWriter, r *http.Request) { BktInfo: bktInfo, Object: reqInfo.ObjectName, Reader: body, - Size: size, Header: metadata, Encryption: encryptionParams, ContentMD5: r.Header.Get(api.ContentMD5), ContentSHA256Hash: r.Header.Get(api.AmzContentSha256), } + if size > 0 { + params.Size = &size + } + params.CopiesNumbers, err = h.pickCopiesNumbers(metadata, reqInfo.Namespace, bktInfo.LocationConstraint) if err != nil { h.logAndSendError(w, "invalid copies number", reqInfo, err) @@ -490,7 +493,7 @@ func (h *handler) PostObject(w http.ResponseWriter, r *http.Request) { BktInfo: bktInfo, Object: reqInfo.ObjectName, Reader: contentReader, - Size: size, + Size: &size, Header: metadata, } diff --git a/api/handler/put_test.go b/api/handler/put_test.go index e3d6c7e..7b92236 100644 --- a/api/handler/put_test.go +++ b/api/handler/put_test.go @@ -163,6 +163,10 @@ func TestPutObjectWithNegativeContentLength(t *testing.T) { tc.Handler().HeadObjectHandler(w, r) assertStatus(t, w, http.StatusOK) require.Equal(t, strconv.Itoa(len(content)), w.Header().Get(api.ContentLength)) + + result := listVersions(t, tc, bktName) + require.Len(t, result.Version, 1) + require.EqualValues(t, len(content), result.Version[0].Size) } func TestPutObjectWithStreamBodyError(t *testing.T) { diff --git a/api/layer/layer.go b/api/layer/layer.go index 85fb7db..b90ccb0 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -97,7 +97,7 @@ type ( PutObjectParams struct { BktInfo *data.BucketInfo Object string - Size uint64 + Size *uint64 Reader io.Reader Header map[string]string Lock *data.ObjectLock @@ -512,7 +512,7 @@ func (n *Layer) CopyObject(ctx context.Context, p *CopyObjectParams) (*data.Exte return n.PutObject(ctx, &PutObjectParams{ BktInfo: p.DstBktInfo, Object: p.DstObject, - Size: p.DstSize, + Size: &p.DstSize, Reader: objPayload, Header: p.Header, Encryption: p.DstEncryption, diff --git a/api/layer/multipart_upload.go b/api/layer/multipart_upload.go index aeb6759..7adfe10 100644 --- a/api/layer/multipart_upload.go +++ b/api/layer/multipart_upload.go @@ -453,7 +453,7 @@ func (n *Layer) CompleteMultipartUpload(ctx context.Context, p *CompleteMultipar Object: p.Info.Key, Reader: bytes.NewReader(partsData), Header: initMetadata, - Size: multipartObjetSize, + Size: &multipartObjetSize, Encryption: p.Info.Encryption, CopiesNumbers: multipartInfo.CopiesNumbers, CompleteMD5Hash: hex.EncodeToString(md5Hash.Sum(nil)) + "-" + strconv.Itoa(len(p.Parts)), diff --git a/api/layer/object.go b/api/layer/object.go index 1b89abf..51ff4cb 100644 --- a/api/layer/object.go +++ b/api/layer/object.go @@ -222,16 +222,20 @@ func (n *Layer) PutObject(ctx context.Context, p *PutObjectParams) (*data.Extend r := p.Reader if p.Encryption.Enabled() { - p.Header[AttributeDecryptedSize] = strconv.FormatUint(p.Size, 10) + var size uint64 + if p.Size != nil { + size = *p.Size + } + p.Header[AttributeDecryptedSize] = strconv.FormatUint(size, 10) if err = addEncryptionHeaders(p.Header, p.Encryption); err != nil { return nil, fmt.Errorf("add encryption header: %w", err) } var encSize uint64 - if r, encSize, err = encryptionReader(p.Reader, p.Size, p.Encryption.Key()); err != nil { + if r, encSize, err = encryptionReader(p.Reader, size, p.Encryption.Key()); err != nil { return nil, fmt.Errorf("create encrypter: %w", err) } - p.Size = encSize + p.Size = &encSize } if r != nil { @@ -250,12 +254,14 @@ func (n *Layer) PutObject(ctx context.Context, p *PutObjectParams) (*data.Extend prm := PrmObjectCreate{ Container: p.BktInfo.CID, - PayloadSize: p.Size, Filepath: p.Object, Payload: r, CreationTime: TimeNow(ctx), CopiesNumber: p.CopiesNumbers, } + if p.Size != nil { + prm.PayloadSize = *p.Size + } prm.Attributes = make([][2]string, 0, len(p.Header)) @@ -303,19 +309,25 @@ func (n *Layer) PutObject(ctx context.Context, p *PutObjectParams) (*data.Extend OID: id, ETag: hex.EncodeToString(hash), FilePath: p.Object, - Size: p.Size, Created: &now, Owner: &n.gateOwner, }, IsUnversioned: !bktSettings.VersioningEnabled(), IsCombined: p.Header[MultipartObjectSize] != "", } + if len(p.CompleteMD5Hash) > 0 { newVersion.MD5 = p.CompleteMD5Hash } else { newVersion.MD5 = hex.EncodeToString(md5Hash) } + if p.Size != nil { + newVersion.Size = *p.Size + } else { + newVersion.Size = size + } + if newVersion.ID, err = n.treeService.AddVersion(ctx, p.BktInfo, newVersion); err != nil { return nil, fmt.Errorf("couldn't add new verion to tree service: %w", err) } diff --git a/api/layer/versioning_test.go b/api/layer/versioning_test.go index d037c03..c22b311 100644 --- a/api/layer/versioning_test.go +++ b/api/layer/versioning_test.go @@ -23,7 +23,7 @@ func (tc *testContext) putObject(content []byte) *data.ObjectInfo { extObjInfo, err := tc.layer.PutObject(tc.ctx, &PutObjectParams{ BktInfo: tc.bktInfo, Object: tc.obj, - Size: uint64(len(content)), + Size: ptr(uint64(len(content))), Reader: bytes.NewReader(content), Header: make(map[string]string), }) @@ -444,3 +444,7 @@ func TestFilterVersionsByMarker(t *testing.T) { }) } } + +func ptr[T any](t T) *T { + return &t +}