[wp-trac] [WordPress Trac] #34028: wp_safe_redirect can return admin_url() when get_admin_url() is used

[WordPress Trac]

#34028: wp_safe_redirect can return admin_url() when get_admin_url() is used
-------------------------------------------------+-------------------------
 Reporter:  layotte                              |       Owner:
     Type:  defect (bug)                         |  aaroncampbell
 Priority:  normal                               |      Status:  reopened
Component:  General                              |   Milestone:  4.5
 Severity:  normal                               |     Version:
 Keywords:  has-patch needs-testing dev-         |  Resolution:
  feedback                                       |     Focuses:
-------------------------------------------------+-------------------------
Changes (by mdawaffe):

 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 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?

--
Ticket URL: <https://core.trac.wordpress.org/ticket/34028#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform