Opened 14 years ago
Last modified 5 years ago
#15001 new defect (bug)
Duplication and incompatibilities in register_new_user() and wp_insert_user()
Reported by: | coffee2code | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.0 |
Component: | Users | Keywords: | has-patch needs-testing needs-refresh needs-unit-tests |
Focuses: | Cc: |
Description
As a result of [12778], the commit of a patch that was part of #11644 (the MU-merge ticket), wp_insert_user()
was modified to introduce user verification checks. The addition of these checks also introduced a number of duplications and incompatibilities between it and register_new_user()
. (Bear in mind that register_new_user()
calls wp_create_user()
which calls wp_insert_user()
.)
These issues include:
- Duplication (both run-time execution and code): both functions perform
username_exists()
andemail_exists()
checks. Ideally, we should only perform each check once, and from a single piece of code. - Whereas both functions perform
username_exists()
andemail_exists()
checks,register_new_user()
performs more checks (empty_username, invalid_username, empty_email, invalid_email). If the former 2 are being checked, all 6 criteria should be checked. wp_insert_user()
can now return a WP_Error object, butregister_new_user()
can't handle it (I reported this in #14290)register_new_user()
generates a new generic error ifwp_create_user()
(viawp_insert_user()
) returns an error (assuming proper patch #14290), rather than passing along the more specific error it was told about
If an error is returned by wp_create_user()
, register_new_user()
throws a generic 'registerfail' error ('<strong>ERROR</strong>: Couldn’t register you... please contact the <a href="mailto:%s">webmaster</a> !'
). However, most likely it received a more informative WP_Error object.
register_new_user()
allows errors to be suppressed via filters, butwp_insert_user()
does not
register_new_user()
has the filter 'registration_errors' which allows plugins to suppress any encountered errors. Having done so, register_new_users()
's subsequent call to wp_create_user()
can generate an error (of the type already suppressed) which is then un-suppressable. This renders the 'registration_errors' filter useless for the username_exists and email_exists errors (and possibly other in the future).
These different issues may warrant separate tickets, but cumulatively I think they point to the need to refactor register_new_user()
and wp_insert_user()
to be more efficient and compatible.
This is a rare instance where I don't include an immediate patch for a ticket, but I wanted to get it out there and see if we can try and get these fixed for 3.1.
Attachments (2)
Change History (10)
#2
@
14 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
#3
@
12 years ago
- Keywords has-patch needs-testing added; needs-patch removed
Combined code from both functions - passes unit tests / needs more testing. Patch attached.
#4
@
11 years ago
- Milestone changed from Future Release to 3.7
Refreshed against trunk, I think this is still a good move - the code between the two funcs should be in sync
Your considerations look very useful to me. Maybe starting with removing code duplication and putting the shared code into functions?