From 0d89acd910f4377e594bc12a4f48129b3d29bcb2 Mon Sep 17 00:00:00 2001
From: Nazar Gargol <nazargargol@gmail.com>
Date: Mon, 1 Apr 2019 12:11:04 +0800
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20redirects=20to=20externa?=
 =?UTF-8?q?l=20URL?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes #10623

- The ability to redirect to external URLs was broken with https://github.com/TryGhost/Ghost/commit/7e211a307c92b48e4a8b0c9b6ce17620211ca018
- Added test coverage for external URL case
---
 .../shared/middlewares/custom-redirects.js    | 22 +++++++-----------
 core/test/regression/site/frontend_spec.js    | 23 +++++++++++++++++++
 core/test/utils/fixtures/data/redirects.json  |  4 ++++
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/core/server/web/shared/middlewares/custom-redirects.js b/core/server/web/shared/middlewares/custom-redirects.js
index cdc19087c2..02966000be 100644
--- a/core/server/web/shared/middlewares/custom-redirects.js
+++ b/core/server/web/shared/middlewares/custom-redirects.js
@@ -22,13 +22,6 @@ _private.registerRoutes = () => {
         validation.validateRedirects(redirects);
 
         redirects.forEach((redirect) => {
-            /**
-             * Extract target info, such as hash.
-             * 
-             * Required to re-formulate the correct endpoint.
-             */
-            const parsedTo = url.parse(redirect.to);
-
             /**
              * Detect case insensitive modifier when regex is enclosed by
              * / ... /i
@@ -56,17 +49,18 @@ _private.registerRoutes = () => {
 
             debug('register', redirect.from);
             customRedirectsRouter.get(new RegExp(redirect.from, options), function (req, res) {
-                const maxAge = redirect.permanent ? config.get('caching:customRedirects:maxAge') : 0,
-                    parsedUrl = url.parse(req.originalUrl);
+                const maxAge = redirect.permanent ? config.get('caching:customRedirects:maxAge') : 0;
+                const fromURL = url.parse(req.originalUrl);
+                const toURL = url.parse(redirect.to);
+
+                toURL.pathname = fromURL.pathname.replace(new RegExp(redirect.from, options), toURL.pathname),
+                toURL.search = fromURL.search;
 
                 res.set({
                     'Cache-Control': `public, max-age=${maxAge}`
                 });
-                res.redirect(redirect.permanent ? 301 : 302, url.format({
-                    pathname: parsedUrl.pathname.replace(new RegExp(redirect.from, options), parsedTo.pathname),
-                    search: parsedUrl.search,
-                    hash: parsedTo.hash
-                }));
+
+                res.redirect(redirect.permanent ? 301 : 302, url.format(toURL));
             });
         });
     } catch (err) {
diff --git a/core/test/regression/site/frontend_spec.js b/core/test/regression/site/frontend_spec.js
index 944a759014..9cc32efe26 100644
--- a/core/test/regression/site/frontend_spec.js
+++ b/core/test/regression/site/frontend_spec.js
@@ -673,6 +673,7 @@ describe('Frontend Routing', function () {
         });
     });
 
+    // TODO: convert to unit tests
     describe('Redirects (use redirects.json from test/utils/fixtures/data)', function () {
         var ghostServer;
 
@@ -923,5 +924,27 @@ describe('Frontend Routing', function () {
                     });
             });
         });
+
+        describe('external url redirect', function () {
+            it('with trailing slash', function (done) {
+                request.get('/external-url/')
+                    .expect(302)
+                    .expect('Cache-Control', testUtils.cacheRules.public)
+                    .end(function (err, res) {
+                        res.headers.location.should.eql('https://ghost.org/');
+                        doEnd(done)(err, res);
+                    });
+            });
+
+            it('without trailing slash', function (done) {
+                request.get('/external-url')
+                    .expect(302)
+                    .expect('Cache-Control', testUtils.cacheRules.public)
+                    .end(function (err, res) {
+                        res.headers.location.should.eql('https://ghost.org/');
+                        doEnd(done)(err, res);
+                    });
+            });
+        });
     });
 });
diff --git a/core/test/utils/fixtures/data/redirects.json b/core/test/utils/fixtures/data/redirects.json
index ace3c1358a..7c828abbb9 100644
--- a/core/test/utils/fixtures/data/redirects.json
+++ b/core/test/utils/fixtures/data/redirects.json
@@ -42,5 +42,9 @@
     {
         "from": "^\\/Default-Sensitive",
         "to": "/redirected-default"
+    },
+    {
+        "from": "/external-url",
+        "to": "https://ghost.org"
     }
 ]