diff --git a/src/file.ts b/src/file.ts index e48cb507b..8b02905ca 100644 --- a/src/file.ts +++ b/src/file.ts @@ -1387,9 +1387,10 @@ class File extends ServiceObject { const throughStream = new PassThroughShim(); - let isServedCompressed = true; + let isCompressed = true; let crc32c = true; let md5 = false; + let safeToValidate = true; if (typeof options.validation === 'string') { const value = options.validation.toLowerCase().trim(); @@ -1489,7 +1490,16 @@ class File extends ServiceObject { rawResponseStream.on('error', onComplete); const headers = rawResponseStream.toJSON().headers; - isServedCompressed = headers['content-encoding'] === 'gzip'; + isCompressed = headers['content-encoding'] === 'gzip'; + + // The object is safe to validate if: + // 1. It was stored gzip and returned to us gzip OR + // 2. It was never stored as gzip + safeToValidate = + (headers['x-goog-stored-content-encoding'] === 'gzip' && + isCompressed) || + headers['x-goog-stored-content-encoding'] === 'identity'; + const transformStreams: Transform[] = []; if (shouldRunValidation) { @@ -1515,7 +1525,7 @@ class File extends ServiceObject { transformStreams.push(validateStream); } - if (isServedCompressed && options.decompress) { + if (isCompressed && options.decompress) { transformStreams.push(zlib.createGunzip()); } @@ -1558,30 +1568,13 @@ class File extends ServiceObject { return; } - // TODO(https://github.com/googleapis/nodejs-storage/issues/709): - // Remove once the backend issue is fixed. - // If object is stored compressed (having - // metadata.contentEncoding === 'gzip') and was served decompressed, - // then skip checksum validation because the remote checksum is computed - // against the compressed version of the object. - if (!isServedCompressed) { - try { - await this.getMetadata({userProject: options.userProject}); - } catch (e) { - throughStream.destroy(e as Error); - return; - } - if (this.metadata.contentEncoding === 'gzip') { - return; - } - } - // If we're doing validation, assume the worst-- a data integrity // mismatch. If not, these tests won't be performed, and we can assume // the best. - let failed = crc32c || md5; - - if (validateStream) { + // We must check if the server decompressed the data on serve because hash + // validation is not possible in this case. + let failed = (crc32c || md5) && safeToValidate; + if (validateStream && safeToValidate) { if (crc32c && hashes.crc32c) { failed = !validateStream.test('crc32c', hashes.crc32c); } diff --git a/test/file.ts b/test/file.ts index fa860e972..73ddef124 100644 --- a/test/file.ts +++ b/test/file.ts @@ -41,7 +41,6 @@ import { // eslint-disable-next-line @typescript-eslint/no-unused-vars File, FileOptions, - GetFileMetadataOptions, PolicyDocument, SetFileMetadataOptions, GetSignedUrlConfig, @@ -1373,6 +1372,7 @@ describe('File', () => { return { headers: { 'x-goog-hash': `crc32c=${responseCRC32C},md5=${responseMD5}`, + 'x-goog-stored-content-encoding': 'identity', }, }; }, @@ -1399,35 +1399,80 @@ describe('File', () => { } describe('server decompression', () => { - it('should skip validation if file was stored compressed', done => { + it('should skip validation if file was stored compressed and served decompressed', done => { file.metadata.crc32c = '.invalid.'; file.metadata.contentEncoding = 'gzip'; + handleRespOverride = ( + err: Error, + res: {}, + body: {}, + callback: Function + ) => { + const rawResponseStream = new PassThrough(); + Object.assign(rawResponseStream, { + toJSON() { + return { + headers: { + 'x-goog-hash': `crc32c=${responseCRC32C},md5=${responseMD5}`, + 'x-goog-stored-content-encoding': 'gzip', + }, + }; + }, + }); + callback(null, null, rawResponseStream); + setImmediate(() => { + rawResponseStream.end(DATA); + }); + }; + file .createReadStream({validation: 'crc32c'}) - .on('error', done) .on('end', done) .resume(); }); }); - it('should emit errors from the validation stream', done => { - const expectedError = new Error('test error'); + it('should perform validation if file was stored compressed and served compressed', done => { + file.metadata.crc32c = '.invalid.'; + file.metadata.contentEncoding = 'gzip'; + handleRespOverride = ( + err: Error, + res: {}, + body: {}, + callback: Function + ) => { + const rawResponseStream = new PassThrough(); + Object.assign(rawResponseStream, { + toJSON() { + return { + headers: { + 'x-goog-hash': `crc32c=${responseCRC32C},md5=${responseMD5}`, + 'x-goog-stored-content-encoding': 'gzip', + 'content-encoding': 'gzip', + }, + }; + }, + }); + callback(null, null, rawResponseStream); + setImmediate(() => { + rawResponseStream.end(DATA); + }); + }; - file.requestStream = getFakeSuccessfulRequest(DATA); + const expectedError = new Error('test error'); setFileValidationToError(expectedError); file - .createReadStream() + .createReadStream({validation: 'crc32c'}) .on('error', (err: Error) => { assert(err === expectedError); - done(); }) .resume(); }); - it('should not handle both error and end events', done => { + it('should emit errors from the validation stream', done => { const expectedError = new Error('test error'); file.requestStream = getFakeSuccessfulRequest(DATA); @@ -1438,45 +1483,26 @@ describe('File', () => { .on('error', (err: Error) => { assert(err === expectedError); - setImmediate(done); - }) - .on('end', () => { - done(new Error('Should not have been called.')); + done(); }) .resume(); }); - it('should pass the userProject to getMetadata', done => { - const fakeOptions = { - userProject: 'grapce-spaceship-123', - }; - - file.getMetadata = (options: GetFileMetadataOptions) => { - assert.strictEqual(options.userProject, fakeOptions.userProject); - setImmediate(done); - return Promise.resolve({ - crc32c: CRC32C_HASH, - }); - }; - - file.requestStream = getFakeSuccessfulRequest(DATA); - - file.createReadStream(fakeOptions).on('error', done).resume(); - }); - - it('should destroy stream from failed metadata fetch', done => { - const error = new Error('Error.'); - file.getMetadata = () => { - return Promise.reject(error); - }; + it('should not handle both error and end events', done => { + const expectedError = new Error('test error'); file.requestStream = getFakeSuccessfulRequest(DATA); + setFileValidationToError(expectedError); file .createReadStream() .on('error', (err: Error) => { - assert.strictEqual(err, error); - done(); + assert(err === expectedError); + + setImmediate(done); + }) + .on('end', () => { + done(new Error('Should not have been called.')); }) .resume(); });