From 9c32bfe6549d3f7651ea9b0e1dd5dd704ae041ae Mon Sep 17 00:00:00 2001 From: Ruslan Matkovskyi Date: Thu, 26 Sep 2024 09:19:40 +0200 Subject: [PATCH 1/6] [sitecore-jss-nextjs]: Redirects with double and more parameters has been fixed for netlify #SXA-7554 --- .../src/middleware/redirects-middleware.ts | 60 +++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts index 86ad0d16dc..8c499fe5c7 100644 --- a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts +++ b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts @@ -203,11 +203,13 @@ export class RedirectsMiddleware extends MiddlewareBase { 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}` - )) && + this.isPermutedQueryMatch({ + pathname: tragetURL, + queryString: targetQS, + pattern: redirect.pattern, + locale: req.nextUrl.locale, + })) && (redirect.locale ? redirect.locale.toLowerCase() === req.nextUrl.locale.toLowerCase() : true) @@ -285,4 +287,54 @@ export class RedirectsMiddleware extends MiddlewareBase { } return redirect; } + + /** + * 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>} - A 2D array where each inner array is a unique permutation of the input. + */ + private 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 this.getPermutations(remaining).map((permutation) => [current, ...permutation]); + }); + } + + /** + * 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 {boolean} - True if any of the query permutations match the provided pattern, false otherwise. + */ + private isPermutedQueryMatch({ + pathname, + queryString, + pattern, + locale, + }: { + pathname: string; + queryString: string; + pattern: string; + locale?: string; + }): boolean { + const paramsArray = Array.from(new URLSearchParams(queryString).entries()); + const listOfPermuted = this.getPermutations(paramsArray).map( + (permutation) => '?' + permutation.map(([key, value]) => `${key}=${value}`).join('&') + ); + + const normalizedPath = pathname.replace(/\/*$/gi, ''); + return listOfPermuted.some((query) => + [ + regexParser(pattern).test(`${normalizedPath}${query}`), + regexParser(pattern).test(`/${locale}${normalizedPath}${query}`), + ].some(Boolean) + ); + } } From 82af114a8a921fafca74e83b6a97eea5c02eea83 Mon Sep 17 00:00:00 2001 From: Ruslan Matkovskyi Date: Thu, 26 Sep 2024 13:37:19 +0200 Subject: [PATCH 2/6] [sitecore-jss-nextjs]: Unit test has been added --- .../middleware/redirects-middleware.test.ts | 61 +++++++++++++++++++ .../src/middleware/redirects-middleware.ts | 28 +++++---- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts index d917059805..a018602e81 100644 --- a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts +++ b/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 = () => {}; diff --git a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts index 8c499fe5c7..64e4f34b85 100644 --- a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts +++ b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts @@ -117,7 +117,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 +183,7 @@ export class RedirectsMiddleware extends MiddlewareBase { private async getExistsRedirect( req: NextRequest, siteName: string - ): Promise { + ): Promise<(RedirectInfo & { matchedQueryString?: string }) | undefined> { const redirects = await this.redirectsService.fetchRedirects(siteName); const normalizedUrl = this.normalizeUrl(req.nextUrl.clone()); const tragetURL = normalizedUrl.pathname; @@ -192,24 +192,26 @@ export class RedirectsMiddleware extends MiddlewareBase { const modifyRedirects = structuredClone(redirects); return modifyRedirects.length - ? modifyRedirects.find((redirect: RedirectInfo) => { + ? modifyRedirects.find((redirect: RedirectInfo & { matchedQueryString?: string }) => { redirect.pattern = redirect.pattern.replace(RegExp(`^[^]?/${language}/`, 'gi'), ''); redirect.pattern = `/^\/${redirect.pattern .replace(/^\/|\/$/g, '') .replace(/^\^\/|\/\$$/g, '') .replace(/^\^|\$$/g, '') .replace(/(? '?' + permutation.map(([key, value]) => `${key}=${value}`).join('&') ); const normalizedPath = pathname.replace(/\/*$/gi, ''); - return listOfPermuted.some((query) => + return listOfPermuted.find((query) => [ regexParser(pattern).test(`${normalizedPath}${query}`), regexParser(pattern).test(`/${locale}${normalizedPath}${query}`), From fbd602302f8386c02123514759a3f9778e37c9fe Mon Sep 17 00:00:00 2001 From: Ruslan Matkovskyi Date: Thu, 26 Sep 2024 13:43:07 +0200 Subject: [PATCH 3/6] CHANGELOG has been changed --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index beaa53643f..380aafe26f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Our versioning strategy is as follows: * `[templates/nextjs]` `[templates/react]` `[templates/angular]` `[templates/vue]` Fixed an issue when environment variable is undefined (not present in .env), that produced an "undefined" value in generated config file ([#1875](https://github.com/Sitecore/jss/pull/1875)) * `[templates/nextjs]` Fix embedded personalization not rendering correctly after navigation through router links. ([#1911](https://github.com/Sitecore/jss/pull/1911)) * `[template/angular]` Prevent client-side dictionary API call when SSR data is available ([#1930](https://github.com/Sitecore/jss/pull/1930)) ([#1932](https://github.com/Sitecore/jss/pull/1932)) +* `[sitecore-jss-nextjs]` 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)) ### 🎉 New Features & Improvements From ec6513ea749927dbce2e058104f1b9038887b71b Mon Sep 17 00:00:00 2001 From: Ruslan Matkovskyi Date: Mon, 7 Oct 2024 01:21:30 +0200 Subject: [PATCH 4/6] [sitecore-jss-nextjs/sitecore-jss]: the method getPermutations has been moved Also the comments has been added --- .../src/middleware/redirects-middleware.ts | 65 ++++++++++++------- packages/sitecore-jss/src/utils/index.ts | 1 + packages/sitecore-jss/src/utils/utils.ts | 15 +++++ 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts index 64e4f34b85..a55d781c93 100644 --- a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts +++ b/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'; @@ -193,21 +194,53 @@ 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'), ''); + + // Prepare the redirect pattern as a regular expression, making it more flexible for matching URLs redirect.pattern = `/^\/${redirect.pattern - .replace(/^\/|\/$/g, '') - .replace(/^\^\/|\/\$$/g, '') - .replace(/^\^|\$$/g, '') - .replace(/(?} array - The array of key-value pairs to permute. - * @returns {Array>} - A 2D array where each inner array is a unique permutation of the input. - */ - private 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 this.getPermutations(remaining).map((permutation) => [current, ...permutation]); - }); - } - /** * 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. @@ -327,12 +345,13 @@ export class RedirectsMiddleware extends MiddlewareBase { locale?: string; }): string | undefined { const paramsArray = Array.from(new URLSearchParams(queryString).entries()); - const listOfPermuted = this.getPermutations(paramsArray).map( - (permutation) => '?' + permutation.map(([key, value]) => `${key}=${value}`).join('&') + const listOfPermuted = getPermutations(paramsArray).map( + (permutation: [string, string][]) => + '?' + permutation.map(([key, value]) => `${key}=${value}`).join('&') ); const normalizedPath = pathname.replace(/\/*$/gi, ''); - return listOfPermuted.find((query) => + return listOfPermuted.find((query: string) => [ regexParser(pattern).test(`${normalizedPath}${query}`), regexParser(pattern).test(`/${locale}${normalizedPath}${query}`), diff --git a/packages/sitecore-jss/src/utils/index.ts b/packages/sitecore-jss/src/utils/index.ts index 1ae53ac897..921cd33d20 100644 --- a/packages/sitecore-jss/src/utils/index.ts +++ b/packages/sitecore-jss/src/utils/index.ts @@ -4,6 +4,7 @@ export { isAbsoluteUrl, isTimeoutError, enforceCors, + getPermutations, EnhancedOmit, getAllowedOriginsFromEnv, } from './utils'; diff --git a/packages/sitecore-jss/src/utils/utils.ts b/packages/sitecore-jss/src/utils/utils.ts index 1796f123f2..f9104b8458 100644 --- a/packages/sitecore-jss/src/utils/utils.ts +++ b/packages/sitecore-jss/src/utils/utils.ts @@ -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>} - 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]); + }); +}; From 9e6ae1ad716d76e7c94252200dbf04390ed1972a Mon Sep 17 00:00:00 2001 From: Ruslan Matkovskyi Date: Mon, 7 Oct 2024 01:23:11 +0200 Subject: [PATCH 5/6] CHANGELOG has been changed --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 380aafe26f..ea5bcd273b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ Our versioning strategy is as follows: * `[templates/nextjs]` `[templates/react]` `[templates/angular]` `[templates/vue]` Fixed an issue when environment variable is undefined (not present in .env), that produced an "undefined" value in generated config file ([#1875](https://github.com/Sitecore/jss/pull/1875)) * `[templates/nextjs]` Fix embedded personalization not rendering correctly after navigation through router links. ([#1911](https://github.com/Sitecore/jss/pull/1911)) * `[template/angular]` Prevent client-side dictionary API call when SSR data is available ([#1930](https://github.com/Sitecore/jss/pull/1930)) ([#1932](https://github.com/Sitecore/jss/pull/1932)) -* `[sitecore-jss-nextjs]` 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)) +* `[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)) ### 🎉 New Features & Improvements From b7ec703ad3749b409b8790d7f712b95b5c7b4b38 Mon Sep 17 00:00:00 2001 From: Ruslan Matkovskyi Date: Tue, 8 Oct 2024 11:05:48 +0200 Subject: [PATCH 6/6] [sitecore-jss]: Unit test has been added for getPermutations function --- packages/sitecore-jss/src/utils/utils.test.ts | 42 ++++++++++++++++++- packages/sitecore-jss/src/utils/utils.ts | 6 +-- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/packages/sitecore-jss/src/utils/utils.test.ts b/packages/sitecore-jss/src/utils/utils.test.ts index dff18894fe..02579a48ae 100644 --- a/packages/sitecore-jss/src/utils/utils.test.ts +++ b/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); + }); + }); }); diff --git a/packages/sitecore-jss/src/utils/utils.ts b/packages/sitecore-jss/src/utils/utils.ts index f9104b8458..dc057957e5 100644 --- a/packages/sitecore-jss/src/utils/utils.ts +++ b/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.