Skip to content

Commit 732bc8d

Browse files
committed
CLDSRV-848: create one global list of checksumed methods
1 parent d8586f6 commit 732bc8d

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',
@@ -208,29 +230,6 @@ async function defaultValidationFunc(request, body, log) {
208230
}
209231
}
210232

211-
const methodValidationFunc = Object.freeze({
212-
'completeMultipartUpload': defaultValidationFunc,
213-
'bucketPutACL': defaultValidationFunc,
214-
'bucketPutCors': defaultValidationFunc,
215-
'bucketPutEncryption': defaultValidationFunc,
216-
'bucketPutLifecycle': defaultValidationFunc,
217-
'bucketPutNotification': defaultValidationFunc,
218-
'bucketPutObjectLock': defaultValidationFunc,
219-
'bucketPutPolicy': defaultValidationFunc,
220-
'bucketPutReplication': defaultValidationFunc,
221-
'bucketPutVersioning': defaultValidationFunc,
222-
'bucketPutWebsite': defaultValidationFunc,
223-
'bucketPutLogging': defaultValidationFunc,
224-
'bucketPutTagging': defaultValidationFunc,
225-
// TODO: DeleteObjects requires a checksum. Should return an error if ChecksumError.MissingChecksum.
226-
'multiObjectDelete': defaultValidationFunc,
227-
'objectPutACL': defaultValidationFunc,
228-
'objectPutLegalHold': defaultValidationFunc,
229-
'objectPutTagging': defaultValidationFunc,
230-
'objectPutRetention': defaultValidationFunc,
231-
'objectRestore': defaultValidationFunc,
232-
});
233-
234233
/**
235234
* validateMethodChecksumsNoChunking - Validate the checksums of a request.
236235
* @param {object} request - http request
@@ -239,12 +238,12 @@ const methodValidationFunc = Object.freeze({
239238
* @return {object} - error
240239
*/
241240
async function validateMethodChecksumNoChunking(request, body, log) {
242-
if (config.integrityChecks[request.apiMethod]) {
243-
const validationFunc = methodValidationFunc[request.apiMethod];
244-
if (!validationFunc) {
245-
return null; //await defaultValidationFunc2(request, body, log);
246-
}
247-
return await validationFunc(request, body, log);
241+
if (config.integrityChecks[request.apiMethod] === false) {
242+
return null;
243+
}
244+
245+
if (request.apiMethod in checksumedMethods) {
246+
return await defaultValidationFunc(request, body, log);
248247
}
249248

250249
return null;
@@ -254,4 +253,5 @@ module.exports = {
254253
ChecksumError,
255254
validateChecksumsNoChunking,
256255
validateMethodChecksumNoChunking,
256+
checksumedMethods,
257257
};

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)