Skip to content

Commit 1b5e6e0

Browse files
committed
CLDSRV-848: create one global list of checksumed methods
1 parent a790686 commit 1b5e6e0

File tree

4 files changed

+37
-75
lines changed

4 files changed

+37
-75
lines changed

lib/Config.js

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -545,27 +545,7 @@ function bucketNotifAssert(bucketNotifConfig) {
545545
}
546546

547547
function parseIntegrityChecks(config) {
548-
const integrityChecks = {
549-
'bucketPutACL': true,
550-
'bucketPutCors': true,
551-
'bucketPutEncryption': true,
552-
'bucketPutLifecycle': true,
553-
'bucketPutNotification': true,
554-
'bucketPutObjectLock': true,
555-
'bucketPutPolicy': true,
556-
'bucketPutReplication': true,
557-
'bucketPutVersioning': true,
558-
'bucketPutWebsite': true,
559-
'bucketPutLogging': true,
560-
'bucketPutTagging': true,
561-
'multiObjectDelete': true,
562-
'objectPutACL': true,
563-
'objectPutLegalHold': true,
564-
'objectPutTagging': true,
565-
'objectPutRetention': true,
566-
'objectRestore': true,
567-
'completeMultipartUpload': true,
568-
};
548+
const integrityChecks = {};
569549

570550
if (config && config.integrityChecks) {
571551
for (const method in integrityChecks) {

lib/api/apiUtils/integrity/validateChecksums.js

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,28 @@ const { CrtCrc64Nvme } = require('@aws-sdk/crc64-nvme-crt');
55
const { errors: ArsenalErrors } = require('arsenal');
66
const { config } = require('../../../Config');
77

8+
const checksumedMethods = Object.freeze({
9+
'completeMultipartUpload': true,
10+
'multiObjectDelete': true,
11+
'bucketPutACL': true,
12+
'bucketPutCors': true,
13+
'bucketPutEncryption': true,
14+
'bucketPutLifecycle': true,
15+
'bucketPutLogging': true,
16+
'bucketPutNotification': true,
17+
'bucketPutPolicy': true,
18+
'bucketPutReplication': true,
19+
'bucketPutTagging': true,
20+
'bucketPutVersioning': true,
21+
'bucketPutWebsite': true,
22+
'objectPutACL': true,
23+
'objectPutLegalHold': true,
24+
'bucketPutObjectLock': true, // PutObjectLockConfiguration
25+
'objectPutRetention': true,
26+
'objectPutTagging': true,
27+
'objectRestore': true,
28+
});
29+
830
const ChecksumError = Object.freeze({
931
MD5Mismatch: 'MD5Mismatch',
1032
MD5Invalid: 'MD5Invalid',
@@ -252,29 +274,6 @@ async function defaultValidationFunc(request, body, log) {
252274
}
253275
}
254276

255-
const methodValidationFunc = Object.freeze({
256-
'completeMultipartUpload': defaultValidationFunc,
257-
'bucketPutACL': defaultValidationFunc,
258-
'bucketPutCors': defaultValidationFunc,
259-
'bucketPutEncryption': defaultValidationFunc,
260-
'bucketPutLifecycle': defaultValidationFunc,
261-
'bucketPutNotification': defaultValidationFunc,
262-
'bucketPutObjectLock': defaultValidationFunc,
263-
'bucketPutPolicy': defaultValidationFunc,
264-
'bucketPutReplication': defaultValidationFunc,
265-
'bucketPutVersioning': defaultValidationFunc,
266-
'bucketPutWebsite': defaultValidationFunc,
267-
'bucketPutLogging': defaultValidationFunc,
268-
'bucketPutTagging': defaultValidationFunc,
269-
// TODO: DeleteObjects requires a checksum. Should return an error if ChecksumError.MissingChecksum.
270-
'multiObjectDelete': defaultValidationFunc,
271-
'objectPutACL': defaultValidationFunc,
272-
'objectPutLegalHold': defaultValidationFunc,
273-
'objectPutTagging': defaultValidationFunc,
274-
'objectPutRetention': defaultValidationFunc,
275-
'objectRestore': defaultValidationFunc,
276-
});
277-
278277
/**
279278
* validateMethodChecksumsNoChunking - Validate the checksums of a request.
280279
* @param {object} request - http request
@@ -283,12 +282,12 @@ const methodValidationFunc = Object.freeze({
283282
* @return {object} - error
284283
*/
285284
async function validateMethodChecksumNoChunking(request, body, log) {
286-
if (config.integrityChecks[request.apiMethod]) {
287-
const validationFunc = methodValidationFunc[request.apiMethod];
288-
if (!validationFunc) {
289-
return null; //await defaultValidationFunc2(request, body, log);
290-
}
291-
return await validationFunc(request, body, log);
285+
if (config.integrityChecks[request.apiMethod] === false) {
286+
return null;
287+
}
288+
289+
if (request.apiMethod in checksumedMethods) {
290+
return await defaultValidationFunc(request, body, log);
292291
}
293292

294293
return null;
@@ -298,4 +297,5 @@ module.exports = {
298297
ChecksumError,
299298
validateChecksumsNoChunking,
300299
validateMethodChecksumNoChunking,
300+
checksumedMethods,
301301
};

tests/unit/Config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -920,7 +920,7 @@ describe('Config', () => {
920920
});
921921

922922
describe('parse integrity checks', () => {
923-
it('should replace default values with new values', () => {
923+
it('should insert values into integrityCheck object', () => {
924924
const newConfig = {
925925
integrityChecks: {
926926
'bucketPutACL': false,

tests/unit/api/apiUtils/integrity/validateChecksums.js

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const assert = require('assert');
22
const crypto = require('crypto');
33
const sinon = require('sinon');
44

5-
const { validateChecksumsNoChunking, ChecksumError, validateMethodChecksumNoChunking } =
5+
const { validateChecksumsNoChunking, ChecksumError, validateMethodChecksumNoChunking, checksumedMethods } =
66
require('../../../../../lib/api/apiUtils/integrity/validateChecksums');
77
const { errors: ArsenalErrors } = require('arsenal');
88
const { config } = require('../../../../../lib/Config');
@@ -278,24 +278,6 @@ describe('validateMethodChecksumNoChunking', () => {
278278
let sandbox;
279279
let originalIntegrityChecks;
280280

281-
const supportedMethods = [
282-
'bucketPutACL',
283-
'bucketPutCors',
284-
'bucketPutEncryption',
285-
'bucketPutLifecycle',
286-
'bucketPutNotification',
287-
'bucketPutObjectLock',
288-
'bucketPutPolicy',
289-
'bucketPutReplication',
290-
'bucketPutVersioning',
291-
'bucketPutWebsite',
292-
'multiObjectDelete',
293-
'objectPutACL',
294-
'objectPutLegalHold',
295-
'objectPutTagging',
296-
'objectPutRetention'
297-
];
298-
299281
beforeEach(() => {
300282
sandbox = sinon.createSandbox();
301283
originalIntegrityChecks = { ...config.integrityChecks };
@@ -307,7 +289,7 @@ describe('validateMethodChecksumNoChunking', () => {
307289
});
308290

309291
describe('when checksum mismatches', () => {
310-
supportedMethods.forEach(method => {
292+
Object.keys(checksumedMethods).forEach(method => {
311293
it(`should return BadDigest error for ${method} when checksum mismatch`, async () => {
312294
config.integrityChecks[method] = true;
313295

@@ -330,7 +312,7 @@ describe('validateMethodChecksumNoChunking', () => {
330312
});
331313

332314
describe('when checksum mismatches', () => {
333-
supportedMethods.forEach(method => {
315+
Object.keys(checksumedMethods).forEach(method => {
334316
it(`should return InvalidDigest error for ${method} when checksum mismatch`, async () => {
335317
config.integrityChecks[method] = true;
336318

@@ -353,7 +335,7 @@ describe('validateMethodChecksumNoChunking', () => {
353335
});
354336

355337
describe('when no checksum is provided', () => {
356-
supportedMethods.forEach(method => {
338+
Object.keys(checksumedMethods).forEach(method => {
357339
it(`should return null for ${method} when no checksum is provided`, async () => {
358340
config.integrityChecks[method] = true;
359341

@@ -373,7 +355,7 @@ describe('validateMethodChecksumNoChunking', () => {
373355
});
374356

375357
describe('when checksum matches', () => {
376-
supportedMethods.forEach(method => {
358+
Object.keys(checksumedMethods).forEach(method => {
377359
it(`should return null for ${method} when checksum matches`, async () => {
378360
config.integrityChecks[method] = true;
379361

@@ -396,7 +378,7 @@ describe('validateMethodChecksumNoChunking', () => {
396378
});
397379

398380
describe('when method is disabled in config', () => {
399-
supportedMethods.forEach(method => {
381+
Object.keys(checksumedMethods).forEach(method => {
400382
it(`should return null for ${method} when disabled, even with checksum mismatch`, async () => {
401383
config.integrityChecks[method] = false;
402384

0 commit comments

Comments
 (0)