From ce563179b890e711896ee2f4caea566fec600941 Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <erisds@gmail.com>
Date: Mon, 8 Jul 2019 17:29:41 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20infinite=20redirect=20fo?=
 =?UTF-8?q?r=20amp=20when=20disabled?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

fixes 10883

- fixed an issue where /amp/ pages would cause an infinite redirect loop
- this only occurred when amp was disabled, and query params were passed to the /amp/ url
- this fix resolves the issue by not assuming /amp/ is the end of the URL
- it also checks for `/amp/` (both slashes) and replaces one
---
 core/frontend/apps/amp/index.js            | 13 +++++++------
 core/test/regression/site/frontend_spec.js | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/core/frontend/apps/amp/index.js b/core/frontend/apps/amp/index.js
index 416b121dbb..5b60a3dc82 100644
--- a/core/frontend/apps/amp/index.js
+++ b/core/frontend/apps/amp/index.js
@@ -1,16 +1,17 @@
-const router = require('./lib/router'),
-    registerHelpers = require('./lib/helpers'),
-    urlUtils = require('../../../server/lib/url-utils'),
+const router = require('./lib/router');
+const registerHelpers = require('./lib/helpers');
+const urlUtils = require('../../../server/lib/url-utils');
 
-    // Dirty requires
-    settingsCache = require('../../../server/services/settings/cache');
+// Dirty requires
+const settingsCache = require('../../../server/services/settings/cache');
 
 function ampRouter(req, res) {
     if (settingsCache.get('amp') === true) {
         return router.apply(this, arguments);
     } else {
         // routeKeywords.amp: 'amp'
-        let redirectUrl = req.originalUrl.replace(/amp\/$/, '');
+        let redirectUrl = req.originalUrl.replace(/\/amp\//, '/');
+
         urlUtils.redirect301(res, redirectUrl);
     }
 }
diff --git a/core/test/regression/site/frontend_spec.js b/core/test/regression/site/frontend_spec.js
index e8ea49d435..c4da7047ca 100644
--- a/core/test/regression/site/frontend_spec.js
+++ b/core/test/regression/site/frontend_spec.js
@@ -262,6 +262,20 @@ describe('Frontend Routing', function () {
                     .expect(301)
                     .end(doEnd(done));
             });
+
+            it('should redirect to regular post with query params when AMP is disabled', function (done) {
+                sinon.stub(settingsCache, 'get').callsFake(function (key, options) {
+                    if (key === 'amp' && !options) {
+                        return false;
+                    }
+                    return origCache.get(key, options);
+                });
+
+                request.get('/welcome/amp/?q=a')
+                    .expect('Location', '/welcome/?q=a')
+                    .expect(301)
+                    .end(doEnd(done));
+            });
         });
 
         describe('Static assets', function () {