From 32cd3b9433c6e5379fe5cc2ac77369d1e51889bf Mon Sep 17 00:00:00 2001
From: Naz <hi@nazavo.com>
Date: Thu, 2 Feb 2023 15:01:20 +0800
Subject: [PATCH] Optimized link redirects handling

refs https://github.com/TryGhost/Toolbox/issues/515

- The link redirect handled was querying database on every single frontend request causing a significant amount of unnecessary traffic
- The optimization is returning early if the incoming URL does not start with a common "r/" prefix
---
 .../LinkRedirectRepository.js                 |  2 +-
 .../lib/LinkRedirectsService.js               | 11 ++++-
 .../test/LinkRedirectsService.test.js         | 42 +++++++++++++++++--
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/ghost/core/core/server/services/link-redirection/LinkRedirectRepository.js b/ghost/core/core/server/services/link-redirection/LinkRedirectRepository.js
index a126540ef8..788ebaf363 100644
--- a/ghost/core/core/server/services/link-redirection/LinkRedirectRepository.js
+++ b/ghost/core/core/server/services/link-redirection/LinkRedirectRepository.js
@@ -23,7 +23,7 @@ module.exports = class LinkRedirectRepository {
      */
     async save(linkRedirect) {
         const model = await this.#LinkRedirect.add({
-            // Only store the parthname (no support for variable query strings)
+            // Only store the pathname (no support for variable query strings)
             from: this.stripSubdirectoryFromPath(linkRedirect.from.pathname),
             to: linkRedirect.to.href
         }, {});
diff --git a/ghost/link-redirects/lib/LinkRedirectsService.js b/ghost/link-redirects/lib/LinkRedirectsService.js
index f40ba699dc..3b234c07c1 100644
--- a/ghost/link-redirects/lib/LinkRedirectsService.js
+++ b/ghost/link-redirects/lib/LinkRedirectsService.js
@@ -17,6 +17,9 @@ class LinkRedirectsService {
     /** @type URL */
     #baseURL;
 
+    /** @type String */
+    #redirectURLPrefix = 'r/';
+
     /**
      * @param {object} deps
      * @param {ILinkRedirectRepository} deps.linkRedirectRepository
@@ -43,7 +46,7 @@ class LinkRedirectsService {
         let url;
         while (!url || await this.#linkRedirectRepository.getByURL(url)) {
             const slug = crypto.randomBytes(4).toString('hex');
-            url = new URL(`r/${slug}`, this.#baseURL);
+            url = new URL(`${this.#redirectURLPrefix}${slug}`, this.#baseURL);
         }
         return url;
     }
@@ -82,6 +85,12 @@ class LinkRedirectsService {
      * @returns {Promise<void>}
      */
     async handleRequest(req, res, next) {
+        // skip handling if original url doesn't match the prefix
+        const fullURLWithRedirectPrefix = `${this.#baseURL.pathname}${this.#redirectURLPrefix}`;
+        if (!req.originalUrl.startsWith(fullURLWithRedirectPrefix)) {
+            return next();
+        }
+
         const url = new URL(req.originalUrl, this.#baseURL);
         const link = await this.#linkRedirectRepository.getByURL(url);
 
diff --git a/ghost/link-redirects/test/LinkRedirectsService.test.js b/ghost/link-redirects/test/LinkRedirectsService.test.js
index 412583b11d..ecb0aacfe7 100644
--- a/ghost/link-redirects/test/LinkRedirectsService.test.js
+++ b/ghost/link-redirects/test/LinkRedirectsService.test.js
@@ -1,4 +1,4 @@
-const LinkRedirectsService = require('../lib/LinkRedirectsService');
+const {LinkRedirectsService} = require('../index');
 const assert = require('assert');
 const sinon = require('sinon');
 const crypto = require('crypto');
@@ -73,7 +73,7 @@ describe('LinkRedirectsService', function () {
         it('redirects if found', async function () {
             const linkRedirectRepository = {
                 getByURL: (url) => {
-                    if (url.toString() === 'https://localhost:2368/a') {
+                    if (url.toString() === 'https://localhost:2368/r/a') {
                         return Promise.resolve({
                             to: new URL('https://localhost:2368/b')
                         });
@@ -88,7 +88,7 @@ describe('LinkRedirectsService', function () {
                 }
             });
             const req = {
-                originalUrl: '/a'
+                originalUrl: '/r/a'
             };
             const res = {
                 redirect: sinon.fake(),
@@ -111,12 +111,46 @@ describe('LinkRedirectsService', function () {
                 }
             });
             const req = {
-                url: '/a'
+                originalUrl: 'r/a'
             };
             const res = {};
             const next = sinon.fake();
             await instance.handleRequest(req, res, next);
             assert.equal(next.callCount, 1);
         });
+
+        it('does not redirect if url does not contain a redirect prefix on site with no subdir', async function () {
+            const instance = new LinkRedirectsService({
+                config: {
+                    baseURL: new URL('https://localhost:2368/')
+                }
+            });
+            const req = {
+                originalUrl: 'no_r/prefix'
+            };
+            const res = {};
+            const next = sinon.fake();
+
+            await instance.handleRequest(req, res, next);
+
+            assert.equal(next.callCount, 1);
+        });
+
+        it('does not redirect if url does not contain a redirect prefix on site with subdir', async function () {
+            const instance = new LinkRedirectsService({
+                config: {
+                    baseURL: new URL('https://localhost:2368/blog')
+                }
+            });
+            const req = {
+                originalUrl: 'blog/no_r/prefix'
+            };
+            const res = {};
+            const next = sinon.fake();
+
+            await instance.handleRequest(req, res, next);
+
+            assert.equal(next.callCount, 1);
+        });
     });
 });