Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#21602 new defect (bug)

redirect_canonical can lead to infinite loop on index navigation if site url is not all lower case

Reported by: sreedoap Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Canonical Keywords: needs-unit-tests needs-refresh
Focuses: Cc:


The function redirect_canonical in wp-includes/canonical.php (WordPress 3.4.1) on line 406 and 422 makes the following check:

if ( !$redirect_url || $redirect_url == $requested_url )
		return false;

This ensures that it does not attempt to redirect you to the page you requested in the first place. However this function is not case sensitive so if the redirect URL is in a different case than the requested URL then the user can enter an infinite redirect loop. (For example if the Site Address (URL) of the site is set to be in all upper case.)

This function should do a case-insensitive string comparison since domain names are case-insensitive.

The issue only appears to happen with certain plugins installed (ShareThis and PilotPress both led to this issue,) I haven't figured out yet why it's only an issue with certain plugins but it should still be fixed in WordPress to make the proper string comparison.

Attachments (2)

canonical.php.patch (959 bytes) - added by sreedoap 3 years ago.
21602.patch (1.5 KB) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (13)

#1 @sreedoap
3 years ago

Correction: The "Theme My Login" plugin is one that will lead to this issue.

#2 @sreedoap
3 years ago

  • Keywords has-patch added

#3 @SergeyBiryukov
3 years ago

  • Component changed from General to Canonical

#4 @sreedoap
3 years ago

Narrowed this down a bit.. this will not happen with the default permalink setting but will happen with all the other ones (Post name, Numeric, etc.)

#5 @sreedoap
3 years ago

Alright.. I've compiled below a complete list of steps to reproduce. Turns out this only will happen if there is a plugin activated that uses the is_front_page() function.

1) Change the URL of the site address/wordpress address setting to have uppercase characters (for example testsite.example.com to TESTSITE.example.com)

2) Change the permalinks setting to an option besides the default (for example post name)

3) Install the following plugin: https://gist.github.com/9649e00c6acec8805f39

4) Attempt to access the sites homepage, a infinite redirect loop will occur.

Version 1, edited 3 years ago by sreedoap (previous) (next) (diff)

#6 @sreedoap
3 years ago

Sorry, forgot to include one step to reproduce.. in settings > reading change the "front page displays" setting to a static page for the front page.

Last edited 3 years ago by sreedoap (previous) (diff)

#7 follow-up: @nacin
3 years ago


All that is necessary is this:

add_action( 'wp', function() {

This is a tricky one.

I haven't tracked down what exactly fails to trigger an infinite redirect, but I do know why is_front_page() is the trigger. In canonical, there is this conditional:

} elseif ( is_page() && !is_feed() && isset($wp_query->queried_object) && 'page' == get_option('show_on_front') && $wp_query->queried_object->ID == get_option('page_on_front') && ! $redirect_url ) {
			$redirect_url = home_url('/');

This is designed to redirect a /front-page/ page that is assigned to the page_on_front to home_url(). But, since it checks if $wp_query->queried_object is set, it doesn't actually work. is_front_page() calls is_page() with the right arguments to trigger a proper get_queried_object(). This then causes the conditional to run.

As to why it spirals into a redirect loop, home_url() returns something different than what was submitted, and all that jazz.

I see the need for multiple unit tests here, both to cover front-page -> / redirect failures, and the domain issue.

#8 in reply to: ↑ 7 @SergeyBiryukov
3 years ago

Replying to nacin:

I haven't tracked down what exactly fails to trigger an infinite redirect

home_url('/') returns http://trunk.WordPress/, wp_redirect() fires, browser converts that to http://trunk.wordpress/, and the loop starts again.

There's a block in line 375 (introduced in [6097] for #4773) to prevent exactly this situation:

However, it doesn't seem to work as intended. As far as I can see, the purpose of the block is to discard the differences in host capitalization, so there shouldn't be a redirect in this case.

What happens instead is $compare_original and $compare_redirect end up being the same, $redirect_url is not rewritten, and the redirect proceeds.

21602.patch moves the block lower, after $compare_original and $compare_redirect are filled.

#9 @SergeyBiryukov
3 years ago

  • Keywords needs-unit-tests added

#10 @meloniq
3 years ago

  • Cc meloniq@… added

#11 @chriscct7
3 months ago

  • Keywords needs-refresh added; has-patch removed
Note: See TracTickets for help on using tickets.