wp_safe_redirect can return admin_url() when get_admin_url() is used

[layotte]

Setup your site like this:

WordPress Address (URL): ​http://yourdomain.tld/ (without www)
Site Address (URL): ​http://www.yourdomain.tld/ (with www)

Example code (yes I know it's stupid code, but it's a working proof-of-concept):

<?php
function unsafe_safe_redirect() {
	$pagenow   = empty( $GLOBALS['pagenow'] ) ? false : $GLOBALS['pagenow'];

	if ( empty( $pagenow ) || 'post-new.php' != $pagenow )
		return;

	// Redirect for add new screen
	if ( 'post-new.php' == $pagenow ) {
		wp_safe_redirect( get_admin_url() . 'plugins.php' );
		die();
	}
}
add_action( 'admin_init', 'unsafe_safe_redirect' );

Visit: ​http://yourdomain.tld/wp-admin/post-new.php
It will redirect you to: ​http://yourdomain.tld/wp-admin/
But it should have redirected you to: ​http://yourdomain.tld/wp-admin/plugins.php

The problem is that wp_validate_redirect() uses home_url() which can be different from site_url() which is used by get_admin_url().

I propose that we use both in wp_validate_redirect(). Diff attached.

[layotte]

Patch to only modify the lines related to this ticket

[aaroncampbell]

In 35792:

Consider both home and site domains to be valid in wp_validate_redirect().

Props layotte.
Fixes #34028.

[mdawaffe]

Correct logic

[mdawaffe]

I believe the logic in [35792] is incorrect and causes a regression for #5114. Can someone please double check?

If

1. your home and site URLs contain uppercase characters, and
2. your home and site URLs are different (as in the www. example in this ticket), and
3. the (valid) URL passed to wp_validate_redirect() is all lowercased, then

the redirect will fail. This is because one of the strtolower( $site['host'] ) or strtolower( $wpp['host'] ) checks will always fail (since the domains are different) and the two are separated by an ||. We're only interested if they both fail: they should be separated by an &&.

To test:

1. Set WordPress Address (URL): ​http://yourDOMAIN.tld/ (mixed case and without www)
2. Set Site Address (URL): ​http://www.yourDOMAIN.tld/ (mixed case and with www)
3. Use the provided test script, but modify the redirect to use strtolower():

   wp_safe_redirect( strtolower( get_admin_url() ) . 'plugins.php' );
   

Updated in attachment:34028.3.diff​

By the way, it seems like we should also call strtolower() on the input URL's host ($lp['host']) and the $allowed_hosts array if we're going to do this at all. @markjaquith, do you recall why you went with [6219] over attachment:pluggable.php.diff:ticket:5114​?

[jorbin]

In 36445:

Revert [35792]

This causes a regression and causes redirects to potentially fail.

See #5114 #34028
props ocean90

[ocean90]

Note that in multisite there is filter on allowed_redirect_hosts named redirect_this_site() which only returns the site URL.