minify: Fix gzip not triggered for packages (#4491)

* caching_middleware: fix gzip compression not triggered

* packages: If a client sets `Accept-Encoding: gzip`, the responseCache will
include `Content-Encoding: gzip` in all future responses, even
if a subsequent request does not set `Accept-Encoding` or another client
requests the file without setting `Accept-Encoding`.
Fix that.

* caching_middleware: use `test` instead of `match`

* add tests

* make code easier to understand

* make the regex more clear

src/node/utils/caching_middleware.js

@@ -20,7 +20,6 @@ var fs = require('fs');
 var path = require('path');
 var zlib = require('zlib');
 var settings = require('./Settings');
-var semver = require('semver');
 var existsSync = require('./path_exists');
 
 /*
@@ -192,11 +191,12 @@ CachingMiddleware.prototype = new function () {
         res.write = old_res.write || res.write;
         res.end = old_res.end || res.end;
 
-        var headers = responseCache[cacheKey].headers;
+        let headers = {};
+        Object.assign(headers, (responseCache[cacheKey].headers || {}));
         var statusCode = responseCache[cacheKey].statusCode;
 
         var pathStr = CACHE_DIR + 'minified_' + cacheKey;
-        if (supportsGzip && (headers['content-type'] || '').match(/^text\//)) {
+        if (supportsGzip && /application\/javascript/.test(headers['content-type'])) {
           pathStr = pathStr + '.gz';
           headers['content-encoding'] = 'gzip';
         }

tests/backend/specs/caching_middleware.js

@@ -0,0 +1,168 @@
+/**
+ * caching_middleware is responsible for serving everything under path `/javascripts/`
+ * That includes packages as defined in `src/node/utils/tar.json` and probably also plugin code
+ *
+ */
+
+const common = require('../common');
+const settings = require('../../../src/node/utils/Settings');
+const assert = require('assert').strict;
+const url = require('url');
+const queryString = require('querystring');
+
+let agent;
+
+/**
+ * Hack! Returns true if the resource is not plaintext
+ * The file should start with the callback method, so we need the
+ * URL.
+ *
+ * @param {string} fileContent the response body
+ * @param {URI} resource resource URI
+ * @returns {boolean} if it is plaintext 
+ */
+function isPlaintextResponse(fileContent, resource){
+  // callback=require.define&v=1234
+  const query = url.parse(resource)['query'];
+  // require.define
+  const jsonp = queryString.parse(query)['callback'];
+
+  // returns true if the first letters in fileContent equal the content of `jsonp`
+  return fileContent.substring(0, jsonp.length) === jsonp;
+}
+
+/**
+ * A hack to disable `superagent`'s auto unzip functionality
+ *
+ * @param {Request} request
+ */
+function disableAutoDeflate(request){
+  request._shouldUnzip = function(){
+    return false
+  }
+}
+
+describe(__filename, function() {
+  const backups = {};
+  const fantasyEncoding = 'brainwaves'; // non-working encoding until https://github.com/visionmedia/superagent/pull/1560 is resolved
+  const packages = [
+    "/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define"
+  , "/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define"
+  , "/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define"
+  , "/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define"
+  ];
+
+  before(async function() {
+    agent = await common.init();
+  });
+  beforeEach(async function() {
+    backups.settings = {};
+    backups.settings['minify'] = settings.minify;
+  });
+  afterEach(async function() {
+    Object.assign(settings, backups.settings);
+  });
+
+  context('when minify is false', function(){
+    before(async function() {
+      settings.minify = false;
+    });
+    it('gets packages uncompressed without Accept-Encoding gzip', async function() {
+      await Promise.all(packages.map(async (resource) => {
+        return agent.get(resource)
+          .set('Accept-Encoding', fantasyEncoding)
+          .use(disableAutoDeflate)
+          .then((res) => {
+            assert.match(res.header['content-type'], /application\/javascript/);
+            assert.equal(res.header['content-encoding'], undefined);
+            assert.equal(isPlaintextResponse(res.text, resource), true);
+            return;
+          })
+      }))
+    })
+
+    it('gets packages compressed with Accept-Encoding gzip', async function() {
+      await Promise.all(packages.map(async (resource) => {
+        return agent.get(resource)
+          .set('Accept-Encoding', 'gzip')
+          .use(disableAutoDeflate)
+          .then((res) => {
+            assert.match(res.header['content-type'], /application\/javascript/);
+            assert.equal(res.header['content-encoding'], 'gzip');
+            assert.equal(isPlaintextResponse(res.text, resource), false);
+            return;
+          })
+      }))
+    })
+
+    it('does not cache content-encoding headers', async function(){
+      await agent.get(packages[0])
+        .set('Accept-Encoding', fantasyEncoding)
+        .then((res) => {
+          return assert.equal(res.header['content-encoding'], undefined);
+        });
+      await agent.get(packages[0])
+        .set('Accept-Encoding', 'gzip')
+        .then((res) => {
+          return assert.equal(res.header['content-encoding'], 'gzip');
+        });
+      await agent.get(packages[0])
+        .set('Accept-Encoding', fantasyEncoding)
+        .then((res) => {
+          return assert.equal(res.header['content-encoding'], undefined);
+        });
+    })
+  });
+
+  context('when minify is true', function(){
+    before(async function() {
+      settings.minify = true;
+    });
+    it('gets packages uncompressed without Accept-Encoding gzip', async function() {
+      await Promise.all(packages.map(async (resource) => {
+        return agent.get(resource)
+          .set('Accept-Encoding', fantasyEncoding)
+          .use(disableAutoDeflate)
+          .then((res) => {
+            assert.match(res.header['content-type'], /application\/javascript/);
+            assert.equal(res.header['content-encoding'], undefined);
+            assert.equal(isPlaintextResponse(res.text, resource), true);
+            return;
+          })
+      }))
+    })
+
+    it('gets packages compressed with Accept-Encoding gzip', async function() {
+      await Promise.all(packages.map(async (resource) => {
+        return agent.get(resource)
+          .set('Accept-Encoding', 'gzip')
+          .use(disableAutoDeflate)
+          .then((res) => {
+            assert.match(res.header['content-type'], /application\/javascript/);
+            assert.equal(res.header['content-encoding'], 'gzip');
+            assert.equal(isPlaintextResponse(res.text, resource), false);
+            return;
+          })
+      }))
+    })
+
+    it('does not cache content-encoding headers', async function(){
+      await agent.get(packages[0])
+        .set('Accept-Encoding', fantasyEncoding)
+        .then((res) => {
+          return assert.equal(res.header['content-encoding'], undefined);
+        });
+      await agent.get(packages[0])
+        .set('Accept-Encoding', 'gzip')
+        .then((res) => {
+          return assert.equal(res.header['content-encoding'], 'gzip');
+        });
+      await agent.get(packages[0])
+        .set('Accept-Encoding', fantasyEncoding)
+        .then((res) => {
+          return assert.equal(res.header['content-encoding'], undefined);
+        });
+    })
+  });
+
+});