[sitecore-jss-nextjs]: Redirects with double and more parameters has …

…been fixed for netlify #SXA-7554 (#1935)

CHANGELOG.md

@@ -9,6 +9,12 @@ Our versioning strategy is as follows:
 - Minor: may include breaking changes in framework packages (e.g. framework upgrades, new features, improvements)
 - Major: may include breaking changes in core packages (e.g. major architectural changes, major features)
 
+## 22.1.4
+
+### 🐛 Bug Fixes
+
+* `[sitecore-jss-nextjs]` `[sitecore-jss]` Resolved an issue with Netlify where URL query parameters were being sorted, causing redirect failures. Added a method to generate all possible permutations of query parameters, ensuring proper matching with URL patterns regardless of their order. ([#1935](https://github.com/Sitecore/jss/pull/1935))
+
 ## 22.1.3
 
 ### 🐛 Bug Fixes

packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts

@@ -1490,6 +1490,67 @@ describe('RedirectsMiddleware', () => {
       expect(finalRes.status).to.equal(expected.status);
     });
 
+    it('should return 301 redirect when queryString is ordered by alphabetic(Netlify feature)', async () => {
+      const setCookies = () => {};
+      const res = createResponse({
+        url: 'http://localhost:3000/found/',
+        status: 301,
+        setCookies,
+      });
+      const nextRedirectStub = sinon.stub(NextResponse, 'redirect').callsFake((url, init) => {
+        const status = typeof init === 'number' ? init : init?.status || 307;
+        return ({
+          url,
+          status,
+          cookies: { set: setCookies },
+          headers: res.headers,
+        } as unknown) as NextResponse;
+      });
+      const req = createRequest({
+        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() {
+            return Object.assign({}, req.nextUrl);
+          },
+        },
+      });
+
+      const { middleware, fetchRedirects, siteResolver } = createMiddleware({
+        pattern: '/not-found?w=1&a=1',
+        target: 'http://localhost:3000/found/',
+        redirectType: REDIRECT_TYPE_301,
+        isQueryStringPreserved: true,
+        locale: 'en',
+      });
+
+      const finalRes = await middleware.getHandler()(req);
+
+      validateDebugLog('redirects middleware start: %o', {
+        hostname: 'foo.net',
+        language: 'en',
+        pathname: '/not-found/',
+      });
+
+      validateEndMessageDebugLog('redirects middleware end in %dms: %o', {
+        headers: {},
+        redirected: undefined,
+        status: 301,
+        url: 'http://localhost:3000/found/',
+      });
+
+      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);
+
+      nextRedirectStub.restore();
+    });
+
     describe('should redirect to normalized path when nextjs specific "path" query string parameter is provided', () => {
       it('should return 301 redirect', async () => {
         const setCookies = () => {};

packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts

@@ -8,6 +8,7 @@ import {
   RedirectInfo,
   SiteInfo,
 } from '@sitecore-jss/sitecore-jss/site';
+import { getPermutations } from '@sitecore-jss/sitecore-jss/utils';
 import { NextRequest, NextResponse } from 'next/server';
 import regexParser from 'regex-parser';
 import { MiddlewareBase, MiddlewareBaseConfig } from './middleware';
@@ -117,7 +118,7 @@ export class RedirectsMiddleware extends MiddlewareBase {
       if (REGEXP_ABSOLUTE_URL.test(existsRedirect.target)) {
         url.href = existsRedirect.target;
       } else {
-        const source = `${url.pathname.replace(/\/*$/gi, '')}${url.search}`;
+        const source = `${url.pathname.replace(/\/*$/gi, '')}${existsRedirect.matchedQueryString}`;
         const urlFirstPart = existsRedirect.target.split('/')[1];
 
         if (this.locales.includes(urlFirstPart)) {
@@ -183,7 +184,7 @@ export class RedirectsMiddleware extends MiddlewareBase {
   private async getExistsRedirect(
     req: NextRequest,
     siteName: string
-  ): Promise<RedirectInfo | undefined> {
+  ): Promise<(RedirectInfo & { matchedQueryString?: string }) | undefined> {
     const redirects = await this.redirectsService.fetchRedirects(siteName);
     const normalizedUrl = this.normalizeUrl(req.nextUrl.clone());
     const tragetURL = normalizedUrl.pathname;
@@ -192,22 +193,58 @@ export class RedirectsMiddleware extends MiddlewareBase {
     const modifyRedirects = structuredClone(redirects);
 
     return modifyRedirects.length
-      ? modifyRedirects.find((redirect: RedirectInfo) => {
+      ? 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'), '');
-          redirect.pattern = `/^\/${redirect.pattern
-            .replace(/^\/|\/$/g, '')
-            .replace(/^\^\/|\/\$$/g, '')
-            .replace(/^\^|\$$/g, '')
-            .replace(/(?<!\\)\?/g, '\\?')
-            .replace(/\$\/gi$/g, '')}[\/]?$/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
+
+          /**
+           * This line checks whether the current URL query string (and all its possible permutations)
+           * matches the redirect pattern.
+           *
+           * Query parameters in URLs can come in different orders, but logically they represent the
+           * same information (e.g., "key1=value1&key2=value2" is the same as "key2=value2&key1=value1").
+           * To account for this, the method `isPermutedQueryMatch` generates all possible permutations
+           * of the query parameters and checks if any of those permutations match the regex pattern for the redirect.
+           *
+           * NOTE: This fix is specifically implemented for Netlify, where query parameters are sometimes
+           * automatically sorted, which can cause issues with matching redirects if the order of query
+           * parameters is important. By checking every possible permutation, we ensure that redirects
+           * work correctly on Netlify despite this behavior.
+           *
+           * It passes several pieces of information to the function:
+           * 1. `pathname`: The normalized URL path without query parameters (e.g., '/about').
+           * 2. `queryString`: The current query string from the URL, which will be permuted and matched (e.g., '?key1=value1&key2=value2').
+           * 3. `pattern`: The regex pattern for the redirect that we are trying to match against the URL (e.g., '/about?key1=value1').
+           * 4. `locale`: The locale part of the URL (if any), which helps support multilingual URLs.
+           *
+           * If one of the permutations of the query string matches the redirect pattern, the function
+           * returns the matched query string, which is stored in `matchedQueryString`. If no match is found,
+           * it returns `undefined`. The `matchedQueryString` is later used to indicate whether the query
+           * string contributed to a successful redirect match.
+           */
+          const matchedQueryString = this.isPermutedQueryMatch({
+            pathname: tragetURL,
+            queryString: targetQS,
+            pattern: redirect.pattern,
+            locale: req.nextUrl.locale,
+          });
+
+          // Save the matched query string (if found) into the redirect object
+          redirect.matchedQueryString = matchedQueryString || '';
+
+          // Return the redirect if the URL path or any query string permutation matches the pattern
           return (
             (regexParser(redirect.pattern).test(tragetURL) ||
-              regexParser(redirect.pattern).test(`${tragetURL.replace(/\/*$/gi, '')}${targetQS}`) ||
               regexParser(redirect.pattern).test(`/${req.nextUrl.locale}${tragetURL}`) ||
-              regexParser(redirect.pattern).test(
-                `/${req.nextUrl.locale}${tragetURL}${targetQS}`
-              )) &&
+              matchedQueryString) &&
             (redirect.locale
               ? redirect.locale.toLowerCase() === req.nextUrl.locale.toLowerCase()
               : true)
@@ -285,4 +322,40 @@ export class RedirectsMiddleware extends MiddlewareBase {
     }
     return redirect;
   }
+
+  /**
+   * Checks if the current URL query matches the provided pattern, considering all permutations of query parameters.
+   * It constructs all possible query parameter permutations and tests them against the pattern.
+   * @param {Object} params - The parameters for the URL match.
+   * @param {string} params.pathname - The current URL pathname.
+   * @param {string} params.queryString - The current URL query string.
+   * @param {string} params.pattern - The regex pattern to test the constructed URLs against.
+   * @param {string} [params.locale] - The locale prefix to include in the URL if present.
+   * @returns {string | undefined} - return query string if any of the query permutations match the provided pattern, undefined otherwise.
+   */
+  private isPermutedQueryMatch({
+    pathname,
+    queryString,
+    pattern,
+    locale,
+  }: {
+    pathname: string;
+    queryString: string;
+    pattern: string;
+    locale?: string;
+  }): string | undefined {
+    const paramsArray = Array.from(new URLSearchParams(queryString).entries());
+    const listOfPermuted = getPermutations(paramsArray).map(
+      (permutation: [string, string][]) =>
+        '?' + permutation.map(([key, value]) => `${key}=${value}`).join('&')
+    );
+
+    const normalizedPath = pathname.replace(/\/*$/gi, '');
+    return listOfPermuted.find((query: string) =>
+      [
+        regexParser(pattern).test(`${normalizedPath}${query}`),
+        regexParser(pattern).test(`/${locale}${normalizedPath}${query}`),
+      ].some(Boolean)
+    );
+  }
 }

packages/sitecore-jss/src/utils/index.ts

@@ -4,6 +4,7 @@ export {
   isAbsoluteUrl,
   isTimeoutError,
   enforceCors,
+  getPermutations,
   EnhancedOmit,
   getAllowedOriginsFromEnv,
 } from './utils';

packages/sitecore-jss/src/utils/utils.test.ts

@@ -1,8 +1,14 @@
 /* eslint-disable no-unused-expressions */
 import { expect, spy } from 'chai';
-import { isServer, resolveUrl } from '.';
-import { enforceCors, getAllowedOriginsFromEnv, isAbsoluteUrl, isTimeoutError } from './utils';
 import { IncomingMessage, OutgoingMessage } from 'http';
+import { isServer, resolveUrl } from '.';
+import {
+  enforceCors,
+  getAllowedOriginsFromEnv,
+  getPermutations,
+  isAbsoluteUrl,
+  isTimeoutError,
+} from './utils';
 
 // must make TypeScript happy with `global` variable modification
 interface CustomWindow {
@@ -203,4 +209,36 @@ describe('utils', () => {
       delete process.env.JSS_ALLOWED_ORIGINS;
     });
   });
+
+  describe('getPermutations', () => {
+    it('should return all permutations of an array with one pair', () => {
+      const input: [string, string][] = [['key1', 'value1']];
+      const expectedOutput = [[['key1', 'value1']]];
+      expect(getPermutations(input)).to.deep.equal(expectedOutput);
+    });
+
+    it('should return all permutations of an array with two pairs', () => {
+      const input: [string, string][] = [
+        ['key1', 'value1'],
+        ['key2', 'value2'],
+      ];
+      const expectedOutput = [
+        [
+          ['key1', 'value1'],
+          ['key2', 'value2'],
+        ],
+        [
+          ['key2', 'value2'],
+          ['key1', 'value1'],
+        ],
+      ];
+      expect(getPermutations(input)).to.deep.equal(expectedOutput);
+    });
+
+    it('should return an empty array when input is empty', () => {
+      const input: [string, string][] = [];
+      const expectedOutput = [[]];
+      expect(getPermutations(input)).to.deep.equal(expectedOutput);
+    });
+  });
 });

packages/sitecore-jss/src/utils/utils.ts

@@ -1,8 +1,8 @@
-import isServer from './is-server';
-import { ParsedUrlQueryInput } from 'querystring';
 import { AxiosError } from 'axios';
-import { ResponseError } from '../data-fetcher';
 import { IncomingMessage, OutgoingMessage } from 'http';
+import { ParsedUrlQueryInput } from 'querystring';
+import { ResponseError } from '../data-fetcher';
+import isServer from './is-server';
 
 /**
  * Omit properties from T that are in K. This is a simplified version of TypeScript's built-in `Omit` utility type.
@@ -153,3 +153,18 @@ export const enforceCors = (
   }
   return false;
 };
+
+  /**
+   * Generates all possible permutations of an array of key-value pairs.
+   * This is used to create every possible combination of URL query parameters.
+   * @param {Array<[string, string]>} array - The array of key-value pairs to permute.
+   * @returns {Array<Array<[string, string]>>} - A 2D array where each inner array is a unique permutation of the input.
+   */
+ export const getPermutations = (array: [string, string][]): [string, string][][] =>{
+  if (array.length <= 1) return [array];
+
+  return array.flatMap((current, i) => {
+    const remaining = array.filter((_, idx) => idx !== i);
+    return getPermutations(remaining).map((permutation) => [current, ...permutation]);
+  });
+};