Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 799 forks source link

Jetpack Search sometimes pulls in "+" signs for spaces #32753

Open dpasque opened 1 year ago

dpasque commented 1 year ago

Impacted plugin

Jetpack, Search

Quick summary

In what seems to be a rare race condition, if you enter a search term and then launch the Instant search modal, the spaces in the search can become replaced with + plus signs (looks like maybe a URL encoding or something).

This is hard to do manually, but our E2E tests trigger it every now and then, so you probably have to be moving pretty fast.

jetpack_search_plus

Here's a full video for reference: jetpack_search_plus_vid.webm

Steps to reproduce

  1. Have a site with a search block somewhere and instant search
  2. Enter a search term with spaces
  3. Launch the modal with that search (button or enter)
  4. Rarely, get pluses!

A clear and concise description of what you expected to happen.

Search value in the modal matches the value in the input outside of the modal exactly.

What actually happened

Spaces were replaced with "+"

Impact

Some (< 50%)

Available workarounds?

Yes, easy to implement

Platform (Simple and/or Atomic)

Simple, Atomic

Logs or notes

Our E2E currently don't hit self-hosted, so not sure if that's affected.

dpasque commented 1 year ago

Weirdly, when I watch the E2E tests that have this bug, the encoding in the URL is in fact different.

When it's working normal, a search of "foo bar" gets encoded in the URL as: https://jetpackedgephp74.wpcomstaging.com/?s=foo%20bar

But, when it has the bug, the actual query param is using the + for space encoding: https://jetpackedgephp74.wpcomstaging.com/?s=foo+bar

Not sure still what is making the space encoding different between those executions!

anomiex commented 1 year ago

Not sure still what is making the space encoding different between those executions!

I think the test is managing to type in the field and submit the form before the JS gets a chance to hook the submit event. So the browser submits, encoding the value using the application/x-www-form-urlencoded variation of percent encoding rather than the "normal" percent encoding the JS expects to receive.

In the linked video you can see that, instead of the Instant Search popup being on top of the blog's front page like normal, it has loaded the "fallback" page from https://jetpackedgephp74.wpcomstaging.com/?s=foo+bar and is displaying the popup on top of that instead.

dpasque commented 1 year ago

I think the test is managing to type in the field and submit the form before the JS gets a chance to hook the submit event. So the browser submits, encoding the value using the application/x-www-form-urlencoded variation of percent encoding rather than the "normal" percent encoding the JS expects to receive.

Ahhh, nice catch, that would make a lot of sense!!

github-actions[bot] commented 8 months ago

This issue has been marked as stale. This happened because:

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

davemart-in commented 2 weeks ago

Assigned to Jetpack search repo and removed from The One Board.