[wp-trac] [WordPress Trac] #45845: Use get_user_by in check_password_reset_key

WordPress Trac noreply at wordpress.org
Thu Mar 7 00:57:19 UTC 2019


#45845: Use get_user_by in check_password_reset_key
--------------------------------------+-----------------------------
 Reporter:  spacedmonkey              |       Owner:  SergeyBiryukov
     Type:  defect (bug)              |      Status:  reopened
 Priority:  normal                    |   Milestone:  5.2
Component:  Users                     |     Version:  3.1
 Severity:  normal                    |  Resolution:
 Keywords:  good-first-bug has-patch  |     Focuses:  performance
--------------------------------------+-----------------------------

Comment (by davidbaumwald):

 Replying to [comment:12 desrosj]:
 > @davidbaumwald I haven’t gotten to return to this yet, but my thinking
 was that digging a bit deeper to understand why the cache does not need to
 be cleared for other similar tests to ensure there is not a bigger issue
 buried in these changes somewhere was a good idea.

 @desrosj That's a good idea.  In scanning the tests in auth.php as an
 example, I see reasons why this test sticks out as having an issue with
 caching.

 All subsequent tests in the file that call `check_password_reset_key` are
 looking for expected errors.  However, they are just `assertInstanceOf`
 using `WP_Error`.  They aren't checking the ''specific'' error.  Since the
 user is cached and the `user_activation_key` is empty, they will pass
 these assertions, albeit for the wrong reasons.

 There is a preceding test case for `test_password_length_limit` in which
 `get_user_by` is also referenced.  That test passses but that's because
 the user cache is reset by the `wp_set_password` before assertions are
 made.

 Let me know if you'd like me to dig into anything else.  Happy to help.

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


More information about the wp-trac mailing list