Merge pull request alibaba#295 from alibaba/304-gzip

do not unzip the response body when it's empty, such as 304

Author: ottomao

lib/requestHandler.js

@@ -93,19 +93,20 @@ function fetchRemoteResponse(protocol, options, reqData, config) {
             fulfill(resDataStream);
           } else {
             const serverResData = Buffer.concat(resDataChunks);
+            const originContentLen = util.getByteSize(serverResData);
+            // set origin content length into header
+            resHeader['x-anyproxy-origin-content-length'] = originContentLen;
 
-            // put origin content length into header
-            resHeader['x-anyproxy-origin-content-length'] = util.getByteSize(serverResData);
-
-            if (ifServerGzipped) {
+            // only do unzip when there is res data
+            if (ifServerGzipped && originContentLen) {
               zlib.gunzip(serverResData, (err, buff) => { // TODO test case to cover
                 if (err) {
                   rejectParsing(err);
                 } else {
                   fulfill(buff);
                 }
               });
-            } else if (isServerDeflated) {
+            } else if (isServerDeflated && originContentLen) {
               zlib.inflateRaw(serverResData, (err, buff) => { // TODO test case to cover
                 if (err) {
                   rejectParsing(err);

test/jasmine.json

@@ -8,6 +8,6 @@
      "../node_modules/babel-register/lib/node.js",
      "../node_modules/babel-polyfill/dist/polyfill.js"
   ],
-  "stopSpecOnExpectationFailure": true,
+  "stopSpecOnExpectationFailure": false,
   "random": false
 }

test/server/server.js

@@ -103,6 +103,11 @@ KoaServer.prototype.constructRouter = function () {
     });
   });
 
+  router.get('/test/response/304', this.logRequest, function *(next) {
+    this.response.set('Content-Encoding', 'gzip');
+    this.status = 304;
+  });
+
   router.get('/test/response/303', function *(next) {
     printLog('now to redirect 303');
     this.redirect('/test');

test/spec_rule/no_rule_spec.js

@@ -212,6 +212,31 @@ function testRequest(protocol = 'http') {
         });
     });
 
+    it('304 should work as direct without proxy rules', (done) => {
+      const url = constructUrl('/test/response/304');
+
+      proxyGet(url, CommonRequestHeader)
+        .then(proxyRes => {
+          directGet(url, CommonRequestHeader)
+            .then(directRes => {
+              expect(directRes.statusCode).toEqual(304);
+              expect(directRes.body).toEqual('');
+
+              expect(directRes.statusCode).toEqual(proxyRes.statusCode);
+              expect(isCommonResHeaderEqual(directRes.headers, proxyRes.headers, url)).toBe(true);
+              expect(directRes.body).toEqual(proxyRes.body);
+              expect(isCommonReqEqual(url, serverInstance)).toBe(true);
+              done();
+            }, error => {
+              console.error('error happened in direct 304 request:', error);
+              done.fail('error happened in direct 304 request');
+            });
+        }, error => {
+          console.error('error happened in proxy 304 request:', error);
+          done.fail('error happened in proxy 304 request');
+        });
+    })
+
     describe('Response code should be honored as direct without proxy rules', () => {
       [301, 302, 303].forEach(code => {
         testRedirect(code);

test/util/CommonUtil.js

@@ -130,13 +130,6 @@ function isCommonReqEqual(url, serverInstance) {
     isEqual = isEqual && proxyReqObj.headers['via-proxy'] === 'true';
     delete proxyReqObj.headers['via-proxy'];
 
-    // exclued accept-encoding from comparing, since the proxy will remove it before sending it out
-    delete directReqObj.headers['accept-encoding'];
-
-    // TODO: 我这里proxy出去的options里没有accept-encoding, 但node自己加上了。Why ?
-    // By 加里 2017.1.31
-    delete proxyReqObj.headers['accept-encoding'];
-
     directReqObj.headers['content-type'] = trimFormContentType(directReqObj.headers['content-type']);
     proxyReqObj.headers['content-type'] = trimFormContentType(proxyReqObj.headers['content-type']);