From 757f7e63c63318991f9980d832d9d0080f883137 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 4 Feb 2026 07:25:22 +0100 Subject: [PATCH 1/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Migrate=20bucketGet=20?= =?UTF-8?q?and=20objectGetLegalHold=20to=20async/await?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-823 --- lib/api/bucketGet.js | 114 +++++++++++++++-------------- lib/api/objectGetLegalHold.js | 130 +++++++++++++++++++--------------- 2 files changed, 134 insertions(+), 110 deletions(-) diff --git a/lib/api/bucketGet.js b/lib/api/bucketGet.js index b504e0f99d..c5cb558cc4 100644 --- a/lib/api/bucketGet.js +++ b/lib/api/bucketGet.js @@ -1,3 +1,4 @@ +const { promisify } = require('util'); const querystring = require('querystring'); const { errors, errorInstances, versioning, s3middleware } = require('arsenal'); const constants = require('../../constants'); @@ -236,8 +237,7 @@ function processMasterVersions(bucketName, listParams, list) { return xml.join(''); } -function handleResult(listParams, requestMaxKeys, encoding, authInfo, - bucketName, list, corsHeaders, log, callback) { +function handleResult(listParams, requestMaxKeys, encoding, authInfo, bucketName, list, log) { // eslint-disable-next-line no-param-reassign listParams.maxKeys = requestMaxKeys; // eslint-disable-next-line no-param-reassign @@ -250,7 +250,7 @@ function handleResult(listParams, requestMaxKeys, encoding, authInfo, } pushMetric('listBucket', log, { authInfo, bucket: bucketName }); monitoring.promMetrics('GET', bucketName, '200', 'listBucket'); - return callback(null, res, corsHeaders); + return res; } /** @@ -259,11 +259,9 @@ function handleResult(listParams, requestMaxKeys, encoding, authInfo, * requester's info * @param {object} request - http request object * @param {function} log - Werelogs request logger - * @param {function} callback - callback to respond to http request - * with either error code or xml response body - * @return {undefined} + * @return {Promise} - object containing xml and additionalResHeaders */ -function bucketGet(authInfo, request, log, callback) { +async function bucketGet(authInfo, request, log) { const params = request.query; const bucketName = request.bucketName; const v2 = params['list-type']; @@ -277,9 +275,9 @@ function bucketGet(authInfo, request, log, callback) { } if (v2 !== undefined && Number.parseInt(v2, 10) !== 2) { - return callback(errorInstances.InvalidArgument.customizeDescription('Invalid ' + - 'List Type specified in Request')); + throw errorInstances.InvalidArgument.customizeDescription('Invalid List Type specified in Request'); } + if (v2) { log.addDefaultFields({ action: 'ListObjectsV2', }); if (request.serverAccessLog) { @@ -297,18 +295,14 @@ function bucketGet(authInfo, request, log, callback) { const encoding = params['encoding-type']; if (encoding !== undefined && encoding !== 'url') { monitoring.promMetrics('GET', bucketName, 400, 'listBucket'); - return callback(errorInstances.InvalidArgument.customizeDescription('Invalid ' + - 'Encoding Method specified in Request')); + throw errorInstances.InvalidArgument.customizeDescription('Invalid Encoding Method specified in Request'); } const requestMaxKeys = params['max-keys'] ? Number.parseInt(params['max-keys'], 10) : 1000; if (Number.isNaN(requestMaxKeys) || requestMaxKeys < 0) { monitoring.promMetrics('GET', bucketName, 400, 'listBucket'); - return callback(errors.InvalidArgument); + throw errors.InvalidArgument; } - // AWS only returns 1000 keys even if max keys are greater. - // Max keys stated in response xml can be greater than actual - // keys returned. const actualMaxKeys = Math.min(constants.listingHardLimit, requestMaxKeys); const metadataValParams = { @@ -336,51 +330,67 @@ function bucketGet(authInfo, request, log, callback) { listParams.marker = params.marker; } - standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { - const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); + let error; + let bucket; - if (err) { - log.debug('error processing request', { error: err }); - monitoring.promMetrics('GET', bucketName, err.code, 'listBucket'); - return callback(err, null, corsHeaders); - } + try { + const standardMetadataValidateBucketPromised = promisify(standardMetadataValidateBucket); + bucket = await standardMetadataValidateBucketPromised(metadataValParams, request.actionImplicitDenies, log); + } catch (err) { + error = err; + } - if (params.versions !== undefined) { - listParams.listingType = 'DelimiterVersions'; - delete listParams.marker; - listParams.keyMarker = params['key-marker']; - listParams.versionIdMarker = params['version-id-marker'] ? - versionIdUtils.decode(params['version-id-marker']) : - undefined; - } + const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); - if (!requestMaxKeys) { - const emptyList = { - CommonPrefixes: [], - Contents: [], - Versions: [], - IsTruncated: false, - }; - return handleResult(listParams, requestMaxKeys, encoding, authInfo, - bucketName, emptyList, corsHeaders, log, callback); - } + if (error) { + log.debug('error processing request', { error }); + monitoring.promMetrics('GET', bucketName, error.code, 'listBucket'); + error.additionalResHeaders = corsHeaders; + throw error; + } + if (params.versions !== undefined) { + listParams.listingType = 'DelimiterVersions'; + delete listParams.marker; + listParams.keyMarker = params['key-marker']; + listParams.versionIdMarker = params['version-id-marker'] ? + versionIdUtils.decode(params['version-id-marker']) : + undefined; + } + if (!requestMaxKeys) { + const emptyList = { + CommonPrefixes: [], + Contents: [], + Versions: [], + IsTruncated: false, + }; + const res = handleResult(listParams, requestMaxKeys, encoding, authInfo, bucketName, emptyList, log); + return [res, corsHeaders]; + } - return services.getObjectListing(bucketName, listParams, log, (err, list) => { - if (err) { - log.debug('error processing request', { error: err }); - monitoring.promMetrics('GET', bucketName, err.code, 'listBucket'); - return callback(err, null, corsHeaders); - } + let list; + try { + list = await promisify(services.getObjectListing)(bucketName, listParams, log); + } catch (err) { + log.debug('error processing request', { error: err }); + monitoring.promMetrics('GET', bucketName, err.code, 'listBucket'); - return handleResult(listParams, requestMaxKeys, encoding, authInfo, - bucketName, list, corsHeaders, log, callback); - }); - }); - return undefined; + err.additionalResHeaders = corsHeaders; + throw err; + } + + const res = handleResult(listParams, requestMaxKeys, encoding, authInfo, bucketName, list, log); + return [res, corsHeaders]; } module.exports = { processVersions, processMasterVersions, - bucketGet, + bucketGet: (...args) => { + const callback = args.at(-1); + const argsWithoutCallback = args.slice(0, -1); + + bucketGet(...argsWithoutCallback) + .then(result => callback(null, ...result)) + .catch(err => callback(err, null, err.additionalResHeaders)); + }, }; diff --git a/lib/api/objectGetLegalHold.js b/lib/api/objectGetLegalHold.js index a403fae458..5fd5c62e93 100644 --- a/lib/api/objectGetLegalHold.js +++ b/lib/api/objectGetLegalHold.js @@ -1,4 +1,3 @@ -const async = require('async'); const { errors, errorInstances, s3middleware } = require('arsenal'); const { decodeVersionId, getVersionIdResHeader } @@ -10,15 +9,22 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const { convertToXml } = s3middleware.objectLegalHold; +const standardMetadataValidateBucketAndObjPromised = (metadataValParams, actionImplicitDenies, log) => + new Promise((resolve, reject) => { + standardMetadataValidateBucketAndObj(metadataValParams, actionImplicitDenies, log, (err, bucket, objectMD) => { + if (err) {return reject(err);} + return resolve({ bucket, objectMD }); + }); + }); + /** * Returns legal hold status of object * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info * @param {object} request - http request object * @param {object} log - Werelogs logger - * @param {function} callback - callback to server - * @return {undefined} + * @return {Promise} - object containing xml and additionalResHeaders */ -function objectGetLegalHold(authInfo, request, log, callback) { +async function objectGetLegalHold(authInfo, request, log) { log.debug('processing request', { method: 'objectGetLegalHold' }); const { bucketName, objectKey, query } = request; @@ -26,7 +32,7 @@ function objectGetLegalHold(authInfo, request, log, callback) { const decodedVidResult = decodeVersionId(query); if (decodedVidResult instanceof Error) { log.trace('invalid versionId query', { versionId: query.versionId, error: decodedVidResult }); - return process.nextTick(() => callback(decodedVidResult)); + throw decodedVidResult; } const versionId = decodedVidResult; @@ -40,60 +46,68 @@ function objectGetLegalHold(authInfo, request, log, callback) { request, }; - return async.waterfall([ - next => standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, - (err, bucket, objectMD) => { - if (err) { - log.trace('request authorization failed', { method: 'objectGetLegalHold', error: err }); - return next(err); - } - if (!objectMD) { - const err = versionId ? errors.NoSuchVersion : errors.NoSuchKey; - log.trace('error no object metadata found', { method: 'objectGetLegalHold', error: err }); - return next(err, bucket); - } - if (objectMD.isDeleteMarker) { - if (versionId) { - log.trace('requested version is delete marker', { method: 'objectGetLegalHold' }); - // FIXME we should return a `x-amz-delete-marker: true` header, see S3C-7592 - return next(errors.MethodNotAllowed); - } - log.trace('most recent version is delete marker', { method: 'objectGetLegalHold' }); - // FIXME we should return a `x-amz-delete-marker: true` header, see S3C-7592 - return next(errors.NoSuchKey); - } - if (!bucket.isObjectLockEnabled()) { - log.trace('object lock not enabled on bucket', { method: 'objectGetRetention' }); - return next(errorInstances.InvalidRequest.customizeDescription( - 'Bucket is missing Object Lock Configuration')); - } - return next(null, bucket, objectMD); - }), - (bucket, objectMD, next) => { - const { legalHold } = objectMD; - const xml = convertToXml(legalHold); - if (xml === '') { - return next(errors.NoSuchObjectLockConfiguration); - } - return next(null, bucket, xml, objectMD); - }, - ], (err, bucket, xml, objectMD) => { - const additionalResHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); - if (err) { - log.trace('error processing request', { error: err, method: 'objectGetLegalHold' }); - } else { - pushMetric('getObjectLegalHold', log, { - authInfo, - bucket: bucketName, - keys: [objectKey], - versionId: objectMD ? objectMD.versionId : undefined, - location: objectMD ? objectMD.dataStoreName : undefined, - }); - const verCfg = bucket.getVersioningConfiguration(); - additionalResHeaders['x-amz-version-id'] = getVersionIdResHeader(verCfg, objectMD); + let bucket, objectMD; + + try { + ({ bucket, objectMD } = await standardMetadataValidateBucketAndObjPromised( + metadataValParams, + request.actionImplicitDenies, + log, + )); + } catch (err) { + log.trace('request authorization failed', { method: 'objectGetLegalHold', error: err }); + throw err; + } + + if (!objectMD) { + const err = versionId ? errors.NoSuchVersion : errors.NoSuchKey; + log.trace('error no object metadata found', { method: 'objectGetLegalHold', error: err }); + throw err; + } + + if (objectMD.isDeleteMarker) { + if (versionId) { + log.trace('requested version is delete marker', { method: 'objectGetLegalHold' }); + // FIXME we should return a `x-amz-delete-marker: true` header, see S3C-7592 + throw errors.MethodNotAllowed; } - return callback(err, xml, additionalResHeaders); + + log.trace('most recent version is delete marker', { method: 'objectGetLegalHold' }); + // FIXME we should return a `x-amz-delete-marker: true` header, see S3C-7592 + throw errors.NoSuchKey; + } + + if (!bucket.isObjectLockEnabled()) { + log.trace('object lock not enabled on bucket', { method: 'objectGetRetention' }); + throw errorInstances.InvalidRequest.customizeDescription('Bucket is missing Object Lock Configuration'); + } + + const { legalHold } = objectMD; + const xml = convertToXml(legalHold); + if (xml === '') { + throw errors.NoSuchObjectLockConfiguration; + } + + const additionalResHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); + + pushMetric('getObjectLegalHold', log, { + authInfo, + bucket: bucketName, + keys: [objectKey], + versionId: objectMD ? objectMD.versionId : undefined, + location: objectMD ? objectMD.dataStoreName : undefined, }); + const verCfg = bucket.getVersioningConfiguration(); + additionalResHeaders['x-amz-version-id'] = getVersionIdResHeader(verCfg, objectMD); + + return [xml, additionalResHeaders]; } -module.exports = objectGetLegalHold; +module.exports = (...args) => { + const callback = args.at(-1); + const argsWithoutCallback = args.slice(0, -1); + + objectGetLegalHold(...argsWithoutCallback) + .then(result => callback(null, ...result)) + .catch(err => callback(err, null, err.additionalResHeaders)); +}; From a24d60c674c024f7c3ac5b45b6ce44d30efce587 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 4 Feb 2026 08:13:21 +0100 Subject: [PATCH 2/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20use=20trampoline=20fun?= =?UTF-8?q?ction=20to=20keep=20backward=20compatibility?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-823 --- lib/api/bucketGet.js | 17 ++++++++--------- lib/api/objectGetLegalHold.js | 8 +++++++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/api/bucketGet.js b/lib/api/bucketGet.js index c5cb558cc4..301067f5f4 100644 --- a/lib/api/bucketGet.js +++ b/lib/api/bucketGet.js @@ -261,7 +261,13 @@ function handleResult(listParams, requestMaxKeys, encoding, authInfo, bucketName * @param {function} log - Werelogs request logger * @return {Promise} - object containing xml and additionalResHeaders */ -async function bucketGet(authInfo, request, log) { +async function bucketGet(authInfo, request, log, callback) { + if (callback) { + return bucketGet(authInfo, request, log) + .then(result => callback(null, ...result)) + .catch(err => callback(err, null, err.additionalResHeaders)); + } + const params = request.query; const bucketName = request.bucketName; const v2 = params['list-type']; @@ -385,12 +391,5 @@ async function bucketGet(authInfo, request, log) { module.exports = { processVersions, processMasterVersions, - bucketGet: (...args) => { - const callback = args.at(-1); - const argsWithoutCallback = args.slice(0, -1); - - bucketGet(...argsWithoutCallback) - .then(result => callback(null, ...result)) - .catch(err => callback(err, null, err.additionalResHeaders)); - }, + bucketGet, }; diff --git a/lib/api/objectGetLegalHold.js b/lib/api/objectGetLegalHold.js index 5fd5c62e93..475260bba4 100644 --- a/lib/api/objectGetLegalHold.js +++ b/lib/api/objectGetLegalHold.js @@ -24,7 +24,13 @@ const standardMetadataValidateBucketAndObjPromised = (metadataValParams, actionI * @param {object} log - Werelogs logger * @return {Promise} - object containing xml and additionalResHeaders */ -async function objectGetLegalHold(authInfo, request, log) { +async function objectGetLegalHold(authInfo, request, log, callback) { + if (callback) { + return objectGetLegalHold(authInfo, request, log) + .then(result => callback(null, ...result)) + .catch(err => callback(err, null, err.additionalResHeaders)); + } + log.debug('processing request', { method: 'objectGetLegalHold' }); const { bucketName, objectKey, query } = request; From d6af6e2e949534058f5393e87e56c9f0e32d9d93 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 4 Feb 2026 08:32:20 +0100 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=93=9D=20update=20JSDoc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-823 --- lib/api/bucketGet.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/api/bucketGet.js b/lib/api/bucketGet.js index 301067f5f4..f695a4f89b 100644 --- a/lib/api/bucketGet.js +++ b/lib/api/bucketGet.js @@ -259,7 +259,9 @@ function handleResult(listParams, requestMaxKeys, encoding, authInfo, bucketName * requester's info * @param {object} request - http request object * @param {function} log - Werelogs request logger + * @param {function} callback - callback optional to keep backward compatibility * @return {Promise} - object containing xml and additionalResHeaders + * @throws {Error} */ async function bucketGet(authInfo, request, log, callback) { if (callback) { From 7bed44798fcd63af0c39062c44450f79e87f2a10 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 4 Feb 2026 08:44:18 +0100 Subject: [PATCH 4/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20factorise=20custom=20p?= =?UTF-8?q?romisify=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-823 --- lib/api/objectGetLegalHold.js | 10 ++-------- lib/metadata/metadataUtils.js | 9 +++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/api/objectGetLegalHold.js b/lib/api/objectGetLegalHold.js index 475260bba4..8940c2ba8c 100644 --- a/lib/api/objectGetLegalHold.js +++ b/lib/api/objectGetLegalHold.js @@ -1,3 +1,4 @@ +const { promisify } = require('util'); const { errors, errorInstances, s3middleware } = require('arsenal'); const { decodeVersionId, getVersionIdResHeader } @@ -9,14 +10,6 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const { convertToXml } = s3middleware.objectLegalHold; -const standardMetadataValidateBucketAndObjPromised = (metadataValParams, actionImplicitDenies, log) => - new Promise((resolve, reject) => { - standardMetadataValidateBucketAndObj(metadataValParams, actionImplicitDenies, log, (err, bucket, objectMD) => { - if (err) {return reject(err);} - return resolve({ bucket, objectMD }); - }); - }); - /** * Returns legal hold status of object * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info @@ -55,6 +48,7 @@ async function objectGetLegalHold(authInfo, request, log, callback) { let bucket, objectMD; try { + const standardMetadataValidateBucketAndObjPromised = promisify(standardMetadataValidateBucketAndObj); ({ bucket, objectMD } = await standardMetadataValidateBucketAndObjPromised( metadataValParams, request.actionImplicitDenies, diff --git a/lib/metadata/metadataUtils.js b/lib/metadata/metadataUtils.js index cf0ef7ce36..2702125209 100644 --- a/lib/metadata/metadataUtils.js +++ b/lib/metadata/metadataUtils.js @@ -1,3 +1,4 @@ +const { promisify } = require('util'); const async = require('async'); const { errors } = require('arsenal'); @@ -429,6 +430,14 @@ function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log, return callback(null, bucket, objMD); }); } +standardMetadataValidateBucketAndObj[promisify.custom] = (...args) => + new Promise((resolve, reject) => { + standardMetadataValidateBucketAndObj(...args, (err, bucket, objectMD) => { + if (err) {return reject(err);} + return resolve({ bucket, objectMD }); + }); + }); + /** standardMetadataValidateBucket - retrieve bucket from metadata and check if user * is authorized to access it * @param {object} params - function parameters From 7b03be0b77e9828ca453864670e9f7c98b0531dd Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 4 Feb 2026 19:05:48 +0100 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=90=9B=20update=20callback=20to=20asy?= =?UTF-8?q?nc/await=20after=20merge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-823 --- lib/api/bucketGet.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/api/bucketGet.js b/lib/api/bucketGet.js index f695a4f89b..2b5216c337 100644 --- a/lib/api/bucketGet.js +++ b/lib/api/bucketGet.js @@ -277,9 +277,7 @@ async function bucketGet(authInfo, request, log, callback) { const optionalAttributes = request.headers['x-amz-optional-object-attributes']?.split(',').map(attr => attr.trim()) ?? []; if (optionalAttributes.some(attr => !attr.startsWith('x-amz-meta-') && attr != 'RestoreStatus')) { - return callback( - errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified') - ); + throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified'); } if (v2 !== undefined && Number.parseInt(v2, 10) !== 2) {