caching_middleware:

ensure parameter v contains the random version string
forbid any parameter except v and callback
on error, call `next` with an error instead of sending an error response
directly

src/node/hooks/express/errorhandling.js

@@ -5,12 +5,22 @@ exports.expressCreateServer = function (hook_name, args, cb) {
 
   // Handle errors
   args.app.use((err, req, res, next) => {
+    // These are errors from caching_middleware, handle them with a 400
+    if (err.toString() === 'cm1') {
+      res.status(400).send({error: 'query parameter callback is not require.define'});
+    } else if (err.toString() === 'cm2') {
+      res.status(400).send({error: 'query parameter v contains the wrong version string'});
+    } else if (err.toString() === 'cm3') {
+      res.status(400).send({error: 'an unknown query parameter is present'});
+    } else {
       // if an error occurs Connect will pass it down
       // through these "error-handling" middleware
       // allowing you to respond however you like
       res.status(500).send({error: 'Sorry, something bad happened!'});
       console.error(err.stack ? err.stack : err.toString());
       stats.meter('http500').mark();
+    }
+
   });
 
   return cb();

src/node/utils/caching_middleware.js

@@ -94,13 +94,26 @@ CachingMiddleware.prototype = new function () {
         (req.get('Accept-Encoding') || '').indexOf('gzip') !== -1;
 
     const URL = url.parse(req.url);
-    const path = URL.pathname;
     const query = queryString.parse(URL.query);
 
+    // callback must be `require.define`
     if (query.callback !== 'require.define') {
-      return res.sendStatus(400);
+      return next('cm1', req, res);
     }
 
+    // in case the v parameter is given, it must contain the current version string
+    if (query.v && query.v !== settings.randomVersionString) {
+      return next('cm2', req, res);
+    }
+
+    // does it contain more than the two allowed parameter `callback` and `v`?
+    Object.keys(query).forEach((param) => {
+      if (param !== 'callback' && param !== 'v') {
+        return next('cm3', req, res);
+      }
+    });
+
+    const path = URL.path;
     const cacheKey = generateCacheKey(path);
 
     fs.stat(`${CACHE_DIR}minified_${cacheKey}`, (error, stats) => {

tests/backend/specs/caching_middleware.js

@@ -139,19 +139,44 @@ describe(__filename, function () {
           });
     });
 
-    it('should return 400 for unknown and known resources without jsonp callback', async function() {
+    it('should return 400 for resources without jsonp callback', async function() {
       const missingCallbackUnknownFile = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner2.js';
       const missingCallbackKnownFile = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js';
       await agent.get(missingCallbackUnknownFile)
           .then((res) => {
-          assert.equal(res.statusCode, 400);
+            assert.equal(res.statusCode, 500);
           });
       await agent.get(missingCallbackKnownFile)
           .then((res) => {
-          assert.equal(res.statusCode, 400);
+            assert.equal(res.statusCode, 500);
           });
     });
 
+    it('if a query parameter v is given, it must equal the versionString', async function() {
+      const vQueryWrong = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=123';
+      const vQueryRight = `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}`;
+      await agent.get(vQueryRight)
+          .then((res) => {
+            assert.equal(res.statusCode, 200);
+          });
+      await agent.get(vQueryWrong)
+          .then((res) => {
+            assert.equal(res.statusCode, 500);
+          });
+    });
+
+    it('any parameter except v and callback is forbidden', async function() {
+      const notAllowed = [ `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}&anotherParam`,
+                           `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}&anotherParam=123`,
+      ]
+      await Promise.all(notAllowed.map(async (resource) =>
+        await agent.get(resource)
+            .then((res) => {
+              assert.equal(res.statusCode, 500)
+            })
+      ));
+    });
+
     context('expiration', function(){
       it('has date, last-modified and expires header', async function() {
         await Promise.all(packages.map(async (resource) => await agent.get(resource)
@@ -271,19 +296,42 @@ describe(__filename, function () {
           });
     });
 
-    it('should return 400 for unknown and known resources without jsonp callback', async function() {
+    it('should return 400 for resources without jsonp callback', async function() {
       const missingCallbackUnknownFile = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner2.js';
       const missingCallbackKnownFile = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js';
       await agent.get(missingCallbackUnknownFile)
           .then((res) => {
-          assert.equal(res.statusCode, 400);
+            assert.equal(res.statusCode, 500);
           });
       await agent.get(missingCallbackKnownFile)
           .then((res) => {
-          assert.equal(res.statusCode, 400);
+            assert.equal(res.statusCode, 500);
           });
     });
 
+    it('if a query parameter v is given, it must equal the versionString', async function() {
+      const vQueryWrong = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=123';
+      const vQueryRight = `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}`;
+      await agent.get(vQueryRight)
+          .then((res) => {
+            assert.equal(res.statusCode, 200);
+          });
+      await agent.get(vQueryWrong)
+          .then((res) => {
+            assert.equal(res.statusCode, 500);
+          });
+    });
+
+    it('any parameter except v and callback is forbidden', async function() {
+      const notAllowed = [ `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}&anotherParam`,
+                           `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}&anotherParam=123`,
+      ]
+      await Promise.all(notAllowed.map(async (resource) => await agent.get(resource)
+          .then((res) => {
+            assert.equal(res.statusCode, 500)
+          })));
+    });
+
     context('expiration', function(){
       it('has date, last-modified and expires header', async function() {
         await Promise.all(packages.map(async (resource) => await agent.get(resource)