Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

Commit 70ab224

Browse files
shaffeeullahgcf-owl-bot[bot]
shaffeeullah
and
gcf-owl-bot[bot]
authored
fix: revert skip validation (#2023)
* fix: revert skip validation * fixed logic * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * made updated change * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * changed to === * fixed implementation * added comment * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * refactored boolean * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fixed bugs * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * linted files * removed console.log * fixed unit tests * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * added test Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent 14c4600 commit 70ab224

2 files changed

Lines changed: 81 additions & 62 deletions

File tree

‎src/file.ts‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,9 +1387,10 @@ class File extends ServiceObject<File> {
13871387

13881388
const throughStream = new PassThroughShim();
13891389

1390-
let isServedCompressed = true;
1390+
let isCompressed = true;
13911391
let crc32c = true;
13921392
let md5 = false;
1393+
let safeToValidate = true;
13931394

13941395
if (typeof options.validation === 'string') {
13951396
const value = options.validation.toLowerCase().trim();
@@ -1489,7 +1490,16 @@ class File extends ServiceObject<File> {
14891490
rawResponseStream.on('error', onComplete);
14901491

14911492
const headers = rawResponseStream.toJSON().headers;
1492-
isServedCompressed = headers['content-encoding'] === 'gzip';
1493+
isCompressed = headers['content-encoding'] === 'gzip';
1494+
1495+
// The object is safe to validate if:
1496+
// 1. It was stored gzip and returned to us gzip OR
1497+
// 2. It was never stored as gzip
1498+
safeToValidate =
1499+
(headers['x-goog-stored-content-encoding'] === 'gzip' &&
1500+
isCompressed) ||
1501+
headers['x-goog-stored-content-encoding'] === 'identity';
1502+
14931503
const transformStreams: Transform[] = [];
14941504

14951505
if (shouldRunValidation) {
@@ -1515,7 +1525,7 @@ class File extends ServiceObject<File> {
15151525
transformStreams.push(validateStream);
15161526
}
15171527

1518-
if (isServedCompressed && options.decompress) {
1528+
if (isCompressed && options.decompress) {
15191529
transformStreams.push(zlib.createGunzip());
15201530
}
15211531

@@ -1558,30 +1568,13 @@ class File extends ServiceObject<File> {
15581568
return;
15591569
}
15601570

1561-
// TODO(https://github.com/googleapis/nodejs-storage/issues/709):
1562-
// Remove once the backend issue is fixed.
1563-
// If object is stored compressed (having
1564-
// metadata.contentEncoding === 'gzip') and was served decompressed,
1565-
// then skip checksum validation because the remote checksum is computed
1566-
// against the compressed version of the object.
1567-
if (!isServedCompressed) {
1568-
try {
1569-
await this.getMetadata({userProject: options.userProject});
1570-
} catch (e) {
1571-
throughStream.destroy(e as Error);
1572-
return;
1573-
}
1574-
if (this.metadata.contentEncoding === 'gzip') {
1575-
return;
1576-
}
1577-
}
1578-
15791571
// If we're doing validation, assume the worst-- a data integrity
15801572
// mismatch. If not, these tests won't be performed, and we can assume
15811573
// the best.
1582-
let failed = crc32c || md5;
1583-
1584-
if (validateStream) {
1574+
// We must check if the server decompressed the data on serve because hash
1575+
// validation is not possible in this case.
1576+
let failed = (crc32c || md5) && safeToValidate;
1577+
if (validateStream && safeToValidate) {
15851578
if (crc32c && hashes.crc32c) {
15861579
failed = !validateStream.test('crc32c', hashes.crc32c);
15871580
}

‎test/file.ts‎

Lines changed: 64 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import {
4141
// eslint-disable-next-line @typescript-eslint/no-unused-vars
4242
File,
4343
FileOptions,
44-
GetFileMetadataOptions,
4544
PolicyDocument,
4645
SetFileMetadataOptions,
4746
GetSignedUrlConfig,
@@ -1373,6 +1372,7 @@ describe('File', () => {
13731372
return {
13741373
headers: {
13751374
'x-goog-hash': `crc32c=${responseCRC32C},md5=${responseMD5}`,
1375+
'x-goog-stored-content-encoding': 'identity',
13761376
},
13771377
};
13781378
},
@@ -1399,35 +1399,80 @@ describe('File', () => {
13991399
}
14001400

14011401
describe('server decompression', () => {
1402-
it('should skip validation if file was stored compressed', done => {
1402+
it('should skip validation if file was stored compressed and served decompressed', done => {
14031403
file.metadata.crc32c = '.invalid.';
14041404
file.metadata.contentEncoding = 'gzip';
14051405

1406+
handleRespOverride = (
1407+
err: Error,
1408+
res: {},
1409+
body: {},
1410+
callback: Function
1411+
) => {
1412+
const rawResponseStream = new PassThrough();
1413+
Object.assign(rawResponseStream, {
1414+
toJSON() {
1415+
return {
1416+
headers: {
1417+
'x-goog-hash': `crc32c=${responseCRC32C},md5=${responseMD5}`,
1418+
'x-goog-stored-content-encoding': 'gzip',
1419+
},
1420+
};
1421+
},
1422+
});
1423+
callback(null, null, rawResponseStream);
1424+
setImmediate(() => {
1425+
rawResponseStream.end(DATA);
1426+
});
1427+
};
1428+
14061429
file
14071430
.createReadStream({validation: 'crc32c'})
1408-
.on('error', done)
14091431
.on('end', done)
14101432
.resume();
14111433
});
14121434
});
14131435

1414-
it('should emit errors from the validation stream', done => {
1415-
const expectedError = new Error('test error');
1436+
it('should perform validation if file was stored compressed and served compressed', done => {
1437+
file.metadata.crc32c = '.invalid.';
1438+
file.metadata.contentEncoding = 'gzip';
1439+
handleRespOverride = (
1440+
err: Error,
1441+
res: {},
1442+
body: {},
1443+
callback: Function
1444+
) => {
1445+
const rawResponseStream = new PassThrough();
1446+
Object.assign(rawResponseStream, {
1447+
toJSON() {
1448+
return {
1449+
headers: {
1450+
'x-goog-hash': `crc32c=${responseCRC32C},md5=${responseMD5}`,
1451+
'x-goog-stored-content-encoding': 'gzip',
1452+
'content-encoding': 'gzip',
1453+
},
1454+
};
1455+
},
1456+
});
1457+
callback(null, null, rawResponseStream);
1458+
setImmediate(() => {
1459+
rawResponseStream.end(DATA);
1460+
});
1461+
};
14161462

1417-
file.requestStream = getFakeSuccessfulRequest(DATA);
1463+
const expectedError = new Error('test error');
14181464
setFileValidationToError(expectedError);
14191465

14201466
file
1421-
.createReadStream()
1467+
.createReadStream({validation: 'crc32c'})
14221468
.on('error', (err: Error) => {
14231469
assert(err === expectedError);
1424-
14251470
done();
14261471
})
14271472
.resume();
14281473
});
14291474

1430-
it('should not handle both error and end events', done => {
1475+
it('should emit errors from the validation stream', done => {
14311476
const expectedError = new Error('test error');
14321477

14331478
file.requestStream = getFakeSuccessfulRequest(DATA);
@@ -1438,45 +1483,26 @@ describe('File', () => {
14381483
.on('error', (err: Error) => {
14391484
assert(err === expectedError);
14401485

1441-
setImmediate(done);
1442-
})
1443-
.on('end', () => {
1444-
done(new Error('Should not have been called.'));
1486+
done();
14451487
})
14461488
.resume();
14471489
});
14481490

1449-
it('should pass the userProject to getMetadata', done => {
1450-
const fakeOptions = {
1451-
userProject: 'grapce-spaceship-123',
1452-
};
1453-
1454-
file.getMetadata = (options: GetFileMetadataOptions) => {
1455-
assert.strictEqual(options.userProject, fakeOptions.userProject);
1456-
setImmediate(done);
1457-
return Promise.resolve({
1458-
crc32c: CRC32C_HASH,
1459-
});
1460-
};
1461-
1462-
file.requestStream = getFakeSuccessfulRequest(DATA);
1463-
1464-
file.createReadStream(fakeOptions).on('error', done).resume();
1465-
});
1466-
1467-
it('should destroy stream from failed metadata fetch', done => {
1468-
const error = new Error('Error.');
1469-
file.getMetadata = () => {
1470-
return Promise.reject(error);
1471-
};
1491+
it('should not handle both error and end events', done => {
1492+
const expectedError = new Error('test error');
14721493

14731494
file.requestStream = getFakeSuccessfulRequest(DATA);
1495+
setFileValidationToError(expectedError);
14741496

14751497
file
14761498
.createReadStream()
14771499
.on('error', (err: Error) => {
1478-
assert.strictEqual(err, error);
1479-
done();
1500+
assert(err === expectedError);
1501+
1502+
setImmediate(done);
1503+
})
1504+
.on('end', () => {
1505+
done(new Error('Should not have been called.'));
14801506
})
14811507
.resume();
14821508
});

0 commit comments

Comments
 (0)
⚡