-
Notifications
You must be signed in to change notification settings - Fork 254
✨ CLDSRV-812: ListObjectsV2 create x-amz-optional-attributes header #6033
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.
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
base: development/9.2
Are you sure you want to change the base?
Changes from 7 commits
19ce8d6
ef5c47a
3573352
acf4175
535b360
d438738
a8df466
aab1833
30a1233
60c61c9
ab9cd5e
7844c33
f626fcb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -284,6 +284,15 @@ function bucketGet(authInfo, request, log, callback) { | |
| const params = request.query; | ||
| const bucketName = request.bucketName; | ||
| const v2 = params['list-type']; | ||
|
|
||
| const optionalAttributes = | ||
| request.headers['x-amz-optional-object-attributes']?.split(',').map(attr => attr.trim()) ?? []; | ||
| if (optionalAttributes.some(attr => !attr.startsWith('x-amz-meta-') && attr != 'RestoreStatus')) { | ||
| return callback( | ||
| errorInstances.InvalidArgument.customizeDescription('Invalid header x-amz-optional-object-attributes') | ||
|
||
| ); | ||
| } | ||
|
|
||
| if (v2 !== undefined && Number.parseInt(v2, 10) !== 2) { | ||
| return callback(errorInstances.InvalidArgument.customizeDescription('Invalid ' + | ||
| 'List Type specified in Request')); | ||
|
|
@@ -352,21 +361,22 @@ function bucketGet(authInfo, request, log, callback) { | |
| } | ||
|
|
||
| standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { | ||
| const corsHeaders = collectCorsHeaders(request.headers.origin, | ||
| request.method, bucket); | ||
| const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); | ||
|
|
||
| if (err) { | ||
| log.debug('error processing request', { error: err }); | ||
| monitoring.promMetrics( | ||
| 'GET', bucketName, err.code, 'listBucket'); | ||
| monitoring.promMetrics('GET', bucketName, err.code, 'listBucket'); | ||
| return callback(err, null, corsHeaders); | ||
| } | ||
|
|
||
| if (params.versions !== undefined) { | ||
| listParams.listingType = 'DelimiterVersions'; | ||
| delete listParams.marker; | ||
| listParams.keyMarker = params['key-marker']; | ||
| listParams.versionIdMarker = params['version-id-marker'] ? | ||
| versionIdUtils.decode(params['version-id-marker']) : undefined; | ||
| } | ||
|
|
||
| if (!requestMaxKeys) { | ||
| const emptyList = { | ||
| CommonPrefixes: [], | ||
|
|
@@ -377,14 +387,14 @@ function bucketGet(authInfo, request, log, callback) { | |
| return handleResult(listParams, requestMaxKeys, encoding, authInfo, | ||
| bucketName, emptyList, corsHeaders, log, callback); | ||
| } | ||
| return services.getObjectListing(bucketName, listParams, log, | ||
| (err, list) => { | ||
|
|
||
| return services.getObjectListing(bucketName, listParams, log, (err, list) => { | ||
| if (err) { | ||
| log.debug('error processing request', { error: err }); | ||
| monitoring.promMetrics( | ||
| 'GET', bucketName, err.code, 'listBucket'); | ||
| monitoring.promMetrics('GET', bucketName, err.code, 'listBucket'); | ||
| return callback(err, null, corsHeaders); | ||
| } | ||
|
|
||
| return handleResult(listParams, requestMaxKeys, encoding, authInfo, | ||
| bucketName, list, corsHeaders, log, callback); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, the reason why RestoreStatus is not lower-cased is because of AWS standard ? why not leaving the other params the same ? Because we store them lower-cased ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for what I understand, you added this since user metadata are expected to be lower case in AWS (did not check, but I think I saw you mention this). In that case,
x-amz-meta-) as well? e.g. if we only consider lower case prefix, no need to convert to lower case for filtering user-metadata properties...(overall, trying to say that we don't need to be too defensive here: if spec & API both only support lower user-metadata, no need to spend cycle trying to accomodate, we can just be strict and expect user to behave correctly)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, user metadata are lowercase. AWS will convert a non lowercase usermetadata to a lowercase (we are doing the same). And yes, this apply to the prefix also.