-
Notifications
You must be signed in to change notification settings - Fork 219
Filter by Stock and Filter by Rating: Fix the potential endless redirection loop when used on a search results page #8784
Conversation
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.
The release ZIP for this PR is accessible via:
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
Size Change: +294 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
…in Filter by Rating
Hey @kmanijak ! Regarding:
Do you mind clarifying in which widget these filters should be added? I added them to the Footer widget (which seems to be the correct one based on your follow-up instruction on item number 5), but when I click on search, none of the filter blocks are rendered. |
I made a mistake and wrote to add a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a mistake and wrote to add a Search block, while it has to be a Product Search block which would then show a product result search where filters appear. I'm editing the testing steps. Apologies!
No worries, thanks for clarifying!
Great work here 🎉 ! I can confirm things are working as expected.
Test Results:
Enable TT1 theme
Go to Appearance -> Widgets
Add there Filter by Stock, Filter by Rating, Active Filters and Product Search blocks
Save and go to your store's shop
Your blocks should be available in the footer
✅ Confirmed these blocks are visible in the footer of my shop (the Active filters block is rendered only when a filter is active):
Input in the search term: ' and then some other random terms including letters, numbers, white space, special characters
Expected: there's no endless redirections
✅ Confirmed the search works as expected with no infinite loops.
Screen.Recording.2023-03-22.at.10.00.11.mov
Click on some filters to Filter by Stock and Filter by Rating and confirm there's no redirection loop
✅ Confirmed there's no redirection loop when the filters are used. Sidenote: I noticed that the filter by rating is redirecting to the single product template after search; unsure if this is expected or not, but documenting it just in case:
Screen.Recording.2023-03-22.at.10.03.47.mov
Regarding:
When reviewing please focus on possible security concerns.
I can't think of any potential security issues by adopting this approach, but I would recommend reaching out for a second opinion just in case 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also tests as described for me. Also ran the added unit tests to confirm everything is passing.
I don't see any security issues that jump out at me here either.
Everything LGTM! 🚀
Issue: #8707
Problem statement
If we have the widget 'Filter Products by Stock' enabled (), when we search products and the string contains the single quote character
'
, the page gets in an infinite reloading loop.Explanation
Single quote character
'
is not encoded bycomponentURIComponents
method (docs), while it is encoded in the URL (%27
). So in the Filter by Stock and Filter by Rating, when there's a comparison of URL after augmenting the query params, the search param was never matching (s='
vss=%27
).Solution
The search term is encoded/normalized using
addQueryArgs
which does the encoding under the hood. It's used ONLY for comparison purposes.One caveat is that calling
addQueryArgs
from@wordpress/url
runs encoding on each of the query param, not only the one that's added.Review
When reviewing please focus on possible security concerns.
Fixes #8707
Screenshots
Testing
Automated Tests
User Facing Testing
'
and then some other random terms including letters, numbers, white space, special charactersWooCommerce Visibility
Performance Impact
Changelog