CLDSRV-909: Reject CopyObject when source exceeds 5 GiB#6177
Draft
tcarmet wants to merge 3 commits into
Draft
Conversation
Contributor
Hello tcarmet,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Contributor
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
|
LGTM |
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 2 files with indirect coverage changes @@ Coverage Diff @@
## development/9.4 #6177 +/- ##
===================================================
+ Coverage 85.25% 85.30% +0.04%
===================================================
Files 208 208
Lines 13919 13925 +6
===================================================
+ Hits 11867 11879 +12
+ Misses 2052 2046 -6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
tcarmet
force-pushed
the
bugfix/CLDSRV-909-enforce-copyobject-source-size-limit
branch
from
0094687 to
be2daf0
Compare
|
LGTM |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| after(() => { | ||
| constants.maximumAllowedUploadSize = originalMaximumUploadSize; | ||
| cleanup(); | ||
| }); |
There was a problem hiding this comment.
config.bypassMaxPutObjectSize is set to true but only reset inside the callback. If the assertion throws, the flag leaks into subsequent test suites sharing this config singleton. Reset it in the after block alongside maximumAllowedUploadSize.
Suggested change
| }); | |
| after(() => { | |
| constants.maximumAllowedUploadSize = originalMaximumUploadSize; | |
| config.bypassMaxPutObjectSize = false; | |
| cleanup(); | |
| }); |
— Claude Code
Review by Claude Code |
Review by Claude Code |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+309
to
-359
| ); | ||
| }, | ||
| function checkSourceAuthorization(destBucketMD, destObjMD, next) { | ||
| return standardMetadataValidateBucketAndObj( | ||
| { | ||
| ...valGetParams, | ||
| destObjMD, | ||
| serverAccessLogOptions: { copySource: true }, | ||
| }, | ||
| request.actionImplicitDenies, | ||
| log, | ||
| (err, sourceBucketMD, sourceObjMD) => { | ||
| if (err) { | ||
| log.debug('error validating get part of request', { error: err }); | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); | ||
| return next(err, null, destBucketMD); | ||
| } | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); | ||
| return next(err, destBucketMD); | ||
| } | ||
| const headerValResult = | ||
| validateHeaders(request.headers, | ||
| sourceObjMD['last-modified'], | ||
| sourceObjMD['content-md5']); | ||
| if (headerValResult.error) { | ||
| request.sourceServerAccessLog |
Comment on lines
+327
to
+482
| if (!sourceObjMD) { | ||
| const err = sourceVersionId ? errors.NoSuchVersion : errors.NoSuchKey; | ||
| log.debug('no source object', { sourceObject }); | ||
| // eslint-disable-next-line no-param-reassign | ||
| && (request.sourceServerAccessLog.error = errors.PreconditionFailed); | ||
| return next(errors.PreconditionFailed, destBucketMD); | ||
| } | ||
| const { storeMetadataParams, error: metadataError, | ||
| sourceLocationConstraintName, backendInfoDest } = | ||
| _prepMetadata(request, sourceObjMD, request.headers, | ||
| sourceIsDestination, authInfo, destObjectKey, | ||
| sourceBucketMD, destBucketMD, sourceVersionId, log); | ||
| if (metadataError) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = metadataError); | ||
| return next(metadataError, destBucketMD); | ||
| } | ||
| if (storeMetadataParams.metaHeaders) { | ||
| dataStoreContext.metaHeaders = | ||
| storeMetadataParams.metaHeaders; | ||
| } | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); | ||
| return next(err, null, destBucketMD); | ||
| } | ||
| // check if object data is in a cold storage | ||
| const coldErr = verifyColdObjectAvailable(sourceObjMD); | ||
| if (coldErr) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = coldErr); | ||
| return next(coldErr, null); | ||
| } | ||
| if (sourceObjMD.isDeleteMarker) { | ||
| log.debug('delete marker on source object', { sourceObject }); | ||
| let err; | ||
| if (sourceVersionId) { | ||
| err = errorInstances.InvalidRequest.customizeDescription( | ||
| 'The source of a copy ' + | ||
| 'request may not specifically refer to a delete' + | ||
| 'marker by version id.', | ||
| ); | ||
| } else { | ||
| // if user specifies a key in a versioned source bucket | ||
| // without specifying a version, and the object has | ||
| // a delete marker, return NoSuchKey | ||
| err = errors.NoSuchKey; | ||
| } | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); | ||
| return next(err, destBucketMD); | ||
| } | ||
| const sourceSize = parseInt(sourceObjMD['content-length'], 10); | ||
| if (sourceSize > constants.maximumAllowedUploadSize && !config.bypassMaxPutObjectSize) { | ||
| log.debug('copy source object too large', { sourceSize }); | ||
| const err = errorInstances.InvalidRequest.customizeDescription( | ||
| 'The specified copy source is larger than the maximum ' + | ||
| `allowable size for a copy source: ${constants.maximumAllowedUploadSize}`, | ||
| ); | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); | ||
| return next(err, destBucketMD); | ||
| } | ||
| const headerValResult = validateHeaders( | ||
| request.headers, | ||
| sourceObjMD['last-modified'], | ||
| sourceObjMD['content-md5'], | ||
| ); | ||
| if (headerValResult.error) { | ||
| request.sourceServerAccessLog && | ||
| // eslint-disable-next-line no-param-reassign | ||
| (request.sourceServerAccessLog.error = errors.PreconditionFailed); | ||
| return next(errors.PreconditionFailed, destBucketMD); | ||
| } | ||
| const { | ||
| storeMetadataParams, | ||
| error: metadataError, | ||
| sourceLocationConstraintName, | ||
| backendInfoDest, | ||
| } = _prepMetadata( | ||
| request, | ||
| sourceObjMD, | ||
| request.headers, | ||
| sourceIsDestination, | ||
| authInfo, | ||
| destObjectKey, | ||
| sourceBucketMD, | ||
| destBucketMD, | ||
| sourceVersionId, | ||
| log, | ||
| ); | ||
| if (metadataError) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = metadataError); | ||
| return next(metadataError, destBucketMD); | ||
| } | ||
| if (storeMetadataParams.metaHeaders) { | ||
| dataStoreContext.metaHeaders = storeMetadataParams.metaHeaders; | ||
| } | ||
|
|
||
| storeMetadataParams.overheadField = constants.overheadField; | ||
|
|
||
| let dataLocator; | ||
| // If 0 byte object just set dataLocator to empty array | ||
| if (!sourceObjMD.location) { | ||
| dataLocator = []; | ||
| } else { | ||
| // To provide for backwards compatibility before | ||
| // md-model-version 2, need to handle cases where | ||
| // objMD.location is just a string | ||
| dataLocator = Array.isArray(sourceObjMD.location) ? | ||
| sourceObjMD.location : [{ key: sourceObjMD.location }]; | ||
| } | ||
| storeMetadataParams.overheadField = constants.overheadField; | ||
|
|
||
| if (sourceObjMD['x-amz-server-side-encryption']) { | ||
| for (let i = 0; i < dataLocator.length; i++) { | ||
| dataLocator[i].masterKeyId = sourceObjMD[ | ||
| 'x-amz-server-side-encryption-aws-kms-key-id']; | ||
| dataLocator[i].algorithm = | ||
| sourceObjMD['x-amz-server-side-encryption']; | ||
| let dataLocator; | ||
| // If 0 byte object just set dataLocator to empty array | ||
| if (!sourceObjMD.location) { | ||
| dataLocator = []; | ||
| } else { | ||
| // To provide for backwards compatibility before | ||
| // md-model-version 2, need to handle cases where | ||
| // objMD.location is just a string | ||
| dataLocator = Array.isArray(sourceObjMD.location) | ||
| ? sourceObjMD.location | ||
| : [{ key: sourceObjMD.location }]; | ||
| } | ||
|
|
||
| if (sourceObjMD['x-amz-server-side-encryption']) { | ||
| for (let i = 0; i < dataLocator.length; i++) { | ||
| dataLocator[i].masterKeyId = sourceObjMD['x-amz-server-side-encryption-aws-kms-key-id']; | ||
| dataLocator[i].algorithm = sourceObjMD['x-amz-server-side-encryption']; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If the destination key already exists | ||
| if (destObjMD) { | ||
| // Re-use creation-time if we can | ||
| if (destObjMD['creation-time']) { | ||
| storeMetadataParams.creationTime = | ||
| destObjMD['creation-time']; | ||
| // Otherwise fallback to last-modified | ||
| // If the destination key already exists | ||
| if (destObjMD) { | ||
| // Re-use creation-time if we can | ||
| if (destObjMD['creation-time']) { | ||
| storeMetadataParams.creationTime = destObjMD['creation-time']; | ||
| // Otherwise fallback to last-modified | ||
| } else { | ||
| storeMetadataParams.creationTime = destObjMD['last-modified']; | ||
| } | ||
| // If this is a new key, create a new timestamp | ||
| } else { | ||
| storeMetadataParams.creationTime = | ||
| destObjMD['last-modified']; | ||
| storeMetadataParams.creationTime = new Date().toJSON(); | ||
| } | ||
| // If this is a new key, create a new timestamp | ||
| } else { | ||
| storeMetadataParams.creationTime = new Date().toJSON(); | ||
| } | ||
|
|
||
| return next(null, storeMetadataParams, dataLocator, | ||
| sourceBucketMD, destBucketMD, destObjMD, | ||
| sourceLocationConstraintName, backendInfoDest); | ||
| }); | ||
| }, | ||
| function getSSEConfiguration(storeMetadataParams, dataLocator, sourceBucketMD, | ||
| destBucketMD, destObjMD, sourceLocationConstraintName, | ||
| backendInfoDest, next) { | ||
| getObjectSSEConfiguration( | ||
| request.headers, | ||
| destBucketMD, | ||
| log, | ||
| (err, sseConfig) => | ||
| next(err, storeMetadataParams, dataLocator, sourceBucketMD, | ||
| destBucketMD, destObjMD, sourceLocationConstraintName, | ||
| backendInfoDest, sseConfig)); | ||
| }, | ||
| function goGetData(storeMetadataParams, dataLocator, sourceBucketMD, | ||
| destBucketMD, destObjMD, sourceLocationConstraintName, | ||
| backendInfoDest, serverSideEncryption, next) { | ||
| const vcfg = destBucketMD.getVersioningConfiguration(); | ||
| const isVersionedObj = vcfg && vcfg.Status === 'Enabled'; | ||
| const destLocationConstraintName = | ||
| storeMetadataParams.dataStoreName; | ||
| const needsEncryption = serverSideEncryption && !!serverSideEncryption.algo; | ||
| // skip if source and dest and location constraint the same and | ||
| // versioning is not enabled | ||
| // still send along serverSideEncryption info so algo | ||
| // and masterKeyId stored properly in metadata | ||
| if (sourceIsDestination && storeMetadataParams.locationMatch | ||
| && !isVersionedObj && !needsEncryption) { | ||
| return next(null, storeMetadataParams, dataLocator, destObjMD, | ||
| serverSideEncryption, destBucketMD); | ||
| } | ||
| return next( | ||
| null, | ||
| storeMetadataParams, | ||
| dataLocator, | ||
| sourceBucketMD, | ||
| destBucketMD, | ||
| destObjMD, | ||
| sourceLocationConstraintName, | ||
| backendInfoDest, | ||
| ); | ||
| }, | ||
| ); | ||
| }, | ||
| function getSSEConfiguration( | ||
| storeMetadataParams, | ||
| dataLocator, | ||
| sourceBucketMD, | ||
| destBucketMD, | ||
| destObjMD, | ||
| sourceLocationConstraintName, | ||
| backendInfoDest, | ||
| next, | ||
| ) { | ||
| getObjectSSEConfiguration(request.headers, destBucketMD, log, (err, sseConfig) => | ||
| next( | ||
| err, | ||
| storeMetadataParams, | ||
| dataLocator, | ||
| sourceBucketMD, | ||
| destBucketMD, | ||
| destObjMD, | ||
| sourceLocationConstraintName, | ||
| backendInfoDest, | ||
| sseConfig, | ||
| ), | ||
| ); | ||
| }, | ||
| function goGetData( | ||
| storeMetadataParams, |
Comment on lines
+483
to
-455
| dataLocator, | ||
| sourceBucketMD, | ||
| destBucketMD, | ||
| destObjMD, | ||
| sourceLocationConstraintName, | ||
| backendInfoDest, | ||
| serverSideEncryption, | ||
| next, | ||
| ) { | ||
| const vcfg = destBucketMD.getVersioningConfiguration(); | ||
| const isVersionedObj = vcfg && vcfg.Status === 'Enabled'; | ||
| const destLocationConstraintName = storeMetadataParams.dataStoreName; | ||
| const needsEncryption = serverSideEncryption && !!serverSideEncryption.algo; | ||
| // skip if source and dest and location constraint the same and | ||
| // versioning is not enabled | ||
| // still send along serverSideEncryption info so algo | ||
| // and masterKeyId stored properly in metadata | ||
| if (sourceIsDestination && storeMetadataParams.locationMatch && !isVersionedObj && !needsEncryption) { | ||
| return next(null, storeMetadataParams, dataLocator, destObjMD, serverSideEncryption, destBucketMD); | ||
| } | ||
|
|
||
| // also skip if 0 byte object, unless location constraint is an | ||
| // external backend and differs from source, in which case put | ||
| // metadata to backend |
Comment on lines
-456
to
-530
| let destLocationConstraintType; | ||
| if (config.backends.data === 'multiple') { | ||
| destLocationConstraintType = | ||
| config.getLocationConstraintType(destLocationConstraintName); | ||
| } | ||
| if (destLocationConstraintType && | ||
| versioningNotImplBackends[destLocationConstraintType] | ||
| && isVersionedObj) { | ||
| log.debug(externalVersioningErrorMessage, | ||
| { method: 'multipleBackendGateway', | ||
| error: errors.NotImplemented }); | ||
| return next(errorInstances.NotImplemented.customizeDescription( | ||
| externalVersioningErrorMessage), destBucketMD); | ||
| } | ||
| if (dataLocator.length === 0) { | ||
| if (!storeMetadataParams.locationMatch && | ||
| destLocationConstraintType && | ||
| constants.externalBackends[destLocationConstraintType]) { | ||
| return data.put(null, null, storeMetadataParams.size, | ||
| dataStoreContext, backendInfoDest, | ||
| log, (error, objectRetrievalInfo) => { | ||
| if (error) { | ||
| return next(error, destBucketMD); | ||
| } | ||
| const putResult = { | ||
| key: objectRetrievalInfo.key, | ||
| dataStoreName: objectRetrievalInfo. | ||
| dataStoreName, | ||
| dataStoreType: objectRetrievalInfo. | ||
| dataStoreType, | ||
| size: storeMetadataParams.size, | ||
| }; | ||
| const putResultArr = [putResult]; | ||
| return next(null, storeMetadataParams, putResultArr, | ||
| destObjMD, serverSideEncryption, destBucketMD); | ||
| }); | ||
| // also skip if 0 byte object, unless location constraint is an | ||
| // external backend and differs from source, in which case put | ||
| // metadata to backend | ||
| let destLocationConstraintType; | ||
| if (config.backends.data === 'multiple') { | ||
| destLocationConstraintType = config.getLocationConstraintType(destLocationConstraintName); | ||
| } | ||
| return next(null, storeMetadataParams, dataLocator, destObjMD, | ||
| serverSideEncryption, destBucketMD); | ||
| } | ||
| const originalIdentityImpDenies = request.actionImplicitDenies; | ||
| // eslint-disable-next-line no-param-reassign | ||
| delete request.actionImplicitDenies; | ||
| return data.copyObject(request, sourceLocationConstraintName, | ||
| storeMetadataParams, dataLocator, dataStoreContext, | ||
| backendInfoDest, sourceBucketMD, destBucketMD, serverSideEncryption, log, | ||
| (err, results) => { | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.actionImplicitDenies = originalIdentityImpDenies; | ||
| if (err) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); | ||
| return next(err, destBucketMD); | ||
| if ( | ||
| destLocationConstraintType && | ||
| versioningNotImplBackends[destLocationConstraintType] && | ||
| isVersionedObj | ||
| ) { | ||
| log.debug(externalVersioningErrorMessage, { | ||
| method: 'multipleBackendGateway', | ||
| error: errors.NotImplemented, | ||
| }); | ||
| return next( | ||
| errorInstances.NotImplemented.customizeDescription(externalVersioningErrorMessage), | ||
| destBucketMD, | ||
| ); | ||
| } | ||
| return next(null, storeMetadataParams, results, | ||
| destObjMD, serverSideEncryption, destBucketMD); | ||
| }); | ||
| }, | ||
| function getVersioningInfo(storeMetadataParams, destDataGetInfoArr, | ||
| destObjMD, serverSideEncryption, destBucketMD, next) { | ||
| if (!destBucketMD.isVersioningEnabled() && destObjMD?.archive?.archiveInfo) { | ||
| // Ensure we trigger a "delete" event in the oplog for the previously archived object | ||
| // eslint-disable-next-line | ||
| storeMetadataParams.needOplogUpdate = 's3:ReplaceArchivedObject'; | ||
| } | ||
| return versioningPreprocessing(destBucketName, | ||
| destBucketMD, destObjectKey, destObjMD, log, | ||
| (err, options) => { | ||
| if (err) { | ||
| log.debug('error processing versioning info', | ||
| { error: err }); | ||
| return next(err, null, destBucketMD); | ||
| if (dataLocator.length === 0) { | ||
| if ( | ||
| !storeMetadataParams.locationMatch && | ||
| destLocationConstraintType && | ||
| constants.externalBackends[destLocationConstraintType] | ||
| ) { | ||
| return data.put( | ||
| null, | ||
| null, | ||
| storeMetadataParams.size, | ||
| dataStoreContext, | ||
| backendInfoDest, | ||
| log, | ||
| (error, objectRetrievalInfo) => { | ||
| if (error) { | ||
| return next(error, destBucketMD); | ||
| } | ||
| const putResult = { | ||
| key: objectRetrievalInfo.key, | ||
| dataStoreName: objectRetrievalInfo.dataStoreName, | ||
| dataStoreType: objectRetrievalInfo.dataStoreType, | ||
| size: storeMetadataParams.size, | ||
| }; | ||
| const putResultArr = [putResult]; | ||
| return next( | ||
| null, | ||
| storeMetadataParams, | ||
| putResultArr, | ||
| destObjMD, | ||
| serverSideEncryption, | ||
| destBucketMD, | ||
| ); | ||
| }, | ||
| ); | ||
| } | ||
| return next(null, storeMetadataParams, dataLocator, destObjMD, serverSideEncryption, destBucketMD); | ||
| } | ||
| const originalIdentityImpDenies = request.actionImplicitDenies; | ||
| // eslint-disable-next-line no-param-reassign | ||
| delete request.actionImplicitDenies; | ||
| return data.copyObject( | ||
| request, | ||
| sourceLocationConstraintName, | ||
| storeMetadataParams, | ||
| dataLocator, | ||
| dataStoreContext, | ||
| backendInfoDest, | ||
| sourceBucketMD, | ||
| destBucketMD, | ||
| serverSideEncryption, | ||
| log, | ||
| (err, results) => { | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.actionImplicitDenies = originalIdentityImpDenies; | ||
| if (err) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); | ||
| return next(err, destBucketMD); | ||
| } | ||
| return next(null, storeMetadataParams, results, destObjMD, serverSideEncryption, destBucketMD); | ||
| }, | ||
| ); | ||
| }, | ||
| function getVersioningInfo( | ||
| storeMetadataParams, | ||
| destDataGetInfoArr, | ||
| destObjMD, | ||
| serverSideEncryption, | ||
| destBucketMD, | ||
| next, | ||
| ) { | ||
| if (!destBucketMD.isVersioningEnabled() && destObjMD?.archive?.archiveInfo) { | ||
| // Ensure we trigger a "delete" event in the oplog for the previously archived object | ||
| // eslint-disable-next-line | ||
| storeMetadataParams.needOplogUpdate = 's3:ReplaceArchivedObject'; | ||
| } | ||
| return versioningPreprocessing( | ||
| destBucketName, | ||
| destBucketMD, | ||
| destObjectKey, | ||
| destObjMD, | ||
| log, | ||
| (err, options) => { | ||
| if (err) { | ||
| log.debug('error processing versioning info', { error: err }); | ||
| return next(err, null, destBucketMD); | ||
| } | ||
|
|
||
| const location = destDataGetInfoArr?.[0]?.dataStoreName; |
Comment on lines
-531
to
-556
| if (location === destBucketMD.getLocationConstraint() && destBucketMD.isIngestionBucket()) { | ||
| // If the object is being written to the "ingested" storage location, keep the same | ||
| // versionId for consistency and to avoid creating an extra version when it gets | ||
| // ingested | ||
| const backendVersionId = decodeVID(destDataGetInfoArr[0].dataStoreVersionId); | ||
| if (!(backendVersionId instanceof Error)) { | ||
| options.versionId = backendVersionId; // eslint-disable-line no-param-reassign | ||
| const location = destDataGetInfoArr?.[0]?.dataStoreName; | ||
| if (location === destBucketMD.getLocationConstraint() && destBucketMD.isIngestionBucket()) { | ||
| // If the object is being written to the "ingested" storage location, keep the same | ||
| // versionId for consistency and to avoid creating an extra version when it gets | ||
| // ingested | ||
| const backendVersionId = decodeVID(destDataGetInfoArr[0].dataStoreVersionId); | ||
| if (!(backendVersionId instanceof Error)) { | ||
| options.versionId = backendVersionId; // eslint-disable-line no-param-reassign | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // eslint-disable-next-line | ||
| storeMetadataParams.versionId = options.versionId; | ||
| // eslint-disable-next-line | ||
| storeMetadataParams.versioning = options.versioning; | ||
| // eslint-disable-next-line | ||
| storeMetadataParams.isNull = options.isNull; | ||
| if (options.extraMD) { | ||
| Object.assign(storeMetadataParams, options.extraMD); | ||
| } | ||
| const dataToDelete = options.dataToDelete; | ||
| return next( | ||
| null, | ||
| storeMetadataParams, | ||
| destDataGetInfoArr, | ||
| destObjMD, | ||
| serverSideEncryption, | ||
| destBucketMD, | ||
| dataToDelete, | ||
| ); | ||
| }, | ||
| ); | ||
| }, | ||
| function storeNewMetadata( | ||
| storeMetadataParams, | ||
| destDataGetInfoArr, | ||
| destObjMD, | ||
| serverSideEncryption, | ||
| destBucketMD, | ||
| dataToDelete, | ||
| next, | ||
| ) { | ||
| if (destObjMD && destObjMD.uploadId) { | ||
| // eslint-disable-next-line | ||
| storeMetadataParams.versionId = options.versionId; | ||
| // eslint-disable-next-line | ||
| storeMetadataParams.versioning = options.versioning; | ||
| // eslint-disable-next-line | ||
| storeMetadataParams.isNull = options.isNull; | ||
| if (options.extraMD) { | ||
| Object.assign(storeMetadataParams, options.extraMD); | ||
| } | ||
| const dataToDelete = options.dataToDelete; | ||
| return next(null, storeMetadataParams, destDataGetInfoArr, | ||
| destObjMD, serverSideEncryption, destBucketMD, | ||
| dataToDelete); | ||
| }); | ||
| }, | ||
| function storeNewMetadata(storeMetadataParams, destDataGetInfoArr, |
Comment on lines
-557
to
-607
| destObjMD, serverSideEncryption, destBucketMD, dataToDelete, next) { | ||
| if (destObjMD && destObjMD.uploadId) { | ||
| // eslint-disable-next-line | ||
| storeMetadataParams.oldReplayId = destObjMD.uploadId; | ||
| } | ||
| storeMetadataParams.oldReplayId = destObjMD.uploadId; | ||
| } | ||
|
|
||
| return services.metadataStoreObject(destBucketName, | ||
| destDataGetInfoArr, serverSideEncryption, | ||
| storeMetadataParams, (err, result) => { | ||
| if (err) { | ||
| log.debug('error storing new metadata', { error: err }); | ||
| return next(err, null, destBucketMD); | ||
| } | ||
| const sourceObjSize = storeMetadataParams.size; | ||
| const destObjPrevSize = (destObjMD && | ||
| destObjMD['content-length'] !== undefined) ? | ||
| destObjMD['content-length'] : null; | ||
|
|
||
| setExpirationHeaders(responseHeaders, { | ||
| lifecycleConfig: destBucketMD.getLifecycleConfiguration(), | ||
| objectParams: { | ||
| key: destObjectKey, | ||
| date: result.lastModified, | ||
| tags: result.tags, | ||
| }, | ||
| }); | ||
| return services.metadataStoreObject( | ||
| destBucketName, | ||
| destDataGetInfoArr, | ||
| serverSideEncryption, | ||
| storeMetadataParams, | ||
| (err, result) => { | ||
| if (err) { | ||
| log.debug('error storing new metadata', { error: err }); | ||
| return next(err, null, destBucketMD); | ||
| } | ||
| const sourceObjSize = storeMetadataParams.size; | ||
| const destObjPrevSize = | ||
| destObjMD && destObjMD['content-length'] !== undefined ? destObjMD['content-length'] : null; | ||
|
|
||
| setExpirationHeaders(responseHeaders, { | ||
| lifecycleConfig: destBucketMD.getLifecycleConfiguration(), | ||
| objectParams: { | ||
| key: destObjectKey, | ||
| date: result.lastModified, | ||
| tags: result.tags, | ||
| }, | ||
| }); | ||
|
|
||
| return next(null, dataToDelete, result, destBucketMD, | ||
| storeMetadataParams, serverSideEncryption, | ||
| sourceObjSize, destObjPrevSize); | ||
| }); | ||
| }, | ||
| function deleteExistingData(dataToDelete, storingNewMdResult, | ||
| destBucketMD, storeMetadataParams, serverSideEncryption, | ||
| sourceObjSize, destObjPrevSize, next) { | ||
| // Clean up any potential orphans in data if object | ||
| // put is an overwrite of already existing | ||
| // object with same name, so long as the source is not | ||
| // the same as the destination | ||
| if (!sourceIsDestination && dataToDelete) { | ||
| const newDataStoreName = storeMetadataParams.dataStoreName; | ||
| return data.batchDelete(dataToDelete, request.method, | ||
| newDataStoreName, log, err => { | ||
| return next( | ||
| null, | ||
| dataToDelete, | ||
| result, | ||
| destBucketMD, | ||
| storeMetadataParams, | ||
| serverSideEncryption, | ||
| sourceObjSize, | ||
| destObjPrevSize, | ||
| ); | ||
| }, | ||
| ); | ||
| }, | ||
| function deleteExistingData( | ||
| dataToDelete, | ||
| storingNewMdResult, | ||
| destBucketMD, | ||
| storeMetadataParams, | ||
| serverSideEncryption, | ||
| sourceObjSize, | ||
| destObjPrevSize, | ||
| next, | ||
| ) { | ||
| // Clean up any potential orphans in data if object | ||
| // put is an overwrite of already existing | ||
| // object with same name, so long as the source is not | ||
| // the same as the destination | ||
| if (!sourceIsDestination && dataToDelete) { | ||
| const newDataStoreName = storeMetadataParams.dataStoreName; | ||
| return data.batchDelete(dataToDelete, request.method, newDataStoreName, log, err => { | ||
| if (err) { | ||
| // if error, log the error and move on as it is not | ||
| // relevant to the client as the client's | ||
| // object already succeeded putting data, metadata | ||
| log.error('error deleting existing data', | ||
| { error: err }); | ||
| log.error('error deleting existing data', { error: err }); | ||
| } | ||
| next(null, |
Comment on lines
-608
to
-620
| storingNewMdResult, destBucketMD, storeMetadataParams, | ||
| serverSideEncryption, sourceObjSize, destObjPrevSize); | ||
| next( | ||
| null, | ||
| storingNewMdResult, | ||
| destBucketMD, | ||
| storeMetadataParams, | ||
| serverSideEncryption, | ||
| sourceObjSize, | ||
| destObjPrevSize, | ||
| ); | ||
| }); | ||
| } | ||
| return next( | ||
| null, | ||
| storingNewMdResult, | ||
| destBucketMD, | ||
| storeMetadataParams, | ||
| serverSideEncryption, | ||
| sourceObjSize, | ||
| destObjPrevSize, | ||
| ); | ||
| }, | ||
| ], | ||
| ( | ||
| err, | ||
| storingNewMdResult, | ||
| destBucketMD, | ||
| storeMetadataParams, | ||
| serverSideEncryption, | ||
| sourceObjSize, | ||
| destObjPrevSize, | ||
| ) => { | ||
| const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, destBucketMD); | ||
|
|
||
| // Store full object size for server access logs | ||
| if (request.serverAccessLog) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| request.serverAccessLog.objectSize = sourceObjSize; | ||
| } | ||
| return next(null, | ||
| storingNewMdResult, destBucketMD, storeMetadataParams, | ||
| serverSideEncryption, sourceObjSize, destObjPrevSize); | ||
| }, | ||
| ], (err, storingNewMdResult, destBucketMD, storeMetadataParams, | ||
| serverSideEncryption, sourceObjSize, destObjPrevSize) => { | ||
| const corsHeaders = collectCorsHeaders(request.headers.origin, | ||
| request.method, destBucketMD); | ||
|
|
tcarmet
force-pushed
the
bugfix/CLDSRV-909-enforce-copyobject-source-size-limit
branch
from
35eb632 to
213648e
Compare
|
tcarmet
force-pushed
the
bugfix/CLDSRV-909-enforce-copyobject-source-size-limit
branch
from
213648e to
f34ae66
Compare
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Align cloudserver's CopyObject with the AWS S3 contract by rejecting requests whose source object exceeds 5 GiB. AWS S3 returns HTTP 400
InvalidRequest("The specified copy source is larger than the maximum allowable size for a copy source: 5368709120") and expects clients to use the multipart copy flow instead; cloudserver currently accepts these requests, leaving a gap with the published contract.Targeting
development/9.4because changing a previously-accepted response is a behavior break for clients that relied on cloudserver's permissive behavior, happy to retargetdevelopment/9.3if we prefer wider rollout.Last commit contains a prettier fmt command and is kept separate from the actual code change for review. We should not focus on it as most of it is existing change.