[wp-trac] [WordPress Trac] #42957: Usernames ending in a period generate invalid reset password links in certain email clients

[WordPress Trac]

#42957: Usernames ending in a period generate invalid reset password links in
certain email clients
-------------------------------------------------+-------------------------
 Reporter:  paulcline                            |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  6.7
Component:  Users                                |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests needs-dev-  |     Focuses:
  note dev-feedback                              |
-------------------------------------------------+-------------------------

Comment (by dmsnell):

 Here are a couple of thoughts on this:

 It seems like a very specific failure and not entirely a bug in WordPress,
 as WordPress is properly generating links. It's the email clients that
 misinterpret them, so this is more like a way for WordPress to
 interoperate better in the ecosystem. To that end I don't know if it makes
 a lot of sense to introduce a new unclear abstraction in `functions.php`.
 Maybe what would do well here is a very specific targeted fix that doesn't
 raise other questions.

 > I also wonder if
 >> $url = str_replace('.', '%2E', $url);
 > will be more logical.

 For example, this will break almost every URL that passes through it, by
 replacing the actual dots in the host part of the URL with percent-
 escapes. There are also problems with running URLs through
 `rawurlencode()`: for one, it's not safe to run the entire URL through
 that function; for two, it will double-escape and corrupt anything that
 was already escaped.

 ----

 So now I wonder if there is a simpler way to address this that resolves
 the issue without raising framework level questions. In the third patch
 there are two changes to generated URLs but only one of the URLs has the
 problem in various email clients. Each URL contains a query arg
 `action=rp`

 What if instead of all of this, we simply moved the `login=" .
 rawurlencode( $user_login ) . "` part to the front and ended each URL with
 `&action=rp`? There would be no conflict, the exact issue would be
 avoided, and the change would require essentially no review because there
 are no new overarching questions or API design work involved.


 {{{
 -       $message .= network_site_url( "wp-
 login.php?action=rp&key=$key&login=" . rawurlencode( $user->user_login ),
 'login' ) . "\r\n\r\n";
 +       $message .= network_site_url( "wp-login.php?login=" .
 rawurlencode( $user->user_login ) . "&key=$key&action=rp", 'login' ) .
 "\r\n\r\n";
 }}}

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