Skip to content

Return user metadata in GetObjectAttributes API #6070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
maeldonn wants to merge 6 commits into
base: feature/CLDSRV-817/get-object-attributes
Choose a base branch
from
+623 −212
Open

Return user metadata in GetObjectAttributes API #6070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b818779
Support user metadata
maeldonn Jan 30, 2026
0dc49e8
Add a new permission to get user metadata
maeldonn Feb 2, 2026
e2a4a81
Parse optional attributes header with utility function
maeldonn Feb 3, 2026
1a3d966
Handle rights for GetObjectAttributes
maeldonn Feb 4, 2026
45e8e2f
fixup! Add a new permission to get user metadata
maeldonn Feb 10, 2026
e040894
fixup! Support user metadata
maeldonn Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions lib/api/apiUtils/authorization/prepareRequestContexts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,23 @@ function prepareRequestContexts(apiMethod, request, sourceBucket,
if (requestedAttributes.filter(attr => attr != 'RestoreStatus').length > 0) {
requestContexts.push(generateRequestContext('listObjectsV2OptionalAttributes'));
}
} else if (apiMethodAfterVersionCheck === 'objectGetAttributes') {
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.trim().toLowerCase().startsWith('x-amz-meta-'))) {
requestContexts.push(generateRequestContext('objectGetAttributesWithUserMetadata'));
}
} else {
const requestContext =
generateRequestContext(apiMethodAfterVersionCheck);
Expand Down
34 changes: 24 additions & 10 deletions lib/api/apiUtils/object/parseAttributesHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,35 @@
const { errorInstances } = require('arsenal');
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
* @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<string>} 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
* @example
* // Input headers:
* { 'headerName': 'ETag, ObjectSize, x-amz-meta-custom' }
*
* // Parsed result:
* ['ETag', 'ObjectSize', 'x-amz-meta-custom']
*/
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())) ?? [];
Copy link
Contributor

@SylvainSenechal SylvainSenechal Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait so it means even if the attribute is not in supportedAttribute, it will be returned as lower cased ?
Shouldn't this be a filter instead ?

I mean, ok I guess the overall function works because later there is the attributes.some() and has() thing, but could it be clearer

Copy link
Contributor

@SylvainSenechal SylvainSenechal Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyways, maybe the funciton is good enough as it is, but if you're able to make it a bit clearer it's nice, cause it's doing multiple thing that aren't completely obvious from the function name, it looks like it's gonna do some generic parsing, then ends up doing lowercasing and having a special case for x-amz-meta. Maybe renaming function name and intermediates variables help

Copy link
Author

@maeldonn maeldonn Feb 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not a supported attribute:

  1. It's user metadata so it's ok
  2. It's something else -> will throw an exception

Copy link
Contributor

@francoisferrand francoisferrand Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems uselessly innefficient: we traverse the list 3 times to do the same thing...

  • trim() can be done in the same iteration, no need to the extra strings/list allocation
  • isn't user metadata already mandated (by spec) to be lower case? do we do the same lower case conversion in other APIs? if not, not need to be more accommodating here
  • if an attribute is in the supportedAttributes, there is no point making another check for the prefix (and rechecking later it is a supported attribute, if there is no prefix)
Suggested change
.map(attr => (supportedAttributes.has(attr) ? attr : attr.toLowerCase())) ?? [];
const attributes =
headers[headerName]?.split(',').map(attr => {
const res = attr.trim())
if (!supportedAttributes.has(res) && !res.startsWith('x-amz-meta-')) {
throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.');
}
return res;
});


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 => !supportedGetObjectAttributes.has(attr))) {
if (attributes.some(attr => !attr.startsWith('x-amz-meta-') && !supportedAttributes.has(attr))) {
throw errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified.');
}

Expand Down
17 changes: 7 additions & 10 deletions lib/api/bucketGet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down Expand Up @@ -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']));
Copy link
Contributor

@francoisferrand francoisferrand Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Set should not be allocated on every call, should be a constant

} catch (err) {
return callback(err);
}

if (v2 !== undefined && Number.parseInt(v2, 10) !== 2) {
Expand Down
30 changes: 29 additions & 1 deletion lib/api/objectGetAttributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 OBJECT_GET_ATTRIBUTES = 'objectGetAttributes';

Expand Down Expand Up @@ -47,10 +48,37 @@ function buildXmlResponse(objMD, attributes) {
attrResp.ObjectSize = objMD['content-length'];
}

if (attributes.some(attr => attr.startsWith('x-amz-meta-'))) {
const userMetadata = extractUserMetadata(objMD, attributes);
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 {string[]} attributes - requested attributes
* @returns {object} - object containing requested user metadata key-value pairs
*/
function extractUserMetadata(objMD, attributes) {
Copy link
Contributor

@francoisferrand francoisferrand Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is already this logic in ListObjectv2, must be factorized...

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
Expand Down Expand Up @@ -137,7 +165,7 @@ async function objectGetAttributes(authInfo, request, log, callback) {
throw err;
}

const attributes = parseAttributesHeaders(headers);
const attributes = parseAttributesHeaders(headers, 'x-amz-object-attributes', supportedGetObjectAttributes, true);
Copy link
Contributor

@francoisferrand francoisferrand Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this true here makes it hard to understand what is happening, and it can just as well be done here (it does not add any value to have a conditional behavior inside the parseAttributesHeaders function):

Suggested change
const attributes = parseAttributesHeaders(headers, 'x-amz-object-attributes', supportedGetObjectAttributes, true);
const attributes = parseAttributesHeaders(headers, 'x-amz-object-attributes', supportedGetObjectAttributes);
if (attributes.length === 0) {
throw ....
}


pushMetric(OBJECT_GET_ATTRIBUTES, log, {
authInfo,
Expand Down
Loading
Loading