Skip to content

Commit 79ecae4

Browse files
committed
Merge branch 'w/8.8/bugfix/CLDSRV-729-remove-md5-header-requirement' into tmp/octopus/w/9.0/bugfix/CLDSRV-729-remove-md5-header-requirement
2 parents 091fd27 + 850e511 commit 79ecae4

File tree

4 files changed

+121
-15
lines changed

4 files changed

+121
-15
lines changed

lib/api/bucketPutCors.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@ function bucketPutCors(authInfo, request, log, callback) {
3333
return callback(errors.MissingRequestBodyError);
3434
}
3535

36-
const md5 = crypto.createHash('md5')
37-
.update(request.post, 'utf8').digest('base64');
38-
if (md5 !== request.headers['content-md5']) {
39-
log.debug('bad md5 digest', { error: errors.BadDigest });
40-
monitoring.promMetrics('PUT', bucketName, 400, 'putBucketCors');
41-
return callback(errors.BadDigest);
36+
if (request.headers['content-md5']) {
37+
const md5 = crypto.createHash('md5')
38+
.update(request.post, 'utf8').digest('base64');
39+
if (md5 !== request.headers['content-md5']) {
40+
log.debug('bad md5 digest', { error: errors.BadDigest });
41+
monitoring.promMetrics('PUT', bucketName, 400, 'putBucketCors');
42+
return callback(errors.BadDigest);
43+
}
4244
}
4345

4446
if (parseInt(request.headers['content-length'], 10) > 65536) {

lib/api/multiObjectDelete.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -474,12 +474,15 @@ function multiObjectDelete(authInfo, request, log, callback) {
474474
'multiObjectDelete');
475475
return callback(errors.MissingRequestBodyError);
476476
}
477-
const md5 = crypto.createHash('md5')
478-
.update(request.post, 'utf8').digest('base64');
479-
if (md5 !== request.headers['content-md5']) {
480-
monitoring.promMetrics('DELETE', request.bucketName, 400,
481-
'multiObjectDelete');
482-
return callback(errors.BadDigest);
477+
478+
if (request.headers['content-md5']) {
479+
const md5 = crypto.createHash('md5')
480+
.update(request.post, 'utf8').digest('base64');
481+
if (md5 !== request.headers['content-md5']) {
482+
monitoring.promMetrics('DELETE', request.bucketName, 400,
483+
'multiObjectDelete');
484+
return callback(errors.BadDigest);
485+
}
483486
}
484487

485488
const inPlayInternal = [];

tests/unit/api/bucketPutCors.js

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('putBucketCORS API', () => {
5555
.createBucketCorsRequest('PUT', bucketName);
5656
bucketPutCors(authInfo, testBucketPutCorsRequest, log, err => {
5757
if (err) {
58-
process.stdout.write(`Err putting website config ${err}`);
58+
process.stdout.write(`Err putting bucket cors ${err}`);
5959
return done(err);
6060
}
6161
return metadata.getBucket(bucketName, log, (err, bucket) => {
@@ -70,11 +70,33 @@ describe('putBucketCORS API', () => {
7070
});
7171
});
7272

73-
it('should return BadDigest if md5 is omitted', done => {
73+
it('should accept request if md5 is omitted', done => {
7474
const corsUtil = new CorsConfigTester();
7575
const testBucketPutCorsRequest = corsUtil
7676
.createBucketCorsRequest('PUT', bucketName);
7777
testBucketPutCorsRequest.headers['content-md5'] = undefined;
78+
bucketPutCors(authInfo, testBucketPutCorsRequest, log, err => {
79+
if (err) {
80+
process.stdout.write(`Err putting bucket cors ${err}`);
81+
return done(err);
82+
}
83+
return metadata.getBucket(bucketName, log, (err, bucket) => {
84+
if (err) {
85+
process.stdout.write(`Err retrieving bucket MD ${err}`);
86+
return done(err);
87+
}
88+
const uploadedCors = bucket.getCors();
89+
assert.deepStrictEqual(uploadedCors, corsUtil.getCors());
90+
return done();
91+
});
92+
});
93+
});
94+
95+
it('should reject request if md5 is mismatch', done => {
96+
const corsUtil = new CorsConfigTester();
97+
const testBucketPutCorsRequest = corsUtil
98+
.createBucketCorsRequest('PUT', bucketName);
99+
testBucketPutCorsRequest.headers['content-md5'] = 'wrong md5';
78100
_testPutBucketCors(authInfo, testBucketPutCorsRequest,
79101
log, 'BadDigest', done);
80102
});

tests/unit/api/multiObjectDelete.js

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ describe('multiObjectDelete function', () => {
383383
objectKey: 'objectname',
384384
parsedHost: 'localhost',
385385
headers: {
386-
'content-md5': crypto.createHash('md5').update(post, 'utf8').digest('base64')
386+
'content-md5': crypto.createHash('md5').update(post, 'utf8').digest('base64'),
387387
},
388388
post,
389389
socket: {
@@ -412,4 +412,83 @@ describe('multiObjectDelete function', () => {
412412
done();
413413
});
414414
});
415+
416+
it('should accept request when content-md5 header is missing', done => {
417+
const post = '<Delete><Object><Key>objectname</Key></Object></Delete>';
418+
const testObjectKey = 'objectname';
419+
const testBucketName = 'test-bucket';
420+
const request = new DummyRequest({
421+
bucketName: testBucketName,
422+
objectKey: testObjectKey,
423+
parsedHost: 'localhost',
424+
headers: {
425+
// No content-md5 header
426+
},
427+
post,
428+
socket: {
429+
remoteAddress: '127.0.0.1',
430+
},
431+
url: `/${testBucketName}`,
432+
});
433+
434+
// Use the same canonicalID for both authInfo and bucket owner to avoid AccessDenied
435+
const testAuthInfo = makeAuthInfo(canonicalID);
436+
437+
// Create bucket with proper ownership
438+
const testBucketRequest = new DummyRequest({
439+
bucketName: testBucketName,
440+
namespace,
441+
headers: {},
442+
url: `/${testBucketName}`,
443+
});
444+
445+
// Create object to delete
446+
const testObjectRequest = new DummyRequest({
447+
bucketName: testBucketName,
448+
namespace,
449+
objectKey: testObjectKey,
450+
headers: {},
451+
url: `/${testBucketName}/${testObjectKey}`,
452+
}, postBody);
453+
454+
bucketPut(testAuthInfo, testBucketRequest, log, () => {
455+
objectPut(testAuthInfo, testObjectRequest, undefined, log, () => {
456+
multiObjectDelete.multiObjectDelete(testAuthInfo, request, log, (err, res) => {
457+
// Request should succeed even without content-md5 header
458+
assert.strictEqual(err, null);
459+
assert.strictEqual(typeof res, 'string');
460+
// Should contain successful deletion response
461+
assert.strictEqual(res.includes('<Deleted><Key>objectname</Key></Deleted>'), true);
462+
done();
463+
});
464+
});
465+
});
466+
});
467+
468+
it('should reject request with BadDigest error when content-md5 header mismatches', done => {
469+
const post = '<Delete><Object><Key>objectname</Key></Object></Delete>';
470+
const incorrectMd5 = 'incorrectMd5Hash';
471+
472+
const request = new DummyRequest({
473+
bucketName: 'bucketname',
474+
objectKey: 'objectname',
475+
parsedHost: 'localhost',
476+
headers: {
477+
'content-md5': incorrectMd5,
478+
},
479+
post,
480+
socket: {
481+
remoteAddress: '127.0.0.1',
482+
},
483+
url: '/bucketname',
484+
});
485+
const authInfo = makeAuthInfo('123456');
486+
487+
multiObjectDelete.multiObjectDelete(authInfo, request, log, (err, res) => {
488+
// Should return BadDigest error for mismatched content-md5
489+
assert.strictEqual(err.is.BadDigest, true);
490+
assert.strictEqual(res, undefined);
491+
done();
492+
});
493+
});
415494
});

0 commit comments

Comments
 (0)