From f6988ad0d358c7e519f0adeaa7bde8bca5bbc661 Mon Sep 17 00:00:00 2001 From: Ruslan Matkovskyi Date: Thu, 15 Aug 2024 15:27:56 +0200 Subject: [PATCH] [sitecore-jss-nextjs]: the problem with query string in target url has been fixed --- .../middleware/redirects-middleware.test.ts | 83 ++++++++++++++++++- .../src/middleware/redirects-middleware.ts | 46 +++++----- 2 files changed, 99 insertions(+), 30 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 b420b3f25e..79d57d0d44 100644 --- a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts +++ b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts @@ -372,6 +372,7 @@ describe('RedirectsMiddleware', () => { const req = createRequest({ nextUrl: { pathname: '/not-found', + origin: 'http://localhost:3000', locale: 'en', href: 'http://localhost:3000/not-found', clone() { @@ -421,6 +422,7 @@ describe('RedirectsMiddleware', () => { url: { pathname: 'http://localhost:3000/ua/found', href: 'http://localhost:3000/not-found', + origin: 'http://localhost:3000', locale: 'en', clone: cloneUrl, }, @@ -440,6 +442,7 @@ describe('RedirectsMiddleware', () => { nextUrl: { pathname: '/not-found', href: 'http://localhost:3000/not-found', + origin: 'http://localhost:3000', locale: 'en', clone: cloneUrl, }, @@ -470,6 +473,7 @@ describe('RedirectsMiddleware', () => { url: { pathname: 'http://localhost:3000/ua/found', href: 'http://localhost:3000/not-found', + origin: 'http://localhost:3000', locale: 'en', clone: cloneUrl, }, @@ -581,6 +585,7 @@ describe('RedirectsMiddleware', () => { search: '?abc=def', href: 'http://localhost:3000/not-found?abc=def', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, @@ -661,6 +666,67 @@ describe('RedirectsMiddleware', () => { expect(finalRes).to.deep.equal(res); }); + it('should redirect, when target uses query string', async () => { + const setCookies = () => {}; + const res = createResponse({ + url: 'http://localhost:3000/found?abc=def', + 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: '?abc=def', + href: 'http://localhost:3000/not-found?abc=def', + locale: 'en', + origin: 'http://localhost:3000', + clone() { + return Object.assign({}, req.nextUrl); + }, + }, + }); + + const { middleware, fetchRedirects, siteResolver } = createMiddleware({ + pattern: 'not-found', + target: 'http://localhost:3000/found?abc=def', + redirectType: REDIRECT_TYPE_301, + isQueryStringPreserved: false, + 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?abc=def', + }); + + 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(); + }); + xit('should redirect uses token in target', async () => { const setCookies = () => {}; const res = createResponse({ @@ -742,6 +808,7 @@ describe('RedirectsMiddleware', () => { pathname: '/not-found', href: 'http://localhost:3000/not-found', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, @@ -802,6 +869,7 @@ describe('RedirectsMiddleware', () => { search: 'abc=def', href: 'http://localhost:3000/not-found', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, @@ -971,6 +1039,7 @@ describe('RedirectsMiddleware', () => { href: 'http://localhost:3000/not-found', pathname: '/not-found', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, @@ -1137,6 +1206,7 @@ describe('RedirectsMiddleware', () => { pathname: '/not-found', href: 'http://localhost:3000/not-found', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, @@ -1199,6 +1269,7 @@ describe('RedirectsMiddleware', () => { pathname: '/not-found', href: 'http://localhost:3000/not-found', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, @@ -1258,6 +1329,7 @@ describe('RedirectsMiddleware', () => { pathname: '/not-found/', href: 'http://localhost:3000/not-found/', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, @@ -1317,9 +1389,11 @@ describe('RedirectsMiddleware', () => { const req = createRequest({ nextUrl: { - pathname: '/not-found?path=not-found', - href: 'http://localhost:3000/not-found?path=not-found', + pathname: '/not-found', + search: '?path=not-found', + href: 'http://localhost:3000/not-found/?path=not-found', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, @@ -1327,7 +1401,7 @@ describe('RedirectsMiddleware', () => { }); const { middleware, fetchRedirects, siteResolver } = createMiddleware({ - pattern: 'not-found', + pattern: '/not-found', target: 'http://localhost:3000/found', redirectType: REDIRECT_TYPE_301, isQueryStringPreserved: false, @@ -1339,7 +1413,7 @@ describe('RedirectsMiddleware', () => { validateDebugLog('redirects middleware start: %o', { hostname: 'foo.net', language: 'en', - pathname: '/not-found?path=not-found', + pathname: '/not-found', }); validateEndMessageDebugLog('redirects middleware end in %dms: %o', { @@ -1380,6 +1454,7 @@ describe('RedirectsMiddleware', () => { search: '?path=not-found', href: 'http://localhost:3000/not-found/?path=not-found', locale: 'en', + origin: 'http://localhost:3000', clone() { return Object.assign({}, req.nextUrl); }, diff --git a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts index a9fded0213..0560bfc128 100644 --- a/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts +++ b/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts @@ -112,16 +112,16 @@ export class RedirectsMiddleware extends MiddlewareBase { ); } - const url = req.nextUrl.clone(); + const url = this.normalizeUrl(req.nextUrl.clone()); if (REGEXP_ABSOLUTE_URL.test(existsRedirect.target)) { url.href = existsRedirect.target; } else { - const source = this.normalizeUrl(url.pathname, url.search); + const source = `${url.pathname.replace(/\/*$/gi, '')}${url.search}`; const urlFirstPart = existsRedirect.target.split('/')[1]; if (this.locales.includes(urlFirstPart)) { - url.locale = urlFirstPart; + req.nextUrl.locale = urlFirstPart; existsRedirect.target = existsRedirect.target.replace(`/${urlFirstPart}`, ''); } @@ -130,17 +130,15 @@ export class RedirectsMiddleware extends MiddlewareBase { .replace(/^\/\//, '/') .split('?'); - url.pathname = `${target[0]}`; - if (target[1]) { - url.search = `?${target[1]}`; + const movedSearchParams = existsRedirect.isQueryStringPreserved + ? url.search.replace(/^\?/gi, '&') + : ''; + url.search = '?' + new URLSearchParams(`${target[1]}${movedSearchParams}`).toString(); } - const newURL = new URL( - `${url.pathname}${existsRedirect.isQueryStringPreserved ? url.search : ''}`, - url.origin - ); - url.href = newURL.href; + const prepareNewURL = new URL(`${target[0]}${url.search}`, url.origin); + url.href = prepareNewURL.href; } const redirectUrl = decodeURIComponent(url.href); @@ -193,10 +191,7 @@ export class RedirectsMiddleware extends MiddlewareBase { siteName: string ): Promise { const redirects = await this.redirectsService.fetchRedirects(siteName); - const normalizedUrl = new URL( - this.normalizeUrl(req.nextUrl.pathname, req.nextUrl.search || ''), - req.nextUrl.href - ); + const normalizedUrl = this.normalizeUrl(req.nextUrl.clone()); const tragetURL = normalizedUrl.pathname; const targetQS = normalizedUrl.search || ''; const language = this.getLanguage(req); @@ -214,7 +209,7 @@ export class RedirectsMiddleware extends MiddlewareBase { return ( (regexParser(redirect.pattern).test(tragetURL) || - regexParser(redirect.pattern).test(`${tragetURL}${targetQS}`) || + 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}` @@ -231,19 +226,18 @@ export class RedirectsMiddleware extends MiddlewareBase { * When a user clicks on a link generated by the Link component from next/link, * Next.js adds special parameters in the route called path. * This method removes these special parameters. - * @param {string} pathname - * @param {string} queryString - * @returns {string} modified url + * @param {URL} url + * @returns {string} normalize url */ - private normalizeUrl(pathname: string, queryString: string) { - if (!queryString) { - return pathname; + private normalizeUrl(url: URL): URL { + if (!url.search) { + return url; } /** * Prepare special parameters for exclusion. */ - const splittedPathname = pathname + const splittedPathname = url.pathname .split('/') .filter((route) => route) .map((route) => `path=${route}`); @@ -254,7 +248,7 @@ export class RedirectsMiddleware extends MiddlewareBase { * When a user clicks on this link, Next.js should generate a link for the middleware, formatted like this: * http://host/about/contact/us?path=about&path=contact&path=us */ - const newQueryString = queryString + const newQueryString = url.search .replace(/^\?/, '') .split('&') .filter((param) => { @@ -266,9 +260,9 @@ export class RedirectsMiddleware extends MiddlewareBase { .join('&'); if (newQueryString) { - return `${pathname}?${newQueryString}`; + return new URL(`${url.pathname}?${newQueryString}`, url.origin); } - return pathname; + return new URL(`${url.pathname}`, url.origin); } }