From 85a66715cef9fde92e47e310c1ebc01f285333c5 Mon Sep 17 00:00:00 2001
From: Jonathan Lui <jonathanlui@google.com>
Date: Wed, 5 Feb 2020 17:54:54 -0800
Subject: [PATCH] fix: skip validation for server decompressed objects

---
 src/file.ts            | 20 +++++++++++---
 system-test/storage.ts | 27 +++++++++++++++++++
 test/file.ts           | 59 +++++++++++++++++++++++++++++++++++++-----
 3 files changed, 97 insertions(+), 9 deletions(-)

diff --git a/src/file.ts b/src/file.ts
index 56db4a542..b5e9904af 100644
--- a/src/file.ts
+++ b/src/file.ts
@@ -1212,6 +1212,7 @@ class File extends ServiceObject<File> {
     // once Node 8 support is discontinued
     const throughStream = streamEvents(through()) as Duplex;
 
+    let isServedCompressed = true;
     let crc32c = true;
     let md5 = false;
 
@@ -1294,7 +1295,7 @@ class File extends ServiceObject<File> {
       //      applicable, to the user.
       const onResponse = (
         err: Error | null,
-        body: ResponseBody,
+        _body: ResponseBody,
         rawResponseStream: Metadata
       ) => {
         if (err) {
@@ -1312,7 +1313,7 @@ class File extends ServiceObject<File> {
         rawResponseStream.on('error', onComplete);
 
         const headers = rawResponseStream.toJSON().headers;
-        const isCompressed = headers['content-encoding'] === 'gzip';
+        isServedCompressed = headers['content-encoding'] === 'gzip';
         const shouldRunValidation = !rangeRequest && (crc32c || md5);
         const throughStreams: Writable[] = [];
 
@@ -1321,7 +1322,7 @@ class File extends ServiceObject<File> {
           throughStreams.push(validateStream);
         }
 
-        if (isCompressed && options.decompress) {
+        if (isServedCompressed && options.decompress) {
           throughStreams.push(zlib.createGunzip());
         }
 
@@ -1369,6 +1370,17 @@ class File extends ServiceObject<File> {
 
         onCompleteCalled = true;
 
+        // TODO: 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.
+        // See https://github.com/googleapis/nodejs-storage/issues/709.
+        if (this.metadata.contentEncoding === 'gzip' && !isServedCompressed) {
+          onCompleteCalled = true;
+          throughStream.end();
+          return;
+        }
+
         const hashes = {
           crc32c: this.metadata.crc32c,
           md5: this.metadata.md5Hash,
@@ -1602,6 +1614,8 @@ class File extends ServiceObject<File> {
    *     supported for composite objects. An error will be raised if MD5 is
    *     specified but is not available. You may also choose to skip validation
    *     completely, however this is **not recommended**.
+   *     NOTE: Validation is automatically skipped for objects that were
+   *     uploaded using the `gzip` option and have already compressed content.
    */
   /**
    * Create a writable stream to overwrite the contents of the file in your
diff --git a/system-test/storage.ts b/system-test/storage.ts
index 374a870cb..856ae0422 100644
--- a/system-test/storage.ts
+++ b/system-test/storage.ts
@@ -2250,6 +2250,33 @@ describe('storage', () => {
       });
     });
 
+    it('should skip validation if file is served decompressed', done => {
+      const filename = 'logo-gzipped.png';
+      bucket
+        .upload(FILES.logo.path, {
+          destination: filename,
+          gzip: true,
+        })
+        .then(() => {
+          tmp.setGracefulCleanup();
+          tmp.file((err, tmpFilePath) => {
+            assert.ifError(err);
+
+            const file = bucket.file(filename)
+            file.createReadStream()
+              .pipe(fs.createWriteStream(tmpFilePath))
+              .on('error', done)
+              .on('response', (err, body, raw) => {
+                assert.strictEqual(raw.toJSON().headers['content-encoding'], undefined);
+              })
+              .on('finish', () => {
+                file.delete(done);
+              });
+          });
+        })
+        .catch(done);
+    });
+
     describe('simple write', () => {
       it('should save arbitrary data', done => {
         const file = bucket.file('TestFile');
diff --git a/test/file.ts b/test/file.ts
index f9df3a7c0..c1ac7e6ff 100644
--- a/test/file.ts
+++ b/test/file.ts
@@ -1293,7 +1293,7 @@ describe('File', () => {
 
     describe('validation', () => {
       const data = 'test';
-      let fakeValidationStream: stream.Stream;
+      let fakeValidationStream: stream.Stream & {'test': Function};
 
       beforeEach(() => {
         file.metadata.mediaLink = 'http://uri';
@@ -1306,16 +1306,63 @@ describe('File', () => {
           callback();
         };
 
-        fakeValidationStream = through();
-        // tslint:disable-next-line no-any
-        (fakeValidationStream as any).test = () => {
-          return true;
-        };
+        fakeValidationStream = Object.assign(
+          through(),
+          {test: () => true},
+        );
         hashStreamValidationOverride = () => {
           return fakeValidationStream;
         };
       });
 
+      describe('server decompression', () => {
+        beforeEach(() => {
+          handleRespOverride = (
+            err: Error,
+            res: {},
+            body: {},
+            callback: Function
+          ) => {
+            const rawResponseStream = through();
+            Object.assign(rawResponseStream, {
+              toJSON() {
+                return {
+                  headers: {},
+                };
+              },
+            });
+            callback(null, null, rawResponseStream);
+            setImmediate(() => {
+              rawResponseStream.end(data);
+            });
+          };
+          file.requestStream = getFakeSuccessfulRequest(data);
+        });
+
+        it('should skip validation if file was stored compressed', done => {
+          const validateStub = sinon.stub().returns(true);
+          fakeValidationStream.test = validateStub;
+
+          file.getMetadata = (options: {}, callback: Function) => {
+            file.metadata = {
+              crc32c: '####wA==',
+              md5Hash: 'CY9rzUYh03PK3k6DJie09g==',
+              contentEncoding: 'gzip',
+            };
+            callback();
+          };
+
+          file
+            .createReadStream({validation: 'crc32c'})
+            .on('error', done)
+            .on('end', () => {
+              assert(validateStub.notCalled);
+              done();
+            })
+            .resume();
+        });
+      });
+
       it('should emit errors from the validation stream', done => {
         const error = new Error('Error.');