#38609 closed defect (bug) (fixed)
REST API: `unfiltered_html` and slashing: posts and attachments
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | REST API | Keywords: | has-patch has-unit-tests dev-feedback |
Focuses: | Cc: |
Description
Migrated from https://github.com/WP-API/WP-API/issues/2787, comments paraphrased from @rachelbacker @kadamwhite and @rmccue.
Match WordPress Core's logic when users with the unfiltered_html capability are creating/updating Posts or Comments
- For Posts: Core removes the appropriate
kses
filter from sanitizingpost_content
andpost_title
. - For Comments: Core changes the active
kses
filter used for sanitizingcomment_content
towp_filter_post_kses()
.
See the kses_init()
function in WP Core for the complete logic.
Per discussion in 2016-10-17 API team chat, the API should honor unfiltered_html
; proposed next steps:
- Let's take a look at introducing a new function into core that checks
unfiltered_html
and sanitizes the input appropriately, and this could be used in core rather than theremove_filter
craziness. This depends on back-compat as to whether core can use it, but is a good path forward. - Let's also switch our behaviour ASAP, then use core's behaviour if/when it exists.
This also came up again in recent Slack discussion regarding inserting data:
URIs, which requires unfiltered_html
.
Attachments (6)
Change History (26)
This ticket was mentioned in Slack in #core by jnylen. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-restapi by jnylen. View the logs.
8 years ago
#5
@
8 years ago
@jnylen0 @westonruter and I chatted on a Hangout about this for ~10 minutes. James is going to focus on putting together a variety of test cases to assert expected behavior for handling slashed data / unfiltered_html
behavior. From that, we'll be able to determine which tests are failing and what we need to do to correct the behavior.
For reference, WP-CLI had this problem with slashed data a while back:
#6
@
8 years ago
It looks to me like @westonruter's patch will pretty much fix all the things here, with some minor tweaks.
In 38609.1.diff
:
- Fix creating posts by calling
wp_slash( (array) $post )
instead ofwp_slash( $post )
- Add a bunch of tests for creating and updating posts with the nastiest HTML I could think of
In trunk, 10/12 of these new tests fail (8/12 in multisite mode). With this patch, they all pass in both modes.
This ticket was mentioned in Slack in #core-restapi by jnylen. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by nerrad. View the logs.
8 years ago
#9
@
8 years ago
In 38609.2.diff
:
- Simplify test harness and test rendered content also
- Add tests for attachments
This patch *includes* https://core.trac.wordpress.org/attachment/ticket/38679/38679.diff because they are closely related. Adding tests for attachments here surfaced the issue in #38679, which I think should be patched separately.
There are a couple of "TODO" / "FIXME" comments in the tests. Right now, the rendered description
for media items includes the <p class="attachment">
tag containing the attachment content itself. Perhaps the filters we apply to attachment description should be tweaked, and this should show up in the content
property instead?
This ticket was mentioned in Slack in #core-restapi by jnylen. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jnylen. View the logs.
8 years ago
#13
@
8 years ago
- Summary changed from REST API should honor the `unfiltered_html` capability to REST API: `unfiltered_html` and slashing: posts and attachments
#14
@
8 years ago
38609.3.diff was a bad patch with a couple of failing tests; please ignore it.
38609.4.diff includes the changes to fix unfiltered_html
and slashing for posts and attachments without touching other object types. I'll create other tickets for those.
Note that 38679.diff is still applied here to fix content handling for attachments.
#15
@
8 years ago
- Owner set to rmccue
- Resolution set to fixed
- Status changed from new to closed
In 39155:
I'd started on this, but there are so many kses functions that it ended up being pretty bloated, so I'd suggest we only implement a subset of these. The issue is that
wp_kses
takes a$allowed_html
parameter, which is usually the context. There are wrappers to set this topost
(wp_kses_post
), the current filter (wp_kses_data
), and slashing variants of all of these.I'd propose we add two more functions:
wp_maybe_kses
wp_maybe_kses_post
Both of this shortcircuit if the user has
unfiltered_html
.Unfortunately, due to the highly undefined nature of the existing filters, likely not easy to migrate these over. These filters all use the slashed variant, and rely on the name of the current filter in
wp_filter_kses
, which seems insane.