From 72c680dd5eb89229be468bd7bd5968d13ce091a8 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 Signed-off-by: Marina Biryukova --- api/handler/handlers_test.go | 2 +- 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 | 2 +- 8 files changed, 63 insertions(+), 12 deletions(-) diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index b7ef210..7052617 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, diff --git a/api/handler/multipart_upload_test.go b/api/handler/multipart_upload_test.go index d1b4a93..4df0b10 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" @@ -621,6 +622,37 @@ func TestMultipartObjectLocation(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 59c1499..8a7a10f 100644 --- a/api/handler/put.go +++ b/api/handler/put.go @@ -248,13 +248,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) @@ -517,7 +520,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 7477f07..570dfb6 100644 --- a/api/handler/put_test.go +++ b/api/handler/put_test.go @@ -250,6 +250,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 57f804d..da3ec6e 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -103,7 +103,7 @@ type ( PutObjectParams struct { BktInfo *data.BucketInfo Object string - Size uint64 + Size *uint64 Reader io.Reader Header map[string]string Lock *data.ObjectLock @@ -528,7 +528,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 c99173d..bc6261d 100644 --- a/api/layer/multipart_upload.go +++ b/api/layer/multipart_upload.go @@ -460,7 +460,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 0a2b6de..fa8b2a9 100644 --- a/api/layer/object.go +++ b/api/layer/object.go @@ -232,16 +232,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 { @@ -260,12 +264,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)) @@ -313,7 +319,6 @@ func (n *Layer) PutObject(ctx context.Context, p *PutObjectParams) (*data.Extend OID: createdObj.ID, ETag: hex.EncodeToString(createdObj.HashSum), FilePath: p.Object, - Size: p.Size, Created: &now, Owner: &n.gateOwner, CreationEpoch: createdObj.CreationEpoch, @@ -321,12 +326,19 @@ func (n *Layer) PutObject(ctx context.Context, p *PutObjectParams) (*data.Extend IsUnversioned: !bktSettings.VersioningEnabled(), IsCombined: p.Header[MultipartObjectSize] != "", } + if len(p.CompleteMD5Hash) > 0 { newVersion.MD5 = p.CompleteMD5Hash } else { newVersion.MD5 = hex.EncodeToString(createdObj.MD5Sum) } + if p.Size != nil { + newVersion.Size = *p.Size + } else { + newVersion.Size = createdObj.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..533a39b 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), })