fix: redirect to 301 instead of 302 in case when only trailing slashe…

…s are present
namecheap · Oct 24, 2024 · 3420279 · 3420279
1 parent b542240
commit 3420279
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 36 deletions.
diff --git a/ilc/server/app.spec.js b/ilc/server/app.spec.js
@@ -4,24 +4,34 @@ const nock = require('nock');
 const config = require('config');
 const helpers = require('../tests/helpers');
 const { context } = require('./context/context');
-
 const createApp = require('./app');
 const sinon = require('sinon');
 
+async function createTestServer(mockRegistryOptions = {}, mockPluginOptions = {}) {
+    const app = createApp(
+        helpers.getRegistryMock(mockRegistryOptions),
+        helpers.getPluginManagerMock(mockPluginOptions),
+        context,
+    );
+
+    await app.ready();
+    app.server.listen(0);
+
+    const { port } = app.server.address();
+    const address = `127.0.0.1:${port}`;
+    const server = supertest(app.server);
+
+    return { app, server, address };
+}
+
 describe('App', () => {
     let app;
     let server;
-    let address;
 
     before(async () => {
-        app = createApp(helpers.getRegistryMock(), helpers.getPluginManagerMock(), context);
-        await app.ready();
-        app.server.listen(0);
-
-        const { port } = app.server.address();
-        address = `127.0.0.1:${port}`;
-
-        server = supertest(app.server);
+        const serverInstance = await createTestServer();
+        app = serverInstance.app;
+        server = serverInstance.server;
     });
 
     after(() => {
@@ -43,4 +53,32 @@ describe('App', () => {
     it('should parse "invalid" urls', async () => {
         await server.get('///').expect(200);
     });
+
+    describe('Redirect with trailing slash', () => {
+        let app;
+        let server;
+
+        beforeEach(async () => {
+            const serverInstance = await createTestServer({
+                settings: {
+                    trailingSlash: 'redirectToTrailingSlash',
+                },
+            });
+
+            app = serverInstance.app;
+            server = serverInstance.server;
+        });
+
+        afterEach(() => {
+            if (app && app.server && app.server.listening) {
+                app.server.close();
+            }
+        });
+
+        it('should reply with 301 in case of redirect /someRoute -> /someRoute/', async () => {
+            const response = await server.get('/someRoute').expect(301);
+
+            chai.expect(response.headers.location).to.be.eql('/someRoute/');
+        });
+    });
 });
diff --git a/ilc/server/i18n.js b/ilc/server/i18n.js
@@ -50,10 +50,6 @@ const onRequestFactory =
                 containsOnlySlashes(fixedUrl) &&
                 containsOnlySlashes(req.raw.url);
 
-            if (containsOnlySlashes(req.raw.url) && !shouldSkipRedirectForSlashes) {
-                return reply.redirect(301, '/');
-            }
-
             if (fixedUrl !== req.raw.url && !shouldSkipRedirectForSlashes) {
                 return reply.redirect(fixedUrl);
             }

diff --git a/ilc/server/i18n.spec.js b/ilc/server/i18n.spec.js
@@ -201,27 +201,6 @@ describe('i18n', () => {
                 sinon.assert.calledWithExactly(reply.redirect, '/ua/test');
             });
 
-            it('redirects trailing slash with 301 code', async () => {
-                const detectedI18nConfig = {
-                    locale: 'en-US',
-                    currency: 'USD',
-                };
-
-                i18nParamsDetectionPlugin.detectI18nConfig.onFirstCall().returns(detectedI18nConfig);
-
-                const req = getReqMock('///');
-
-                await onRequest(req, reply);
-
-                const [providedReqRaw, , providedI18nConfig] =
-                    i18nParamsDetectionPlugin.detectI18nConfig.getCalls()[0].args;
-
-                chai.expect(providedReqRaw).to.be.eql(req.raw);
-                chai.expect(providedI18nConfig).to.be.eql(i18nConfig.default);
-
-                sinon.assert.calledWithExactly(reply.redirect, 301, '/');
-            });
-
             it('ua-ua, redirects to URL with correct lang code', async () => {
                 const detectedI18nConfig = {
                     locale: 'ua-ua',

diff --git a/ilc/server/routes/wildcardRequestHandlerFactory.ts b/ilc/server/routes/wildcardRequestHandlerFactory.ts
@@ -43,8 +43,12 @@ export function wildcardRequestHandlerFactory(
         const url = req.raw.url;
         const urlProcessor = new UrlProcessor(registryConfig.settings.trailingSlash);
         const processedUrl = urlProcessor.process(url);
+
         if (processedUrl !== url) {
-            reply.redirect(processedUrl);
+            reply.redirect(
+                registryConfig.settings.trailingSlash === UrlProcessor.routerHasTo.redirectToTrailingSlash ? 301 : 302,
+                processedUrl,
+            );
             return;
         }