Opened 10 years ago
Closed 7 years ago
#5971 closed enhancement (maybelater)
Usage of wp_filter_kses is inconsistent for XProfile fields
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Priority: | normal | |
| Severity: | normal | Version: | 2.1 |
| Component: | Extended Profile | Keywords: | needs-patch, trac-tidy-2018 |
| Cc: |
Description
In bp-xprofile-admin.php on line 246 wp_filter_kses is applied to the field description before saving. This is redundant as the xprofile_field_description_before_save filter already has wp_filter_kses attached. The same goes for the field name.
On the other hand fieldtype and required are ksesed in bp-xprofile-admin.php but do not have wp_filter_kses attached as a filter.
Wouldn't it be better from an encapsulation perspective to do all the input sanitizing in the save function of the BP_XProfile_Field class?
Additionally this makes it just a little bit harder to use the wp_kses_allowed_html filter to allow more html in the description as one has to watch for 2 contexts.
Change History (4)
#2
@
10 years ago
- Keywords needs-patch added
I don't think there's a non-terrible way to test xprofile_admin_manage_field(), but I definitely don't see any harm in removing the redundant calls there.
#3
@
7 years ago
- Keywords trac-tidy-2018 added
We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.
Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.
If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.
For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/
I agree. :)
The change doesn't look too complicated to make. Since we're messing with kses, I would like us to add an integration test here, but I don't know if that's possible as the function is messy. Perhaps we just document a list of things on this ticket that we need test manually before/after making the change. Maybe @boonebgorges has thoughts on if/how/should we be testing this.