#54987 closed defect (bug) (fixed)
Check if 'user_nicename' is not too long after the 'pre_user_nicename' filter
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Users | Keywords: | good-first-bug has-patch |
Focuses: | Cc: |
Description
Background: #33793, #44107.
In wp_insert_user()
, there are a few checks to make sure the values are not too long to fit into the database:
user_login
may not be longer than 60 characters.user_nicename
may not be longer than 50 characters.user_url
may not be longer than 100 characters.
Some history: [34218], [34626], [52650].
While the user_login
and user_url
checks run after the pre_user_login
and pre_user_url
filters, respectively, the user_nicename
check for some reason runs before the pre_user_nicename
filter.
For consistency with the other filters, and to make sure the filtered value is still no longer than 50 characters, the mb_strlen( $user_nicename ) > 50
check should run after the pre_user_nicename
filter.
Attachments (6)
Change History (21)
This ticket was mentioned in PR #2244 on WordPress/wordpress-develop by MuhammadFaizanHaidar.
3 years ago
#1
- Keywords has-patch added; needs-patch removed
Added Check if 'user_nicename' is not too long after the 'pre_user_nicename' filter in user.php function wp_insert_user( $userdata )
Trac ticket: https://core.trac.wordpress.org/ticket/54987
#2
@
3 years ago
@SergeyBiryukov did not notice it already had a patch but I have removed size check from earlier location and added a more strict check after the 'pre_user_nicename' filter
Following added.
<?php // Remove any non-printable chars from the user_nicename string to see if we have ended up with an empty username. $user_nicename = trim( $user_nicename ); // user_nicename must be between 0 and 50 characters. if ( empty( $user_nicename ) ) { return new WP_Error( 'empty_user_nicename', __( 'Cannot create a user with an empty nicename.' ) ); } elseif ( mb_strlen( $user_nicename ) > 50 ) { return new WP_Error( 'user_nicename_too_long', __( 'Nicename may not be longer than 50 characters.' ) ); }
#3
@
3 years ago
Hi there!
54987.2.patch introduce an other approach for the solution. Please have a look
#4
@
3 years ago
@mukesh27 I think user_nicename check should run after after the 'pre_user_nicename' filter according to the ticket.
#5
@
3 years ago
@mukesh27
Please review my patch as @muhammadfaizanhaidar.
What's happen when any one post user_nicename more then 50 caracter using the filter pre_user_nicename, So code condition come after the filter code.
#6
@
3 years ago
As per the comment, I have already checked pre_user_nicename and pre_user_url also checked pre_user_login filter. pre_user_login already validate that's why I just added validation after pre_user_nicename filter and pre_user_url filter.
This ticket was mentioned in Slack in #core by csesumonpro. View the logs.
3 years ago
This ticket was mentioned in Slack in #core by csesumonpro. View the logs.
3 years ago
#10
follow-up:
↓ 12
@
3 years ago
Thanks for the patches! It looks like most of them have changes that are a bit out of scope for this ticket:
- 54987.patch introduces a duplicate check instead of moving the existing one.
- 54987.2.patch moves the existing check, but no longer throws a
WP_Error
if the nicename is too long. - 54987.diff has some formatting changes and renames a variable, which does not seem necessary here. It also adds validation for
$user_url
, but that was already added in [52650] / #44107. - PR #2244 introduces a new check that
$user_nicename
cannot be empty. I think this would be better discussed in a separate ticket.
For this ticket, I was thinking of just moving the existing check after the filter, like in 54987.2.diff.
This ticket was mentioned in Slack in #core by csesumonpro. View the logs.
3 years ago
#12
in reply to:
↑ 10
@
3 years ago
Replying to SergeyBiryukov:
- PR #2244 introduces a new check that
$user_nicename
cannot be empty. I think this would be better discussed in a separate ticket.
I don't think this worth another ticket, there is already a control above to make sure the nicename won't be empty
For this ticket, I was thinking of just moving the existing check after the filter, like in 54987.2.diff.
Agree. This LGTM.
dream-encode commented on PR #2244:
3 years ago
#14
Thanks for the PR! This was merged into Core in https://core.trac.wordpress.org/changeset/52954.
MuhammadFaizanHaidar commented on PR #2244:
3 years ago
#15
Thanks for the PR! This was merged into Core in https://core.trac.wordpress.org/changeset/52954.
Pleasure is all mine.
As per comment i have added a condiiton after pre_user_nicename filter