From 0c71ce3398ec3dca81ce3f7bb829a3f0ed5967c4 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 24 May 2020 11:19:17 -0700 Subject: [PATCH] fix size accounting for encrypted/compressed objects (#9690) size calculation in crawler was using the real size of the object instead of its actual size i.e either a decrypted or uncompressed size. this is needed to make sure all other accounting such as bucket quota and mcs UI to display the correct values. --- cmd/api-headers.go | 17 +----- cmd/bucket-listobjects-handlers.go | 90 +++++++++--------------------- cmd/notification.go | 14 +---- cmd/object-api-utils.go | 28 ++++++---- cmd/object-api-utils_test.go | 2 +- cmd/object-handlers.go | 7 +-- cmd/posix.go | 10 +++- cmd/web-handlers.go | 22 +++----- go.mod | 2 +- 9 files changed, 68 insertions(+), 124 deletions(-) diff --git a/cmd/api-headers.go b/cmd/api-headers.go index 59485addd..518289b1a 100644 --- a/cmd/api-headers.go +++ b/cmd/api-headers.go @@ -120,20 +120,9 @@ func setObjectHeaders(w http.ResponseWriter, objInfo ObjectInfo, rs *HTTPRangeSp w.Header().Set(k, v) } - var totalObjectSize int64 - switch { - case crypto.IsEncrypted(objInfo.UserDefined): - totalObjectSize, err = objInfo.DecryptedSize() - if err != nil { - return err - } - case objInfo.IsCompressed(): - totalObjectSize = objInfo.GetActualSize() - if totalObjectSize < 0 { - return errInvalidDecompressedSize - } - default: - totalObjectSize = objInfo.Size + totalObjectSize, err := objInfo.GetActualSize() + if err != nil { + return err } // for providing ranged content diff --git a/cmd/bucket-listobjects-handlers.go b/cmd/bucket-listobjects-handlers.go index 8724fea94..ad03b1fd5 100644 --- a/cmd/bucket-listobjects-handlers.go +++ b/cmd/bucket-listobjects-handlers.go @@ -99,24 +99,13 @@ func (api objectAPIHandlers) ListBucketObjectVersionsHandler(w http.ResponseWrit } for i := range listObjectsInfo.Objects { - var actualSize int64 - if listObjectsInfo.Objects[i].IsCompressed() { - // Read the decompressed size from the meta.json. - actualSize = listObjectsInfo.Objects[i].GetActualSize() - if actualSize < 0 { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidDecompressedSize), - r.URL, guessIsBrowserReq(r)) - return - } - // Set the info.Size to the actualSize. - listObjectsInfo.Objects[i].Size = actualSize - } else if crypto.IsEncrypted(listObjectsInfo.Objects[i].UserDefined) { + if crypto.IsEncrypted(listObjectsInfo.Objects[i].UserDefined) { listObjectsInfo.Objects[i].ETag = getDecryptedETag(r.Header, listObjectsInfo.Objects[i], false) - listObjectsInfo.Objects[i].Size, err = listObjectsInfo.Objects[i].DecryptedSize() - if err != nil { - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) - return - } + } + listObjectsInfo.Objects[i].Size, err = listObjectsInfo.Objects[i].GetActualSize() + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return } } @@ -181,23 +170,13 @@ func (api objectAPIHandlers) ListObjectsV2MHandler(w http.ResponseWriter, r *htt } for i := range listObjectsV2Info.Objects { - var actualSize int64 - if listObjectsV2Info.Objects[i].IsCompressed() { - // Read the decompressed size from the meta.json. - actualSize = listObjectsV2Info.Objects[i].GetActualSize() - if actualSize < 0 { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidDecompressedSize), r.URL, guessIsBrowserReq(r)) - return - } - // Set the info.Size to the actualSize. - listObjectsV2Info.Objects[i].Size = actualSize - } else if crypto.IsEncrypted(listObjectsV2Info.Objects[i].UserDefined) { + if crypto.IsEncrypted(listObjectsV2Info.Objects[i].UserDefined) { listObjectsV2Info.Objects[i].ETag = getDecryptedETag(r.Header, listObjectsV2Info.Objects[i], false) - listObjectsV2Info.Objects[i].Size, err = listObjectsV2Info.Objects[i].DecryptedSize() - if err != nil { - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) - return - } + } + listObjectsV2Info.Objects[i].Size, err = listObjectsV2Info.Objects[i].GetActualSize() + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return } } @@ -265,23 +244,13 @@ func (api objectAPIHandlers) ListObjectsV2Handler(w http.ResponseWriter, r *http } for i := range listObjectsV2Info.Objects { - var actualSize int64 - if listObjectsV2Info.Objects[i].IsCompressed() { - // Read the decompressed size from the meta.json. - actualSize = listObjectsV2Info.Objects[i].GetActualSize() - if actualSize < 0 { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidDecompressedSize), r.URL, guessIsBrowserReq(r)) - return - } - // Set the info.Size to the actualSize. - listObjectsV2Info.Objects[i].Size = actualSize - } else if crypto.IsEncrypted(listObjectsV2Info.Objects[i].UserDefined) { + if crypto.IsEncrypted(listObjectsV2Info.Objects[i].UserDefined) { listObjectsV2Info.Objects[i].ETag = getDecryptedETag(r.Header, listObjectsV2Info.Objects[i], false) - listObjectsV2Info.Objects[i].Size, err = listObjectsV2Info.Objects[i].DecryptedSize() - if err != nil { - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) - return - } + } + listObjectsV2Info.Objects[i].Size, err = listObjectsV2Info.Objects[i].GetActualSize() + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return } } @@ -344,25 +313,16 @@ func (api objectAPIHandlers) ListObjectsV1Handler(w http.ResponseWriter, r *http } for i := range listObjectsInfo.Objects { - var actualSize int64 - if listObjectsInfo.Objects[i].IsCompressed() { - // Read the decompressed size from the meta.json. - actualSize = listObjectsInfo.Objects[i].GetActualSize() - if actualSize < 0 { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidDecompressedSize), r.URL, guessIsBrowserReq(r)) - return - } - // Set the info.Size to the actualSize. - listObjectsInfo.Objects[i].Size = actualSize - } else if crypto.IsEncrypted(listObjectsInfo.Objects[i].UserDefined) { + if crypto.IsEncrypted(listObjectsInfo.Objects[i].UserDefined) { listObjectsInfo.Objects[i].ETag = getDecryptedETag(r.Header, listObjectsInfo.Objects[i], false) - listObjectsInfo.Objects[i].Size, err = listObjectsInfo.Objects[i].DecryptedSize() - if err != nil { - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) - return - } + } + listObjectsInfo.Objects[i].Size, err = listObjectsInfo.Objects[i].GetActualSize() + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return } } + response := generateListObjectsV1Response(bucket, prefix, marker, delimiter, encodingType, maxKeys, listObjectsInfo) // Write success response. diff --git a/cmd/notification.go b/cmd/notification.go index d1b91a965..66fae45b5 100644 --- a/cmd/notification.go +++ b/cmd/notification.go @@ -1260,9 +1260,6 @@ func (args eventArgs) ToEvent(escape bool) event.Event { if args.EventName != event.ObjectRemovedDelete { newEvent.S3.Object.ETag = args.Object.ETag newEvent.S3.Object.Size = args.Object.Size - if args.Object.IsCompressed() { - newEvent.S3.Object.Size = args.Object.GetActualSize() - } newEvent.S3.Object.ContentType = args.Object.ContentType newEvent.S3.Object.UserMetadata = args.Object.UserDefined } @@ -1271,16 +1268,9 @@ func (args eventArgs) ToEvent(escape bool) event.Event { } func sendEvent(args eventArgs) { - // remove sensitive encryption entries in metadata. - switch { - case crypto.IsEncrypted(args.Object.UserDefined): - if totalObjectSize, err := args.Object.DecryptedSize(); err == nil { - args.Object.Size = totalObjectSize - } - case args.Object.IsCompressed(): - args.Object.Size = args.Object.GetActualSize() - } + args.Object.Size, _ = args.Object.GetActualSize() + // remove sensitive encryption entries in metadata. crypto.RemoveSensitiveEntries(args.Object.UserDefined) crypto.RemoveInternalEntries(args.Object.UserDefined) diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go index db3b845cd..90a1f9a84 100644 --- a/cmd/object-api-utils.go +++ b/cmd/object-api-utils.go @@ -372,17 +372,23 @@ func (o ObjectInfo) IsCompressedOK() (bool, error) { return true, fmt.Errorf("unknown compression scheme: %s", scheme) } -// GetActualSize - read the decompressed size from the meta json. -func (o ObjectInfo) GetActualSize() int64 { - metadata := o.UserDefined - sizeStr, ok := metadata[ReservedMetadataPrefix+"actual-size"] - if ok { +// GetActualSize - returns the actual size of the stored object +func (o ObjectInfo) GetActualSize() (int64, error) { + if crypto.IsEncrypted(o.UserDefined) { + return o.DecryptedSize() + } + if o.IsCompressed() { + sizeStr, ok := o.UserDefined[ReservedMetadataPrefix+"actual-size"] + if !ok { + return -1, errInvalidDecompressedSize + } size, err := strconv.ParseInt(sizeStr, 10, 64) - if err == nil { - return size + if err != nil { + return -1, errInvalidDecompressedSize } + return size, nil } - return -1 + return o.Size, nil } // Disabling compression for encrypted enabled requests. @@ -608,9 +614,9 @@ func NewGetObjectReader(rs *HTTPRangeSpec, oi ObjectInfo, opts ObjectOptions, cl } case isCompressed: // Read the decompressed size from the meta.json. - actualSize := oi.GetActualSize() - if actualSize < 0 { - return nil, 0, 0, errInvalidDecompressedSize + actualSize, err := oi.GetActualSize() + if err != nil { + return nil, 0, 0, err } off, length = int64(0), oi.Size decOff, decLength := int64(0), actualSize diff --git a/cmd/object-api-utils_test.go b/cmd/object-api-utils_test.go index 3ca33ce11..9a98e4bfe 100644 --- a/cmd/object-api-utils_test.go +++ b/cmd/object-api-utils_test.go @@ -521,7 +521,7 @@ func TestGetActualSize(t *testing.T) { }, } for i, test := range testCases { - got := test.objInfo.GetActualSize() + got, _ := test.objInfo.GetActualSize() if got != test.result { t.Errorf("Test %d - expected %d but received %d", i+1, test.result, got) diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index eb9659dc6..95e8d8412 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1188,10 +1188,6 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re // Write success response. writeSuccessResponseXML(w, encodedSuccessResponse) - if objInfo.IsCompressed() { - objInfo.Size = actualSize - } - // Notify object created event. sendEvent(eventArgs{ EventName: event.ObjectCreatedCopy, @@ -1508,6 +1504,9 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req writeSuccessResponseHeadersOnly(w) + // Set the etag sent to the client as part of the event. + objInfo.ETag = etag + // Notify object created event. sendEvent(eventArgs{ EventName: event.ObjectCreatedPut, diff --git a/cmd/posix.go b/cmd/posix.go index d4454c5de..8c017b748 100644 --- a/cmd/posix.go +++ b/cmd/posix.go @@ -383,7 +383,15 @@ func (s *posix) CrawlAndGetDataUsage(ctx context.Context, cache dataUsageCache) return 0, nil } - return meta.Stat.Size, nil + // we don't necessarily care about the names + // of bucket and object, only interested in size. + // so use some dummy names. + size, err := meta.ToObjectInfo("dummy", "dummy").GetActualSize() + if err != nil { + return 0, errSkipFile + } + return size, nil + }) if err != nil { return dataUsageInfo, err diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index ddabe2954..e30c1ed6d 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -529,17 +529,9 @@ func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, r return &json2.Error{Message: err.Error()} } for i := range lo.Objects { - if crypto.IsEncrypted(lo.Objects[i].UserDefined) { - lo.Objects[i].Size, err = lo.Objects[i].DecryptedSize() - if err != nil { - return toJSONError(ctx, err) - } - } else if lo.Objects[i].IsCompressed() { - actualSize := lo.Objects[i].GetActualSize() - if actualSize < 0 { - return toJSONError(ctx, errInvalidDecompressedSize) - } - lo.Objects[i].Size = actualSize + lo.Objects[i].Size, err = lo.Objects[i].GetActualSize() + if err != nil { + return toJSONError(ctx, err) } } @@ -1505,10 +1497,10 @@ func (web *webAPIHandlers) DownloadZip(w http.ResponseWriter, r *http.Request) { info := gr.ObjInfo // filter object lock metadata if permission does not permit info.UserDefined = objectlock.FilterObjectLockMetadata(info.UserDefined, getRetPerms[i] != ErrNone, legalHoldPerms[i] != ErrNone) - - if info.IsCompressed() { - // For reporting, set the file size to the uncompressed size. - info.Size = info.GetActualSize() + // For reporting, set the file size to the uncompressed size. + info.Size, err = info.GetActualSize() + if err != nil { + return err } header := &zip.FileHeader{ Name: strings.TrimPrefix(objectName, args.Prefix), diff --git a/go.mod b/go.mod index bb21b2d3f..4a62c0cd6 100644 --- a/go.mod +++ b/go.mod @@ -106,7 +106,7 @@ require ( github.com/stretchr/testify v1.5.1 // indirect github.com/tinylib/msgp v1.1.1 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 // indirect - github.com/ugorji/go/codec v1.1.5-pre // indirect + github.com/ugorji/go v1.1.5-pre // indirect github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a github.com/willf/bitset v1.1.10 // indirect github.com/willf/bloom v2.0.3+incompatible