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

[sreedoap]

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.

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

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.)

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 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.

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.

Confirmed.

All that is necessary is this:

add_action( 'wp', function() {
   is_front_page();
});

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.

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:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/canonical.php#L375

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.