Merged v4.17.1 into main

v4.17.1

core/client

@@ -1 +1 @@
-Subproject commit 70dac42ffd8e694ead98640fab11e957f79b51e2
+Subproject commit c3145d4397e93ea67625e664f47be2ed1a391788

core/server/services/redirects/validation.js

@@ -4,6 +4,7 @@ const errors = require('@tryghost/errors');
 
 const messages = {
     redirectsWrongFormat: 'Incorrect redirects file format.',
+    invalidRedirectsFromRegex: 'Incorrect RegEx in redirects file.',
     redirectsHelp: 'https://ghost.org/docs/themes/routing/#redirects'
 };
 /**
@@ -26,6 +27,17 @@ const validate = (redirects) => {
                 help: tpl(messages.redirectsHelp)
             });
         }
+
+        try {
+            // each 'from' property should be a valid RegExp string
+            new RegExp(redirect.from);
+        } catch (error) {
+            throw new errors.ValidationError({
+                message: tpl(messages.invalidRedirectsFromRegex),
+                context: redirect,
+                help: tpl(messages.redirectsHelp)
+            });
+        }
     });
 };
 

core/server/web/shared/middlewares/custom-redirects.js

@@ -19,14 +19,14 @@ let customRedirectsRouter;
 
 /**
  *
+ * @param {Object} router instance of Express Router to decorate with redirects
  * @param {Object} redirects valid redirects JSON
+ *
  * @returns {Object} instance of express.Router express router handling redirects based on config
  */
-_private.registerRoutes = (redirects) => {
+_private.registerRoutes = (router, redirects) => {
     debug('redirects loading');
 
-    const redirectsRouter = express.Router('redirects');
-
     if (labsService.isSet('offers')) {
         redirects.unshift({
             from: '/zimo50',
@@ -61,7 +61,7 @@ _private.registerRoutes = (redirects) => {
         }
 
         debug('register', redirect.from);
-        redirectsRouter.get(new RegExp(redirect.from, options), function (req, res) {
+        router.get(new RegExp(redirect.from, options), function (req, res) {
             const maxAge = redirect.permanent ? config.get('caching:customRedirects:maxAge') : 0;
             const toURL = url.parse(redirect.to);
             const toURLParams = querystring.parse(toURL.query);
@@ -91,16 +91,17 @@ _private.registerRoutes = (redirects) => {
 
     debug('redirects loaded');
 
-    return redirectsRouter;
+    return router;
 };
 
 const loadRoutes = () => {
     try {
+        customRedirectsRouter = express.Router('redirects');
+
         const redirects = redirectsService.loadRedirectsFile();
         redirectsService.validate(redirects);
 
-        const redirectsRouter = _private.registerRoutes(redirects);
-        customRedirectsRouter = redirectsRouter;
+        _private.registerRoutes(customRedirectsRouter, redirects);
     } catch (err) {
         if (errors.utils.isIgnitionError(err)) {
             logging.error(err);

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ghost",
-  "version": "4.17.0",
+  "version": "4.17.1",
   "description": "The professional publishing platform",
   "author": "Ghost Foundation",
   "homepage": "https://ghost.org",

test/unit/services/redirects/validation.test.js

@@ -0,0 +1,44 @@
+const should = require('should');
+
+const {validate} = require('../../../../core/server/services/redirects/validation');
+
+describe('UNIT: custom redirects validation', function () {
+    it('passes validation for a valid redirects config', function () {
+        const config = [{
+            permanent: true,
+            from: '/test-params',
+            to: '/result?q=abc'
+        }];
+
+        validate(config);
+    });
+
+    it('throws for an invalid redirects config format missing from parameter', function () {
+        const config = [{
+            permanent: true,
+            to: '/'
+        }];
+
+        try {
+            validate(config);
+            should.fail('should have thrown');
+        } catch (err) {
+            err.message.should.equal('Incorrect redirects file format.');
+        }
+    });
+
+    it('throws for an invalid redirects config having invalid RegExp in from field', function () {
+        const config = [{
+            permanent: true,
+            from: '/invalid_regex/(/size/[a-zA-Z0-9_-.]*/[a-zA-Z0-9_-.]*/[0-9]*/[0-9]*/)([a-zA-Z0-9_-.]*)',
+            to: '/'
+        }];
+
+        try {
+            validate(config);
+            should.fail('should have thrown');
+        } catch (err) {
+            err.message.should.equal('Incorrect RegEx in redirects file.');
+        }
+    });
+});

test/unit/web/shared/middleware/custom-redirects.test.js

@@ -1,6 +1,8 @@
 const should = require('should');
 const sinon = require('sinon');
 const rewire = require('rewire');
+const express = require('express');
+const supertest = require('supertest');
 
 const customRedirects = rewire('../../../../../core/server/web/shared/middlewares/custom-redirects');
 const registerRoutes = customRedirects.__get__('_private.registerRoutes');
@@ -16,7 +18,8 @@ describe('UNIT: custom redirects', function () {
         };
         res = {
             redirect: sinon.spy(),
-            set: sinon.spy()
+            set: sinon.spy(),
+            writeHead: sinon.spy()
         };
 
         next = sinon.spy();
@@ -33,7 +36,7 @@ describe('UNIT: custom redirects', function () {
             from: '/test-params',
             to: '/result?q=abc'
         }];
-        const redirect = registerRoutes(redirectsConfig);
+        const redirect = registerRoutes(new express.Router(), redirectsConfig);
 
         req.url = '/test-params/?q=123&lang=js';
         redirect(req, res, next);
@@ -45,4 +48,22 @@ describe('UNIT: custom redirects', function () {
             'Cache-Control': `public, max-age=0`
         });
     });
+
+    it('the parent app functions even when the middleware gets an invalid redirects configuration', function (done) {
+        const redirectsConfig = [{
+            permanent: true,
+            from: '/invalid_regex/(/size/[a-zA-Z0-9_-.]*/[a-zA-Z0-9_-.]*/[0-9]*/[0-9]*/)([a-zA-Z0-9_-.]*)',
+            to: '/'
+        }];
+        const redirectsService = customRedirects.__get__('redirectsService');
+        sinon.stub(redirectsService, 'loadRedirectsFile').returns(redirectsConfig);
+
+        const app = express('test');
+        customRedirects.use(app);
+        app.get('/', (_req, _res) => _res.status(200).end());
+
+        supertest(app)
+            .get('/')
+            .expect(200, done);
+    });
 });