caching_middleware: ignore query string when generating cacheKey

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.

src/node/utils/caching_middleware.js

@@ -20,8 +20,8 @@ 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');
+const url = require('url');
 
 /*
  * The crypto module can be absent on reduced node installations.
@@ -92,8 +92,14 @@ CachingMiddleware.prototype = new function () {
     var supportsGzip =
         (req.get('Accept-Encoding') || '').indexOf('gzip') != -1;
 
-    var path = require('url').parse(req.url).path;
-    var cacheKey = generateCacheKey(path);
+    /**
+     * `req.url` is either /file?callback=require.define&v=versionString
+     * or /file.
+     *
+     * invalid.invalid is just a placeholder
+     */
+    let path = new url.URL(req.url, 'http://invalid.invalid').pathname;
+    let cacheKey = generateCacheKey(path);
 
     fs.stat(CACHE_DIR + 'minified_' + cacheKey, function (error, stats) {
       var modifiedSince = (req.headers['if-modified-since']
@@ -192,11 +198,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,186 @@
+/**
+ * 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');
+const fs = require('fs');
+const path = require('path');
+
+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);
+        });
+    })
+  });
+
+  context("different callback or version parameter", function(){
+    it('should never create new files', async function(){
+      const pathToVar = path.join(__filename, '../../../../var');
+
+      // the number of files in ./var should not change during this test
+      const minifiedFilesBefore = (await fs.promises.readdir(pathToVar)).length;
+      const resource1 = "/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define";
+      const resource2 = "/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require";
+      const resource3 = "/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=1234";
+      await agent.get(resource1);
+      await agent.get(resource2);
+      await agent.get(resource3);
+      const minifiedFilesAfter = (await fs.promises.readdir(pathToVar)).length;
+      assert.equal(minifiedFilesBefore, minifiedFilesAfter);
+    })
+  })
+});