[sitecore-jss-nextjs]: Updated logic to properly handle cases like […

…/]? #SXA-6320 (#1999) * [sitecore-jss-nextjs]: Updated logic to properly handle cases like [/]? * CHANGELOG has been changed --------- Co-authored-by: Ruslan Matkovskyi <[email protected]>
Sitecore · Dec 16, 2024 · 20c3932 · 20c3932
1 parent 3b5b780
commit 20c3932
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,10 @@ Our versioning strategy is as follows:
 
 * `[templates/nextjs-sxa]` Fixed font-awesome import issue in custom workspace configuration ([#1998](https://github.com/Sitecore/jss/pull/1998))
 
+### 🐛 Bug Fixes
+
+* `[sitecore-jss-nextjs]` Fixed handling of ? inside square brackets [] in regex patterns to prevent incorrect escaping ([#1999](https://github.com/Sitecore/jss/pull/1999))
+
 ## 22.3.0 / 22.3.1
 
 ### 🐛 Bug Fixes

diff --git a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts
@@ -1411,6 +1411,57 @@ describe('RedirectsMiddleware', () => {
         expect(finalRes).to.deep.equal(res);
         expect(finalRes.status).to.equal(res.status);
       });
+
+      it('should return 301 redirect when pattern has special symbols "?"', async () => {
+        const cloneUrl = () => Object.assign({}, req.nextUrl);
+        const url = {
+          clone: cloneUrl,
+          href: 'http://localhost:3000/found?a=1&w=1',
+          locale: 'en',
+          origin: 'http://localhost:3000',
+          search: '?a=1&w=1',
+          pathname: '/found',
+        };
+
+        const { res, req } = createTestRequestResponse({
+          response: { url },
+          request: {
+            nextUrl: {
+              pathname: '/not-found/',
+              search: '?a=1&w=1',
+              href: 'http://localhost:3000/not-found/?a=1&w=1',
+              locale: 'en',
+              origin: 'http://localhost:3000',
+              clone: cloneUrl,
+            },
+          },
+        });
+        setupRedirectStub(301);
+
+        const { finalRes, fetchRedirects, siteResolver } = await runTestWithRedirect(
+          {
+            pattern: '/[/]?not-found?a=1&w=1/',
+            target: '/found',
+            redirectType: REDIRECT_TYPE_301,
+            isQueryStringPreserved: true,
+            locale: 'en',
+          },
+          req
+        );
+
+        validateEndMessageDebugLog('redirects middleware end in %dms: %o', {
+          headers: {},
+          redirected: undefined,
+          status: 301,
+          url,
+        });
+
+        expect(siteResolver.getByHost).to.be.calledWith(hostname);
+        // eslint-disable-next-line no-unused-expressions
+        expect(fetchRedirects.called).to.be.true;
+        expect(finalRes).to.deep.equal(res);
+        expect(finalRes.status).to.equal(res.status);
+      });
     });
 
     describe('should redirect to normalized path when nextjs specific "path" query string parameter is provided', () => {

diff --git a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts
@@ -199,14 +199,16 @@ export class RedirectsMiddleware extends MiddlewareBase {
     return modifyRedirects.length
       ? modifyRedirects.find((redirect: RedirectInfo & { matchedQueryString?: string }) => {
           // Modify the redirect pattern to ignore the language prefix in the path
-          redirect.pattern = redirect.pattern.replace(RegExp(`^[^]?/${language}/`, 'gi'), '');
+          // And escapes non-special "?" characters in a string or regex.
+          redirect.pattern = this.escapeNonSpecialQuestionMarks(
+            redirect.pattern.replace(RegExp(`^[^]?/${language}/`, 'gi'), '')
+          );
 
           // Prepare the redirect pattern as a regular expression, making it more flexible for matching URLs
           redirect.pattern = `/^\/${redirect.pattern
             .replace(/^\/|\/$/g, '') // Removes leading and trailing slashes
             .replace(/^\^\/|\/\$$/g, '') // Removes unnecessary start (^) and end ($) anchors
             .replace(/^\^|\$$/g, '') // Further cleans up anchors
-            .replace(/(?<!\\)\?/g, '\\?') // Escapes question marks in the pattern
             .replace(/\$\/gi$/g, '')}[\/]?$/i`; // Ensures the pattern allows an optional trailing slash
 
           /**
@@ -272,7 +274,7 @@ export class RedirectsMiddleware extends MiddlewareBase {
      */
     const splittedPathname = url.pathname
       .split('/')
-      .filter((route) => route)
+      .filter((route: string) => route)
       .map((route) => `path=${route}`);
 
     /**
@@ -362,4 +364,51 @@ export class RedirectsMiddleware extends MiddlewareBase {
       ].some(Boolean)
     );
   }
+
+  /**
+   * Escapes non-special "?" characters in a string or regex.
+   *
+   * - For regular strings, it escapes all unescaped "?" characters by adding a backslash (`\`).
+   * - For regex patterns (strings enclosed in `/.../`), it analyzes each "?" to determine if it has special meaning
+   *   (e.g., `?` in `(abc)?`, `.*?`) or is just a literal character. Only literal "?" characters are escaped.
+   * @param {string} input - The input string or regex pattern.
+   * @returns {string} - The modified string or regex with non-special "?" characters escaped.
+   **/
+  private escapeNonSpecialQuestionMarks(input: string): string {
+    const regexPattern = /(?<!\\)\?/g; // Find unescaped "?" characters
+    const isRegex = input.startsWith('/') && input.endsWith('/'); // Check if the string is a regex
+
+    if (!isRegex) {
+      // If not a regex, escape all unescaped "?" characters
+      return input.replace(regexPattern, '\\?');
+    }
+
+    // If it's a regex, analyze each "?" character
+    let result = '';
+    let lastIndex = 0;
+
+    let match;
+    while ((match = regexPattern.exec(input)) !== null) {
+      const index = match.index; // Position of "?" in the string
+      const before = input.slice(0, index).replace(/\s+$/, ''); // Context before "?"
+      const lastChar = before.slice(-1); // Last character before "?"
+
+      // Determine if the "?" is a special regex symbol
+      const isSpecialRegexSymbol = /[\.\*\+\)\[\]]$/.test(lastChar);
+
+      if (isSpecialRegexSymbol) {
+        // If it's special, keep it as is
+        result += input.slice(lastIndex, index + 1);
+      } else {
+        // If it's not special, escape it
+        result += input.slice(lastIndex, index) + '\\?';
+      }
+      lastIndex = index + 1;
+    }
+
+    // Append the remaining part of the string
+    result += input.slice(lastIndex);
+
+    return result;
+  }
 }