From ad89265c0152fa445ea92acb8e0409ca04e31696 Mon Sep 17 00:00:00 2001 From: Karol Manijak Date: Sun, 19 Mar 2023 21:44:31 +0100 Subject: [PATCH 1/5] Replace single quote with the encoded version %27 for URL comparison This is required as as removeQueryArgs() function uses decodeURIcomponent method which doesn't encode single quotes (') while it was still encoded in the original URL (%27). So when the single quote was in a query param, for example as a search term, it caused endless redirection loop. --- assets/js/blocks/stock-filter/block.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/assets/js/blocks/stock-filter/block.tsx b/assets/js/blocks/stock-filter/block.tsx index 37d8dda9da9..ef365615bc7 100644 --- a/assets/js/blocks/stock-filter/block.tsx +++ b/assets/js/blocks/stock-filter/block.tsx @@ -219,10 +219,17 @@ const StockStatusFilterBlock = ( { return; } if ( checkedOptions.length === 0 ) { + /** + * .replace() was added as removeQueryArgs() function uses decodeURIcomponent method + * which doesn't encode single quotes (') while it was still encoded in the original URL (%27). + * So when the single quote was in a query param, for example as a search term, it caused + * endless redirection loop. + * More context: https://github.com/woocommerce/woocommerce-blocks/issues/8707 + */ const url = removeQueryArgs( window.location.href, QUERY_PARAM_KEY - ); + ).replace( /'/g, '%27' ); if ( url !== window.location.href ) { changeUrl( url ); From e042bb8ecd9d6a6004fab8f3cc5fe509ce38296d Mon Sep 17 00:00:00 2001 From: Karol Manijak Date: Mon, 20 Mar 2023 11:08:41 +0100 Subject: [PATCH 2/5] Replace single quote with the encoded version %27 for URL comparison in Filter by Rating --- assets/js/blocks/rating-filter/block.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/assets/js/blocks/rating-filter/block.tsx b/assets/js/blocks/rating-filter/block.tsx index 3b9894b07cc..ee4a8755109 100644 --- a/assets/js/blocks/rating-filter/block.tsx +++ b/assets/js/blocks/rating-filter/block.tsx @@ -139,10 +139,17 @@ const RatingFilterBlock = ( { } if ( checkedRatings.length === 0 ) { + /** + * .replace() was added as removeQueryArgs() function uses decodeURIcomponent method + * which doesn't encode single quotes (') while it was still encoded in the original URL (%27). + * So when the single quote was in a query param, for example as a search term, it caused + * endless redirection loop. + * More context: https://github.com/woocommerce/woocommerce-blocks/issues/8707 + */ const url = removeQueryArgs( window.location.href, QUERY_PARAM_KEY - ); + ).replace( /'/g, '%27' ); if ( url !== window.location.href ) { changeUrl( url ); From 7e59eddb742dd577158498aba994d0e4aae8ffd5 Mon Sep 17 00:00:00 2001 From: Karol Manijak Date: Mon, 20 Mar 2023 12:37:42 +0100 Subject: [PATCH 3/5] refactor the solution so it encodes the href rather than decode the newly created URL --- assets/js/blocks/rating-filter/block.tsx | 15 ++++----------- assets/js/blocks/stock-filter/block.tsx | 19 ++++++++----------- assets/js/utils/filters.ts | 17 ++++++++++++++++- 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/assets/js/blocks/rating-filter/block.tsx b/assets/js/blocks/rating-filter/block.tsx index ee4a8755109..9c95b438943 100644 --- a/assets/js/blocks/rating-filter/block.tsx +++ b/assets/js/blocks/rating-filter/block.tsx @@ -26,7 +26,7 @@ import FilterSubmitButton from '@woocommerce/base-components/filter-submit-butto import FilterResetButton from '@woocommerce/base-components/filter-reset-button'; import FormTokenField from '@woocommerce/base-components/form-token-field'; import { addQueryArgs, removeQueryArgs } from '@wordpress/url'; -import { changeUrl } from '@woocommerce/utils'; +import { changeUrl, encodeSearchTerm } from '@woocommerce/utils'; import classnames from 'classnames'; import { difference } from 'lodash'; import type { ReactElement } from 'react'; @@ -139,19 +139,12 @@ const RatingFilterBlock = ( { } if ( checkedRatings.length === 0 ) { - /** - * .replace() was added as removeQueryArgs() function uses decodeURIcomponent method - * which doesn't encode single quotes (') while it was still encoded in the original URL (%27). - * So when the single quote was in a query param, for example as a search term, it caused - * endless redirection loop. - * More context: https://github.com/woocommerce/woocommerce-blocks/issues/8707 - */ const url = removeQueryArgs( window.location.href, QUERY_PARAM_KEY - ).replace( /'/g, '%27' ); + ); - if ( url !== window.location.href ) { + if ( url !== encodeSearchTerm( window.location.href ) ) { changeUrl( url ); } @@ -162,7 +155,7 @@ const RatingFilterBlock = ( { [ QUERY_PARAM_KEY ]: checkedRatings.join( ',' ), } ); - if ( newUrl === window.location.href ) { + if ( newUrl === encodeSearchTerm( window.location.href ) ) { return; } diff --git a/assets/js/blocks/stock-filter/block.tsx b/assets/js/blocks/stock-filter/block.tsx index ef365615bc7..443ccc210b9 100644 --- a/assets/js/blocks/stock-filter/block.tsx +++ b/assets/js/blocks/stock-filter/block.tsx @@ -32,7 +32,11 @@ import isShallowEqual from '@wordpress/is-shallow-equal'; import { decodeEntities } from '@wordpress/html-entities'; import { isBoolean, objectHasProp } from '@woocommerce/types'; import { addQueryArgs, removeQueryArgs } from '@wordpress/url'; -import { changeUrl, PREFIX_QUERY_ARG_FILTER_TYPE } from '@woocommerce/utils'; +import { + changeUrl, + PREFIX_QUERY_ARG_FILTER_TYPE, + encodeSearchTerm, +} from '@woocommerce/utils'; import { difference } from 'lodash'; import classnames from 'classnames'; @@ -219,19 +223,12 @@ const StockStatusFilterBlock = ( { return; } if ( checkedOptions.length === 0 ) { - /** - * .replace() was added as removeQueryArgs() function uses decodeURIcomponent method - * which doesn't encode single quotes (') while it was still encoded in the original URL (%27). - * So when the single quote was in a query param, for example as a search term, it caused - * endless redirection loop. - * More context: https://github.com/woocommerce/woocommerce-blocks/issues/8707 - */ const url = removeQueryArgs( window.location.href, QUERY_PARAM_KEY - ).replace( /'/g, '%27' ); + ); - if ( url !== window.location.href ) { + if ( url !== encodeSearchTerm( window.location.href ) ) { changeUrl( url ); } @@ -242,7 +239,7 @@ const StockStatusFilterBlock = ( { [ QUERY_PARAM_KEY ]: checkedOptions.join( ',' ), } ); - if ( newUrl === window.location.href ) { + if ( newUrl === encodeSearchTerm( window.location.href ) ) { return; } diff --git a/assets/js/utils/filters.ts b/assets/js/utils/filters.ts index 98548288fba..371bb0f431b 100644 --- a/assets/js/utils/filters.ts +++ b/assets/js/utils/filters.ts @@ -1,7 +1,7 @@ /** * External dependencies */ -import { getQueryArg } from '@wordpress/url'; +import { getQueryArg, addQueryArgs } from '@wordpress/url'; import { getSettingWithCoercion } from '@woocommerce/settings'; import { isBoolean } from '@woocommerce/types'; @@ -39,3 +39,18 @@ export function changeUrl( newUrl: string ) { window.history.replaceState( {}, '', newUrl ); } } + +/** + * Extract the search term from query params and encode it. + * + * @param {string} url URL to encode the search param from. + */ +export const encodeSearchTerm = ( url: string ) => { + const searchTerm = getQueryArg( url, 's' ); + if ( searchTerm ) { + return addQueryArgs( url, { + s: searchTerm, + } ); + } + return url; +}; From 15fe5673c1e0d3b4e87556f642ccffaaf12355be Mon Sep 17 00:00:00 2001 From: Karol Manijak Date: Mon, 20 Mar 2023 13:40:15 +0100 Subject: [PATCH 4/5] Refactor the normalisation --- assets/js/blocks/rating-filter/block.tsx | 6 +++--- assets/js/blocks/stock-filter/block.tsx | 6 +++--- assets/js/utils/filters.ts | 15 +++++---------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/assets/js/blocks/rating-filter/block.tsx b/assets/js/blocks/rating-filter/block.tsx index 9c95b438943..64c8559504a 100644 --- a/assets/js/blocks/rating-filter/block.tsx +++ b/assets/js/blocks/rating-filter/block.tsx @@ -26,7 +26,7 @@ import FilterSubmitButton from '@woocommerce/base-components/filter-submit-butto import FilterResetButton from '@woocommerce/base-components/filter-reset-button'; import FormTokenField from '@woocommerce/base-components/form-token-field'; import { addQueryArgs, removeQueryArgs } from '@wordpress/url'; -import { changeUrl, encodeSearchTerm } from '@woocommerce/utils'; +import { changeUrl, normalizeQueryParams } from '@woocommerce/utils'; import classnames from 'classnames'; import { difference } from 'lodash'; import type { ReactElement } from 'react'; @@ -144,7 +144,7 @@ const RatingFilterBlock = ( { QUERY_PARAM_KEY ); - if ( url !== encodeSearchTerm( window.location.href ) ) { + if ( url !== normalizeQueryParams( window.location.href ) ) { changeUrl( url ); } @@ -155,7 +155,7 @@ const RatingFilterBlock = ( { [ QUERY_PARAM_KEY ]: checkedRatings.join( ',' ), } ); - if ( newUrl === encodeSearchTerm( window.location.href ) ) { + if ( newUrl === normalizeQueryParams( window.location.href ) ) { return; } diff --git a/assets/js/blocks/stock-filter/block.tsx b/assets/js/blocks/stock-filter/block.tsx index 443ccc210b9..12e4dd6433c 100644 --- a/assets/js/blocks/stock-filter/block.tsx +++ b/assets/js/blocks/stock-filter/block.tsx @@ -35,7 +35,7 @@ import { addQueryArgs, removeQueryArgs } from '@wordpress/url'; import { changeUrl, PREFIX_QUERY_ARG_FILTER_TYPE, - encodeSearchTerm, + normalizeQueryParams, } from '@woocommerce/utils'; import { difference } from 'lodash'; import classnames from 'classnames'; @@ -228,7 +228,7 @@ const StockStatusFilterBlock = ( { QUERY_PARAM_KEY ); - if ( url !== encodeSearchTerm( window.location.href ) ) { + if ( url !== normalizeQueryParams( window.location.href ) ) { changeUrl( url ); } @@ -239,7 +239,7 @@ const StockStatusFilterBlock = ( { [ QUERY_PARAM_KEY ]: checkedOptions.join( ',' ), } ); - if ( newUrl === encodeSearchTerm( window.location.href ) ) { + if ( newUrl === normalizeQueryParams( window.location.href ) ) { return; } diff --git a/assets/js/utils/filters.ts b/assets/js/utils/filters.ts index 371bb0f431b..5faf1743bb3 100644 --- a/assets/js/utils/filters.ts +++ b/assets/js/utils/filters.ts @@ -1,7 +1,7 @@ /** * External dependencies */ -import { getQueryArg, addQueryArgs } from '@wordpress/url'; +import { getQueryArg, getQueryArgs, addQueryArgs } from '@wordpress/url'; import { getSettingWithCoercion } from '@woocommerce/settings'; import { isBoolean } from '@woocommerce/types'; @@ -41,16 +41,11 @@ export function changeUrl( newUrl: string ) { } /** - * Extract the search term from query params and encode it. + * Run the query params through buildQueryString to normalise the params. * * @param {string} url URL to encode the search param from. */ -export const encodeSearchTerm = ( url: string ) => { - const searchTerm = getQueryArg( url, 's' ); - if ( searchTerm ) { - return addQueryArgs( url, { - s: searchTerm, - } ); - } - return url; +export const normalizeQueryParams = ( url: string ) => { + const queryArgs = getQueryArgs( url ); + return addQueryArgs( url, queryArgs ); }; From c7a64595c8d424272dd54aab51cb1d6a4be3e8aa Mon Sep 17 00:00:00 2001 From: Karol Manijak Date: Mon, 20 Mar 2023 13:40:37 +0100 Subject: [PATCH 5/5] Add tests to normalisation function --- assets/js/utils/test/filters.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 assets/js/utils/test/filters.js diff --git a/assets/js/utils/test/filters.js b/assets/js/utils/test/filters.js new file mode 100644 index 00000000000..6fff2418715 --- /dev/null +++ b/assets/js/utils/test/filters.js @@ -0,0 +1,27 @@ +/** + * Internal dependencies + */ +import { normalizeQueryParams } from '../filters'; + +describe( 'normalizeQueryParams', () => { + test( 'does not change url if there is no query params', () => { + const input = 'https://example.com'; + const expected = 'https://example.com'; + + expect( normalizeQueryParams( input ) ).toBe( expected ); + } ); + + test( 'does not change search term if there is no special character', () => { + const input = 'https://example.com?foo=bar&s=asdf1234&baz=qux'; + const expected = 'https://example.com?foo=bar&s=asdf1234&baz=qux'; + + expect( normalizeQueryParams( input ) ).toBe( expected ); + } ); + + test( 'decodes single quote characters', () => { + const input = 'https://example.com?foo=bar%27&s=asd%27f1234&baz=qux%27'; + const expected = "https://example.com?foo=bar'&s=asd'f1234&baz=qux'"; + + expect( normalizeQueryParams( input ) ).toBe( expected ); + } ); +} );