From b5ba3a4884ba5e4cb5fe393c96e94c2666480f3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Tue, 20 Jan 2026 19:45:21 +0100 Subject: [PATCH 01/17] Support the new API GetObjectAttributes Issue: CLDSRV-817 --- constants.js | 8 + lib/api/api.js | 2 + .../apiUtils/object/parseAttributesHeader.js | 32 + lib/api/objectGetAttributes.js | 146 ++++ package.json | 2 +- .../test/object/objectGetAttributes.js | 272 +++++++ .../test/versioning/objectGetAttributes.js | 145 ++++ .../apiUtils/object/parseAttributesHeader.js | 192 +++++ tests/unit/api/objectGetAttributes.js | 672 ++++++++++++++++++ yarn.lock | 4 +- 10 files changed, 1472 insertions(+), 3 deletions(-) create mode 100644 lib/api/apiUtils/object/parseAttributesHeader.js create mode 100644 lib/api/objectGetAttributes.js create mode 100644 tests/functional/aws-node-sdk/test/object/objectGetAttributes.js create mode 100644 tests/functional/aws-node-sdk/test/versioning/objectGetAttributes.js create mode 100644 tests/unit/api/apiUtils/object/parseAttributesHeader.js create mode 100644 tests/unit/api/objectGetAttributes.js diff --git a/constants.js b/constants.js index b9cdd49287..4824109d69 100644 --- a/constants.js +++ b/constants.js @@ -275,6 +275,14 @@ const constants = { 'bucketPutLogging', 'bucketGetLogging', ], + // Metadata allowed to be returned by getObjectAttributes API + allowedObjectAttributes: new Set([ + 'StorageClass', + 'ObjectSize', + 'ObjectParts', + 'Checksum', + 'ETag', + ]), }; module.exports = constants; diff --git a/lib/api/api.js b/lib/api/api.js index 6ab4fcc77f..de7c954abf 100644 --- a/lib/api/api.js +++ b/lib/api/api.js @@ -53,6 +53,7 @@ const { objectDelete } = require('./objectDelete'); const objectDeleteTagging = require('./objectDeleteTagging'); const objectGet = require('./objectGet'); const objectGetACL = require('./objectGetACL'); +const objectGetAttributes = require('./objectGetAttributes.js'); const objectGetLegalHold = require('./objectGetLegalHold'); const objectGetRetention = require('./objectGetRetention'); const objectGetTagging = require('./objectGetTagging'); @@ -384,6 +385,7 @@ const api = { objectDeleteTagging, objectGet, objectGetACL, + objectGetAttributes, objectGetLegalHold, objectGetRetention, objectGetTagging, diff --git a/lib/api/apiUtils/object/parseAttributesHeader.js b/lib/api/apiUtils/object/parseAttributesHeader.js new file mode 100644 index 0000000000..92f7509251 --- /dev/null +++ b/lib/api/apiUtils/object/parseAttributesHeader.js @@ -0,0 +1,32 @@ +const { errorInstances } = require('arsenal'); +const { allowedObjectAttributes } = require('../../../../constants'); + +/** + * parseAttributesHeaders - Parse and validate the x-amz-object-attributes header + * @param {object} headers - request headers + * @returns {string[]} - array of valid attribute names + * @throws {Error} - InvalidRequest if header is missing/empty, InvalidArgument if attribute is invalid + */ +function parseAttributesHeaders(headers) { + const raw = headers['x-amz-object-attributes'] || ''; + + const attributes = raw + .split(',') + .map(s => s.trim()) + .filter(s => s !== ''); + + if (attributes.length === 0) { + throw errorInstances.InvalidRequest.customizeDescription( + 'The x-amz-object-attributes header specifying the attributes to be retrieved is either missing or empty', + ); + } + + const invalids = attributes.filter(s => !allowedObjectAttributes.has(s)); + if (invalids.length > 0) { + throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.'); + } + + return attributes; +} + +module.exports = parseAttributesHeaders; diff --git a/lib/api/objectGetAttributes.js b/lib/api/objectGetAttributes.js new file mode 100644 index 0000000000..8c232dd2f1 --- /dev/null +++ b/lib/api/objectGetAttributes.js @@ -0,0 +1,146 @@ +const { promisify } = require('util'); +const xml2js = require('xml2js'); +const { errors } = require('arsenal'); +const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); +const collectCorsHeaders = require('../utilities/collectCorsHeaders'); +const parseAttributesHeaders = require('./apiUtils/object/parseAttributesHeader'); +const { decodeVersionId, getVersionIdResHeader } = require('./apiUtils/object/versioning'); +const { checkExpectedBucketOwner } = require('./apiUtils/authorization/bucketOwner'); +const { pushMetric } = require('../utapi/utilities'); +const { getPartCountFromMd5 } = require('./apiUtils/object/partInfo'); + +const OBJECT_GET_ATTRIBUTES = 'objectGetAttributes'; + +const checkExpectedBucketOwnerPromise = promisify(checkExpectedBucketOwner); + +/** + * validateBucketAndObjPromise - Promisified wrapper for standardMetadataValidateBucketAndObj + * @param {object} params - validation parameters + * @param {boolean} actionImplicitDenies - whether action has implicit denies + * @param {object} log - Werelogs logger + * @returns {Promise<{bucket: BucketInfo, objMD: object}>} - bucket and object metadata + * @throws {Error} - rejects with error from standardMetadataValidateBucketAndObj + */ +function validateBucketAndObjPromise(params, actionImplicitDenies, log) { + return new Promise((resolve, reject) => { + standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log, (err, bucket, objMD) => { + if (err) { + return reject(err); + } + return resolve({ bucket, objMD }); + }); + }); +} + +/** + * buildXmlResponse - Build XML response for GetObjectAttributes + * @param {object} objMD - object metadata + * @param {array} attributes - requested attributes + * @returns {string} XML response + */ +function buildXmlResponse(objMD, attributes) { + const attrResp = {}; + + if (attributes.includes('ETag')) { + attrResp.ETag = objMD['content-md5']; + } + + // NOTE: Checksum is not implemented + if (attributes.includes('Checksum')) { + attrResp.Checksum = {}; + } + + if (attributes.includes('ObjectParts')) { + const partCount = getPartCountFromMd5(objMD); + if (partCount) { + attrResp.ObjectParts = { PartsCount: partCount }; + } + } + + if (attributes.includes('StorageClass')) { + attrResp.StorageClass = objMD['x-amz-storage-class']; + } + + if (attributes.includes('ObjectSize')) { + attrResp.ObjectSize = objMD['content-length']; + } + + const builder = new xml2js.Builder(); + return builder.buildObject({ GetObjectAttributesResponse: attrResp }); +} + +/** + * objectGetAttributes - Retrieves all metadata from an object without returning the object itself + * @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} + */ +async function objectGetAttributes(authInfo, request, log, callback) { + log.trace('processing request', { method: OBJECT_GET_ATTRIBUTES }); + const { bucketName, objectKey, headers, actionImplicitDenies } = request; + + let responseHeaders = {}; + + const versionId = decodeVersionId(request.query); + if (versionId instanceof Error) { + log.debug('invalid versionId query', { versionId: request.query.versionId, error: versionId }); + throw versionId; + } + + const metadataValParams = { + authInfo, + bucketName, + objectKey, + versionId, + getDeleteMarker: true, + requestType: request.apiMethods || OBJECT_GET_ATTRIBUTES, + request, + }; + + try { + const { bucket, objMD } = await validateBucketAndObjPromise(metadataValParams, actionImplicitDenies, log); + await checkExpectedBucketOwnerPromise(headers, bucket, log); + + responseHeaders = collectCorsHeaders(headers.origin, request.method, bucket); + if (objMD) { + responseHeaders['x-amz-version-id'] = getVersionIdResHeader(bucket.getVersioningConfiguration(), objMD); + responseHeaders['Last-Modified'] = objMD['last-modified'] && new Date(objMD['last-modified']).toUTCString(); + } + + if (!objMD) { + const err = versionId ? errors.NoSuchVersion : errors.NoSuchKey; + log.debug('object not found', { bucket: bucketName, key: objectKey, versionId }); + throw err; + } + + if (objMD.isDeleteMarker) { + log.debug('attempt to get attributes of a delete marker', { bucket: bucketName, key: objectKey, versionId }); + responseHeaders['x-amz-delete-marker'] = true; + throw errors.MethodNotAllowed; + } + + const attributes = parseAttributesHeaders(headers); + + pushMetric(OBJECT_GET_ATTRIBUTES, log, { + authInfo, + bucket: bucketName, + keys: [objectKey], + versionId: objMD?.versionId, + location: objMD?.dataStoreName, + }); + + const xml = buildXmlResponse(objMD, attributes); + return callback(null, xml, responseHeaders); + } catch (err) { + log.debug('error processing request', { + error: err, + method: OBJECT_GET_ATTRIBUTES, + }); + + return callback(err, null, responseHeaders); + } +} + +module.exports = objectGetAttributes; diff --git a/package.json b/package.json index df84ef014b..25e4285073 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "dependencies": { "@azure/storage-blob": "^12.28.0", "@hapi/joi": "^17.1.1", - "arsenal": "git+https://github.com/scality/arsenal#8.2.44", + "arsenal": "git+https://github.com/scality/Arsenal#feature/ARSN-549/get-object-attributes", "async": "2.6.4", "aws-sdk": "^2.1692.0", "bucketclient": "scality/bucketclient#8.2.7", diff --git a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js new file mode 100644 index 0000000000..b969e9926b --- /dev/null +++ b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js @@ -0,0 +1,272 @@ +const assert = require('assert'); +const { S3 } = require('aws-sdk'); +const getConfig = require('../support/config'); + +const bucket = 'testbucket'; +const key = 'testobject'; +const body = 'hello world!'; +const expectedMD5 = 'fc3ff98e8c6a0d3087d515c0473f8677'; + +describe('Test get object attributes', () => { + let s3; + + before(() => { + const config = getConfig('default', { signatureVersion: 'v4' }); + s3 = new S3(config); + }); + + beforeEach(async () => { + await s3.createBucket({ Bucket: bucket }).promise(); + await s3.putObject({ Bucket: bucket, Key: key, Body: body }).promise(); + }); + + afterEach(async () => { + await s3.deleteObject({ Bucket: bucket, Key: key }).promise(); + await s3.deleteBucket({ Bucket: bucket }).promise(); + }); + + it('should fail because a bad bucket owner', async () => { + try { + await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['ETag'], + ExpectedBucketOwner: 'wrongAccountId', + }) + .promise(); + assert.fail('Expected AccessDenied error'); + } catch (err) { + assert.strictEqual(err.code, 'AccessDenied'); + assert.strictEqual(err.message, 'Access Denied'); + } + }); + + it('should fail because attributes header is missing', async () => { + try { + await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: [], + }) + .promise(); + assert.fail('Expected InvalidRequest error'); + } catch (err) { + assert.strictEqual(err.code, 'InvalidRequest'); + assert.strictEqual( + err.message, + 'The x-amz-object-attributes header specifying the attributes to be retrieved is either missing or empty', + ); + } + }); + + it('should fail because attribute name is invalid', async () => { + try { + await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['InvalidAttribute'], + }) + .promise(); + assert.fail('Expected InvalidArgument error'); + } catch (err) { + assert.strictEqual(err.code, 'InvalidArgument'); + assert.strictEqual(err.message, 'Invalid attribute name specified.'); + } + }); + + it('should return NoSuchKey for non-existent object', async () => { + try { + await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: 'nonexistent', + ObjectAttributes: ['ETag'], + }) + .promise(); + assert.fail('Expected NoSuchKey error'); + } catch (err) { + assert.strictEqual(err.code, 'NoSuchKey'); + assert.strictEqual(err.message, 'The specified key does not exist.'); + } + }); + + it('should return all attributes', async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['ETag', 'Checksum', 'ObjectParts', 'StorageClass', 'ObjectSize'], + }) + .promise(); + + assert.strictEqual(data.ETag, expectedMD5); + assert.strictEqual(data.StorageClass, 'STANDARD'); + assert.strictEqual(data.ObjectSize, body.length); + assert(data.Checksum, 'Checksum should be present'); + assert(!data.ObjectParts, "ObjectParts shouldn't be present for non-MPU object"); + assert(data.LastModified, 'LastModified should be present'); + }); + + it('should return ETag', async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['ETag'], + }) + .promise(); + + assert.strictEqual(data.ETag, expectedMD5); + }); + + it('should return Checksum', async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['Checksum'], + }) + .promise(); + + assert(data.Checksum, 'Checksum should be present'); + }); + + it("shouldn't return ObjectParts", async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['ObjectParts'], + }) + .promise(); + + // ObjectParts may be empty for non-MPU objects + assert(!data.ObjectParts, "ObjectParts shouldn't be present"); + }); + + it('should return StorageClass', async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['StorageClass'], + }) + .promise(); + + assert.strictEqual(data.StorageClass, 'STANDARD'); + }); + + it('should return ObjectSize', async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['ObjectSize'], + }) + .promise(); + + assert.strictEqual(data.ObjectSize, body.length); + }); + + it('should return LastModified', async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['ETag'], + }) + .promise(); + + assert(data.LastModified, 'LastModified should be present'); + assert(data.LastModified instanceof Date, 'LastModified should be a Date'); + assert(!isNaN(data.LastModified.getTime()), 'LastModified should be a valid date'); + }); +}); + +describe('Test get object attributes with multipart upload', () => { + let s3; + const mpuKey = 'mpuObject'; + const partSize = 5 * 1024 * 1024; // Minimum part size is 5MB + const partCount = 3; + + before(async () => { + const config = getConfig('default', { signatureVersion: 'v4' }); + s3 = new S3(config); + + // Create bucket + await s3.createBucket({ Bucket: bucket }).promise(); + + // Create multipart upload + const createResult = await s3 + .createMultipartUpload({ + Bucket: bucket, + Key: mpuKey, + }) + .promise(); + const uploadId = createResult.UploadId; + + // Upload parts + const partData = Buffer.alloc(partSize, 'a'); + const parts = []; + for (let i = 1; i <= partCount; i++) { + const uploadResult = await s3 + .uploadPart({ + Bucket: bucket, + Key: mpuKey, + PartNumber: i, + UploadId: uploadId, + Body: partData, + }) + .promise(); + parts.push({ PartNumber: i, ETag: uploadResult.ETag }); + } + + // Complete multipart upload + await s3 + .completeMultipartUpload({ + Bucket: bucket, + Key: mpuKey, + UploadId: uploadId, + MultipartUpload: { Parts: parts }, + }) + .promise(); + }); + + after(async () => { + await s3.deleteObject({ Bucket: bucket, Key: mpuKey }).promise(); + await s3.deleteBucket({ Bucket: bucket }).promise(); + }); + + it('should return TotalPartsCount for MPU object', async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: mpuKey, + ObjectAttributes: ['ObjectParts'], + }) + .promise(); + + assert(data.ObjectParts, 'ObjectParts should be present'); + assert.strictEqual(data.ObjectParts.TotalPartsCount, partCount); + }); + + it('should return TotalPartsCount along with other attributes for MPU object', async () => { + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: mpuKey, + ObjectAttributes: ['ETag', 'ObjectParts', 'ObjectSize', 'StorageClass'], + }) + .promise(); + + assert(data.ETag, 'ETag should be present'); + assert(data.ETag.includes(`-${partCount}`), `ETag should indicate MPU with ${partCount} parts`); + assert(data.ObjectParts, 'ObjectParts should be present'); + assert.strictEqual(data.ObjectParts.TotalPartsCount, partCount); + assert.strictEqual(data.ObjectSize, partSize * partCount); + assert.strictEqual(data.StorageClass, 'STANDARD'); + }); +}); diff --git a/tests/functional/aws-node-sdk/test/versioning/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/versioning/objectGetAttributes.js new file mode 100644 index 0000000000..92c1308de6 --- /dev/null +++ b/tests/functional/aws-node-sdk/test/versioning/objectGetAttributes.js @@ -0,0 +1,145 @@ +const assert = require('assert'); +const { promisify } = require('util'); +const { S3 } = require('aws-sdk'); +const getConfig = require('../support/config'); +const { removeAllVersions, versioningEnabled } = require('../../lib/utility/versioning-util.js'); + +const removeAllVersionsPromise = promisify(removeAllVersions); + +const bucket = 'testbucket'; +const key = 'testobject'; +const body = 'hello world!'; +const expectedMD5 = 'fc3ff98e8c6a0d3087d515c0473f8677'; + +describe('Test get object attributes with versioning', () => { + let s3; + + before(() => { + const config = getConfig('default', { signatureVersion: 'v4' }); + s3 = new S3(config); + }); + + beforeEach(async () => { + await s3.createBucket({ Bucket: bucket }).promise(); + await s3 + .putBucketVersioning({ + Bucket: bucket, + VersioningConfiguration: versioningEnabled, + }) + .promise(); + }); + + afterEach(async () => { + await removeAllVersionsPromise({ Bucket: bucket }); + await s3.deleteBucket({ Bucket: bucket }).promise(); + }); + + it('should return NoSuchVersion for non-existent versionId', async () => { + await s3 + .putObject({ + Bucket: bucket, + Key: key, + Body: body, + }) + .promise(); + + // Use a properly formatted but non-existent version ID + const fakeVersionId = '111111111111111111111111111111111111111175636f7270'; + + try { + await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + VersionId: fakeVersionId, + ObjectAttributes: ['ETag'], + }) + .promise(); + assert.fail('Expected NoSuchVersion error'); + } catch (err) { + assert.strictEqual(err.code, 'NoSuchVersion'); + assert.strictEqual( + err.message, + 'Indicates that the version ID specified in the request does not match an existing version.', + ); + } + }); + + it('should return MethodNotAllowed for delete marker', async () => { + await s3 + .putObject({ + Bucket: bucket, + Key: key, + Body: body, + }) + .promise(); + + // Delete creates a delete marker + await s3 + .deleteObject({ + Bucket: bucket, + Key: key, + }) + .promise(); + + // Request without versionId targets the delete marker + try { + await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['ETag'], + }) + .promise(); + assert.fail('Expected MethodNotAllowed error'); + } catch (err) { + assert.strictEqual(err.code, 'MethodNotAllowed'); + assert.strictEqual(err.message, 'The specified method is not allowed against this resource.'); + } + }); + + it('should return attributes for specific version', async () => { + const putResult = await s3 + .putObject({ + Bucket: bucket, + Key: key, + Body: body, + }) + .promise(); + const versionId = putResult.VersionId; + + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + VersionId: versionId, + ObjectAttributes: ['ETag', 'ObjectSize'], + }) + .promise(); + + assert.strictEqual(data.ETag, expectedMD5); + assert.strictEqual(data.ObjectSize, body.length); + assert(data.LastModified, 'LastModified should be present'); + }); + + it('should return VersionId for versioned object', async () => { + const putResult = await s3 + .putObject({ + Bucket: bucket, + Key: key, + Body: body, + }) + .promise(); + const versionId = putResult.VersionId; + + const data = await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['ETag'], + }) + .promise(); + + assert.strictEqual(data.VersionId, versionId); + }); +}); diff --git a/tests/unit/api/apiUtils/object/parseAttributesHeader.js b/tests/unit/api/apiUtils/object/parseAttributesHeader.js new file mode 100644 index 0000000000..dc09292649 --- /dev/null +++ b/tests/unit/api/apiUtils/object/parseAttributesHeader.js @@ -0,0 +1,192 @@ +const assert = require('assert'); + +const parseAttributesHeaders = require('../../../../../lib/api/apiUtils/object/parseAttributesHeader'); + +describe('parseAttributesHeaders', () => { + describe('missing or empty header', () => { + it('should throw InvalidRequest error when header is missing', () => { + const headers = {}; + + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidRequest, true); + assert(err.description.includes('missing or empty')); + return true; + }, + ); + }); + + it('should throw InvalidRequest error when header is empty string', () => { + const headers = { 'x-amz-object-attributes': '' }; + + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidRequest, true); + assert(err.description.includes('missing or empty')); + return true; + }, + ); + }); + + it('should throw InvalidRequest error when header contains only whitespace', () => { + const headers = { 'x-amz-object-attributes': ' ' }; + + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidRequest, true); + return true; + }, + ); + }); + + it('should throw InvalidRequest error when header contains only commas', () => { + const headers = { 'x-amz-object-attributes': ',,,' }; + + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidRequest, true); + return true; + }, + ); + }); + }); + + describe('invalid attribute names', () => { + it('should throw InvalidArgument error for single invalid attribute', () => { + const headers = { 'x-amz-object-attributes': 'InvalidAttribute' }; + + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidArgument, true); + assert(err.description.includes('Invalid attribute name')); + return true; + }, + ); + }); + + it('should throw InvalidArgument error when one attribute is invalid among valid ones', () => { + const headers = { 'x-amz-object-attributes': 'ETag,InvalidAttribute,ObjectSize' }; + + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidArgument, true); + return true; + }, + ); + }); + + it('should throw InvalidArgument error for multiple invalid attributes', () => { + const headers = { 'x-amz-object-attributes': 'Invalid1,Invalid2' }; + + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidArgument, true); + return true; + }, + ); + }); + }); + + describe('valid attribute names', () => { + it('should return array with single valid attribute ETag', () => { + const headers = { 'x-amz-object-attributes': 'ETag' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['ETag']); + }); + + it('should return array with single valid attribute StorageClass', () => { + const headers = { 'x-amz-object-attributes': 'StorageClass' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['StorageClass']); + }); + + it('should return array with single valid attribute ObjectSize', () => { + const headers = { 'x-amz-object-attributes': 'ObjectSize' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['ObjectSize']); + }); + + it('should return array with single valid attribute ObjectParts', () => { + const headers = { 'x-amz-object-attributes': 'ObjectParts' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['ObjectParts']); + }); + + it('should return array with single valid attribute Checksum', () => { + const headers = { 'x-amz-object-attributes': 'Checksum' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['Checksum']); + }); + + it('should return array with multiple valid attributes', () => { + const headers = { 'x-amz-object-attributes': 'ETag,ObjectSize,StorageClass' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['ETag', 'ObjectSize', 'StorageClass']); + }); + + it('should return array with all valid attributes', () => { + const headers = { 'x-amz-object-attributes': 'StorageClass,ObjectSize,ObjectParts,Checksum,ETag' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.strictEqual(result.length, 5); + assert(result.includes('StorageClass')); + assert(result.includes('ObjectSize')); + assert(result.includes('ObjectParts')); + assert(result.includes('Checksum')); + assert(result.includes('ETag')); + }); + }); + + describe('whitespace handling', () => { + it('should trim whitespace around attribute names', () => { + const headers = { 'x-amz-object-attributes': ' ETag , ObjectSize ' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['ETag', 'ObjectSize']); + }); + + it('should handle extra commas between attributes', () => { + const headers = { 'x-amz-object-attributes': 'ETag,,ObjectSize' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['ETag', 'ObjectSize']); + }); + + it('should handle leading and trailing commas', () => { + const headers = { 'x-amz-object-attributes': ',ETag,ObjectSize,' }; + const result = parseAttributesHeaders(headers); + + assert(Array.isArray(result)); + assert.deepStrictEqual(result, ['ETag', 'ObjectSize']); + }); + }); +}); diff --git a/tests/unit/api/objectGetAttributes.js b/tests/unit/api/objectGetAttributes.js new file mode 100644 index 0000000000..702a3863c6 --- /dev/null +++ b/tests/unit/api/objectGetAttributes.js @@ -0,0 +1,672 @@ +const assert = require('assert'); +const async = require('async'); +const crypto = require('crypto'); +const { parseString } = require('xml2js'); + +const { bucketPut } = require('../../../lib/api/bucketPut'); +const bucketPutVersioning = require('../../../lib/api/bucketPutVersioning'); +const { cleanup, DummyRequestLogger, makeAuthInfo, versioningTestUtils } = require('../helpers'); +const completeMultipartUpload = require('../../../lib/api/completeMultipartUpload'); +const DummyRequest = require('../DummyRequest'); +const initiateMultipartUpload = require('../../../lib/api/initiateMultipartUpload'); +const objectPut = require('../../../lib/api/objectPut'); +const { objectDelete } = require('../../../lib/api/objectDelete'); +const objectGetAttributes = require('../../../lib/api/objectGetAttributes'); +const objectPutPart = require('../../../lib/api/objectPutPart'); + +const log = new DummyRequestLogger(); +const authInfo = makeAuthInfo('accessKey1'); +const namespace = 'default'; +const bucketName = 'bucketname'; +const objectName = 'objectName'; +const body = 'hello world!'; +const postBody = Buffer.from(body, 'utf8'); +const expectedMD5 = 'fc3ff98e8c6a0d3087d515c0473f8677'; + +describe('objectGetAttributes API', () => { + let testPutObjectRequest; + + const testPutBucketRequest = { + bucketName, + namespace, + headers: { host: `${bucketName}.s3.amazonaws.com` }, + url: `/${bucketName}`, + actionImplicitDenies: false, + }; + + beforeEach(() => { + cleanup(); + testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + }); + + const createGetAttributesRequest = (attributes, options = {}) => ({ + bucketName, + namespace, + objectKey: options.objectKey || objectName, + headers: { + 'x-amz-object-attributes': attributes.join(','), + ...options.headers, + }, + url: `/${bucketName}/${options.objectKey || objectName}`, + query: options.query || {}, + actionImplicitDenies: false, + }); + + it('should fail because attributes header is missing', done => { + const testGetRequest = { + bucketName, + namespace, + objectKey: objectName, + headers: {}, + url: `/${bucketName}/${objectName}`, + query: {}, + actionImplicitDenies: false, + }; + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, err => { + assert.strictEqual(err.is.InvalidRequest, true); + assert.strictEqual( + err.description, + 'The x-amz-object-attributes header specifying the attributes ' + + 'to be retrieved is either missing or empty', + ); + done(); + }); + }); + }); + }); + + it('should fail because attributes header is empty', done => { + const testGetRequest = { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'x-amz-object-attributes': '', + }, + url: `/${bucketName}/${objectName}`, + query: {}, + actionImplicitDenies: false, + }; + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, err => { + assert.strictEqual(err.is.InvalidRequest, true); + assert.strictEqual( + err.description, + 'The x-amz-object-attributes header specifying the attributes ' + + 'to be retrieved is either missing or empty', + ); + done(); + }); + }); + }); + }); + + it('should fail because attribute name is invalid', done => { + const testGetRequest = createGetAttributesRequest(['InvalidAttribute']); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, err => { + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); + done(); + }); + }); + }); + }); + + it('should return NoSuchKey for non-existent object', done => { + const testGetRequest = createGetAttributesRequest(['ETag'], { + objectKey: 'nonexistent', + }); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectGetAttributes(authInfo, testGetRequest, log, err => { + assert.strictEqual(err.is.NoSuchKey, true); + assert.strictEqual(err.description, 'The specified key does not exist.'); + done(); + }); + }); + }); + + it('should fail because of bad bucket owner', done => { + const testGetRequest = createGetAttributesRequest(['ETag'], { + headers: { + 'x-amz-expected-bucket-owner': 'wrongAccountId', + }, + }); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, err => { + assert.strictEqual(err.is.AccessDenied, true); + assert.strictEqual(err.description, 'Access Denied'); + done(); + }); + }); + }); + }); + + it('should return all attributes', done => { + const testGetRequest = createGetAttributesRequest([ + 'ETag', + 'Checksum', + 'ObjectParts', + 'StorageClass', + 'ObjectSize', + ]); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, (err, xml, headers) => { + assert.ifError(err); + assert(xml, 'Response XML should be present'); + assert(headers['Last-Modified'], 'Last-Modified header should be present'); + + parseString(xml, (err, result) => { + const response = result.GetObjectAttributesResponse; + + assert.ifError(err); + assert.strictEqual(response.ETag[0], expectedMD5); + assert.strictEqual(response.StorageClass[0], 'STANDARD'); + assert.strictEqual(response.ObjectSize[0], String(body.length)); + assert(response.Checksum, 'Checksum should be present'); + assert(!response.ObjectParts, 'ObjectParts should not be present for non-MPU object'); + assert(headers['Last-Modified'], 'LastModified should be present'); + done(); + }); + }); + }); + }); + }); + + it('should return ETag', done => { + const testGetRequest = createGetAttributesRequest(['ETag']); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { + assert.ifError(err); + parseString(xml, (err, result) => { + assert.ifError(err); + assert.strictEqual(result.GetObjectAttributesResponse.ETag[0], expectedMD5); + done(); + }); + }); + }); + }); + }); + + it('should return Checksum', done => { + const testGetRequest = createGetAttributesRequest(['Checksum']); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { + assert.ifError(err); + parseString(xml, (err, result) => { + assert.ifError(err); + assert(result.GetObjectAttributesResponse.Checksum, 'Checksum should be present'); + done(); + }); + }); + }); + }); + }); + + it('should not return ObjectParts for non-MPU object', done => { + const testGetRequest = createGetAttributesRequest(['ObjectParts']); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { + assert.ifError(err); + parseString(xml, (err, result) => { + assert.ifError(err); + assert(!result.GetObjectAttributesResponse.ObjectParts, 'ObjectParts should not be present'); + done(); + }); + }); + }); + }); + }); + + it('should return StorageClass', done => { + const testGetRequest = createGetAttributesRequest(['StorageClass']); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { + assert.ifError(err); + parseString(xml, (err, result) => { + assert.ifError(err); + assert.strictEqual(result.GetObjectAttributesResponse.StorageClass[0], 'STANDARD'); + done(); + }); + }); + }); + }); + }); + + it('should return ObjectSize', done => { + const testGetRequest = createGetAttributesRequest(['ObjectSize']); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { + assert.ifError(err); + parseString(xml, (err, result) => { + assert.ifError(err); + assert.strictEqual(result.GetObjectAttributesResponse.ObjectSize[0], String(body.length)); + done(); + }); + }); + }); + }); + }); + + it('should return LastModified in response headers', done => { + const testGetRequest = createGetAttributesRequest(['ETag']); + + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + objectGetAttributes(authInfo, testGetRequest, log, (err, _xml, headers) => { + assert.ifError(err); + assert(headers['Last-Modified'], 'Last-Modified should be present'); + assert(!isNaN(new Date(headers['Last-Modified']).getTime()), 'Last-Modified should be a valid date'); + done(); + }); + }); + }); + }); +}); + +describe('objectGetAttributes API with multipart upload', () => { + const mpuObjectName = 'mpuObject'; + const partCount = 2; + const partBody = Buffer.from('I am a part\n', 'utf8'); + + const testPutBucketRequest = { + bucketName, + namespace, + headers: { host: `${bucketName}.s3.amazonaws.com` }, + url: `/${bucketName}`, + actionImplicitDenies: false, + }; + + beforeEach(done => { + cleanup(); + bucketPut(authInfo, testPutBucketRequest, log, done); + }); + + const createMpuObject = callback => { + const initiateRequest = { + bucketName, + namespace, + objectKey: mpuObjectName, + headers: { host: `${bucketName}.s3.amazonaws.com` }, + url: `/${mpuObjectName}?uploads`, + actionImplicitDenies: false, + }; + + async.waterfall( + [ + next => initiateMultipartUpload(authInfo, initiateRequest, log, next), + (result, _corsHeaders, next) => parseString(result, next), + (json, next) => { + const testUploadId = json.InitiateMultipartUploadResult.UploadId[0]; + const partHash = crypto.createHash('md5').update(partBody).digest('hex'); + + // Upload first part (minimum 5MB for non-last part) + const part1Request = new DummyRequest( + { + bucketName, + namespace, + objectKey: mpuObjectName, + headers: { + host: `${bucketName}.s3.amazonaws.com`, + 'content-length': '5242880', + }, + parsedContentLength: 5242880, + url: `/${mpuObjectName}?partNumber=1&uploadId=${testUploadId}`, + query: { + partNumber: '1', + uploadId: testUploadId, + }, + partHash, + }, + partBody, + ); + + objectPutPart(authInfo, part1Request, undefined, log, () => { + next(null, testUploadId, partHash); + }); + }, + (testUploadId, partHash, next) => { + // Upload second part + const part2Request = new DummyRequest( + { + bucketName, + namespace, + objectKey: mpuObjectName, + headers: { + host: `${bucketName}.s3.amazonaws.com`, + 'content-length': `${partBody.length}`, + }, + parsedContentLength: partBody.length, + url: `/${mpuObjectName}?partNumber=2&uploadId=${testUploadId}`, + query: { + partNumber: '2', + uploadId: testUploadId, + }, + partHash, + }, + partBody, + ); + + objectPutPart(authInfo, part2Request, undefined, log, () => { + next(null, testUploadId, partHash); + }); + }, + (testUploadId, partHash, next) => { + // Complete the multipart upload + const completeBody = + '' + + '' + + '1' + + `"${partHash}"` + + '' + + '' + + '2' + + `"${partHash}"` + + '' + + ''; + + const completeRequest = { + bucketName, + namespace, + objectKey: mpuObjectName, + parsedHost: 's3.amazonaws.com', + url: `/${mpuObjectName}?uploadId=${testUploadId}`, + headers: { host: `${bucketName}.s3.amazonaws.com` }, + query: { uploadId: testUploadId }, + post: completeBody, + actionImplicitDenies: false, + }; + + completeMultipartUpload(authInfo, completeRequest, log, err => { + next(err); + }); + }, + ], + callback, + ); + }; + + const createGetAttributesRequest = attributes => ({ + bucketName, + namespace, + objectKey: mpuObjectName, + headers: { + 'x-amz-object-attributes': attributes.join(','), + }, + url: `/${bucketName}/${mpuObjectName}`, + query: {}, + actionImplicitDenies: false, + }); + + it('should return TotalPartsCount for MPU object', done => { + createMpuObject(err => { + assert.ifError(err); + const testGetRequest = createGetAttributesRequest(['ObjectParts']); + + objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { + assert.ifError(err); + parseString(xml, (err, result) => { + const response = result.GetObjectAttributesResponse; + + assert.ifError(err); + assert(response.ObjectParts, 'ObjectParts should be present'); + assert.strictEqual(response.ObjectParts[0].PartsCount[0], String(partCount)); + done(); + }); + }); + }); + }); + + it('should return TotalPartsCount along with other attributes for MPU object', done => { + createMpuObject(err => { + assert.ifError(err); + const testGetRequest = createGetAttributesRequest(['ETag', 'ObjectParts', 'ObjectSize', 'StorageClass']); + + objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { + assert.ifError(err); + parseString(xml, (err, result) => { + const response = result.GetObjectAttributesResponse; + + assert.ifError(err); + assert(response.ETag, 'ETag should be present'); + assert(response.ETag[0].includes(`-${partCount}`), `ETag should indicate MPU with ${partCount} parts`); + assert(response.ObjectParts, 'ObjectParts should be present'); + assert.strictEqual(response.ObjectParts[0].PartsCount[0], String(partCount)); + assert(response.ObjectSize, 'ObjectSize should be present'); + assert.strictEqual(response.StorageClass[0], 'STANDARD'); + done(); + }); + }); + }); + }); +}); + +describe('objectGetAttributes API with versioning', () => { + const enableVersioningRequest = versioningTestUtils.createBucketPutVersioningReq(bucketName, 'Enabled'); + + const testPutBucketRequest = { + bucketName, + namespace, + headers: { host: `${bucketName}.s3.amazonaws.com` }, + url: `/${bucketName}`, + actionImplicitDenies: false, + }; + + beforeEach(done => { + cleanup(); + async.series( + [ + next => bucketPut(authInfo, testPutBucketRequest, log, next), + next => bucketPutVersioning(authInfo, enableVersioningRequest, log, next), + ], + done, + ); + }); + + const createGetAttributesRequest = (attributes, options = {}) => ({ + bucketName, + namespace, + objectKey: options.objectKey || objectName, + headers: { + 'x-amz-object-attributes': attributes.join(','), + ...options.headers, + }, + url: `/${bucketName}/${options.objectKey || objectName}`, + query: options.query || {}, + actionImplicitDenies: false, + }); + + it('should return NoSuchVersion for non-existent versionId', done => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + + // Use a properly formatted but non-existent version ID + const fakeVersionId = '111111111111111111111111111111111111111175636f7270'; + + objectPut(authInfo, testPutObjectRequest, undefined, log, err => { + assert.ifError(err); + const testGetRequest = createGetAttributesRequest(['ETag'], { + query: { versionId: fakeVersionId }, + }); + + objectGetAttributes(authInfo, testGetRequest, log, err => { + assert.strictEqual(err.is.NoSuchVersion, true); + assert.strictEqual( + err.description, + 'Indicates that the version ID specified in the request does not match an existing version.', + ); + done(); + }); + }); + }); + + it('should return MethodNotAllowed for delete marker', done => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + + const testDeleteRequest = { + bucketName, + namespace, + objectKey: objectName, + headers: {}, + url: `/${bucketName}/${objectName}`, + actionImplicitDenies: false, + }; + + async.series( + [ + next => objectPut(authInfo, testPutObjectRequest, undefined, log, next), + next => objectDelete(authInfo, testDeleteRequest, log, next), + ], + err => { + assert.ifError(err); + // Request without versionId targets the delete marker + const testGetRequest = createGetAttributesRequest(['ETag']); + + objectGetAttributes(authInfo, testGetRequest, log, (err, _xml, headers) => { + assert.strictEqual(err.is.MethodNotAllowed, true); + assert.strictEqual(err.description, 'The specified method is not allowed against this resource.'); + assert.strictEqual(headers['x-amz-delete-marker'], true); + done(); + }); + }, + ); + }); + + it('should return attributes for specific version', done => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + + objectPut(authInfo, testPutObjectRequest, undefined, log, (err, resHeaders) => { + assert.ifError(err); + const versionId = resHeaders['x-amz-version-id']; + assert(versionId, 'Version ID should be present'); + + const testGetRequest = createGetAttributesRequest(['ETag', 'ObjectSize'], { + query: { versionId }, + }); + + objectGetAttributes(authInfo, testGetRequest, log, (err, xml, headers) => { + assert.ifError(err); + assert(headers['Last-Modified'], 'Last-Modified should be present'); + + parseString(xml, (err, result) => { + const response = result.GetObjectAttributesResponse; + + assert.ifError(err); + assert.strictEqual(response.ETag[0], expectedMD5); + assert.strictEqual(response.ObjectSize[0], String(body.length)); + done(); + }); + }); + }); + }); + + it('should return VersionId in response headers for versioned object', done => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + + objectPut(authInfo, testPutObjectRequest, undefined, log, (err, resHeaders) => { + assert.ifError(err); + const versionId = resHeaders['x-amz-version-id']; + assert(versionId, 'Version ID should be present from PUT'); + + const testGetRequest = createGetAttributesRequest(['ETag']); + + objectGetAttributes(authInfo, testGetRequest, log, (err, _xml, headers) => { + assert.ifError(err); + assert.strictEqual(headers['x-amz-version-id'], versionId); + done(); + }); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 2dfce61f24..cb032cd9e2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1495,9 +1495,9 @@ arraybuffer.prototype.slice@^1.0.4: optionalDependencies: ioctl "^2.0.2" -"arsenal@git+https://github.com/scality/arsenal#8.2.44": +"arsenal@git+https://github.com/scality/Arsenal#feature/ARSN-549/get-object-attributes": version "8.2.44" - resolved "git+https://github.com/scality/arsenal#960e77028b6eb614dde297e50a1530dcd9015f16" + resolved "git+https://github.com/scality/Arsenal#f0e0fea7ae19df55ce0239b8475d7594a358c253" dependencies: "@azure/identity" "^4.13.0" "@azure/storage-blob" "^12.28.0" From 15650ee3bb009bac251253ec0e869e440c746c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Wed, 28 Jan 2026 15:12:28 +0100 Subject: [PATCH 02/17] fixup! Support the new API GetObjectAttributes --- lib/api/objectGetAttributes.js | 115 +++++++++--------- lib/metadata/metadataUtils.js | 11 ++ .../test/object/objectGetAttributes.js | 19 ++- .../test/versioning/objectGetAttributes.js | 3 - tests/unit/api/objectGetAttributes.js | 5 - yarn.lock | 10 +- 6 files changed, 78 insertions(+), 85 deletions(-) diff --git a/lib/api/objectGetAttributes.js b/lib/api/objectGetAttributes.js index 8c232dd2f1..86f09fe31f 100644 --- a/lib/api/objectGetAttributes.js +++ b/lib/api/objectGetAttributes.js @@ -12,25 +12,7 @@ const { getPartCountFromMd5 } = require('./apiUtils/object/partInfo'); const OBJECT_GET_ATTRIBUTES = 'objectGetAttributes'; const checkExpectedBucketOwnerPromise = promisify(checkExpectedBucketOwner); - -/** - * validateBucketAndObjPromise - Promisified wrapper for standardMetadataValidateBucketAndObj - * @param {object} params - validation parameters - * @param {boolean} actionImplicitDenies - whether action has implicit denies - * @param {object} log - Werelogs logger - * @returns {Promise<{bucket: BucketInfo, objMD: object}>} - bucket and object metadata - * @throws {Error} - rejects with error from standardMetadataValidateBucketAndObj - */ -function validateBucketAndObjPromise(params, actionImplicitDenies, log) { - return new Promise((resolve, reject) => { - standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log, (err, bucket, objMD) => { - if (err) { - return reject(err); - } - return resolve({ bucket, objMD }); - }); - }); -} +const validateBucketAndObj = promisify(standardMetadataValidateBucketAndObj); /** * buildXmlResponse - Build XML response for GetObjectAttributes @@ -74,15 +56,15 @@ function buildXmlResponse(objMD, attributes) { * @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} + * @returns {Promise} - { xml, responseHeaders } + * @throws {ArsenalError} NoSuchVersion - if versionId specified but not found + * @throws {ArsenalError} NoSuchKey - if object not found + * @throws {ArsenalError} MethodNotAllowed - if object is a delete marker */ -async function objectGetAttributes(authInfo, request, log, callback) { +async function objectGetAttributes(authInfo, request, log) { log.trace('processing request', { method: OBJECT_GET_ATTRIBUTES }); const { bucketName, objectKey, headers, actionImplicitDenies } = request; - let responseHeaders = {}; - const versionId = decodeVersionId(request.query); if (versionId instanceof Error) { log.debug('invalid versionId query', { versionId: request.query.versionId, error: versionId }); @@ -99,48 +81,61 @@ async function objectGetAttributes(authInfo, request, log, callback) { request, }; - try { - const { bucket, objMD } = await validateBucketAndObjPromise(metadataValParams, actionImplicitDenies, log); - await checkExpectedBucketOwnerPromise(headers, bucket, log); + const { bucket, objMD } = await validateBucketAndObj(metadataValParams, actionImplicitDenies, log); + await checkExpectedBucketOwnerPromise(headers, bucket, log); - responseHeaders = collectCorsHeaders(headers.origin, request.method, bucket); - if (objMD) { - responseHeaders['x-amz-version-id'] = getVersionIdResHeader(bucket.getVersioningConfiguration(), objMD); - responseHeaders['Last-Modified'] = objMD['last-modified'] && new Date(objMD['last-modified']).toUTCString(); - } + const responseHeaders = collectCorsHeaders(headers.origin, request.method, bucket); - if (!objMD) { - const err = versionId ? errors.NoSuchVersion : errors.NoSuchKey; - log.debug('object not found', { bucket: bucketName, key: objectKey, versionId }); - throw err; - } + if (!objMD) { + const err = versionId ? errors.NoSuchVersion : errors.NoSuchKey; + log.debug('object not found', { bucket: bucketName, key: objectKey, versionId }); + err.responseHeaders = responseHeaders; + throw err; + } - if (objMD.isDeleteMarker) { - log.debug('attempt to get attributes of a delete marker', { bucket: bucketName, key: objectKey, versionId }); - responseHeaders['x-amz-delete-marker'] = true; - throw errors.MethodNotAllowed; - } + responseHeaders['x-amz-version-id'] = getVersionIdResHeader(bucket.getVersioningConfiguration(), objMD); + responseHeaders['Last-Modified'] = objMD['last-modified'] && new Date(objMD['last-modified']).toUTCString(); + + if (objMD.isDeleteMarker) { + log.debug('attempt to get attributes of a delete marker', { bucket: bucketName, key: objectKey, versionId }); + responseHeaders['x-amz-delete-marker'] = true; + const err = errors.MethodNotAllowed; + err.responseHeaders = responseHeaders; + throw err; + } - const attributes = parseAttributesHeaders(headers); + const attributes = parseAttributesHeaders(headers); - pushMetric(OBJECT_GET_ATTRIBUTES, log, { - authInfo, - bucket: bucketName, - keys: [objectKey], - versionId: objMD?.versionId, - location: objMD?.dataStoreName, - }); + pushMetric(OBJECT_GET_ATTRIBUTES, log, { + authInfo, + bucket: bucketName, + keys: [objectKey], + versionId: objMD?.versionId, + location: objMD?.dataStoreName, + }); - const xml = buildXmlResponse(objMD, attributes); - return callback(null, xml, responseHeaders); - } catch (err) { - log.debug('error processing request', { - error: err, - method: OBJECT_GET_ATTRIBUTES, - }); + const xml = buildXmlResponse(objMD, attributes); + return { xml, responseHeaders }; +} - return callback(err, null, responseHeaders); - } +/** + * objectGetAttributesCallback - Callback wrapper for objectGetAttributes + * @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 (err, xml, responseHeaders) + * @return {undefined} + */ +function objectGetAttributesCallback(authInfo, request, log, callback) { + objectGetAttributes(authInfo, request, log) + .then(result => callback(null, result.xml, result.responseHeaders)) + .catch(err => { + log.debug('error processing request', { + error: err, + method: OBJECT_GET_ATTRIBUTES, + }); + return callback(err, null, err.responseHeaders || {}); + }); } -module.exports = objectGetAttributes; +module.exports = objectGetAttributesCallback; diff --git a/lib/metadata/metadataUtils.js b/lib/metadata/metadataUtils.js index 442af70e0a..1360baf1f7 100644 --- a/lib/metadata/metadataUtils.js +++ b/lib/metadata/metadataUtils.js @@ -1,4 +1,5 @@ const async = require('async'); +const { promisify } = require('util'); const { errors } = require('arsenal'); const metadata = require('./wrapper'); @@ -281,6 +282,16 @@ function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log, return callback(null, bucket, objMD); }); } + +standardMetadataValidateBucketAndObj[promisify.custom] = (params, action, log) => new Promise((resolve, reject) => { + standardMetadataValidateBucketAndObj(params, action, log, (err, bucket, objMD) => { + if (err) { + return reject(err); + } + return resolve({ bucket, objMD }); + }); +}); + /** standardMetadataValidateBucket - retrieve bucket from metadata and check if user * is authorized to access it * @param {object} params - function parameters diff --git a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js index b969e9926b..6af868181c 100644 --- a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js +++ b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js @@ -7,7 +7,7 @@ const key = 'testobject'; const body = 'hello world!'; const expectedMD5 = 'fc3ff98e8c6a0d3087d515c0473f8677'; -describe('Test get object attributes', () => { +describe('objectGetAttributes', () => { let s3; before(() => { @@ -25,7 +25,7 @@ describe('Test get object attributes', () => { await s3.deleteBucket({ Bucket: bucket }).promise(); }); - it('should fail because a bad bucket owner', async () => { + it('should fail with a wrong bucket owner header', async () => { try { await s3 .getObjectAttributes({ @@ -105,8 +105,8 @@ describe('Test get object attributes', () => { assert.strictEqual(data.ETag, expectedMD5); assert.strictEqual(data.StorageClass, 'STANDARD'); assert.strictEqual(data.ObjectSize, body.length); - assert(data.Checksum, 'Checksum should be present'); - assert(!data.ObjectParts, "ObjectParts shouldn't be present for non-MPU object"); + assert.deepStrictEqual(data.Checksum, {}, 'Checksum should be present'); + assert.strictEqual(data.ObjectParts, undefined, "ObjectParts shouldn't be present for non-MPU object"); assert(data.LastModified, 'LastModified should be present'); }); @@ -131,10 +131,10 @@ describe('Test get object attributes', () => { }) .promise(); - assert(data.Checksum, 'Checksum should be present'); + assert.deepStrictEqual(data.Checksum, {}, 'Checksum should be present'); }); - it("shouldn't return ObjectParts", async () => { + it("shouldn't return ObjectParts for non-MPU objects", async () => { const data = await s3 .getObjectAttributes({ Bucket: bucket, @@ -143,8 +143,7 @@ describe('Test get object attributes', () => { }) .promise(); - // ObjectParts may be empty for non-MPU objects - assert(!data.ObjectParts, "ObjectParts shouldn't be present"); + assert.strictEqual(data.ObjectParts, undefined, "ObjectParts shouldn't be present"); }); it('should return StorageClass', async () => { @@ -196,10 +195,8 @@ describe('Test get object attributes with multipart upload', () => { const config = getConfig('default', { signatureVersion: 'v4' }); s3 = new S3(config); - // Create bucket await s3.createBucket({ Bucket: bucket }).promise(); - // Create multipart upload const createResult = await s3 .createMultipartUpload({ Bucket: bucket, @@ -208,7 +205,6 @@ describe('Test get object attributes with multipart upload', () => { .promise(); const uploadId = createResult.UploadId; - // Upload parts const partData = Buffer.alloc(partSize, 'a'); const parts = []; for (let i = 1; i <= partCount; i++) { @@ -224,7 +220,6 @@ describe('Test get object attributes with multipart upload', () => { parts.push({ PartNumber: i, ETag: uploadResult.ETag }); } - // Complete multipart upload await s3 .completeMultipartUpload({ Bucket: bucket, diff --git a/tests/functional/aws-node-sdk/test/versioning/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/versioning/objectGetAttributes.js index 92c1308de6..8b932cdfae 100644 --- a/tests/functional/aws-node-sdk/test/versioning/objectGetAttributes.js +++ b/tests/functional/aws-node-sdk/test/versioning/objectGetAttributes.js @@ -43,7 +43,6 @@ describe('Test get object attributes with versioning', () => { }) .promise(); - // Use a properly formatted but non-existent version ID const fakeVersionId = '111111111111111111111111111111111111111175636f7270'; try { @@ -74,7 +73,6 @@ describe('Test get object attributes with versioning', () => { }) .promise(); - // Delete creates a delete marker await s3 .deleteObject({ Bucket: bucket, @@ -82,7 +80,6 @@ describe('Test get object attributes with versioning', () => { }) .promise(); - // Request without versionId targets the delete marker try { await s3 .getObjectAttributes({ diff --git a/tests/unit/api/objectGetAttributes.js b/tests/unit/api/objectGetAttributes.js index 702a3863c6..ba868af59a 100644 --- a/tests/unit/api/objectGetAttributes.js +++ b/tests/unit/api/objectGetAttributes.js @@ -345,7 +345,6 @@ describe('objectGetAttributes API with multipart upload', () => { const testUploadId = json.InitiateMultipartUploadResult.UploadId[0]; const partHash = crypto.createHash('md5').update(partBody).digest('hex'); - // Upload first part (minimum 5MB for non-last part) const part1Request = new DummyRequest( { bucketName, @@ -371,7 +370,6 @@ describe('objectGetAttributes API with multipart upload', () => { }); }, (testUploadId, partHash, next) => { - // Upload second part const part2Request = new DummyRequest( { bucketName, @@ -397,7 +395,6 @@ describe('objectGetAttributes API with multipart upload', () => { }); }, (testUploadId, partHash, next) => { - // Complete the multipart upload const completeBody = '' + '' + @@ -536,7 +533,6 @@ describe('objectGetAttributes API with versioning', () => { postBody, ); - // Use a properly formatted but non-existent version ID const fakeVersionId = '111111111111111111111111111111111111111175636f7270'; objectPut(authInfo, testPutObjectRequest, undefined, log, err => { @@ -587,7 +583,6 @@ describe('objectGetAttributes API with versioning', () => { ], err => { assert.ifError(err); - // Request without versionId targets the delete marker const testGetRequest = createGetAttributesRequest(['ETag']); objectGetAttributes(authInfo, testGetRequest, log, (err, _xml, headers) => { diff --git a/yarn.lock b/yarn.lock index cb032cd9e2..ec0fd06486 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1496,8 +1496,8 @@ arraybuffer.prototype.slice@^1.0.4: ioctl "^2.0.2" "arsenal@git+https://github.com/scality/Arsenal#feature/ARSN-549/get-object-attributes": - version "8.2.44" - resolved "git+https://github.com/scality/Arsenal#f0e0fea7ae19df55ce0239b8475d7594a358c253" + version "8.2.43" + resolved "git+https://github.com/scality/Arsenal#ec21fa885c611498584ef3c56bfd62047c640e9e" dependencies: "@azure/identity" "^4.13.0" "@azure/storage-blob" "^12.28.0" @@ -3791,9 +3791,9 @@ ioredis@^5.6.1: standard-as-callback "^2.1.0" ioredis@^5.8.1: - version "5.8.2" - resolved "https://registry.yarnpkg.com/ioredis/-/ioredis-5.8.2.tgz#c7a228a26cf36f17a5a8011148836877780e2e14" - integrity sha512-C6uC+kleiIMmjViJINWk80sOQw5lEzse1ZmvD+S/s8p8CWapftSaC+kocGTx6xrbrJ4WmYQGC08ffHLr6ToR6Q== + version "5.9.2" + resolved "https://registry.yarnpkg.com/ioredis/-/ioredis-5.9.2.tgz#ffdce2a019950299716e88ee56cd5802b399b108" + integrity sha512-tAAg/72/VxOUW7RQSX1pIxJVucYKcjFjfvj60L57jrZpYCHC3XN0WCQ3sNYL4Gmvv+7GPvTAjc+KSdeNuE8oWQ== dependencies: "@ioredis/commands" "1.4.0" cluster-key-slot "^1.1.0" From c7a132b5c73c23e6be9f8110167146f533cb9115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Wed, 28 Jan 2026 18:04:38 +0100 Subject: [PATCH 03/17] fixup! Support the new API GetObjectAttributes --- tests/unit/api/objectGetAttributes.js | 969 +++++++++++--------------- 1 file changed, 397 insertions(+), 572 deletions(-) diff --git a/tests/unit/api/objectGetAttributes.js b/tests/unit/api/objectGetAttributes.js index ba868af59a..33c678be4f 100644 --- a/tests/unit/api/objectGetAttributes.js +++ b/tests/unit/api/objectGetAttributes.js @@ -1,7 +1,6 @@ const assert = require('assert'); -const async = require('async'); const crypto = require('crypto'); -const { parseString } = require('xml2js'); +const { parseStringPromise } = require('xml2js'); const { bucketPut } = require('../../../lib/api/bucketPut'); const bucketPutVersioning = require('../../../lib/api/bucketPutVersioning'); @@ -23,645 +22,471 @@ const body = 'hello world!'; const postBody = Buffer.from(body, 'utf8'); const expectedMD5 = 'fc3ff98e8c6a0d3087d515c0473f8677'; -describe('objectGetAttributes API', () => { - let testPutObjectRequest; +// Promisify helper for functions with non-standard callback signatures +const promisify = fn => (...args) => new Promise((resolve, reject) => { + fn(...args, (err, ...results) => { + if (err) { + reject(err); + } else { + resolve(results); + } + }); +}); - const testPutBucketRequest = { +const bucketPutAsync = promisify(bucketPut); +const bucketPutVersioningAsync = promisify(bucketPutVersioning); +const objectPutAsync = promisify(objectPut); +const objectDeleteAsync = promisify(objectDelete); +const objectGetAttributesAsync = promisify(objectGetAttributes); +const initiateMultipartUploadAsync = promisify(initiateMultipartUpload); +const objectPutPartAsync = promisify(objectPutPart); +const completeMultipartUploadAsync = promisify(completeMultipartUpload); + +const testPutBucketRequest = { bucketName, namespace, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${bucketName}`, actionImplicitDenies: false, - }; +}; - beforeEach(() => { - cleanup(); - testPutObjectRequest = new DummyRequest( - { +const createGetAttributesRequest = (attributes, options = {}) => { + const key = options.objectKey || objectName; + return { bucketName, namespace, - objectKey: objectName, + objectKey: key, headers: { - 'content-length': `${postBody.length}`, + 'x-amz-object-attributes': attributes.join(','), + ...options.headers, }, - parsedContentLength: postBody.length, - url: `/${bucketName}/${objectName}`, - }, - postBody, - ); - }); - - const createGetAttributesRequest = (attributes, options = {}) => ({ - bucketName, - namespace, - objectKey: options.objectKey || objectName, - headers: { - 'x-amz-object-attributes': attributes.join(','), - ...options.headers, - }, - url: `/${bucketName}/${options.objectKey || objectName}`, - query: options.query || {}, - actionImplicitDenies: false, - }); - - it('should fail because attributes header is missing', done => { - const testGetRequest = { - bucketName, - namespace, - objectKey: objectName, - headers: {}, - url: `/${bucketName}/${objectName}`, - query: {}, - actionImplicitDenies: false, + url: `/${bucketName}/${key}`, + query: options.query || {}, + actionImplicitDenies: false, }; +}; - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, err => { - assert.strictEqual(err.is.InvalidRequest, true); - assert.strictEqual( - err.description, - 'The x-amz-object-attributes header specifying the attributes ' + - 'to be retrieved is either missing or empty', - ); - done(); - }); - }); +describe('objectGetAttributes API', () => { + beforeEach(async () => { + cleanup(); + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + await bucketPutAsync(authInfo, testPutBucketRequest, log); + await objectPutAsync(authInfo, testPutObjectRequest, undefined, log); }); - }); - - it('should fail because attributes header is empty', done => { - const testGetRequest = { - bucketName, - namespace, - objectKey: objectName, - headers: { - 'x-amz-object-attributes': '', - }, - url: `/${bucketName}/${objectName}`, - query: {}, - actionImplicitDenies: false, - }; - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, err => { - assert.strictEqual(err.is.InvalidRequest, true); - assert.strictEqual( - err.description, - 'The x-amz-object-attributes header specifying the attributes ' + - 'to be retrieved is either missing or empty', - ); - done(); - }); - }); - }); - }); - - it('should fail because attribute name is invalid', done => { - const testGetRequest = createGetAttributesRequest(['InvalidAttribute']); - - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, err => { - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - done(); - }); - }); + it('should fail because attributes header is missing', async () => { + const testGetRequest = { + bucketName, + namespace, + objectKey: objectName, + headers: {}, + url: `/${bucketName}/${objectName}`, + query: {}, + actionImplicitDenies: false, + }; + + try { + await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert.fail('Expected error was not thrown'); + } catch (err) { + assert.strictEqual(err.is.InvalidRequest, true); + assert.strictEqual( + err.description, + 'The x-amz-object-attributes header specifying the attributes ' + + 'to be retrieved is either missing or empty', + ); + } }); - }); - it('should return NoSuchKey for non-existent object', done => { - const testGetRequest = createGetAttributesRequest(['ETag'], { - objectKey: 'nonexistent', + it('should fail because attributes header is empty', async () => { + const testGetRequest = { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'x-amz-object-attributes': '', + }, + url: `/${bucketName}/${objectName}`, + query: {}, + actionImplicitDenies: false, + }; + + try { + await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert.fail('Expected error was not thrown'); + } catch (err) { + assert.strictEqual(err.is.InvalidRequest, true); + assert.strictEqual( + err.description, + 'The x-amz-object-attributes header specifying the attributes ' + + 'to be retrieved is either missing or empty', + ); + } }); - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectGetAttributes(authInfo, testGetRequest, log, err => { - assert.strictEqual(err.is.NoSuchKey, true); - assert.strictEqual(err.description, 'The specified key does not exist.'); - done(); - }); - }); - }); + it('should fail because attribute name is invalid', async () => { + const testGetRequest = createGetAttributesRequest(['InvalidAttribute']); - it('should fail because of bad bucket owner', done => { - const testGetRequest = createGetAttributesRequest(['ETag'], { - headers: { - 'x-amz-expected-bucket-owner': 'wrongAccountId', - }, + try { + await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert.fail('Expected error was not thrown'); + } catch (err) { + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); + } }); - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, err => { - assert.strictEqual(err.is.AccessDenied, true); - assert.strictEqual(err.description, 'Access Denied'); - done(); - }); - }); - }); - }); - - it('should return all attributes', done => { - const testGetRequest = createGetAttributesRequest([ - 'ETag', - 'Checksum', - 'ObjectParts', - 'StorageClass', - 'ObjectSize', - ]); - - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, (err, xml, headers) => { - assert.ifError(err); - assert(xml, 'Response XML should be present'); - assert(headers['Last-Modified'], 'Last-Modified header should be present'); - - parseString(xml, (err, result) => { - const response = result.GetObjectAttributesResponse; - - assert.ifError(err); - assert.strictEqual(response.ETag[0], expectedMD5); - assert.strictEqual(response.StorageClass[0], 'STANDARD'); - assert.strictEqual(response.ObjectSize[0], String(body.length)); - assert(response.Checksum, 'Checksum should be present'); - assert(!response.ObjectParts, 'ObjectParts should not be present for non-MPU object'); - assert(headers['Last-Modified'], 'LastModified should be present'); - done(); - }); + it('should return NoSuchKey for non-existent object', async () => { + const testGetRequest = createGetAttributesRequest(['ETag'], { + objectKey: 'nonexistent', }); - }); + + try { + await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert.fail('Expected error was not thrown'); + } catch (err) { + assert.strictEqual(err.is.NoSuchKey, true); + assert.strictEqual(err.description, 'The specified key does not exist.'); + } }); - }); - - it('should return ETag', done => { - const testGetRequest = createGetAttributesRequest(['ETag']); - - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { - assert.ifError(err); - parseString(xml, (err, result) => { - assert.ifError(err); - assert.strictEqual(result.GetObjectAttributesResponse.ETag[0], expectedMD5); - done(); - }); + + it('should fail because of bad bucket owner', async () => { + const testGetRequest = createGetAttributesRequest(['ETag'], { + headers: { + 'x-amz-expected-bucket-owner': 'wrongAccountId', + }, }); - }); + + try { + await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert.fail('Expected error was not thrown'); + } catch (err) { + assert.strictEqual(err.is.AccessDenied, true); + assert.strictEqual(err.description, 'Access Denied'); + } }); - }); - - it('should return Checksum', done => { - const testGetRequest = createGetAttributesRequest(['Checksum']); - - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { - assert.ifError(err); - parseString(xml, (err, result) => { - assert.ifError(err); - assert(result.GetObjectAttributesResponse.Checksum, 'Checksum should be present'); - done(); - }); - }); - }); + + it('should return all attributes', async () => { + const testGetRequest = createGetAttributesRequest([ + 'ETag', + 'Checksum', + 'ObjectParts', + 'StorageClass', + 'ObjectSize', + ]); + + const [xml, headers] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert(xml, 'Response XML should be present'); + assert(headers['Last-Modified'], 'Last-Modified header should be present'); + + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response.ETag[0], expectedMD5); + assert.strictEqual(response.StorageClass[0], 'STANDARD'); + assert.strictEqual(response.ObjectSize[0], String(body.length)); + assert.deepStrictEqual(response.Checksum[0], '', 'Checksum should be empty'); + assert.strictEqual(response.ObjectParts, undefined, "ObjectParts shouldn't be present for non-MPU object"); + assert(headers['Last-Modified'], 'LastModified should be present'); }); - }); - - it('should not return ObjectParts for non-MPU object', done => { - const testGetRequest = createGetAttributesRequest(['ObjectParts']); - - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { - assert.ifError(err); - parseString(xml, (err, result) => { - assert.ifError(err); - assert(!result.GetObjectAttributesResponse.ObjectParts, 'ObjectParts should not be present'); - done(); - }); - }); - }); + + it('should return ETag', async () => { + const testGetRequest = createGetAttributesRequest(['ETag']); + + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + assert.strictEqual(result.GetObjectAttributesResponse.ETag[0], expectedMD5); }); - }); - - it('should return StorageClass', done => { - const testGetRequest = createGetAttributesRequest(['StorageClass']); - - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { - assert.ifError(err); - parseString(xml, (err, result) => { - assert.ifError(err); - assert.strictEqual(result.GetObjectAttributesResponse.StorageClass[0], 'STANDARD'); - done(); - }); - }); - }); + + it('should return Checksum', async () => { + const testGetRequest = createGetAttributesRequest(['Checksum']); + + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + assert.deepStrictEqual(result.GetObjectAttributesResponse.Checksum[0], '', 'Checksum should be empty'); }); - }); - - it('should return ObjectSize', done => { - const testGetRequest = createGetAttributesRequest(['ObjectSize']); - - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { - assert.ifError(err); - parseString(xml, (err, result) => { - assert.ifError(err); - assert.strictEqual(result.GetObjectAttributesResponse.ObjectSize[0], String(body.length)); - done(); - }); - }); - }); + + it("shouldn't return ObjectParts for non-MPU object", async () => { + const testGetRequest = createGetAttributesRequest(['ObjectParts']); + + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + assert.strictEqual( + result.GetObjectAttributesResponse.ObjectParts, + undefined, + "ObjectParts shouldn't be present", + ); }); - }); - - it('should return LastModified in response headers', done => { - const testGetRequest = createGetAttributesRequest(['ETag']); - - bucketPut(authInfo, testPutBucketRequest, log, () => { - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - objectGetAttributes(authInfo, testGetRequest, log, (err, _xml, headers) => { - assert.ifError(err); - assert(headers['Last-Modified'], 'Last-Modified should be present'); - assert(!isNaN(new Date(headers['Last-Modified']).getTime()), 'Last-Modified should be a valid date'); - done(); - }); - }); + + it('should return StorageClass', async () => { + const testGetRequest = createGetAttributesRequest(['StorageClass']); + + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + assert.strictEqual(result.GetObjectAttributesResponse.StorageClass[0], 'STANDARD'); }); - }); -}); -describe('objectGetAttributes API with multipart upload', () => { - const mpuObjectName = 'mpuObject'; - const partCount = 2; - const partBody = Buffer.from('I am a part\n', 'utf8'); + it('should return ObjectSize', async () => { + const testGetRequest = createGetAttributesRequest(['ObjectSize']); - const testPutBucketRequest = { - bucketName, - namespace, - headers: { host: `${bucketName}.s3.amazonaws.com` }, - url: `/${bucketName}`, - actionImplicitDenies: false, - }; - - beforeEach(done => { - cleanup(); - bucketPut(authInfo, testPutBucketRequest, log, done); - }); - - const createMpuObject = callback => { - const initiateRequest = { - bucketName, - namespace, - objectKey: mpuObjectName, - headers: { host: `${bucketName}.s3.amazonaws.com` }, - url: `/${mpuObjectName}?uploads`, - actionImplicitDenies: false, - }; + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + assert.strictEqual(result.GetObjectAttributesResponse.ObjectSize[0], String(body.length)); + }); - async.waterfall( - [ - next => initiateMultipartUpload(authInfo, initiateRequest, log, next), - (result, _corsHeaders, next) => parseString(result, next), - (json, next) => { - const testUploadId = json.InitiateMultipartUploadResult.UploadId[0]; - const partHash = crypto.createHash('md5').update(partBody).digest('hex'); + it('should return LastModified in response headers', async () => { + const testGetRequest = createGetAttributesRequest(['ETag']); - const part1Request = new DummyRequest( - { - bucketName, - namespace, - objectKey: mpuObjectName, - headers: { - host: `${bucketName}.s3.amazonaws.com`, - 'content-length': '5242880', - }, - parsedContentLength: 5242880, - url: `/${mpuObjectName}?partNumber=1&uploadId=${testUploadId}`, - query: { - partNumber: '1', - uploadId: testUploadId, - }, - partHash, - }, - partBody, - ); + const [, headers] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert(headers['Last-Modified'], 'Last-Modified should be present'); + assert(!isNaN(new Date(headers['Last-Modified']).getTime()), 'Last-Modified should be a valid date'); + }); +}); - objectPutPart(authInfo, part1Request, undefined, log, () => { - next(null, testUploadId, partHash); - }); - }, - (testUploadId, partHash, next) => { - const part2Request = new DummyRequest( - { - bucketName, - namespace, - objectKey: mpuObjectName, - headers: { - host: `${bucketName}.s3.amazonaws.com`, - 'content-length': `${partBody.length}`, - }, - parsedContentLength: partBody.length, - url: `/${mpuObjectName}?partNumber=2&uploadId=${testUploadId}`, - query: { - partNumber: '2', - uploadId: testUploadId, - }, - partHash, - }, - partBody, - ); +describe('objectGetAttributes API with multipart upload', () => { + const partCount = 2; + const partBody = Buffer.from('I am a part\n', 'utf8'); - objectPutPart(authInfo, part2Request, undefined, log, () => { - next(null, testUploadId, partHash); - }); - }, - (testUploadId, partHash, next) => { - const completeBody = - '' + - '' + - '1' + - `"${partHash}"` + - '' + - '' + - '2' + - `"${partHash}"` + - '' + - ''; - - const completeRequest = { + const createMpuObject = async () => { + const initiateRequest = { + bucketName, + namespace, + objectKey: objectName, + headers: { host: `${bucketName}.s3.amazonaws.com` }, + url: `/${objectName}?uploads`, + actionImplicitDenies: false, + }; + + const [result] = await initiateMultipartUploadAsync(authInfo, initiateRequest, log); + const json = await parseStringPromise(result); + const testUploadId = json.InitiateMultipartUploadResult.UploadId[0]; + const partHash = crypto.createHash('md5').update(partBody).digest('hex'); + + const completeParts = []; + for (let i = 1; i <= partCount; i++) { + const partRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + host: `${bucketName}.s3.amazonaws.com`, + 'content-length': '5242880', + }, + parsedContentLength: 5242880, + url: `/${objectName}?partNumber=${i}&uploadId=${testUploadId}`, + query: { + partNumber: String(i), + uploadId: testUploadId, + }, + partHash, + }, + partBody, + ); + await objectPutPartAsync(authInfo, partRequest, undefined, log); + completeParts.push(`${i}"${partHash}"`); + } + + const completeBody = + `${completeParts.join('')}`; + + const completeRequest = { bucketName, namespace, - objectKey: mpuObjectName, + objectKey: objectName, parsedHost: 's3.amazonaws.com', - url: `/${mpuObjectName}?uploadId=${testUploadId}`, + url: `/${objectName}?uploadId=${testUploadId}`, headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, actionImplicitDenies: false, - }; + }; - completeMultipartUpload(authInfo, completeRequest, log, err => { - next(err); - }); - }, - ], - callback, - ); - }; + await completeMultipartUploadAsync(authInfo, completeRequest, log); + }; - const createGetAttributesRequest = attributes => ({ - bucketName, - namespace, - objectKey: mpuObjectName, - headers: { - 'x-amz-object-attributes': attributes.join(','), - }, - url: `/${bucketName}/${mpuObjectName}`, - query: {}, - actionImplicitDenies: false, - }); - - it('should return TotalPartsCount for MPU object', done => { - createMpuObject(err => { - assert.ifError(err); - const testGetRequest = createGetAttributesRequest(['ObjectParts']); - - objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { - assert.ifError(err); - parseString(xml, (err, result) => { - const response = result.GetObjectAttributesResponse; - - assert.ifError(err); - assert(response.ObjectParts, 'ObjectParts should be present'); - assert.strictEqual(response.ObjectParts[0].PartsCount[0], String(partCount)); - done(); - }); - }); + beforeEach(async () => { + cleanup(); + await bucketPutAsync(authInfo, testPutBucketRequest, log); + await createMpuObject(); }); - }); - - it('should return TotalPartsCount along with other attributes for MPU object', done => { - createMpuObject(err => { - assert.ifError(err); - const testGetRequest = createGetAttributesRequest(['ETag', 'ObjectParts', 'ObjectSize', 'StorageClass']); - - objectGetAttributes(authInfo, testGetRequest, log, (err, xml) => { - assert.ifError(err); - parseString(xml, (err, result) => { - const response = result.GetObjectAttributesResponse; - - assert.ifError(err); - assert(response.ETag, 'ETag should be present'); - assert(response.ETag[0].includes(`-${partCount}`), `ETag should indicate MPU with ${partCount} parts`); - assert(response.ObjectParts, 'ObjectParts should be present'); - assert.strictEqual(response.ObjectParts[0].PartsCount[0], String(partCount)); - assert(response.ObjectSize, 'ObjectSize should be present'); - assert.strictEqual(response.StorageClass[0], 'STANDARD'); - done(); - }); - }); + + it('should return TotalPartsCount for MPU object', async () => { + const testGetRequest = createGetAttributesRequest(['ObjectParts']); + + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert(response.ObjectParts, 'ObjectParts should be present'); + assert.strictEqual(response.ObjectParts[0].PartsCount[0], String(partCount)); + }); + + it('should return TotalPartsCount along with other attributes for MPU object', async () => { + const testGetRequest = createGetAttributesRequest(['ETag', 'ObjectParts', 'ObjectSize', 'StorageClass']); + + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert(response.ETag, 'ETag should be present'); + assert(response.ETag[0].includes(`-${partCount}`), `ETag should indicate MPU with ${partCount} parts`); + assert(response.ObjectParts, 'ObjectParts should be present'); + assert.strictEqual(response.ObjectParts[0].PartsCount[0], String(partCount)); + assert(response.ObjectSize, 'ObjectSize should be present'); + assert.strictEqual(response.StorageClass[0], 'STANDARD'); }); - }); }); describe('objectGetAttributes API with versioning', () => { - const enableVersioningRequest = versioningTestUtils.createBucketPutVersioningReq(bucketName, 'Enabled'); + const enableVersioningRequest = versioningTestUtils.createBucketPutVersioningReq(bucketName, 'Enabled'); - const testPutBucketRequest = { - bucketName, - namespace, - headers: { host: `${bucketName}.s3.amazonaws.com` }, - url: `/${bucketName}`, - actionImplicitDenies: false, - }; - - beforeEach(done => { - cleanup(); - async.series( - [ - next => bucketPut(authInfo, testPutBucketRequest, log, next), - next => bucketPutVersioning(authInfo, enableVersioningRequest, log, next), - ], - done, - ); - }); - - const createGetAttributesRequest = (attributes, options = {}) => ({ - bucketName, - namespace, - objectKey: options.objectKey || objectName, - headers: { - 'x-amz-object-attributes': attributes.join(','), - ...options.headers, - }, - url: `/${bucketName}/${options.objectKey || objectName}`, - query: options.query || {}, - actionImplicitDenies: false, - }); + beforeEach(async () => { + cleanup(); + await bucketPutAsync(authInfo, testPutBucketRequest, log); + await bucketPutVersioningAsync(authInfo, enableVersioningRequest, log); + }); - it('should return NoSuchVersion for non-existent versionId', done => { - const testPutObjectRequest = new DummyRequest( - { - bucketName, - namespace, - objectKey: objectName, - headers: { - 'content-length': `${postBody.length}`, - }, - parsedContentLength: postBody.length, - url: `/${bucketName}/${objectName}`, - }, - postBody, - ); - - const fakeVersionId = '111111111111111111111111111111111111111175636f7270'; - - objectPut(authInfo, testPutObjectRequest, undefined, log, err => { - assert.ifError(err); - const testGetRequest = createGetAttributesRequest(['ETag'], { - query: { versionId: fakeVersionId }, - }); - - objectGetAttributes(authInfo, testGetRequest, log, err => { - assert.strictEqual(err.is.NoSuchVersion, true); - assert.strictEqual( - err.description, - 'Indicates that the version ID specified in the request does not match an existing version.', + it('should return NoSuchVersion for non-existent versionId', async () => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, ); - done(); - }); + + const fakeVersionId = '111111111111111111111111111111111111111175636f7270'; + + await objectPutAsync(authInfo, testPutObjectRequest, undefined, log); + const testGetRequest = createGetAttributesRequest(['ETag'], { + query: { versionId: fakeVersionId }, + }); + + try { + await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert.fail('Expected error was not thrown'); + } catch (err) { + assert.strictEqual(err.is.NoSuchVersion, true); + assert.strictEqual( + err.description, + 'Indicates that the version ID specified in the request does not match an existing version.', + ); + } }); - }); - it('should return MethodNotAllowed for delete marker', done => { - const testPutObjectRequest = new DummyRequest( - { - bucketName, - namespace, - objectKey: objectName, - headers: { - 'content-length': `${postBody.length}`, - }, - parsedContentLength: postBody.length, - url: `/${bucketName}/${objectName}`, - }, - postBody, - ); - - const testDeleteRequest = { - bucketName, - namespace, - objectKey: objectName, - headers: {}, - url: `/${bucketName}/${objectName}`, - actionImplicitDenies: false, - }; + it('should return MethodNotAllowed for delete marker', async () => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + + const testDeleteRequest = { + bucketName, + namespace, + objectKey: objectName, + headers: {}, + url: `/${bucketName}/${objectName}`, + actionImplicitDenies: false, + }; + + await objectPutAsync(authInfo, testPutObjectRequest, undefined, log); + await objectDeleteAsync(authInfo, testDeleteRequest, log); - async.series( - [ - next => objectPut(authInfo, testPutObjectRequest, undefined, log, next), - next => objectDelete(authInfo, testDeleteRequest, log, next), - ], - err => { - assert.ifError(err); const testGetRequest = createGetAttributesRequest(['ETag']); - objectGetAttributes(authInfo, testGetRequest, log, (err, _xml, headers) => { - assert.strictEqual(err.is.MethodNotAllowed, true); - assert.strictEqual(err.description, 'The specified method is not allowed against this resource.'); - assert.strictEqual(headers['x-amz-delete-marker'], true); - done(); + try { + await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert.fail('Expected error was not thrown'); + } catch (err) { + assert.strictEqual(err.is.MethodNotAllowed, true); + assert.strictEqual(err.description, 'The specified method is not allowed against this resource.'); + assert.strictEqual(err.responseHeaders['x-amz-delete-marker'], true); + } + }); + + it('should return attributes for specific version', async () => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + + const [resHeaders] = await objectPutAsync(authInfo, testPutObjectRequest, undefined, log); + const versionId = resHeaders['x-amz-version-id']; + assert(versionId, 'Version ID should be present'); + + const testGetRequest = createGetAttributesRequest(['ETag', 'ObjectSize'], { + query: { versionId }, }); - }, - ); - }); - it('should return attributes for specific version', done => { - const testPutObjectRequest = new DummyRequest( - { - bucketName, - namespace, - objectKey: objectName, - headers: { - 'content-length': `${postBody.length}`, - }, - parsedContentLength: postBody.length, - url: `/${bucketName}/${objectName}`, - }, - postBody, - ); - - objectPut(authInfo, testPutObjectRequest, undefined, log, (err, resHeaders) => { - assert.ifError(err); - const versionId = resHeaders['x-amz-version-id']; - assert(versionId, 'Version ID should be present'); - - const testGetRequest = createGetAttributesRequest(['ETag', 'ObjectSize'], { - query: { versionId }, - }); - - objectGetAttributes(authInfo, testGetRequest, log, (err, xml, headers) => { - assert.ifError(err); + const [xml, headers] = await objectGetAttributesAsync(authInfo, testGetRequest, log); assert(headers['Last-Modified'], 'Last-Modified should be present'); - parseString(xml, (err, result) => { - const response = result.GetObjectAttributesResponse; + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; - assert.ifError(err); - assert.strictEqual(response.ETag[0], expectedMD5); - assert.strictEqual(response.ObjectSize[0], String(body.length)); - done(); - }); - }); + assert.strictEqual(response.ETag[0], expectedMD5); + assert.strictEqual(response.ObjectSize[0], String(body.length)); }); - }); - it('should return VersionId in response headers for versioned object', done => { - const testPutObjectRequest = new DummyRequest( - { - bucketName, - namespace, - objectKey: objectName, - headers: { - 'content-length': `${postBody.length}`, - }, - parsedContentLength: postBody.length, - url: `/${bucketName}/${objectName}`, - }, - postBody, - ); + it('should return VersionId in response headers for versioned object', async () => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); - objectPut(authInfo, testPutObjectRequest, undefined, log, (err, resHeaders) => { - assert.ifError(err); - const versionId = resHeaders['x-amz-version-id']; - assert(versionId, 'Version ID should be present from PUT'); + const [resHeaders] = await objectPutAsync(authInfo, testPutObjectRequest, undefined, log); + const versionId = resHeaders['x-amz-version-id']; + assert(versionId, 'Version ID should be present from PUT'); - const testGetRequest = createGetAttributesRequest(['ETag']); + const testGetRequest = createGetAttributesRequest(['ETag']); - objectGetAttributes(authInfo, testGetRequest, log, (err, _xml, headers) => { - assert.ifError(err); + const [, headers] = await objectGetAttributesAsync(authInfo, testGetRequest, log); assert.strictEqual(headers['x-amz-version-id'], versionId); - done(); - }); }); - }); }); From 392ad9ba6d53f2a5318ac78dcfdc7c828676a71f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Fri, 30 Jan 2026 15:34:08 +0100 Subject: [PATCH 04/17] fixup! Support the new API GetObjectAttributes --- .../apiUtils/object/parseAttributesHeader.js | 11 +--- .../test/object/objectGetAttributes.js | 9 ++-- .../apiUtils/object/parseAttributesHeader.js | 53 +++++++++++++------ tests/unit/api/objectGetAttributes.js | 8 +-- 4 files changed, 43 insertions(+), 38 deletions(-) diff --git a/lib/api/apiUtils/object/parseAttributesHeader.js b/lib/api/apiUtils/object/parseAttributesHeader.js index 92f7509251..0abe19a697 100644 --- a/lib/api/apiUtils/object/parseAttributesHeader.js +++ b/lib/api/apiUtils/object/parseAttributesHeader.js @@ -8,21 +8,14 @@ const { allowedObjectAttributes } = require('../../../../constants'); * @throws {Error} - InvalidRequest if header is missing/empty, InvalidArgument if attribute is invalid */ function parseAttributesHeaders(headers) { - const raw = headers['x-amz-object-attributes'] || ''; - - const attributes = raw - .split(',') - .map(s => s.trim()) - .filter(s => s !== ''); - + const attributes = headers['x-amz-object-attributes']?.split(',').map(attr => attr.trim()) ?? []; if (attributes.length === 0) { throw errorInstances.InvalidRequest.customizeDescription( 'The x-amz-object-attributes header specifying the attributes to be retrieved is either missing or empty', ); } - const invalids = attributes.filter(s => !allowedObjectAttributes.has(s)); - if (invalids.length > 0) { + if (attributes.some(attr => !allowedObjectAttributes.has(attr))) { throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.'); } diff --git a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js index 6af868181c..0eb10e561e 100644 --- a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js +++ b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js @@ -51,13 +51,10 @@ describe('objectGetAttributes', () => { ObjectAttributes: [], }) .promise(); - assert.fail('Expected InvalidRequest error'); + assert.fail('Expected InvalidArgument error'); } catch (err) { - assert.strictEqual(err.code, 'InvalidRequest'); - assert.strictEqual( - err.message, - 'The x-amz-object-attributes header specifying the attributes to be retrieved is either missing or empty', - ); + assert.strictEqual(err.code, 'InvalidArgument'); + assert.strictEqual(err.message, 'Invalid attribute name specified.'); } }); diff --git a/tests/unit/api/apiUtils/object/parseAttributesHeader.js b/tests/unit/api/apiUtils/object/parseAttributesHeader.js index dc09292649..68acf14a57 100644 --- a/tests/unit/api/apiUtils/object/parseAttributesHeader.js +++ b/tests/unit/api/apiUtils/object/parseAttributesHeader.js @@ -12,47 +12,52 @@ describe('parseAttributesHeaders', () => { err => { assert(err.is); assert.strictEqual(err.is.InvalidRequest, true); - assert(err.description.includes('missing or empty')); + assert.strictEqual( + err.description, + 'The x-amz-object-attributes header specifying the attributes to be retrieved is either missing or empty', + ); return true; }, ); }); - it('should throw InvalidRequest error when header is empty string', () => { + it('should throw InvalidArgument error when header is empty string', () => { const headers = { 'x-amz-object-attributes': '' }; assert.throws( () => parseAttributesHeaders(headers), err => { assert(err.is); - assert.strictEqual(err.is.InvalidRequest, true); - assert(err.description.includes('missing or empty')); + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); return true; }, ); }); - it('should throw InvalidRequest error when header contains only whitespace', () => { + it('should throw InvalidArgument error when header contains only whitespace', () => { const headers = { 'x-amz-object-attributes': ' ' }; assert.throws( () => parseAttributesHeaders(headers), err => { assert(err.is); - assert.strictEqual(err.is.InvalidRequest, true); + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); return true; }, ); }); - it('should throw InvalidRequest error when header contains only commas', () => { + it('should throw InvalidArgument error when header contains only commas', () => { const headers = { 'x-amz-object-attributes': ',,,' }; assert.throws( () => parseAttributesHeaders(headers), err => { assert(err.is); - assert.strictEqual(err.is.InvalidRequest, true); + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); return true; }, ); @@ -68,7 +73,7 @@ describe('parseAttributesHeaders', () => { err => { assert(err.is); assert.strictEqual(err.is.InvalidArgument, true); - assert(err.description.includes('Invalid attribute name')); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); return true; }, ); @@ -82,6 +87,7 @@ describe('parseAttributesHeaders', () => { err => { assert(err.is); assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); return true; }, ); @@ -95,6 +101,7 @@ describe('parseAttributesHeaders', () => { err => { assert(err.is); assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); return true; }, ); @@ -173,20 +180,32 @@ describe('parseAttributesHeaders', () => { assert.deepStrictEqual(result, ['ETag', 'ObjectSize']); }); - it('should handle extra commas between attributes', () => { + it('should throw InvalidArgument for extra commas between attributes', () => { const headers = { 'x-amz-object-attributes': 'ETag,,ObjectSize' }; - const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['ETag', 'ObjectSize']); + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); + return true; + }, + ); }); - it('should handle leading and trailing commas', () => { + it('should throw InvalidArgument for leading and trailing commas', () => { const headers = { 'x-amz-object-attributes': ',ETag,ObjectSize,' }; - const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['ETag', 'ObjectSize']); + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); + return true; + }, + ); }); }); }); diff --git a/tests/unit/api/objectGetAttributes.js b/tests/unit/api/objectGetAttributes.js index 33c678be4f..2e2064d15d 100644 --- a/tests/unit/api/objectGetAttributes.js +++ b/tests/unit/api/objectGetAttributes.js @@ -127,12 +127,8 @@ describe('objectGetAttributes API', () => { await objectGetAttributesAsync(authInfo, testGetRequest, log); assert.fail('Expected error was not thrown'); } catch (err) { - assert.strictEqual(err.is.InvalidRequest, true); - assert.strictEqual( - err.description, - 'The x-amz-object-attributes header specifying the attributes ' + - 'to be retrieved is either missing or empty', - ); + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); } }); From 6e342f38c7281a082d26235ec003aa6ef708afdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Mon, 2 Feb 2026 18:11:01 +0100 Subject: [PATCH 05/17] fixup! Support the new API GetObjectAttributes --- yarn.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/yarn.lock b/yarn.lock index ec0fd06486..861038a1ce 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1496,8 +1496,8 @@ arraybuffer.prototype.slice@^1.0.4: ioctl "^2.0.2" "arsenal@git+https://github.com/scality/Arsenal#feature/ARSN-549/get-object-attributes": - version "8.2.43" - resolved "git+https://github.com/scality/Arsenal#ec21fa885c611498584ef3c56bfd62047c640e9e" + version "8.2.44" + resolved "git+https://github.com/scality/Arsenal#46757d474c28d616548446331722bd858b5672bb" dependencies: "@azure/identity" "^4.13.0" "@azure/storage-blob" "^12.28.0" From 92bd9bed25627c2a1ddd95ae49585c252cc10ccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Wed, 4 Feb 2026 15:33:47 +0100 Subject: [PATCH 06/17] fixup! Support the new API GetObjectAttributes --- lib/api/objectGetAttributes.js | 67 ++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/lib/api/objectGetAttributes.js b/lib/api/objectGetAttributes.js index 86f09fe31f..ed22ab0e45 100644 --- a/lib/api/objectGetAttributes.js +++ b/lib/api/objectGetAttributes.js @@ -56,18 +56,29 @@ function buildXmlResponse(objMD, attributes) { * @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 optional to keep backward compatibility * @returns {Promise} - { xml, responseHeaders } * @throws {ArsenalError} NoSuchVersion - if versionId specified but not found * @throws {ArsenalError} NoSuchKey - if object not found * @throws {ArsenalError} MethodNotAllowed - if object is a delete marker */ -async function objectGetAttributes(authInfo, request, log) { +async function objectGetAttributes(authInfo, request, log, callback) { + if (callback) { + return objectGetAttributes(authInfo, request, log) + .then(result => callback(null, result.xml, result.responseHeaders)) + .catch(err => callback(err, null, err.responseHeaders ?? {})); + } + log.trace('processing request', { method: OBJECT_GET_ATTRIBUTES }); const { bucketName, objectKey, headers, actionImplicitDenies } = request; const versionId = decodeVersionId(request.query); if (versionId instanceof Error) { - log.debug('invalid versionId query', { versionId: request.query.versionId, error: versionId }); + log.debug('invalid versionId query', { + method: OBJECT_GET_ATTRIBUTES, + versionId: request.query.versionId, + error: versionId, + }); throw versionId; } @@ -81,14 +92,31 @@ async function objectGetAttributes(authInfo, request, log) { request, }; - const { bucket, objMD } = await validateBucketAndObj(metadataValParams, actionImplicitDenies, log); - await checkExpectedBucketOwnerPromise(headers, bucket, log); + let bucket, objMD; + try { + ({ bucket, objMD } = await validateBucketAndObj(metadataValParams, actionImplicitDenies, log)); + await checkExpectedBucketOwnerPromise(headers, bucket, log); + } catch (err) { + log.debug('error validating bucket and object', { + method: OBJECT_GET_ATTRIBUTES, + bucket: bucketName, + key: objectKey, + versionId, + error: err, + }); + throw err; + } const responseHeaders = collectCorsHeaders(headers.origin, request.method, bucket); if (!objMD) { + log.debug('object not found', { + method: OBJECT_GET_ATTRIBUTES, + bucket: bucketName, + key: objectKey, + versionId, + }); const err = versionId ? errors.NoSuchVersion : errors.NoSuchKey; - log.debug('object not found', { bucket: bucketName, key: objectKey, versionId }); err.responseHeaders = responseHeaders; throw err; } @@ -97,7 +125,12 @@ async function objectGetAttributes(authInfo, request, log) { responseHeaders['Last-Modified'] = objMD['last-modified'] && new Date(objMD['last-modified']).toUTCString(); if (objMD.isDeleteMarker) { - log.debug('attempt to get attributes of a delete marker', { bucket: bucketName, key: objectKey, versionId }); + log.debug('attempt to get attributes of a delete marker', { + method: OBJECT_GET_ATTRIBUTES, + bucket: bucketName, + key: objectKey, + versionId, + }); responseHeaders['x-amz-delete-marker'] = true; const err = errors.MethodNotAllowed; err.responseHeaders = responseHeaders; @@ -118,24 +151,4 @@ async function objectGetAttributes(authInfo, request, log) { return { xml, responseHeaders }; } -/** - * objectGetAttributesCallback - Callback wrapper for objectGetAttributes - * @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 (err, xml, responseHeaders) - * @return {undefined} - */ -function objectGetAttributesCallback(authInfo, request, log, callback) { - objectGetAttributes(authInfo, request, log) - .then(result => callback(null, result.xml, result.responseHeaders)) - .catch(err => { - log.debug('error processing request', { - error: err, - method: OBJECT_GET_ATTRIBUTES, - }); - return callback(err, null, err.responseHeaders || {}); - }); -} - -module.exports = objectGetAttributesCallback; +module.exports = objectGetAttributes; From 1f928b7ba902ee8ddfe7e96f380e40d0c77f129a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Wed, 4 Feb 2026 16:49:48 +0100 Subject: [PATCH 07/17] fixup! Support the new API GetObjectAttributes --- yarn.lock | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/yarn.lock b/yarn.lock index 861038a1ce..c3066dec2e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -602,9 +602,9 @@ integrity sha512-EKQmr16tM8s16vTT3cA5L0kZZcTMU5DUOZTuvpnY738m+jyP3JIUj+Mm1xc1rsLkGBQ/gVnfKYPwOmPg1tUR4Q== "@hapi/tlds@^1.1.1": - version "1.1.3" - resolved "https://registry.yarnpkg.com/@hapi/tlds/-/tlds-1.1.3.tgz#bf5fee927d213f140cd54d4650965e504a546789" - integrity sha512-QIvUMB5VZ8HMLZF9A2oWr3AFM430QC8oGd0L35y2jHpuW6bIIca6x/xL7zUf4J7L9WJ3qjz+iJII8ncaeMbpSg== + version "1.1.4" + resolved "https://registry.yarnpkg.com/@hapi/tlds/-/tlds-1.1.4.tgz#df4a7b59082b54ba4f3b7b38f781e2ac3cbc359a" + integrity sha512-Fq+20dxsxLaUn5jSSWrdtSRcIUba2JquuorF9UW1wIJS5cSUwxIsO2GIhaWynPRflvxSzFN+gxKte2HEW1OuoA== "@hapi/topo@^5.0.0", "@hapi/topo@^5.1.0": version "5.1.0" @@ -648,16 +648,21 @@ resolved "https://registry.yarnpkg.com/@humanwhocodes/retry/-/retry-0.4.2.tgz#1860473de7dfa1546767448f333db80cb0ff2161" integrity sha512-xeO57FpIu4p1Ri3Jq/EXq4ClRm86dVF2z/+kvFnyqVYRavTZmaFaUBbWCOuuTh0o/g7DSsk6kc2vrS4Vl5oPOQ== -"@ioredis/commands@1.4.0", "@ioredis/commands@^1.3.0": - version "1.4.0" - resolved "https://registry.yarnpkg.com/@ioredis/commands/-/commands-1.4.0.tgz#9f657d51cdd5d2fdb8889592aa4a355546151f25" - integrity sha512-aFT2yemJJo+TZCmieA7qnYGQooOS7QfNmYrzGtsYd3g9j5iDP8AimYYAesf79ohjbLG12XxC4nG5DyEnC88AsQ== +"@ioredis/commands@1.5.0": + version "1.5.0" + resolved "https://registry.yarnpkg.com/@ioredis/commands/-/commands-1.5.0.tgz#3dddcea446a4b1dc177d0743a1e07ff50691652a" + integrity sha512-eUgLqrMf8nJkZxT24JvVRrQya1vZkQh8BBeYNwGDqa5I0VUi8ACx7uFvAaLxintokpTenkK6DASvo/bvNbBGow== "@ioredis/commands@^1.1.1": version "1.2.0" resolved "https://registry.yarnpkg.com/@ioredis/commands/-/commands-1.2.0.tgz#6d61b3097470af1fdbbe622795b8921d42018e11" integrity sha512-Sx1pU8EM64o2BrqNpEO1CNLtKQwyhuXuqyfH7oGKCk+1a33d2r5saW8zNwm3j6BTExtjrv2BxTgzzkMwts6vGg== +"@ioredis/commands@^1.3.0": + version "1.4.0" + resolved "https://registry.yarnpkg.com/@ioredis/commands/-/commands-1.4.0.tgz#9f657d51cdd5d2fdb8889592aa4a355546151f25" + integrity sha512-aFT2yemJJo+TZCmieA7qnYGQooOS7QfNmYrzGtsYd3g9j5iDP8AimYYAesf79ohjbLG12XxC4nG5DyEnC88AsQ== + "@isaacs/cliui@^8.0.2": version "8.0.2" resolved "https://registry.yarnpkg.com/@isaacs/cliui/-/cliui-8.0.2.tgz#b37667b7bc181c168782259bab42474fbf52b550" @@ -1050,9 +1055,9 @@ integrity sha512-9BCxFwvbGg/RsZK9tjXd8s4UcwR0MWeFQ1XEKIQVVvAGJyINdrqKMcTRyLoK8Rse1GjzLV9cwjWV1olXRWEXVA== "@standard-schema/spec@^1.0.0": - version "1.0.0" - resolved "https://registry.yarnpkg.com/@standard-schema/spec/-/spec-1.0.0.tgz#f193b73dc316c4170f2e82a881da0f550d551b9c" - integrity sha512-m2bOd0f2RT9k8QJx1JN85cZYyH1RqFBdlwtkSlf4tBDYLCiiZnv1fIIwacK6cqwXavOydf0NPToMQgpKq+dVlA== + version "1.1.0" + resolved "https://registry.yarnpkg.com/@standard-schema/spec/-/spec-1.1.0.tgz#a79b55dbaf8604812f52d140b2c9ab41bc150bb8" + integrity sha512-l2aFy5jALhniG5HgqrD6jXLi/rUWrKvqN/qJx6yoJsgKhblVd+iqqU4RCXavm/jPityDo5TCvKMnpjKnOriy0w== "@types/async@^3.2.24": version "3.2.24" @@ -1497,7 +1502,7 @@ arraybuffer.prototype.slice@^1.0.4: "arsenal@git+https://github.com/scality/Arsenal#feature/ARSN-549/get-object-attributes": version "8.2.44" - resolved "git+https://github.com/scality/Arsenal#46757d474c28d616548446331722bd858b5672bb" + resolved "git+https://github.com/scality/Arsenal#f02179acbfb6aff963c2fc72146f091544ddf86c" dependencies: "@azure/identity" "^4.13.0" "@azure/storage-blob" "^12.28.0" @@ -3795,7 +3800,7 @@ ioredis@^5.8.1: resolved "https://registry.yarnpkg.com/ioredis/-/ioredis-5.9.2.tgz#ffdce2a019950299716e88ee56cd5802b399b108" integrity sha512-tAAg/72/VxOUW7RQSX1pIxJVucYKcjFjfvj60L57jrZpYCHC3XN0WCQ3sNYL4Gmvv+7GPvTAjc+KSdeNuE8oWQ== dependencies: - "@ioredis/commands" "1.4.0" + "@ioredis/commands" "1.5.0" cluster-key-slot "^1.1.0" debug "^4.3.4" denque "^2.1.0" @@ -4318,9 +4323,9 @@ joi@^17.13.3: "@sideway/pinpoint" "^2.0.0" joi@^18.0.1: - version "18.0.1" - resolved "https://registry.yarnpkg.com/joi/-/joi-18.0.1.tgz#1e1885d035cc6ca1624e81bf22112e7c1ee38e1b" - integrity sha512-IiQpRyypSnLisQf3PwuN2eIHAsAIGZIrLZkd4zdvIar2bDyhM91ubRjy8a3eYablXsh9BeI/c7dmPYHca5qtoA== + version "18.0.2" + resolved "https://registry.yarnpkg.com/joi/-/joi-18.0.2.tgz#30ced6aed00a7848cc11f92859515258301dc3a4" + integrity sha512-RuCOQMIt78LWnktPoeBL0GErkNaJPTBGcYuyaBvUOQSpcpcLfWrHPPihYdOGbV5pam9VTWbeoF7TsGiHugcjGA== dependencies: "@hapi/address" "^5.1.1" "@hapi/formula" "^3.0.2" @@ -5150,7 +5155,7 @@ mongodb@^6.11.0: bson "^6.10.3" mongodb-connection-string-url "^3.0.0" -mongodb@^6.17.0, mongodb@^6.20.0: +mongodb@^6.17.0: version "6.20.0" resolved "https://registry.yarnpkg.com/mongodb/-/mongodb-6.20.0.tgz#5212dcf512719385287aa4574265352eefb01d8e" integrity sha512-Tl6MEIU3K4Rq3TSHd+sZQqRBoGlFsOgNrH5ltAcFBV62Re3Fd+FcaVf8uSEQFOJ51SDowDVttBTONMfoYWrWlQ== @@ -5159,6 +5164,15 @@ mongodb@^6.17.0, mongodb@^6.20.0: bson "^6.10.4" mongodb-connection-string-url "^3.0.2" +mongodb@^6.20.0: + version "6.21.0" + resolved "https://registry.yarnpkg.com/mongodb/-/mongodb-6.21.0.tgz#f83355905900f2e7a912593f0315d5e2e0bda576" + integrity sha512-URyb/VXMjJ4da46OeSXg+puO39XH9DeQpWCslifrRn9JWugy0D+DvvBvkm2WxmHe61O/H19JM66p1z7RHVkZ6A== + dependencies: + "@mongodb-js/saslprep" "^1.3.0" + bson "^6.10.4" + mongodb-connection-string-url "^3.0.2" + ms@2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/ms/-/ms-2.0.0.tgz#5608aeadfc00be6c2901df5f9861788de0d597c8" From 33e7f329f7e7a96df74f85ce41a663cd88da049d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Mon, 9 Feb 2026 15:36:55 +0100 Subject: [PATCH 08/17] fixup! Support the new API GetObjectAttributes --- constants.js | 4 ++-- lib/api/apiUtils/object/parseAttributesHeader.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/constants.js b/constants.js index 4824109d69..5d53003f65 100644 --- a/constants.js +++ b/constants.js @@ -275,8 +275,8 @@ const constants = { 'bucketPutLogging', 'bucketGetLogging', ], - // Metadata allowed to be returned by getObjectAttributes API - allowedObjectAttributes: new Set([ + // Supported attributes for the GetObjectAttributes 'x-amz-optional-attributes' header. + supportedGetObjectAttributes: new Set([ 'StorageClass', 'ObjectSize', 'ObjectParts', diff --git a/lib/api/apiUtils/object/parseAttributesHeader.js b/lib/api/apiUtils/object/parseAttributesHeader.js index 0abe19a697..21dd83181a 100644 --- a/lib/api/apiUtils/object/parseAttributesHeader.js +++ b/lib/api/apiUtils/object/parseAttributesHeader.js @@ -1,5 +1,5 @@ const { errorInstances } = require('arsenal'); -const { allowedObjectAttributes } = require('../../../../constants'); +const { supportedGetObjectAttributes } = require('../../../../constants'); /** * parseAttributesHeaders - Parse and validate the x-amz-object-attributes header @@ -15,7 +15,7 @@ function parseAttributesHeaders(headers) { ); } - if (attributes.some(attr => !allowedObjectAttributes.has(attr))) { + if (attributes.some(attr => !supportedGetObjectAttributes.has(attr))) { throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.'); } From c762cd80344dbe367b0d46beae6e772edbcc26ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Thu, 12 Feb 2026 10:37:38 +0100 Subject: [PATCH 09/17] fixup! Support the new API GetObjectAttributes --- .../apiUtils/object/parseAttributesHeader.js | 4 +- lib/api/objectGetAttributes.js | 76 ++++++++++--------- lib/metadata/metadataUtils.js | 15 ++-- .../test/object/objectGetAttributes.js | 27 ++++--- .../apiUtils/object/parseAttributesHeader.js | 56 +++++++------- tests/unit/api/objectGetAttributes.js | 14 ++-- yarn.lock | 8 +- 7 files changed, 105 insertions(+), 95 deletions(-) diff --git a/lib/api/apiUtils/object/parseAttributesHeader.js b/lib/api/apiUtils/object/parseAttributesHeader.js index 21dd83181a..685ac93884 100644 --- a/lib/api/apiUtils/object/parseAttributesHeader.js +++ b/lib/api/apiUtils/object/parseAttributesHeader.js @@ -4,7 +4,7 @@ const { supportedGetObjectAttributes } = require('../../../../constants'); /** * parseAttributesHeaders - Parse and validate the x-amz-object-attributes header * @param {object} headers - request headers - * @returns {string[]} - array of valid attribute names + * @returns {Set} - set of requested attribute names * @throws {Error} - InvalidRequest if header is missing/empty, InvalidArgument if attribute is invalid */ function parseAttributesHeaders(headers) { @@ -19,7 +19,7 @@ function parseAttributesHeaders(headers) { throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.'); } - return attributes; + return new Set(attributes); } module.exports = parseAttributesHeaders; diff --git a/lib/api/objectGetAttributes.js b/lib/api/objectGetAttributes.js index ed22ab0e45..696f613baa 100644 --- a/lib/api/objectGetAttributes.js +++ b/lib/api/objectGetAttributes.js @@ -9,44 +9,38 @@ const { checkExpectedBucketOwner } = require('./apiUtils/authorization/bucketOwn const { pushMetric } = require('../utapi/utilities'); const { getPartCountFromMd5 } = require('./apiUtils/object/partInfo'); -const OBJECT_GET_ATTRIBUTES = 'objectGetAttributes'; - const checkExpectedBucketOwnerPromise = promisify(checkExpectedBucketOwner); const validateBucketAndObj = promisify(standardMetadataValidateBucketAndObj); +const OBJECT_GET_ATTRIBUTES = 'objectGetAttributes'; +const ATTRIBUTE_HANDLERS = { + ETag: objMD => objMD['content-md5'], + ObjectParts: objMD => { + const partCount = getPartCountFromMd5(objMD); + return partCount ? { PartsCount: partCount } : undefined; + }, + StorageClass: objMD => objMD['x-amz-storage-class'], + ObjectSize: objMD => objMD['content-length'], +}; + /** * buildXmlResponse - Build XML response for GetObjectAttributes * @param {object} objMD - object metadata - * @param {array} attributes - requested attributes + * @param {Set} requestedAttrs - set of requested attribute names * @returns {string} XML response */ -function buildXmlResponse(objMD, attributes) { +function buildXmlResponse(objMD, requestedAttrs) { const attrResp = {}; - if (attributes.includes('ETag')) { - attrResp.ETag = objMD['content-md5']; - } - - // NOTE: Checksum is not implemented - if (attributes.includes('Checksum')) { - attrResp.Checksum = {}; - } - - if (attributes.includes('ObjectParts')) { - const partCount = getPartCountFromMd5(objMD); - if (partCount) { - attrResp.ObjectParts = { PartsCount: partCount }; + for (const [attr, handler] of Object.entries(ATTRIBUTE_HANDLERS)) { + if (requestedAttrs.has(attr)) { + const value = handler(objMD); + if (value !== undefined) { + attrResp[attr] = value; + } } } - if (attributes.includes('StorageClass')) { - attrResp.StorageClass = objMD['x-amz-storage-class']; - } - - if (attributes.includes('ObjectSize')) { - attrResp.ObjectSize = objMD['content-length']; - } - const builder = new xml2js.Builder(); return builder.buildObject({ GetObjectAttributesResponse: attrResp }); } @@ -92,9 +86,9 @@ async function objectGetAttributes(authInfo, request, log, callback) { request, }; - let bucket, objMD; + let bucket, objectMD; try { - ({ bucket, objMD } = await validateBucketAndObj(metadataValParams, actionImplicitDenies, log)); + ({ bucket, objectMD } = await validateBucketAndObj(metadataValParams, actionImplicitDenies, log)); await checkExpectedBucketOwnerPromise(headers, bucket, log); } catch (err) { log.debug('error validating bucket and object', { @@ -109,7 +103,7 @@ async function objectGetAttributes(authInfo, request, log, callback) { const responseHeaders = collectCorsHeaders(headers.origin, request.method, bucket); - if (!objMD) { + if (!objectMD) { log.debug('object not found', { method: OBJECT_GET_ATTRIBUTES, bucket: bucketName, @@ -121,10 +115,10 @@ async function objectGetAttributes(authInfo, request, log, callback) { throw err; } - responseHeaders['x-amz-version-id'] = getVersionIdResHeader(bucket.getVersioningConfiguration(), objMD); - responseHeaders['Last-Modified'] = objMD['last-modified'] && new Date(objMD['last-modified']).toUTCString(); + responseHeaders['x-amz-version-id'] = getVersionIdResHeader(bucket.getVersioningConfiguration(), objectMD); + responseHeaders['Last-Modified'] = objectMD['last-modified'] && new Date(objectMD['last-modified']).toUTCString(); - if (objMD.isDeleteMarker) { + if (objectMD.isDeleteMarker) { log.debug('attempt to get attributes of a delete marker', { method: OBJECT_GET_ATTRIBUTES, bucket: bucketName, @@ -137,17 +131,29 @@ async function objectGetAttributes(authInfo, request, log, callback) { throw err; } - const attributes = parseAttributesHeaders(headers); + const requestedAttrs = parseAttributesHeaders(headers); + + if (requestedAttrs.has('Checksum')) { + log.debug('Checksum attribute requested but not implemented', { + method: OBJECT_GET_ATTRIBUTES, + bucket: bucketName, + key: objectKey, + versionId, + }); + const err = errors.NotImplemented.customizeDescription('Checksum attribute is not implemented'); + err.responseHeaders = responseHeaders; + throw err; + } pushMetric(OBJECT_GET_ATTRIBUTES, log, { authInfo, bucket: bucketName, keys: [objectKey], - versionId: objMD?.versionId, - location: objMD?.dataStoreName, + versionId: objectMD?.versionId, + location: objectMD?.dataStoreName, }); - const xml = buildXmlResponse(objMD, attributes); + const xml = buildXmlResponse(objectMD, requestedAttrs); return { xml, responseHeaders }; } diff --git a/lib/metadata/metadataUtils.js b/lib/metadata/metadataUtils.js index 1360baf1f7..d2a01fcb55 100644 --- a/lib/metadata/metadataUtils.js +++ b/lib/metadata/metadataUtils.js @@ -1,5 +1,5 @@ -const async = require('async'); const { promisify } = require('util'); +const async = require('async'); const { errors } = require('arsenal'); const metadata = require('./wrapper'); @@ -283,14 +283,13 @@ function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log, }); } -standardMetadataValidateBucketAndObj[promisify.custom] = (params, action, log) => new Promise((resolve, reject) => { - standardMetadataValidateBucketAndObj(params, action, log, (err, bucket, objMD) => { - if (err) { - return reject(err); - } - return resolve({ 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 diff --git a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js index 0eb10e561e..5f99e31097 100644 --- a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js +++ b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js @@ -95,14 +95,13 @@ describe('objectGetAttributes', () => { .getObjectAttributes({ Bucket: bucket, Key: key, - ObjectAttributes: ['ETag', 'Checksum', 'ObjectParts', 'StorageClass', 'ObjectSize'], + ObjectAttributes: ['ETag', 'ObjectParts', 'StorageClass', 'ObjectSize'], }) .promise(); assert.strictEqual(data.ETag, expectedMD5); assert.strictEqual(data.StorageClass, 'STANDARD'); assert.strictEqual(data.ObjectSize, body.length); - assert.deepStrictEqual(data.Checksum, {}, 'Checksum should be present'); assert.strictEqual(data.ObjectParts, undefined, "ObjectParts shouldn't be present for non-MPU object"); assert(data.LastModified, 'LastModified should be present'); }); @@ -119,16 +118,20 @@ describe('objectGetAttributes', () => { assert.strictEqual(data.ETag, expectedMD5); }); - it('should return Checksum', async () => { - const data = await s3 - .getObjectAttributes({ - Bucket: bucket, - Key: key, - ObjectAttributes: ['Checksum'], - }) - .promise(); - - assert.deepStrictEqual(data.Checksum, {}, 'Checksum should be present'); + it('should fail with NotImplemented when Checksum is requested', async () => { + try { + await s3 + .getObjectAttributes({ + Bucket: bucket, + Key: key, + ObjectAttributes: ['Checksum'], + }) + .promise(); + assert.fail('Expected NotImplemented error'); + } catch (err) { + assert.strictEqual(err.code, 'NotImplemented'); + assert.strictEqual(err.message, 'Checksum attribute is not implemented'); + } }); it("shouldn't return ObjectParts for non-MPU objects", async () => { diff --git a/tests/unit/api/apiUtils/object/parseAttributesHeader.js b/tests/unit/api/apiUtils/object/parseAttributesHeader.js index 68acf14a57..b722668880 100644 --- a/tests/unit/api/apiUtils/object/parseAttributesHeader.js +++ b/tests/unit/api/apiUtils/object/parseAttributesHeader.js @@ -109,65 +109,65 @@ describe('parseAttributesHeaders', () => { }); describe('valid attribute names', () => { - it('should return array with single valid attribute ETag', () => { + it('should return set with single valid attribute ETag', () => { const headers = { 'x-amz-object-attributes': 'ETag' }; const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['ETag']); + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['ETag'])); }); - it('should return array with single valid attribute StorageClass', () => { + it('should return set with single valid attribute StorageClass', () => { const headers = { 'x-amz-object-attributes': 'StorageClass' }; const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['StorageClass']); + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['StorageClass'])); }); - it('should return array with single valid attribute ObjectSize', () => { + it('should return set with single valid attribute ObjectSize', () => { const headers = { 'x-amz-object-attributes': 'ObjectSize' }; const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['ObjectSize']); + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['ObjectSize'])); }); - it('should return array with single valid attribute ObjectParts', () => { + it('should return set with single valid attribute ObjectParts', () => { const headers = { 'x-amz-object-attributes': 'ObjectParts' }; const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['ObjectParts']); + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['ObjectParts'])); }); - it('should return array with single valid attribute Checksum', () => { + it('should return set with single valid attribute Checksum', () => { const headers = { 'x-amz-object-attributes': 'Checksum' }; const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['Checksum']); + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['Checksum'])); }); - it('should return array with multiple valid attributes', () => { + it('should return set with multiple valid attributes', () => { const headers = { 'x-amz-object-attributes': 'ETag,ObjectSize,StorageClass' }; const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['ETag', 'ObjectSize', 'StorageClass']); + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['ETag', 'ObjectSize', 'StorageClass'])); }); - it('should return array with all valid attributes', () => { + it('should return set with all valid attributes', () => { const headers = { 'x-amz-object-attributes': 'StorageClass,ObjectSize,ObjectParts,Checksum,ETag' }; const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.strictEqual(result.length, 5); - assert(result.includes('StorageClass')); - assert(result.includes('ObjectSize')); - assert(result.includes('ObjectParts')); - assert(result.includes('Checksum')); - assert(result.includes('ETag')); + assert(result instanceof Set); + assert.strictEqual(result.size, 5); + assert(result.has('StorageClass')); + assert(result.has('ObjectSize')); + assert(result.has('ObjectParts')); + assert(result.has('Checksum')); + assert(result.has('ETag')); }); }); @@ -176,8 +176,8 @@ describe('parseAttributesHeaders', () => { const headers = { 'x-amz-object-attributes': ' ETag , ObjectSize ' }; const result = parseAttributesHeaders(headers); - assert(Array.isArray(result)); - assert.deepStrictEqual(result, ['ETag', 'ObjectSize']); + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['ETag', 'ObjectSize'])); }); it('should throw InvalidArgument for extra commas between attributes', () => { diff --git a/tests/unit/api/objectGetAttributes.js b/tests/unit/api/objectGetAttributes.js index 2e2064d15d..b9e756f991 100644 --- a/tests/unit/api/objectGetAttributes.js +++ b/tests/unit/api/objectGetAttributes.js @@ -177,7 +177,6 @@ describe('objectGetAttributes API', () => { it('should return all attributes', async () => { const testGetRequest = createGetAttributesRequest([ 'ETag', - 'Checksum', 'ObjectParts', 'StorageClass', 'ObjectSize', @@ -193,7 +192,6 @@ describe('objectGetAttributes API', () => { assert.strictEqual(response.ETag[0], expectedMD5); assert.strictEqual(response.StorageClass[0], 'STANDARD'); assert.strictEqual(response.ObjectSize[0], String(body.length)); - assert.deepStrictEqual(response.Checksum[0], '', 'Checksum should be empty'); assert.strictEqual(response.ObjectParts, undefined, "ObjectParts shouldn't be present for non-MPU object"); assert(headers['Last-Modified'], 'LastModified should be present'); }); @@ -206,12 +204,16 @@ describe('objectGetAttributes API', () => { assert.strictEqual(result.GetObjectAttributesResponse.ETag[0], expectedMD5); }); - it('should return Checksum', async () => { + it('should fail with NotImplemented when Checksum is requested', async () => { const testGetRequest = createGetAttributesRequest(['Checksum']); - const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); - const result = await parseStringPromise(xml); - assert.deepStrictEqual(result.GetObjectAttributesResponse.Checksum[0], '', 'Checksum should be empty'); + try { + await objectGetAttributesAsync(authInfo, testGetRequest, log); + assert.fail('Expected error was not thrown'); + } catch (err) { + assert.strictEqual(err.is.NotImplemented, true); + assert.strictEqual(err.description, 'Checksum attribute is not implemented'); + } }); it("shouldn't return ObjectParts for non-MPU object", async () => { diff --git a/yarn.lock b/yarn.lock index c3066dec2e..497d3a7199 100644 --- a/yarn.lock +++ b/yarn.lock @@ -602,9 +602,9 @@ integrity sha512-EKQmr16tM8s16vTT3cA5L0kZZcTMU5DUOZTuvpnY738m+jyP3JIUj+Mm1xc1rsLkGBQ/gVnfKYPwOmPg1tUR4Q== "@hapi/tlds@^1.1.1": - version "1.1.4" - resolved "https://registry.yarnpkg.com/@hapi/tlds/-/tlds-1.1.4.tgz#df4a7b59082b54ba4f3b7b38f781e2ac3cbc359a" - integrity sha512-Fq+20dxsxLaUn5jSSWrdtSRcIUba2JquuorF9UW1wIJS5cSUwxIsO2GIhaWynPRflvxSzFN+gxKte2HEW1OuoA== + version "1.1.5" + resolved "https://registry.yarnpkg.com/@hapi/tlds/-/tlds-1.1.5.tgz#3eff5d8a3bd20833a2d779e0f839d25177fee722" + integrity sha512-Vq/1gnIIsvFUpKlDdfrPd/ssHDpAyBP/baVukh3u2KSG2xoNjsnRNjQiPmuyPPGqsn1cqVWWhtZHfOBaLizFRQ== "@hapi/topo@^5.0.0", "@hapi/topo@^5.1.0": version "5.1.0" @@ -1502,7 +1502,7 @@ arraybuffer.prototype.slice@^1.0.4: "arsenal@git+https://github.com/scality/Arsenal#feature/ARSN-549/get-object-attributes": version "8.2.44" - resolved "git+https://github.com/scality/Arsenal#f02179acbfb6aff963c2fc72146f091544ddf86c" + resolved "git+https://github.com/scality/Arsenal#475f746b6d201baaa46cc9ac2cdc0a2ed121fa43" dependencies: "@azure/identity" "^4.13.0" "@azure/storage-blob" "^12.28.0" From b2ef4fd27c63f3db4a133021423877656cbb266d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Fri, 30 Jan 2026 17:05:54 +0100 Subject: [PATCH 10/17] Support user metadata Issue: CLDSRV-844 --- .../apiUtils/object/parseAttributesHeader.js | 2 +- lib/api/objectGetAttributes.js | 23 +++ .../test/object/objectGetAttributes.js | 194 ++++++++++++++++++ .../apiUtils/object/parseAttributesHeader.js | 52 +++++ tests/unit/api/objectGetAttributes.js | 126 ++++++++++++ 5 files changed, 396 insertions(+), 1 deletion(-) diff --git a/lib/api/apiUtils/object/parseAttributesHeader.js b/lib/api/apiUtils/object/parseAttributesHeader.js index 685ac93884..103b550b9d 100644 --- a/lib/api/apiUtils/object/parseAttributesHeader.js +++ b/lib/api/apiUtils/object/parseAttributesHeader.js @@ -15,7 +15,7 @@ function parseAttributesHeaders(headers) { ); } - if (attributes.some(attr => !supportedGetObjectAttributes.has(attr))) { + if (attributes.some(attr => !attr.startsWith('x-amz-meta-') && !supportedGetObjectAttributes.has(attr))) { throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.'); } diff --git a/lib/api/objectGetAttributes.js b/lib/api/objectGetAttributes.js index 696f613baa..20d75db54a 100644 --- a/lib/api/objectGetAttributes.js +++ b/lib/api/objectGetAttributes.js @@ -41,10 +41,33 @@ function buildXmlResponse(objMD, requestedAttrs) { } } + if (Array.from(requestedAttrs).some(attr => attr.startsWith('x-amz-meta-'))) { + const userMetadata = parseUserMetadata(objMD, requestedAttrs); + Object.assign(attrResp, userMetadata); + } + const builder = new xml2js.Builder(); return builder.buildObject({ GetObjectAttributesResponse: attrResp }); } +/** + * parseUserMetadata - Extract user metadata from object metadata + * @param {object} objMD - object metadata + * @param {Set} attributes - requested attributes + * @returns {object} - object containing requested user metadata key-value pairs + */ +function parseUserMetadata(objMD, attributes) { + const sourceKeys = attributes.has('x-amz-meta-*') ? Object.keys(objMD) : Array.from(attributes); + + return sourceKeys + .filter(key => key.startsWith('x-amz-meta-') && key !== 'x-amz-meta-*' && objMD[key]) + .reduce((acc, key) => { + // eslint-disable-next-line no-param-reassign + acc[key] = objMD[key]; + return acc; + }, {}); +} + /** * objectGetAttributes - Retrieves all metadata from an object without returning the object itself * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info diff --git a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js index 5f99e31097..21cee46fa7 100644 --- a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js +++ b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const { parseStringPromise } = require('xml2js'); const { S3 } = require('aws-sdk'); const getConfig = require('../support/config'); @@ -265,3 +266,196 @@ describe('Test get object attributes with multipart upload', () => { assert.strictEqual(data.StorageClass, 'STANDARD'); }); }); + +describe('objectGetAttributes with user metadata', () => { + let s3; + + const getObjectAttributesWithUserAttributes = (s3, params, attributes) => new Promise((resolve, reject) => { + const request = s3.makeRequest('getObjectAttributes', { + Bucket: params.Bucket, + Key: params.Key, + ObjectAttributes: ['ETag'], + }); + + request.on('build', () => { + request.httpRequest.headers['x-amz-object-attributes'] = attributes; + }); + + request.on('error', err => reject(err)); + + request.on('success', async response => { + try { + const body = response.httpResponse.body.toString(); + const parsedXml = await parseStringPromise(body); + + const data = response.data; + const parsedData = parsedXml.GetObjectAttributesResponse; + + if (!data || !parsedData) { + return resolve(response.data); + } + + Object.keys(parsedData).forEach(key => { + if (key.startsWith('x-amz-meta-')) { + data[key] = parsedData[key][0]; + } + }); + + return resolve(response.data); + } catch (err) { + return reject(err); + } + }); + + request.send(); + }); + + before(() => { + const config = getConfig('default', { signatureVersion: 'v4' }); + s3 = new S3(config); + }); + + beforeEach(async () => { + await s3.createBucket({ Bucket: bucket }).promise(); + }); + + afterEach(async () => { + await s3.deleteObject({ Bucket: bucket, Key: key }).promise(); + await s3.deleteBucket({ Bucket: bucket }).promise(); + }); + + it('should return specific user metadata when requested', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + 'custom-key': 'custom-value', + 'another-key': 'another-value', + }, + }).promise(); + + const response = await getObjectAttributesWithUserAttributes(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-custom-key'); + + assert.strictEqual(response['x-amz-meta-custom-key'], 'custom-value'); + }); + + it('should return multiple user metadata when requested', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + foo: 'foo-value', + bar: 'bar-value', + baz: 'baz-value', + }, + }).promise(); + + const response = await getObjectAttributesWithUserAttributes(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-foo,x-amz-meta-bar'); + + assert.strictEqual(response['x-amz-meta-foo'], 'foo-value'); + assert.strictEqual(response['x-amz-meta-bar'], 'bar-value'); + }); + + it('should return all user metadata when x-amz-meta-* is requested', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + key1: 'value1', + key2: 'value2', + key3: 'value3', + }, + }).promise(); + + const response = await getObjectAttributesWithUserAttributes(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-*'); + + assert.strictEqual(response['x-amz-meta-key1'], 'value1'); + assert.strictEqual(response['x-amz-meta-key2'], 'value2'); + assert.strictEqual(response['x-amz-meta-key3'], 'value3'); + }); + + it('should return empty response when object has no user metadata and x-amz-meta-* is requested', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + }).promise(); + + const response = await getObjectAttributesWithUserAttributes(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-*'); + + const metadataKeys = Object.keys(response).filter(k => k.startsWith('x-amz-meta-')); + assert.strictEqual(metadataKeys.length, 0); + }); + + it('should return empty response when requested metadata key does not exist', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + existing: 'value', + }, + }).promise(); + + const response = await getObjectAttributesWithUserAttributes(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-nonexistent'); + + assert.strictEqual(response['x-amz-meta-nonexistent'], undefined); + }); + + it('should return user metadata along with standard attributes', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + custom: 'custom-value', + }, + }).promise(); + + const response = await getObjectAttributesWithUserAttributes(s3, { + Bucket: bucket, + Key: key, + }, 'ETag,x-amz-meta-custom,ObjectSize'); + + assert.strictEqual(response.ETag, expectedMD5); + assert.strictEqual(response.ObjectSize, body.length); + assert.strictEqual(response['x-amz-meta-custom'], 'custom-value'); + }); + + it('should not include x-amz-meta-* marker in response when wildcard is used', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + test: 'test-value', + }, + }).promise(); + + const response = await getObjectAttributesWithUserAttributes(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-*'); + + assert.strictEqual(response['x-amz-meta-*'], undefined); + assert.strictEqual(response['x-amz-meta-test'], 'test-value'); + }); +}); diff --git a/tests/unit/api/apiUtils/object/parseAttributesHeader.js b/tests/unit/api/apiUtils/object/parseAttributesHeader.js index b722668880..a7bce6af70 100644 --- a/tests/unit/api/apiUtils/object/parseAttributesHeader.js +++ b/tests/unit/api/apiUtils/object/parseAttributesHeader.js @@ -171,6 +171,58 @@ describe('parseAttributesHeaders', () => { }); }); + describe('user metadata attributes (x-amz-meta-*)', () => { + it('should return array with single user metadata attribute', () => { + const headers = { 'x-amz-object-attributes': 'x-amz-meta-custom' }; + const result = parseAttributesHeaders(headers); + + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['x-amz-meta-custom'])); + }); + + it('should return array with multiple user metadata attributes', () => { + const headers = { 'x-amz-object-attributes': 'x-amz-meta-foo,x-amz-meta-bar' }; + const result = parseAttributesHeaders(headers); + + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['x-amz-meta-foo', 'x-amz-meta-bar'])); + }); + + it('should return array with mixed valid attributes and user metadata', () => { + const headers = { 'x-amz-object-attributes': 'ETag,x-amz-meta-custom,ObjectSize' }; + const result = parseAttributesHeaders(headers); + + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['ETag', 'x-amz-meta-custom', 'ObjectSize'])); + }); + + it('should allow user metadata with special characters in name', () => { + const headers = { 'x-amz-object-attributes': 'x-amz-meta-*' }; + const result = parseAttributesHeaders(headers); + + assert(result instanceof Set); + assert.deepStrictEqual(result, new Set(['x-amz-meta-*'])); + }); + + it('should reject attributes without the required x-amz-meta- prefix', () => { + const invalidAttributes = ['x-amz-met', 'x-amz-other']; + + invalidAttributes.forEach(attr => { + const headers = { 'x-amz-object-attributes': attr }; + + assert.throws( + () => parseAttributesHeaders(headers), + err => { + assert(err.is); + assert.strictEqual(err.is.InvalidArgument, true); + assert.strictEqual(err.description, 'Invalid attribute name specified.'); + return true; + }, + ); + }); + }); + }); + describe('whitespace handling', () => { it('should trim whitespace around attribute names', () => { const headers = { 'x-amz-object-attributes': ' ETag , ObjectSize ' }; diff --git a/tests/unit/api/objectGetAttributes.js b/tests/unit/api/objectGetAttributes.js index b9e756f991..cb3a7bb1b6 100644 --- a/tests/unit/api/objectGetAttributes.js +++ b/tests/unit/api/objectGetAttributes.js @@ -348,6 +348,132 @@ describe('objectGetAttributes API with multipart upload', () => { }); }); +describe('objectGetAttributes API with user metadata', () => { + beforeEach(async () => { + cleanup(); + await bucketPutAsync(authInfo, testPutBucketRequest, log); + }); + + const createObjectWithMetadata = async (metadata = {}) => { + const testPutObjectRequest = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'content-length': `${postBody.length}`, + ...metadata, + }, + parsedContentLength: postBody.length, + url: `/${bucketName}/${objectName}`, + }, + postBody, + ); + await objectPutAsync(authInfo, testPutObjectRequest, undefined, log); + }; + + it('should return specific user metadata when requested', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-custom-key': 'custom-value', + 'x-amz-meta-another-key': 'another-value', + }); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-custom-key']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response['x-amz-meta-custom-key'][0], 'custom-value'); + }); + + it('should return multiple user metadata when requested', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-foo': 'foo-value', + 'x-amz-meta-bar': 'bar-value', + 'x-amz-meta-baz': 'baz-value', + }); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-foo', 'x-amz-meta-bar']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response['x-amz-meta-foo'][0], 'foo-value'); + assert.strictEqual(response['x-amz-meta-bar'][0], 'bar-value'); + }); + + it('should return all user metadata when x-amz-meta-* is requested', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-key1': 'value1', + 'x-amz-meta-key2': 'value2', + 'x-amz-meta-key3': 'value3', + }); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-*']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response['x-amz-meta-key1'][0], 'value1'); + assert.strictEqual(response['x-amz-meta-key2'][0], 'value2'); + assert.strictEqual(response['x-amz-meta-key3'][0], 'value3'); + }); + + it('should return empty response when object has no user metadata and x-amz-meta-* is requested', async () => { + await createObjectWithMetadata({}); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-*']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + const metadataKeys = Object.keys(response).filter(k => k.startsWith('x-amz-meta-')); + assert.strictEqual(metadataKeys.length, 0); + }); + + it('should return empty response when requested metadata key does not exist', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-existing': 'value', + }); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-nonexistent']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response['x-amz-meta-nonexistent'], undefined); + }); + + it('should return user metadata along with standard attributes', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-custom': 'custom-value', + }); + + const testGetRequest = createGetAttributesRequest(['ETag', 'x-amz-meta-custom', 'ObjectSize']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response.ETag[0], expectedMD5); + assert.strictEqual(response.ObjectSize[0], String(body.length)); + assert.strictEqual(response['x-amz-meta-custom'][0], 'custom-value'); + }); + + it('should not include x-amz-meta-* marker in response when wildcard is used', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-test': 'test-value', + }); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-*']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response['x-amz-meta-*'], undefined); + assert.strictEqual(response['x-amz-meta-test'][0], 'test-value'); + }); +}); + describe('objectGetAttributes API with versioning', () => { const enableVersioningRequest = versioningTestUtils.createBucketPutVersioningReq(bucketName, 'Enabled'); From c9fddbac40450beaadae7ca83d6e06454463be94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Mon, 2 Feb 2026 19:21:31 +0100 Subject: [PATCH 11/17] Add a new permission to get user metadata Issue: CLDSRV-844 --- .../authorization/prepareRequestContexts.js | 7 +++ .../test/object/objectGetAttributes.js | 16 +++---- .../authorization/prepareRequestContexts.js | 48 +++++++++++++++++++ 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/lib/api/apiUtils/authorization/prepareRequestContexts.js b/lib/api/apiUtils/authorization/prepareRequestContexts.js index c62ace060a..578036eae0 100644 --- a/lib/api/apiUtils/authorization/prepareRequestContexts.js +++ b/lib/api/apiUtils/authorization/prepareRequestContexts.js @@ -261,6 +261,13 @@ function prepareRequestContexts(apiMethod, request, sourceBucket, if (requestedAttributes.filter(attr => attr != 'RestoreStatus').length > 0) { requestContexts.push(generateRequestContext('listObjectsV2OptionalAttributes')); } + } else if (apiMethodAfterVersionCheck === 'objectGetAttributes') { + requestContexts.push(generateRequestContext(apiMethodAfterVersionCheck)); + + const attributes = request.headers['x-amz-object-attributes']?.split(',') ?? []; + if (attributes.some(attr => attr.startsWith('x-amz-meta-'))) { + requestContexts.push(generateRequestContext('getObjectAttributes')); + } } else { const requestContext = generateRequestContext(apiMethodAfterVersionCheck); diff --git a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js index 21cee46fa7..8aa3c14f99 100644 --- a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js +++ b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js @@ -270,7 +270,7 @@ describe('Test get object attributes with multipart upload', () => { describe('objectGetAttributes with user metadata', () => { let s3; - const getObjectAttributesWithUserAttributes = (s3, params, attributes) => new Promise((resolve, reject) => { + const getObjectAttributesWithUserMetadata = (s3, params, attributes) => new Promise((resolve, reject) => { const request = s3.makeRequest('getObjectAttributes', { Bucket: params.Bucket, Key: params.Key, @@ -335,7 +335,7 @@ describe('objectGetAttributes with user metadata', () => { }, }).promise(); - const response = await getObjectAttributesWithUserAttributes(s3, { + const response = await getObjectAttributesWithUserMetadata(s3, { Bucket: bucket, Key: key, }, 'x-amz-meta-custom-key'); @@ -355,7 +355,7 @@ describe('objectGetAttributes with user metadata', () => { }, }).promise(); - const response = await getObjectAttributesWithUserAttributes(s3, { + const response = await getObjectAttributesWithUserMetadata(s3, { Bucket: bucket, Key: key, }, 'x-amz-meta-foo,x-amz-meta-bar'); @@ -376,7 +376,7 @@ describe('objectGetAttributes with user metadata', () => { }, }).promise(); - const response = await getObjectAttributesWithUserAttributes(s3, { + const response = await getObjectAttributesWithUserMetadata(s3, { Bucket: bucket, Key: key, }, 'x-amz-meta-*'); @@ -393,7 +393,7 @@ describe('objectGetAttributes with user metadata', () => { Body: body, }).promise(); - const response = await getObjectAttributesWithUserAttributes(s3, { + const response = await getObjectAttributesWithUserMetadata(s3, { Bucket: bucket, Key: key, }, 'x-amz-meta-*'); @@ -412,7 +412,7 @@ describe('objectGetAttributes with user metadata', () => { }, }).promise(); - const response = await getObjectAttributesWithUserAttributes(s3, { + const response = await getObjectAttributesWithUserMetadata(s3, { Bucket: bucket, Key: key, }, 'x-amz-meta-nonexistent'); @@ -430,7 +430,7 @@ describe('objectGetAttributes with user metadata', () => { }, }).promise(); - const response = await getObjectAttributesWithUserAttributes(s3, { + const response = await getObjectAttributesWithUserMetadata(s3, { Bucket: bucket, Key: key, }, 'ETag,x-amz-meta-custom,ObjectSize'); @@ -450,7 +450,7 @@ describe('objectGetAttributes with user metadata', () => { }, }).promise(); - const response = await getObjectAttributesWithUserAttributes(s3, { + const response = await getObjectAttributesWithUserMetadata(s3, { Bucket: bucket, Key: key, }, 'x-amz-meta-*'); diff --git a/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js b/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js index 0a08b104f3..9453fdb4f0 100644 --- a/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js +++ b/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js @@ -396,4 +396,52 @@ describe('prepareRequestContexts', () => { }); }); }); + + describe('objectGetAttributes', () => { + describe('x-amz-object-attributes header', () => { + it('should request for specific permission if the header is set', () => { + const apiMethod = 'objectGetAttributes'; + const request = makeRequest({ + 'x-amz-object-attributes': 'x-amz-meta-department', + }); + const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); + + assert.strictEqual(results.length, 2); + assert.strictEqual(results[0].getAction(), 's3:GetObjectAttributes'); + assert.strictEqual(results[1].getAction(), 'scality:GetObjectAttributes'); + }); + + it('should request for specific permission if the header is set with multiple value', () => { + const apiMethod = 'objectGetAttributes'; + const request = makeRequest({ + 'x-amz-object-attributes': 'x-amz-meta-department,ETag', + }); + const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); + + assert.strictEqual(results.length, 2); + assert.strictEqual(results[0].getAction(), 's3:GetObjectAttributes'); + assert.strictEqual(results[1].getAction(), 'scality:GetObjectAttributes'); + }); + + it('should not request permission if the header contains only RestoreStatus', () => { + const apiMethod = 'objectGetAttributes'; + const request = makeRequest({ + 'x-amz-object-attributes': 'RestoreStatus', + }); + const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); + + assert.strictEqual(results.length, 1); + assert.strictEqual(results[0].getAction(), 's3:GetObjectAttributes'); + }); + + it('should not request permission if the header does not exists', () => { + const apiMethod = 'objectGetAttributes'; + const request = makeRequest({}); + const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); + + assert.strictEqual(results.length, 1); + assert.strictEqual(results[0].getAction(), 's3:GetObjectAttributes'); + }); + }); + }); }); From 8d118687d27afb6b409386de48bd1230a02f2b59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Tue, 3 Feb 2026 12:07:11 +0100 Subject: [PATCH 12/17] Parse optional attributes header with utility function --- .../apiUtils/object/parseAttributesHeader.js | 28 +- lib/api/bucketGet.js | 17 +- lib/api/objectGetAttributes.js | 3 +- .../apiUtils/object/parseAttributesHeader.js | 278 +++--------------- 4 files changed, 63 insertions(+), 263 deletions(-) diff --git a/lib/api/apiUtils/object/parseAttributesHeader.js b/lib/api/apiUtils/object/parseAttributesHeader.js index 103b550b9d..81c22dc4b2 100644 --- a/lib/api/apiUtils/object/parseAttributesHeader.js +++ b/lib/api/apiUtils/object/parseAttributesHeader.js @@ -1,21 +1,29 @@ const { errorInstances } = require('arsenal'); -const { supportedGetObjectAttributes } = require('../../../../constants'); /** - * parseAttributesHeaders - Parse and validate the x-amz-object-attributes header - * @param {object} headers - request headers - * @returns {Set} - set of requested attribute names - * @throws {Error} - InvalidRequest if header is missing/empty, InvalidArgument if attribute is invalid + * Parse and validate attribute headers from a request. + * @param {object} headers - Request headers object + * @param {string} headerName - Name of the header to parse (e.g., 'x-amz-object-attributes') + * @param {Set} supportedAttributes - Set of valid attribute names + * @param {boolean} [isRequired=false] - If true, throws when header is missing/empty + * @returns {string[]} Array of validated attribute names + * @throws {arsenal.errors.InvalidRequest} When header is required but missing/empty + * @throws {arsenal.errors.InvalidArgument} When an invalid attribute name is specified */ -function parseAttributesHeaders(headers) { - const attributes = headers['x-amz-object-attributes']?.split(',').map(attr => attr.trim()) ?? []; - if (attributes.length === 0) { +function parseAttributesHeaders(headers, headerName, supportedAttributes, isRequired = false) { + const attributes = + headers[headerName] + ?.split(',') + .map(attr => attr.trim()) + .map(attr => (supportedAttributes.has(attr) ? attr : attr.toLowerCase())) ?? []; + + if (isRequired && attributes.length === 0) { throw errorInstances.InvalidRequest.customizeDescription( - 'The x-amz-object-attributes header specifying the attributes to be retrieved is either missing or empty', + `The ${headerName} header specifying the attributes to be retrieved is either missing or empty`, ); } - if (attributes.some(attr => !attr.startsWith('x-amz-meta-') && !supportedGetObjectAttributes.has(attr))) { + if (attributes.some(attr => !attr.startsWith('x-amz-meta-') && !supportedAttributes.has(attr))) { throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.'); } diff --git a/lib/api/bucketGet.js b/lib/api/bucketGet.js index 3f5e352b75..343722b670 100644 --- a/lib/api/bucketGet.js +++ b/lib/api/bucketGet.js @@ -10,6 +10,7 @@ const versionIdUtils = versioning.VersionID; const monitoring = require('../utilities/monitoringHandler'); const { generateToken, decryptToken } = require('../api/apiUtils/object/continueToken'); +const parseAttributesHeaders = require('./apiUtils/object/parseAttributesHeader'); // do not url encode the continuation tokens const skipUrlEncoding = new Set([ @@ -332,16 +333,12 @@ function bucketGet(authInfo, request, log, callback) { const bucketName = request.bucketName; const v2 = params['list-type']; - const optionalAttributes = - request.headers['x-amz-optional-object-attributes'] - ?.split(',') - .map(attr => attr.trim()) - .map(attr => attr !== 'RestoreStatus' ? attr.toLowerCase() : attr) - ?? []; - if (optionalAttributes.some(attr => !attr.startsWith('x-amz-meta-') && attr != 'RestoreStatus')) { - return callback( - errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified') - ); + let optionalAttributes; + try { + const headerName = 'x-amz-optional-object-attributes'; + optionalAttributes = parseAttributesHeaders(request.headers, headerName, new Set(['RestoreStatus'])); + } catch (err) { + return callback(err); } if (v2 !== undefined && Number.parseInt(v2, 10) !== 2) { diff --git a/lib/api/objectGetAttributes.js b/lib/api/objectGetAttributes.js index 20d75db54a..33a26dd9c7 100644 --- a/lib/api/objectGetAttributes.js +++ b/lib/api/objectGetAttributes.js @@ -8,6 +8,7 @@ const { decodeVersionId, getVersionIdResHeader } = require('./apiUtils/object/ve const { checkExpectedBucketOwner } = require('./apiUtils/authorization/bucketOwner'); const { pushMetric } = require('../utapi/utilities'); const { getPartCountFromMd5 } = require('./apiUtils/object/partInfo'); +const { supportedGetObjectAttributes } = require('../../constants'); const checkExpectedBucketOwnerPromise = promisify(checkExpectedBucketOwner); const validateBucketAndObj = promisify(standardMetadataValidateBucketAndObj); @@ -154,7 +155,7 @@ async function objectGetAttributes(authInfo, request, log, callback) { throw err; } - const requestedAttrs = parseAttributesHeaders(headers); + const requestedAttrs = parseAttributesHeaders(headers, 'x-amz-object-attributes', supportedGetObjectAttributes, true); if (requestedAttrs.has('Checksum')) { log.debug('Checksum attribute requested but not implemented', { diff --git a/tests/unit/api/apiUtils/object/parseAttributesHeader.js b/tests/unit/api/apiUtils/object/parseAttributesHeader.js index a7bce6af70..75b8772fc1 100644 --- a/tests/unit/api/apiUtils/object/parseAttributesHeader.js +++ b/tests/unit/api/apiUtils/object/parseAttributesHeader.js @@ -2,262 +2,56 @@ const assert = require('assert'); const parseAttributesHeaders = require('../../../../../lib/api/apiUtils/object/parseAttributesHeader'); -describe('parseAttributesHeaders', () => { - describe('missing or empty header', () => { - it('should throw InvalidRequest error when header is missing', () => { - const headers = {}; - - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidRequest, true); - assert.strictEqual( - err.description, - 'The x-amz-object-attributes header specifying the attributes to be retrieved is either missing or empty', - ); - return true; - }, - ); - }); - - it('should throw InvalidArgument error when header is empty string', () => { - const headers = { 'x-amz-object-attributes': '' }; - - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); - - it('should throw InvalidArgument error when header contains only whitespace', () => { - const headers = { 'x-amz-object-attributes': ' ' }; +const headerName = 'x-amz-object-attributes'; +const allowedAttributes = new Set(['ETag', 'StorageClass', 'ObjectSize']); - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); - - it('should throw InvalidArgument error when header contains only commas', () => { - const headers = { 'x-amz-object-attributes': ',,,' }; - - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); +describe('parseAttributesHeaders', () => { + it('should throw InvalidRequest when header is missing and isRequired is true', () => { + assert.throws( + () => parseAttributesHeaders({}, headerName, allowedAttributes, true), + err => { + assert.strictEqual(err.is.InvalidRequest, true); + return true; + }, + ); }); - describe('invalid attribute names', () => { - it('should throw InvalidArgument error for single invalid attribute', () => { - const headers = { 'x-amz-object-attributes': 'InvalidAttribute' }; - - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); + it('should throw InvalidArgument when attribute is invalid', () => { + const headers = { [headerName]: 'InvalidAttribute' }; - it('should throw InvalidArgument error when one attribute is invalid among valid ones', () => { - const headers = { 'x-amz-object-attributes': 'ETag,InvalidAttribute,ObjectSize' }; - - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); - - it('should throw InvalidArgument error for multiple invalid attributes', () => { - const headers = { 'x-amz-object-attributes': 'Invalid1,Invalid2' }; - - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); + assert.throws( + () => parseAttributesHeaders(headers, headerName, allowedAttributes), + err => { + assert.strictEqual(err.is.InvalidArgument, true); + return true; + }, + ); }); - describe('valid attribute names', () => { - it('should return set with single valid attribute ETag', () => { - const headers = { 'x-amz-object-attributes': 'ETag' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['ETag'])); - }); - - it('should return set with single valid attribute StorageClass', () => { - const headers = { 'x-amz-object-attributes': 'StorageClass' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['StorageClass'])); - }); - - it('should return set with single valid attribute ObjectSize', () => { - const headers = { 'x-amz-object-attributes': 'ObjectSize' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['ObjectSize'])); - }); - - it('should return set with single valid attribute ObjectParts', () => { - const headers = { 'x-amz-object-attributes': 'ObjectParts' }; - const result = parseAttributesHeaders(headers); + it('should return empty array when header is missing and isRequired is false', () => { + const result = parseAttributesHeaders({}, headerName, allowedAttributes); - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['ObjectParts'])); - }); - - it('should return set with single valid attribute Checksum', () => { - const headers = { 'x-amz-object-attributes': 'Checksum' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['Checksum'])); - }); - - it('should return set with multiple valid attributes', () => { - const headers = { 'x-amz-object-attributes': 'ETag,ObjectSize,StorageClass' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['ETag', 'ObjectSize', 'StorageClass'])); - }); - - it('should return set with all valid attributes', () => { - const headers = { 'x-amz-object-attributes': 'StorageClass,ObjectSize,ObjectParts,Checksum,ETag' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.strictEqual(result.size, 5); - assert(result.has('StorageClass')); - assert(result.has('ObjectSize')); - assert(result.has('ObjectParts')); - assert(result.has('Checksum')); - assert(result.has('ETag')); - }); + assert.deepStrictEqual(result, new Set([])); }); - describe('user metadata attributes (x-amz-meta-*)', () => { - it('should return array with single user metadata attribute', () => { - const headers = { 'x-amz-object-attributes': 'x-amz-meta-custom' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['x-amz-meta-custom'])); - }); - - it('should return array with multiple user metadata attributes', () => { - const headers = { 'x-amz-object-attributes': 'x-amz-meta-foo,x-amz-meta-bar' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['x-amz-meta-foo', 'x-amz-meta-bar'])); - }); + it('should parse valid attributes', () => { + const headers = { [headerName]: 'ETag,ObjectSize,x-amz-meta-custom,x-amz-meta-*' }; + const result = parseAttributesHeaders(headers, headerName, allowedAttributes); - it('should return array with mixed valid attributes and user metadata', () => { - const headers = { 'x-amz-object-attributes': 'ETag,x-amz-meta-custom,ObjectSize' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['ETag', 'x-amz-meta-custom', 'ObjectSize'])); - }); - - it('should allow user metadata with special characters in name', () => { - const headers = { 'x-amz-object-attributes': 'x-amz-meta-*' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['x-amz-meta-*'])); - }); - - it('should reject attributes without the required x-amz-meta- prefix', () => { - const invalidAttributes = ['x-amz-met', 'x-amz-other']; - - invalidAttributes.forEach(attr => { - const headers = { 'x-amz-object-attributes': attr }; - - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); - }); + assert.deepStrictEqual(result, new Set(['ETag', 'ObjectSize', 'x-amz-meta-custom', 'x-amz-meta-*'])); }); - describe('whitespace handling', () => { - it('should trim whitespace around attribute names', () => { - const headers = { 'x-amz-object-attributes': ' ETag , ObjectSize ' }; - const result = parseAttributesHeaders(headers); - - assert(result instanceof Set); - assert.deepStrictEqual(result, new Set(['ETag', 'ObjectSize'])); - }); + it('should lowercase attributes not in allowedAttributes', () => { + const headers = { [headerName]: 'ETag,X-AMZ-META-CUSTOM' }; + const result = parseAttributesHeaders(headers, headerName, allowedAttributes); - it('should throw InvalidArgument for extra commas between attributes', () => { - const headers = { 'x-amz-object-attributes': 'ETag,,ObjectSize' }; - - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); + assert.deepStrictEqual(result, new Set(['ETag', 'x-amz-meta-custom'])); + }); - it('should throw InvalidArgument for leading and trailing commas', () => { - const headers = { 'x-amz-object-attributes': ',ETag,ObjectSize,' }; + it('should trim whitespace around attribute names', () => { + const headers = { [headerName]: ' ETag , ObjectSize ' }; + const result = parseAttributesHeaders(headers, headerName, allowedAttributes); - assert.throws( - () => parseAttributesHeaders(headers), - err => { - assert(err.is); - assert.strictEqual(err.is.InvalidArgument, true); - assert.strictEqual(err.description, 'Invalid attribute name specified.'); - return true; - }, - ); - }); + assert.deepStrictEqual(result, new Set(['ETag', 'ObjectSize'])); }); }); From cdd214259837d10edfe44be0d8cf072609fda1db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Wed, 4 Feb 2026 16:22:31 +0100 Subject: [PATCH 13/17] Handle rights for GetObjectAttributes Issue: CLDSRV-844 --- .../authorization/prepareRequestContexts.js | 14 ++++- .../authorization/prepareRequestContexts.js | 60 ++++++++++++++----- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/lib/api/apiUtils/authorization/prepareRequestContexts.js b/lib/api/apiUtils/authorization/prepareRequestContexts.js index 578036eae0..f30b0ab56b 100644 --- a/lib/api/apiUtils/authorization/prepareRequestContexts.js +++ b/lib/api/apiUtils/authorization/prepareRequestContexts.js @@ -262,11 +262,21 @@ function prepareRequestContexts(apiMethod, request, sourceBucket, requestContexts.push(generateRequestContext('listObjectsV2OptionalAttributes')); } } else if (apiMethodAfterVersionCheck === 'objectGetAttributes') { - requestContexts.push(generateRequestContext(apiMethodAfterVersionCheck)); + if (request.headers['x-amz-version-id']) { + requestContexts.push( + generateRequestContext('objectGetVersion'), + generateRequestContext('objectGetVersionAttributes'), + ); + } else { + requestContexts.push( + generateRequestContext('objectGet'), + generateRequestContext('objectGetAttributes'), + ); + } const attributes = request.headers['x-amz-object-attributes']?.split(',') ?? []; if (attributes.some(attr => attr.startsWith('x-amz-meta-'))) { - requestContexts.push(generateRequestContext('getObjectAttributes')); + requestContexts.push(generateRequestContext('objectGetAttributesWithUserMetadata')); } } else { const requestContext = diff --git a/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js b/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js index 9453fdb4f0..5272b1934e 100644 --- a/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js +++ b/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js @@ -399,48 +399,80 @@ describe('prepareRequestContexts', () => { describe('objectGetAttributes', () => { describe('x-amz-object-attributes header', () => { - it('should request for specific permission if the header is set', () => { + it('should include scality:GetObjectAttributes with x-amz-meta attribute', () => { const apiMethod = 'objectGetAttributes'; const request = makeRequest({ 'x-amz-object-attributes': 'x-amz-meta-department', }); const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); - assert.strictEqual(results.length, 2); - assert.strictEqual(results[0].getAction(), 's3:GetObjectAttributes'); - assert.strictEqual(results[1].getAction(), 'scality:GetObjectAttributes'); + assert.strictEqual(results.length, 3); + assert.strictEqual(results[0].getAction(), 's3:GetObject'); + assert.strictEqual(results[1].getAction(), 's3:GetObjectAttributes'); + assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributes'); }); - it('should request for specific permission if the header is set with multiple value', () => { + it('should include scality:GetObjectAttributes with multiple attributes', () => { const apiMethod = 'objectGetAttributes'; const request = makeRequest({ 'x-amz-object-attributes': 'x-amz-meta-department,ETag', }); const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); - assert.strictEqual(results.length, 2); - assert.strictEqual(results[0].getAction(), 's3:GetObjectAttributes'); - assert.strictEqual(results[1].getAction(), 'scality:GetObjectAttributes'); + assert.strictEqual(results.length, 3); + assert.strictEqual(results[0].getAction(), 's3:GetObject'); + assert.strictEqual(results[1].getAction(), 's3:GetObjectAttributes'); + assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributes'); }); - it('should not request permission if the header contains only RestoreStatus', () => { + it('should not include scality:GetObjectAttributes with only RestoreStatus', () => { const apiMethod = 'objectGetAttributes'; const request = makeRequest({ 'x-amz-object-attributes': 'RestoreStatus', }); const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); - assert.strictEqual(results.length, 1); - assert.strictEqual(results[0].getAction(), 's3:GetObjectAttributes'); + assert.strictEqual(results.length, 2); + assert.strictEqual(results[0].getAction(), 's3:GetObject'); + assert.strictEqual(results[1].getAction(), 's3:GetObjectAttributes'); }); - it('should not request permission if the header does not exists', () => { + it('should not include scality:GetObjectAttributes without header', () => { const apiMethod = 'objectGetAttributes'; const request = makeRequest({}); const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); - assert.strictEqual(results.length, 1); - assert.strictEqual(results[0].getAction(), 's3:GetObjectAttributes'); + assert.strictEqual(results.length, 2); + assert.strictEqual(results[0].getAction(), 's3:GetObject'); + assert.strictEqual(results[1].getAction(), 's3:GetObjectAttributes'); + }); + }); + + describe('x-amz-version-id header', () => { + it('should return version-specific actions with x-amz-version-id', () => { + const apiMethod = 'objectGetAttributes'; + const request = makeRequest({ + 'x-amz-version-id': '0987654323456789', + }); + const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); + + assert.strictEqual(results.length, 2); + assert.strictEqual(results[0].getAction(), 's3:GetObjectVersion'); + assert.strictEqual(results[1].getAction(), 's3:GetObjectVersionAttributes'); + }); + + it('should include scality:GetObjectAttributes with x-amz-version-id and x-amz-meta', () => { + const apiMethod = 'objectGetAttributes'; + const request = makeRequest({ + 'x-amz-version-id': '0987654323456789', + 'x-amz-object-attributes': 'x-amz-meta-department', + }); + const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); + + assert.strictEqual(results.length, 3); + assert.strictEqual(results[0].getAction(), 's3:GetObjectVersion'); + assert.strictEqual(results[1].getAction(), 's3:GetObjectVersionAttributes'); + assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributes'); }); }); }); From 1d41b400b12a77543071bdc101fdbace5e70dfcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Tue, 10 Feb 2026 18:22:56 +0100 Subject: [PATCH 14/17] fixup! Add a new permission to get user metadata --- lib/api/apiUtils/authorization/prepareRequestContexts.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/apiUtils/authorization/prepareRequestContexts.js b/lib/api/apiUtils/authorization/prepareRequestContexts.js index f30b0ab56b..b6831d2a54 100644 --- a/lib/api/apiUtils/authorization/prepareRequestContexts.js +++ b/lib/api/apiUtils/authorization/prepareRequestContexts.js @@ -275,7 +275,7 @@ function prepareRequestContexts(apiMethod, request, sourceBucket, } const attributes = request.headers['x-amz-object-attributes']?.split(',') ?? []; - if (attributes.some(attr => attr.startsWith('x-amz-meta-'))) { + if (attributes.some(attr => attr.trim().toLowerCase().startsWith('x-amz-meta-'))) { requestContexts.push(generateRequestContext('objectGetAttributesWithUserMetadata')); } } else { From d507149ee48a1b0516a811df97b16f3ac4d15bae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Tue, 10 Feb 2026 18:24:28 +0100 Subject: [PATCH 15/17] fixup! Support user metadata --- .../apiUtils/object/parseAttributesHeader.js | 6 ++ lib/api/objectGetAttributes.js | 28 +++++---- .../test/object/objectGetAttributes.js | 62 +++++++++++++++++++ tests/unit/api/objectGetAttributes.js | 47 ++++++++++++++ 4 files changed, 131 insertions(+), 12 deletions(-) diff --git a/lib/api/apiUtils/object/parseAttributesHeader.js b/lib/api/apiUtils/object/parseAttributesHeader.js index 81c22dc4b2..ef7f67288e 100644 --- a/lib/api/apiUtils/object/parseAttributesHeader.js +++ b/lib/api/apiUtils/object/parseAttributesHeader.js @@ -9,6 +9,12 @@ const { errorInstances } = require('arsenal'); * @returns {string[]} Array of validated attribute names * @throws {arsenal.errors.InvalidRequest} When header is required but missing/empty * @throws {arsenal.errors.InvalidArgument} When an invalid attribute name is specified + * @example + * // Input headers: + * { 'headerName': 'ETag, ObjectSize, x-amz-meta-custom' } + * + * // Parsed result: + * ['ETag', 'ObjectSize', 'x-amz-meta-custom'] */ function parseAttributesHeaders(headers, headerName, supportedAttributes, isRequired = false) { const attributes = diff --git a/lib/api/objectGetAttributes.js b/lib/api/objectGetAttributes.js index 33a26dd9c7..c64e553aa0 100644 --- a/lib/api/objectGetAttributes.js +++ b/lib/api/objectGetAttributes.js @@ -43,7 +43,7 @@ function buildXmlResponse(objMD, requestedAttrs) { } if (Array.from(requestedAttrs).some(attr => attr.startsWith('x-amz-meta-'))) { - const userMetadata = parseUserMetadata(objMD, requestedAttrs); + const userMetadata = extractUserMetadata(objMD, requestedAttrs); Object.assign(attrResp, userMetadata); } @@ -52,21 +52,25 @@ function buildXmlResponse(objMD, requestedAttrs) { } /** - * parseUserMetadata - Extract user metadata from object metadata + * extractUserMetadata - Extract requested user metadata from object metadata * @param {object} objMD - object metadata * @param {Set} attributes - requested attributes * @returns {object} - object containing requested user metadata key-value pairs */ -function parseUserMetadata(objMD, attributes) { - const sourceKeys = attributes.has('x-amz-meta-*') ? Object.keys(objMD) : Array.from(attributes); - - return sourceKeys - .filter(key => key.startsWith('x-amz-meta-') && key !== 'x-amz-meta-*' && objMD[key]) - .reduce((acc, key) => { - // eslint-disable-next-line no-param-reassign - acc[key] = objMD[key]; - return acc; - }, {}); +function extractUserMetadata(objMD, attributes) { + const result = {}; + + const isWildcard = attributes.includes('x-amz-meta-*'); + const sourceKeys = isWildcard ? Object.keys(objMD) : attributes; + + for (const key of sourceKeys) { + const isValidKey = isWildcard ? key.startsWith('x-amz-meta-') : true; + if (isValidKey && objMD[key]) { + result[key] = objMD[key]; + } + } + + return result; } /** diff --git a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js index 8aa3c14f99..3e5646830a 100644 --- a/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js +++ b/tests/functional/aws-node-sdk/test/object/objectGetAttributes.js @@ -458,4 +458,66 @@ describe('objectGetAttributes with user metadata', () => { assert.strictEqual(response['x-amz-meta-*'], undefined); assert.strictEqual(response['x-amz-meta-test'], 'test-value'); }); + + it('should return all metadata when wildcard is combined with specific metadata key', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + key1: 'value1', + key2: 'value2', + key3: 'value3', + }, + }).promise(); + + const response = await getObjectAttributesWithUserMetadata(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-*,x-amz-meta-key1'); + + assert.strictEqual(response['x-amz-meta-key1'], 'value1'); + assert.strictEqual(response['x-amz-meta-key2'], 'value2'); + assert.strictEqual(response['x-amz-meta-key3'], 'value3'); + }); + + it('should handle duplicate wildcard requests without duplicating results', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + key1: 'value1', + key2: 'value2', + }, + }).promise(); + + const response = await getObjectAttributesWithUserMetadata(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-*,x-amz-meta-*'); + + assert.strictEqual(response['x-amz-meta-key1'], 'value1'); + assert.strictEqual(response['x-amz-meta-key2'], 'value2'); + }); + + it('should handle duplicate specific metadata requests without duplicating results', async () => { + await s3.putObject({ + Bucket: bucket, + Key: key, + Body: body, + Metadata: { + key1: 'value1', + key2: 'value2', + }, + }).promise(); + + const response = await getObjectAttributesWithUserMetadata(s3, { + Bucket: bucket, + Key: key, + }, 'x-amz-meta-key1,x-amz-meta-key1'); + + assert.strictEqual(response['x-amz-meta-key1'], 'value1'); + assert.strictEqual(response['x-amz-meta-key2'], undefined); + }); }); diff --git a/tests/unit/api/objectGetAttributes.js b/tests/unit/api/objectGetAttributes.js index cb3a7bb1b6..04ab1f2129 100644 --- a/tests/unit/api/objectGetAttributes.js +++ b/tests/unit/api/objectGetAttributes.js @@ -472,6 +472,53 @@ describe('objectGetAttributes API with user metadata', () => { assert.strictEqual(response['x-amz-meta-*'], undefined); assert.strictEqual(response['x-amz-meta-test'][0], 'test-value'); }); + + it('should return all metadata when wildcard is combined with specific metadata key', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-key1': 'value1', + 'x-amz-meta-key2': 'value2', + 'x-amz-meta-key3': 'value3', + }); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-*', 'x-amz-meta-key1']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response['x-amz-meta-key1'][0], 'value1'); + assert.strictEqual(response['x-amz-meta-key2'][0], 'value2'); + assert.strictEqual(response['x-amz-meta-key3'][0], 'value3'); + }); + + it('should handle duplicate wildcard requests without duplicating results', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-key1': 'value1', + 'x-amz-meta-key2': 'value2', + }); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-*', 'x-amz-meta-*']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response['x-amz-meta-key1'][0], 'value1'); + assert.strictEqual(response['x-amz-meta-key2'][0], 'value2'); + }); + + it('should handle duplicate specific metadata requests without duplicating results', async () => { + await createObjectWithMetadata({ + 'x-amz-meta-key1': 'value1', + 'x-amz-meta-key2': 'value2', + }); + + const testGetRequest = createGetAttributesRequest(['x-amz-meta-key1', 'x-amz-meta-key1']); + const [xml] = await objectGetAttributesAsync(authInfo, testGetRequest, log); + const result = await parseStringPromise(xml); + const response = result.GetObjectAttributesResponse; + + assert.strictEqual(response['x-amz-meta-key1'][0], 'value1'); + assert.strictEqual(response['x-amz-meta-key2'], undefined); + }); }); describe('objectGetAttributes API with versioning', () => { From 85d5719e1d719d3fdc18ffe4e8c7f9b399ef4844 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Thu, 12 Feb 2026 14:58:53 +0100 Subject: [PATCH 16/17] fixup! Handle rights for GetObjectAttributes --- lib/api/apiUtils/authorization/prepareRequestContexts.js | 2 +- .../api/apiUtils/authorization/prepareRequestContexts.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/api/apiUtils/authorization/prepareRequestContexts.js b/lib/api/apiUtils/authorization/prepareRequestContexts.js index b6831d2a54..eece6848e9 100644 --- a/lib/api/apiUtils/authorization/prepareRequestContexts.js +++ b/lib/api/apiUtils/authorization/prepareRequestContexts.js @@ -276,7 +276,7 @@ function prepareRequestContexts(apiMethod, request, sourceBucket, const attributes = request.headers['x-amz-object-attributes']?.split(',') ?? []; if (attributes.some(attr => attr.trim().toLowerCase().startsWith('x-amz-meta-'))) { - requestContexts.push(generateRequestContext('objectGetAttributesWithUserMetadata')); + requestContexts.push(generateRequestContext('objectGetAttributesCustom')); } } else { const requestContext = diff --git a/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js b/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js index 5272b1934e..865480d61e 100644 --- a/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js +++ b/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js @@ -409,7 +409,7 @@ describe('prepareRequestContexts', () => { assert.strictEqual(results.length, 3); assert.strictEqual(results[0].getAction(), 's3:GetObject'); assert.strictEqual(results[1].getAction(), 's3:GetObjectAttributes'); - assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributes'); + assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributesCustom'); }); it('should include scality:GetObjectAttributes with multiple attributes', () => { @@ -422,7 +422,7 @@ describe('prepareRequestContexts', () => { assert.strictEqual(results.length, 3); assert.strictEqual(results[0].getAction(), 's3:GetObject'); assert.strictEqual(results[1].getAction(), 's3:GetObjectAttributes'); - assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributes'); + assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributesCustom'); }); it('should not include scality:GetObjectAttributes with only RestoreStatus', () => { @@ -472,7 +472,7 @@ describe('prepareRequestContexts', () => { assert.strictEqual(results.length, 3); assert.strictEqual(results[0].getAction(), 's3:GetObjectVersion'); assert.strictEqual(results[1].getAction(), 's3:GetObjectVersionAttributes'); - assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributes'); + assert.strictEqual(results[2].getAction(), 'scality:GetObjectAttributesCustom'); }); }); }); From 63d9d12ff14b5bdda71a38d9abaff368add66944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Thu, 12 Feb 2026 14:59:33 +0100 Subject: [PATCH 17/17] fixup! Parse optional attributes header with utility function --- .../apiUtils/object/extractUserMetadata.js | 23 ++++ .../apiUtils/object/parseAttributesHeader.js | 34 ++--- lib/api/bucketGet.js | 47 +++---- lib/api/objectGetAttributes.js | 38 ++---- lib/routes/veeam/list.js | 2 +- .../apiUtils/object/extractUserMetadata.js | 117 ++++++++++++++++++ .../apiUtils/object/parseAttributesHeader.js | 12 +- 7 files changed, 192 insertions(+), 81 deletions(-) create mode 100644 lib/api/apiUtils/object/extractUserMetadata.js create mode 100644 tests/unit/api/apiUtils/object/extractUserMetadata.js diff --git a/lib/api/apiUtils/object/extractUserMetadata.js b/lib/api/apiUtils/object/extractUserMetadata.js new file mode 100644 index 0000000000..06aa1b9a79 --- /dev/null +++ b/lib/api/apiUtils/object/extractUserMetadata.js @@ -0,0 +1,23 @@ +/** + * extractUserMetadata - Extract requested user metadata from object metadata + * @param {object} metadata - source metadata object with x-amz-meta-* keys + * @param {Set} attributes - requested attributes (with x-amz-meta- prefix) + * @returns {object} - object containing requested user metadata key-value pairs + */ +function extractUserMetadata(metadata, attributes) { + const result = {}; + + const isWildcard = attributes.has('x-amz-meta-*'); + const sourceKeys = isWildcard ? Object.keys(metadata) : attributes; + + for (const key of sourceKeys) { + const isValidKey = isWildcard ? key.startsWith('x-amz-meta-') : true; + if (isValidKey && metadata[key] != null) { + result[key] = metadata[key]; + } + } + + return result; +} + +module.exports = extractUserMetadata; diff --git a/lib/api/apiUtils/object/parseAttributesHeader.js b/lib/api/apiUtils/object/parseAttributesHeader.js index ef7f67288e..90627bf520 100644 --- a/lib/api/apiUtils/object/parseAttributesHeader.js +++ b/lib/api/apiUtils/object/parseAttributesHeader.js @@ -5,7 +5,6 @@ const { errorInstances } = require('arsenal'); * @param {object} headers - Request headers object * @param {string} headerName - Name of the header to parse (e.g., 'x-amz-object-attributes') * @param {Set} supportedAttributes - Set of valid attribute names - * @param {boolean} [isRequired=false] - If true, throws when header is missing/empty * @returns {string[]} Array of validated attribute names * @throws {arsenal.errors.InvalidRequest} When header is required but missing/empty * @throws {arsenal.errors.InvalidArgument} When an invalid attribute name is specified @@ -16,24 +15,29 @@ const { errorInstances } = require('arsenal'); * // Parsed result: * ['ETag', 'ObjectSize', 'x-amz-meta-custom'] */ -function parseAttributesHeaders(headers, headerName, supportedAttributes, isRequired = false) { - const attributes = - headers[headerName] - ?.split(',') - .map(attr => attr.trim()) - .map(attr => (supportedAttributes.has(attr) ? attr : attr.toLowerCase())) ?? []; - - if (isRequired && attributes.length === 0) { - throw errorInstances.InvalidRequest.customizeDescription( - `The ${headerName} header specifying the attributes to be retrieved is either missing or empty`, - ); +function parseAttributesHeaders(headers, headerName, supportedAttributes) { + const rawValue = headers[headerName]; + if (rawValue === null || rawValue === undefined) { + return new Set(); } - if (attributes.some(attr => !attr.startsWith('x-amz-meta-') && !supportedAttributes.has(attr))) { - throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.'); + const result = new Set(); + + for (const rawAttr of rawValue.split(',')) { + let attr = rawAttr.trim(); + + if (!supportedAttributes.has(attr)) { + attr = attr.toLowerCase(); + } + + if (!attr.startsWith('x-amz-meta-') && !supportedAttributes.has(attr)) { + throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.'); + } + + result.add(attr); } - return new Set(attributes); + return result; } module.exports = parseAttributesHeaders; diff --git a/lib/api/bucketGet.js b/lib/api/bucketGet.js index 343722b670..362116fdcd 100644 --- a/lib/api/bucketGet.js +++ b/lib/api/bucketGet.js @@ -11,6 +11,9 @@ const monitoring = require('../utilities/monitoringHandler'); const { generateToken, decryptToken } = require('../api/apiUtils/object/continueToken'); const parseAttributesHeaders = require('./apiUtils/object/parseAttributesHeader'); +const extractUserMetadata = require('./apiUtils/object/extractUserMetadata'); + +const OPTIONAL_ATTRIBUTES = new Set(['RestoreStatus']); // do not url encode the continuation tokens const skipUrlEncoding = new Set([ @@ -265,34 +268,21 @@ function processMasterVersions(bucketName, listParams, list) { function processOptionalAttributes(item, optionalAttributes) { const xml = []; - const userMetadata = new Set(); - - for (const attribute of optionalAttributes) { - switch (attribute) { - case 'RestoreStatus': - xml.push(''); - xml.push(`${!!item.restoreStatus?.inProgress}`); - - if (item.restoreStatus?.expiryDate) { - xml.push(`${item.restoreStatus?.expiryDate}`); - } - - xml.push(''); - break; - case 'x-amz-meta-*': - for (const key of Object.keys(item.userMetadata)) { - userMetadata.add(key); - } - break; - default: - if (item.userMetadata?.[attribute]) { - userMetadata.add(attribute); - } + + if (optionalAttributes.has('RestoreStatus')) { + xml.push(''); + xml.push(`${!!item.restoreStatus?.inProgress}`); + + if (item.restoreStatus?.expiryDate) { + xml.push(`${item.restoreStatus?.expiryDate}`); } + + xml.push(''); } - for (const key of userMetadata) { - xml.push(`<${key}>${item.userMetadata[key]}`); + const userMetadata = extractUserMetadata(item.userMetadata || {}, optionalAttributes); + for (const [key, value] of Object.entries(userMetadata)) { + xml.push(`<${key}>${value}`); } return xml; @@ -335,8 +325,11 @@ function bucketGet(authInfo, request, log, callback) { let optionalAttributes; try { - const headerName = 'x-amz-optional-object-attributes'; - optionalAttributes = parseAttributesHeaders(request.headers, headerName, new Set(['RestoreStatus'])); + optionalAttributes = parseAttributesHeaders( + request.headers, + 'x-amz-optional-object-attributes', + OPTIONAL_ATTRIBUTES, + ); } catch (err) { return callback(err); } diff --git a/lib/api/objectGetAttributes.js b/lib/api/objectGetAttributes.js index c64e553aa0..7af64ff05d 100644 --- a/lib/api/objectGetAttributes.js +++ b/lib/api/objectGetAttributes.js @@ -1,6 +1,6 @@ const { promisify } = require('util'); const xml2js = require('xml2js'); -const { errors } = require('arsenal'); +const { errors, errorInstances } = require('arsenal'); const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const parseAttributesHeaders = require('./apiUtils/object/parseAttributesHeader'); @@ -9,6 +9,7 @@ const { checkExpectedBucketOwner } = require('./apiUtils/authorization/bucketOwn const { pushMetric } = require('../utapi/utilities'); const { getPartCountFromMd5 } = require('./apiUtils/object/partInfo'); const { supportedGetObjectAttributes } = require('../../constants'); +const extractUserMetadata = require('./apiUtils/object/extractUserMetadata'); const checkExpectedBucketOwnerPromise = promisify(checkExpectedBucketOwner); const validateBucketAndObj = promisify(standardMetadataValidateBucketAndObj); @@ -42,37 +43,13 @@ function buildXmlResponse(objMD, requestedAttrs) { } } - if (Array.from(requestedAttrs).some(attr => attr.startsWith('x-amz-meta-'))) { - const userMetadata = extractUserMetadata(objMD, requestedAttrs); - Object.assign(attrResp, userMetadata); - } + const userMetadata = extractUserMetadata(objMD, requestedAttrs); + Object.assign(attrResp, userMetadata); const builder = new xml2js.Builder(); return builder.buildObject({ GetObjectAttributesResponse: attrResp }); } -/** - * extractUserMetadata - Extract requested user metadata from object metadata - * @param {object} objMD - object metadata - * @param {Set} attributes - requested attributes - * @returns {object} - object containing requested user metadata key-value pairs - */ -function extractUserMetadata(objMD, attributes) { - const result = {}; - - const isWildcard = attributes.includes('x-amz-meta-*'); - const sourceKeys = isWildcard ? Object.keys(objMD) : attributes; - - for (const key of sourceKeys) { - const isValidKey = isWildcard ? key.startsWith('x-amz-meta-') : true; - if (isValidKey && objMD[key]) { - result[key] = objMD[key]; - } - } - - return result; -} - /** * objectGetAttributes - Retrieves all metadata from an object without returning the object itself * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info @@ -159,6 +136,13 @@ async function objectGetAttributes(authInfo, request, log, callback) { throw err; } + const attrHeader = headers['x-amz-object-attributes']; + if (attrHeader === undefined) { + throw errorInstances.InvalidRequest.customizeDescription( + 'The x-amz-object-attributes header specifying the attributes to be retrieved is either missing or empty', + ); + } + const requestedAttrs = parseAttributesHeaders(headers, 'x-amz-object-attributes', supportedGetObjectAttributes, true); if (requestedAttrs.has('Checksum')) { diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index 5fd8585bfb..1c4c760259 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -25,7 +25,7 @@ function buildXMLResponse(request, arrayOfFiles, versioned = false) { prefix: validPath, maxKeys: parsedQs['max-keys'] || 1000, delimiter: '/', - optionalAttributes: [], + optionalAttributes: new Set(), }; const list = { IsTruncated: false, diff --git a/tests/unit/api/apiUtils/object/extractUserMetadata.js b/tests/unit/api/apiUtils/object/extractUserMetadata.js new file mode 100644 index 0000000000..67d4a88b23 --- /dev/null +++ b/tests/unit/api/apiUtils/object/extractUserMetadata.js @@ -0,0 +1,117 @@ +const assert = require('assert'); + +const extractUserMetadata = require('../../../../../lib/api/apiUtils/object/extractUserMetadata'); + +describe('extractUserMetadata', () => { + const metadata = { + 'x-amz-meta-color': 'red', + 'x-amz-meta-size': 'large', + 'x-amz-meta-count': '42', + 'content-length': 1024, + 'content-type': 'application/json', + }; + + describe('with wildcard x-amz-meta-*', () => { + it('should return all x-amz-meta-* keys', () => { + const attributes = new Set(['x-amz-meta-*']); + const result = extractUserMetadata(metadata, attributes); + + assert.deepStrictEqual(result, { + 'x-amz-meta-color': 'red', + 'x-amz-meta-size': 'large', + 'x-amz-meta-count': '42', + }); + }); + + it('should not include non-metadata keys', () => { + const attributes = new Set(['x-amz-meta-*']); + const result = extractUserMetadata(metadata, attributes); + + assert.strictEqual(result['content-length'], undefined); + assert.strictEqual(result['content-type'], undefined); + }); + + it('should return empty object when no metadata keys exist', () => { + const noMetadata = { 'content-length': 1024 }; + const attributes = new Set(['x-amz-meta-*']); + const result = extractUserMetadata(noMetadata, attributes); + + assert.deepStrictEqual(result, {}); + }); + }); + + describe('with specific attribute names', () => { + it('should return only requested metadata keys', () => { + const attributes = new Set(['x-amz-meta-color']); + const result = extractUserMetadata(metadata, attributes); + + assert.deepStrictEqual(result, { + 'x-amz-meta-color': 'red', + }); + }); + + it('should return multiple requested keys', () => { + const attributes = new Set(['x-amz-meta-color', 'x-amz-meta-count']); + const result = extractUserMetadata(metadata, attributes); + + assert.deepStrictEqual(result, { + 'x-amz-meta-color': 'red', + 'x-amz-meta-count': '42', + }); + }); + + it('should ignore non-existent keys', () => { + const attributes = new Set(['x-amz-meta-color', 'x-amz-meta-nonexistent']); + const result = extractUserMetadata(metadata, attributes); + + assert.deepStrictEqual(result, { + 'x-amz-meta-color': 'red', + }); + }); + + it('should ignore non-metadata attributes in the set', () => { + const attributes = new Set(['x-amz-meta-color', 'RestoreStatus', 'ETag']); + const result = extractUserMetadata(metadata, attributes); + + assert.deepStrictEqual(result, { + 'x-amz-meta-color': 'red', + }); + }); + }); + + describe('edge cases', () => { + it('should return empty object for empty metadata', () => { + const attributes = new Set(['x-amz-meta-*']); + const result = extractUserMetadata({}, attributes); + + assert.deepStrictEqual(result, {}); + }); + + it('should return empty object for empty attributes', () => { + const attributes = new Set(); + const result = extractUserMetadata(metadata, attributes); + + assert.deepStrictEqual(result, {}); + }); + + it('should return keys with falsy values except null and undefined', () => { + const metadataWithFalsy = { + 'x-amz-meta-empty': '', + 'x-amz-meta-null': null, + 'x-amz-meta-undefined': undefined, + 'x-amz-meta-zero': 0, + 'x-amz-meta-false': false, + 'x-amz-meta-valid': 'value', + }; + const attributes = new Set(['x-amz-meta-*']); + const result = extractUserMetadata(metadataWithFalsy, attributes); + + assert.deepStrictEqual(result, { + 'x-amz-meta-empty': '', + 'x-amz-meta-zero': 0, + 'x-amz-meta-false': false, + 'x-amz-meta-valid': 'value', + }); + }); + }); +}); diff --git a/tests/unit/api/apiUtils/object/parseAttributesHeader.js b/tests/unit/api/apiUtils/object/parseAttributesHeader.js index 75b8772fc1..76413f8d64 100644 --- a/tests/unit/api/apiUtils/object/parseAttributesHeader.js +++ b/tests/unit/api/apiUtils/object/parseAttributesHeader.js @@ -6,16 +6,6 @@ const headerName = 'x-amz-object-attributes'; const allowedAttributes = new Set(['ETag', 'StorageClass', 'ObjectSize']); describe('parseAttributesHeaders', () => { - it('should throw InvalidRequest when header is missing and isRequired is true', () => { - assert.throws( - () => parseAttributesHeaders({}, headerName, allowedAttributes, true), - err => { - assert.strictEqual(err.is.InvalidRequest, true); - return true; - }, - ); - }); - it('should throw InvalidArgument when attribute is invalid', () => { const headers = { [headerName]: 'InvalidAttribute' }; @@ -28,7 +18,7 @@ describe('parseAttributesHeaders', () => { ); }); - it('should return empty array when header is missing and isRequired is false', () => { + it('should return empty array when header is missing', () => { const result = parseAttributesHeaders({}, headerName, allowedAttributes); assert.deepStrictEqual(result, new Set([]));