search

Cizzuk / CSE

This is an extension to customize Safari's search engine.
https://cizzuk.net/en/app/cse/
Mozilla Public License 2.0
13 stars 0 forks source link

[BUG] Infinite redirect loop when top URL includes supported search engine #4

Closed DavidSchechter closed 3 weeks ago

DavidSchechter commented 1 month ago

Root cause: Top of url included support url. Redirect / reset of url is done always without ignoring / excluding “top of url”.

Sets to reproduce:

  1. Set top of url to - https://google.com/search?udm=14&q= (this is a redirect to the new “web” tab in google search).
  2. Set default search engine as - Google - both in CSE and Safari Settings
  3. Attempt to make a search

Issue isn’t reproducible if DuckDuckGo is set as the default search engine both in CSE and Safari Settings.

Thank you in advance

DavidSchechter commented 1 month ago

IMG_1250

Also being presented by this captcha

Cizzuk commented 1 month ago

This is a bug but probably cannot be fixed.

Some search engines have specific queries in URLs when searching from Safari search bar. In this case, CSE will check this before redirecting. https://github.com/Cizzuk/CSE/blob/bdc7407bf1ae798b1ab59825d71eb1f96a081fe2/Custom%20Search%20Engine%20Extension/Resources/content.js#L33

However, some search engines, including Google, do not have this query and CSE cannot recognize how to search.

There are workarounds. If you want to set your custom search engine to Google, you can set your default search engine to Bing, and redirect loop will not occur.

Cizzuk commented 1 month ago

I will try to find a way to fix this bug, but it will take some time.

DavidSchechter commented 1 month ago

Understandable that there are challenges in identifying specific queries.

Potential way to resolve: Check if ‘Path’ contains ‘Top of url’. If it does already contains it, bail out and stop attempting to replace/redirect the url.

This check would be done prior to any domain specific checks.

Could this approach potentially work?

Cizzuk commented 1 month ago

Maybe, yes. But I think it should check both Top and Suffix URLs. And I don't think it works correctly when redirected from google.com to www.google.com.

But I will try this, thank you!

DavidSchechter commented 1 month ago

Agreed that it should check for both ‘top’ and ‘suffix’.

For www.google.com vs google.com - the beginning of the ‘Path’ can be filter out (remove http://www or https://www) and make the contains check without the prefix

DavidSchechter commented 3 weeks ago

Hi, Any update on this issue?

Thanks

Cizzuk commented 3 weeks ago

https://github.com/Cizzuk/CSE/issues/4#issuecomment-2144026867 I have tried this and was able to stop the redirect loop. I am making some changes to clean up content.js to fix this issue. This, including testing, will take some time, but will be useful for future fixes!

Cizzuk commented 3 weeks ago

https://github.com/Cizzuk/CSE/commit/aa535294c6bc382d1dd843f92805df4bd1ad5b8a This bug has been fixed. @DavidSchechter, Thank you!