[Hotfix] search redirect loop - #788
Open
mikeyarce wants to merge 3 commits into
Open
Conversation
On single-site installs, every search request entered an infinite 302 loop. The amnesty_maybe_override_search_uri filter on home_url rewrites any /search/<term>/ path to the bare search page URL, and current_url() routes through home_url() when not on multisite. The reported current URL therefore never matched the redirect target computed by Search_Filters::prettify_search(), so the page redirected to itself. Suspend the override while current_url() builds its result: the current URL must always be the literal request URL. Only re-add the filter when it was actually registered, so intentional removals of the override by child themes or plugins are not silently undone. Multisite is unaffected: current_url() builds from the network siteurl option on that path and never calls home_url().
The search results pattern called switch_to_blog(), restore_current_blog(), and get_blog_permalink() unconditionally. None of these functions exist on single-site installs, so rendering search results caused a fatal error. This was previously masked by the search redirect loop, which made the results page unreachable. Add a get_blog_post_term() helper, following the existing get_blog_post_meta() convention, which only switches site context when running on multisite and the result belongs to a different site. Use it in the pattern, and fall back to get_permalink() for result links when no cross-site switch is needed. Behaviour on multisite is preserved: previously the pattern switched to the current site (a no-op) when a result carried no blog_id; now it skips the switch entirely in that case.
current_url() forces the https scheme, while amnesty_search_url() follows the site's configured scheme. On plain-http sites the strpos() guards in title_tag() and results_title() could therefore never match, so the search results title and the month/year suffixes silently stopped applying. Normalise both sides with set_url_scheme() before comparing, as prettify_search() already does.
Author
|
cc @jaymcp |
Contributor
|
Hi @mikeyarce Thank you for submitting your first PR, and apologies for the delayed response. I'll endeavour to get this reviewed today :) Best |
Contributor
|
Hi @mikeyarce We will be inotuch soon with some feedback on this PR, thanks again for contrubuting, we really apereciate it! :-) |
Author
|
@AmnestyAM no problem! Let me know if I can help in any other way, happy to jump in to help more. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ref: #785
Hi! first time contributor here 😄
Fixes the endless redirect loop when searching on a non-multisite install, plus two more single-site search bugs that were hiding behind it.
What's going on
amnesty_maybe_override_search_uri()is hooked intohome_urland rewrites any/search/<term>/path to the bare search page URL. On single-site,current_url()goes throughhome_url(), so on/search/foo/it reported/search/which never matches the target URLprettify_search()wants you on, so it 302s you to the page you're already on, forever. Multisite doesn't loop becausecurrent_url()builds from the network siteurl option and never toucheshome_url()on that path.I looked at dropping the non-multisite branch in
current_url()as suggested in the issue, but building from thesiteurloption would break single-site installs where WordPress lives in its own directory (like a Bedrock setup) they would loop again. Suspending the override whilecurrent_url()builds its result feels like the safest fix: the current URL should always be the literal request URL. I left thehome_urlfilter itself alone, since form actions andget_pagenum_link()and derived URLs depend on its rewriting.With the loop fixed, two more single-site problems surfaced:
switch_to_blog()/get_blog_permalink()unconditionally, and those don't exist outside multisite. Added aget_blog_post_term()helper (modelled on the existingget_blog_post_meta()) that only switches when a result actually comes from another site.title_tag()/results_title()) never applied on plain-http sites:current_url()forces https whileamnesty_search_url()follows the site scheme, so theirstrpos()guards couldn't match. Normalised both sides withset_url_scheme(), asprettify_search()already does.Steps to test:
/?s=fooor/search/foo/( before this PR the browser dies in a redirect loop and after, there's a single 302 to/search/foo/and results render)Considerations:
current_url()'s multisite branch is untouched. The results pattern still switches sites when a result comes from another site on the network, it now just skips the switch when the result is already from the current site, which previously did nothing anyway.home_urloverride is intentionally keptdevelopinstead ofmainbecausemainhas some commits that were not merged back intodevelop.Video
humanity-theme-785-search-fix-before-after.webm