WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 2 years ago

#20067 new defect (bug)

/search/.+ takes priority over pages since 3.3 - Breaks some sites

Reported by: ronnieg Owned by:
Milestone: Awaiting Review Priority: normal
Severity: major Version: 3.3
Component: Rewrite Rules Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Was on WP 3.2.1 and upgraded to 3.3.1. Immediately started experiencing problems with WP intercepting page urls such as: www.mydomain.com/search/advanced-search.html, or any other url whose first level folder was /search/. This is a valid folder and url set on my real estate site, and it all worked fine in 3.2.1. But WP 3.3.1 redirected to some kind of search process, and returned summaries of various pages on the site. As a test, I located and renamed the search.php in my theme folder. After that, the content and formatting of the results changed, but it still incorrectly redirected to some kind of search process, probably the WP core search since the one in the theme folder had been renamed. During this entire process, no other changes were made. The exact same plugins were installed and active, and no changes were made to .htaccess. .htaccess was reviewed, and there were no redirects that could have caused this. No redirect or site search plugins were ever installed or active. I re-installed WP 3.2.1 core files and all of the improper redirecting to search issues went away. Did 3.3.1 have some kind of new internal rewrite rules regarding when/how to do a site search? Affected site is www.denverhomevalue.com.

Attachments (1)

20067.diff (1021 bytes) - added by duck_ 2 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 duck_2 years ago

  • Component changed from General to Rewrite Rules
  • Version changed from 3.3.1 to 3.3

#16687 changed the order of rewrite rules for verbose page structures (%postname%, %category/%postname%, etc.) moving the rules for pages below those for search. This brought them in line with non-verbose pages rules.

The page rules could be moved back before $root_rewrite again, but this would cost an extra database query for many more URLs to check if a page exists and it's also weird to have different priority of rewrite rules depending on the structure used.

duck_2 years ago

comment:2 duck_2 years ago

  • Keywords has-patch 2nd-opinion added

Attached patch doing what I mentioned in the previous comment.

comment:3 follow-up: kawauso2 years ago

I don't understand how the -f flag in the .htaccess rules isn't preventing a valid file URL request being rewritten onto WP's index.php in the first place? Unless there's a circumstance where -f returns false such as when a 403 would occur (from experience).

Or is /search/advanced-search.html actually a WP page that has had the rewrite rules tweaked?

I can't reproduce with /%post_name%/ rewrites on 3.3.1.

Last edited 2 years ago by kawauso (previous) (diff)

comment:4 in reply to: ↑ 3 duck_2 years ago

Replying to kawauso:

Or is /search/advanced-search.html actually a WP page that has had the rewrite rules tweaked?

Yes this is when dealing with WordPress pages. Static files on the system wouldn't have the problem.

I tested with /%postname%/ and a page called "blah" with the parent "search".

comment:5 follow-up: ronnieg2 years ago

Thanks for the patch duck. Will try it out on my dev site today.

On my DenverHomeValue.com, and also on many of my other RE client's WP sites, there are actually several child pages under the /search/ parent, not just the one example I cited. I set all of my real estate client sites up that way. The parent page, /search.html, is not affected, but all of its child pages are. There are no external redirects for any of the pages affected, except a few that redirect old urls (from the old Joomla/html versions of the site) TO the affected urls.

I read the explanation in #16687, and while I agree that improving performance is important, where possible, I do not agree that it should be at the expense of breaking a site's existing permalink structure. Use of a page and structure /search/ is very common in the real estate community, so the negative impact could be tremendous, affecting hundreds if not thousands of other real estate WP sites.

IMO, if search is to be a special case, then it needs its own, unique rules and triggers, triggers that should not limit a site owner's ability to design and use whatever post/page names they want and need to. How are you going to advertise to the world, especially the real estate world, that WP is no longer capable of supporting top level page/post names of /search/ without breaking the site design? I haven't tested yet, but at what level does this issue go away? 2nd level page/post? What happens with deeper structures that have search as their page/post name? Will be trying out some of that later today.

comment:6 in reply to: ↑ 5 duck_2 years ago

Replying to ronnieg:

but at what level does this issue go away? 2nd level page/post?

It doesn't. The search regular expression slurps up everything after /search/:

^search/(.+)/?$

What happens with deeper structures that have search as their page/post name?

They shouldn't be affected. The regular expression is anchored to match "search" at the very beginning of the URL.

comment:7 follow-up: ronnieg2 years ago

WP is actually hijacking it then. Nasty. Tried the patch and no joy. Disabled all plugins, switched theme to twentyeleven. No joy. Changed search permalink base: (line 427 in 3.3.1 stable) to: var $search_base = 'wpsearch'; and when that didn't work, deleted variable $search_rewrite from the rewrite rules lists (both) altogether, since my site users should never need search, and still no joy.

comment:8 nacin2 years ago

Interesting that this hasn't been reported previously. I would have guessed that a "search" static page would be more common.

comment:9 ronnieg2 years ago

Most real estate agents are not their own webmasters, so they don't update their site's plugins and release levels very often. This may be reported again and more often as they and their webmasters get around to it.

I have restored my dev1.denverhomevalue.com from a backup of my www production site and 3.2.1 so I can move on with other development and testing work. I will continue testing this issue in a new sub-domain, with clean 3.3.1 install, default twentyeleven theme and bare-bones content, and will advise if I can come up with a solution.

Last edited 2 years ago by ronnieg (previous) (diff)

comment:10 in reply to: ↑ 7 ; follow-up: duck_2 years ago

Replying to ronnieg:

Tried the patch and no joy.

You have to visit the permalinks settings page to regenerate rewrites rules for it to take effect (wp-admin/options-permalink.php).

comment:11 in reply to: ↑ 10 ronnieg2 years ago

Replying to duck_:
Thanks. That did it. BTW line# in 3.3.1 stable/production was 1544.

comment:12 ronnieg2 years ago

Question for all:
Is there any reason $search_base = 'wpsearch' could not be the default, thereby making it obviously "owned" by WP hooks, and avoiding conflict with other user page/folder names? Or some other way for users to set $search_base to something else if it is a conflict with specific sites?

Last edited 2 years ago by ronnieg (previous) (diff)

comment:13 ronnieg2 years ago

Related question: I have built and installed a custom 404 page that I want all users to be redirected to if a page/post url is not found, and I use the standard search widget in a sidebar for users to be able to do keyword searches. So please explain why WP would or should default to some kind of internal search mechanism? Legacy functionality / compatibility?

comment:14 follow-up: dd322 years ago

Is there any reason $search_base = 'wpsearch' could not be the default,

Other than the fact /search/...../ has been the Rewrite permalinks for search results since WordPress 1.5 or so, It just so happens, that due to (well, what I would call a bug) pages took priority over /search/ when %category% or %postname% was the first token in the URL, to make that clearer, someone using a url structure such as /%year%/%month%/%day%/%postname%/ would never have been able to use /search/xxxxx as a page (and still can't).

I've seen plenty of cases where someone uses /search/ as an advanced search form, relying on the fact that child "pages" (in the url) will be search results..

comment:15 in reply to: ↑ 14 ronnieg2 years ago

Replying to dd32:

"that due to (well, what I would call a bug) pages took priority over /search/ when %category% or %postname% was the first token in the URL,.."

And I might argue, in a friendly way, that if a webmaster has explicitly specified %category% or %postname% for their custom permalink structure, then that's exactly what they intended, so it should take priority, and is NOT a bug. Now, the recent change, well intentioned as it may have been, negates the webmaster's intent, and also has the potential to cause major issues, simply because it will break sites like mine, where the "bug" was actually performing as intended.

I'm pretty reasonable, and would just like to have a user friendly and supported way to make it work as before, without a core code hack.

BTW: I did change the default from search to wpsearch, and restored the order of the rewrites to the current/original 3.3.1, and that by itself cleared up the issue, at least for me.


Last edited 2 years ago by ronnieg (previous) (diff)

comment:16 dd322 years ago

And I might argue, in a friendly way, that if a webmaster has explicitly specified %category% or %postname% for their custom permalink structure, then that's exactly what they intended

Except for the obvious, that the permastructure only applies to Posts, not Pages, so choosing a different permastructure should not change the way that Pages act, doing so adds confusion as to why 2 sites do not act the same.

comment:17 dd322 years ago

  • Summary changed from WP Hijacks / Redirects actual page urls to search to /search/.+ takes priority over pages since 3.3

comment:18 ronnieg2 years ago

  • Summary changed from /search/.+ takes priority over pages since 3.3 to /search/.+ takes priority over pages since 3.3 - Breaks some sites

comment:19 DrewAPicture2 years ago

  • Cc xoodrew@… added
Note: See TracTickets for help on using tickets.