From 7e556d84dec735fd336a4aced41688a7cad133e8 Mon Sep 17 00:00:00 2001
From: Simon Backx <simon@ghost.org>
Date: Thu, 24 Mar 2022 09:54:19 +0100
Subject: [PATCH] Fixed adding same redirect multiple times throws an error on
 removal  (#379)

refs https://ghost.slack.com/archives/C02G9E68C/p1647599592576139

When you add a redirect multiple times, and remove it afterwards, an error is thrown:
`Cannot destructure property 'fromRegex' of 'this.redirects[redirectId]' as it is undefined.`
This was caused by `redirectIds` that contained the same id multiple times.

* Added a test for adding a redirect multiple times and removing it once
* Fixed adding same redirect multiple times throws an error on removal
---
 .../lib/DynamicRedirectManager.js                |  5 ++++-
 .../test/DynamicRedirectManager.test.js          | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/ghost/express-dynamic-redirects/lib/DynamicRedirectManager.js b/ghost/express-dynamic-redirects/lib/DynamicRedirectManager.js
index e9211451fd..b184f16a34 100644
--- a/ghost/express-dynamic-redirects/lib/DynamicRedirectManager.js
+++ b/ghost/express-dynamic-redirects/lib/DynamicRedirectManager.js
@@ -97,7 +97,10 @@ class DynamicRedirectManager {
             const fromRegex = this.buildRegex(from);
             const redirectId = from;
 
-            this.redirectIds.push(redirectId);
+            if (!this.redirectIds.includes(redirectId)) {
+                this.redirectIds.push(redirectId);
+            }
+            
             this.redirects[redirectId] = {
                 fromRegex,
                 to,
diff --git a/ghost/express-dynamic-redirects/test/DynamicRedirectManager.test.js b/ghost/express-dynamic-redirects/test/DynamicRedirectManager.test.js
index d6357a03b6..a05bb61bca 100644
--- a/ghost/express-dynamic-redirects/test/DynamicRedirectManager.test.js
+++ b/ghost/express-dynamic-redirects/test/DynamicRedirectManager.test.js
@@ -76,6 +76,22 @@ describe('DynamicRedirectManager', function () {
             should.equal(location, null);
         });
 
+        it('Can add same redirect multiple times and remove it once', function () {
+            manager.addRedirect('/test-params', '/result?q=abc', {permanent: true});
+            const id = manager.addRedirect('/test-params', '/result?q=abc', {permanent: true});
+            manager.removeRedirect(id);
+
+            req.url = '/test-params/?q=123&lang=js';
+
+            manager.handleRequest(req, res, function next() {
+                should.ok(true, 'next should have been called');
+            });
+
+            should.equal(headers, null);
+            should.equal(status, null);
+            should.equal(location, null);
+        });
+
         it('The routing works when passed an invalid regexp for the from parameter', function () {
             const from = '/invalid_regex/(/size/[a-zA-Z0-9_-.]*/[a-zA-Z0-9_-.]*/[0-9]*/[0-9]*/)([a-zA-Z0-9_-.]*)';
             const to = '/';