-
Notifications
You must be signed in to change notification settings - Fork 58
fix(guest-contributors): preserve special characters in guest contributor display names #4411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(guest-contributors): preserve special characters in guest contributor display names #4411
Conversation
…utor display names
leogermani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!
Let me share what I did to check that this change was safe.
I looked at the hook that this function is tied to: https://developer.wordpress.org/reference/hooks/user_profile_update_errors/, and checked the source on WordPress to see where it was invoked.
It's invoked in https://developer.wordpress.org/reference/functions/edit_user/
My concern was that perhaps this hook could also be triggered in other contexts where there was nothing in the $_POST, like direct CLI commands or other cases. But looks like we're safe, edit_user() is precisely used to process form submissions and reads stuff from $_POST
(You might have gone the same route, just sharing my line of thought here as it might be helpful to see what I was looking for)
|
Excellent. Thank you @leogermani!
Thank you for sharing all of the details on where to directly look! Very helpful to hear/read your process. One thought does come to mind this morning... the code is currently this Should we safeguard this and change that first line to the following, replacing '' with a better fallback of $user->user_login This scenario shouldn't happen, but it feels safer to do it. The Display Name will lack accents, of course, but that's better than the potential of setting $user->user_login to ''. Should I make this modification? |
|
Mases sense @wil-gerken , I think it's a good change, yes. |
|
Done. That's now in place. |
|
Hey @wil-gerken, good job getting this PR merged! 🎉 Now, the Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label. If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label. Thank you! ❤️ |
All Submissions:
Changes proposed in this Pull Request:
This PR fixes a bug where special characters (accented letters, diacritics, punctuation, etc.) were being stripped from Guest Contributor display names during user creation.
NPPM-2360The root cause
When creating a Guest Contributor, the Display name value shares the username field. Because the username is passed through
sanitize_user(), accented characters and other special characters are stripped from the Display name as well.The proposed solution
$_POST['user_login']before WordPress appliessanitize_user()sanitize_text_field()withwp_unslash(), which preserves international characters while still preventing XSS/HTML injectionsanitize_user()for the generated username — no change hereHow to test the changes in this Pull Request:
Before applying this PR (to reproduce the issue):
Iñigo Montoya)inigo-montoya)After applying this PR:
Pepé Le Pew)pepe-le-pew)Please also test with additional names and characters. For example:
Sévêrüs Snāpê, Ph.D.Security Considerations:
This fix uses
wp_unslash()andsanitize_text_field()to safely process user input while preserving accented characters. This approach prevents XSS and HTML injection while allowing legitimate display names with accents, diacritics, and punctuation.Note on
phpcs:ignoreA
phpcs:ignore WordPress.Security.NonceVerification.Missingcomment was added for this change.This logic runs on the
user_profile_update_errorshook, which is triggered fromwp-admin/user-new.phpafter WordPress has already verified the nonce. Since this hook does not have direct access to the earlier nonce verification, the ignore comment seems appropriate here, but I wanted to explicitly call it out for review.Other information: