WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#20210 closed defect (bug) (fixed)

Always allow the standard attributes for all elements when filtering content using kses for posts.

Reported by: westi Owned by:
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4
Component: Formatting Keywords: needs-patch
Focuses: Cc:

Description (last modified by westi)

The list of allowed attributes to configure kses for post content filtering is hit and miss.

We should always allow the standard attributes: class, id, style and title.

We should also make any other extend attributes consistent.

Related: #18649, #18133 / #17977

Attachments (6)

20210.diff (5.1 KB) - added by ryan 2 years ago.
Parity with wordpress.com
20210.2.diff (12.9 KB) - added by ryan 2 years ago.
Introduce wp_kses_allowed_html()
20210-allowed-html-ut.diff (1.1 KB) - added by ryan 2 years ago.
20210.3.diff (12.9 KB) - added by ryan 2 years ago.
20210.4.diff (13.1 KB) - added by ryan 2 years ago.
Run _wp_add_global_attributes() one time only. Handle an array being passed to wp_kses_allowed_html().
20210.5.diff (13.4 KB) - added by ryan 2 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 @westi3 years ago

[UT569] is a start on some tests for these kind of issues.

comment:2 @westi3 years ago

  • Description modified (diff)

comment:3 in reply to: ↑ description @duck_3 years ago

Good idea.

What would be nice is if we can come up with a way adding these globally allowed attributes on the fly (e.g. in wp_kses_attr()) rather than having to specify them over and over again that monster global array. The problem there is continuing to disallow them for commenters.

Replying to westi:

We should always allow the standard attributes: class, id, style and title.

For reference: http://dev.w3.org/html5/spec/Overview.html#global-attributes

I think the four mentioned here are the best to start with.

comment:4 follow-up: @westi3 years ago

While it would be nice to specify them on the fly it would break any code which overwrites the global.

I guess we could build the global array using a function at include time though.

comment:5 @nacin3 years ago

I chatted a bit with ryan (in the context of #17977) about updating kses to stop relying on global arrays, and in general make customization here suck a bit less. We had some ideas but decided not to run with it for 3.4. It would be nice to change how things work rather than hacking around the existing global arrays.

comment:6 @solarissmoke3 years ago

Related: #16713

Can probably close that ticket as one specific instance of the issue raised here.

comment:8 in reply to: ↑ 4 @scribu2 years ago

  • Keywords needs-patch added

Replying to westi:

While it would be nice to specify them on the fly it would break any code which overwrites the global.

I guess we could build the global array using a function at include time though.

That's exactly what I was thinking of. +1

@ryan2 years ago

Parity with wordpress.com

comment:9 @ryan2 years ago

That's what wp.com has been running with for a long while. It adds a few tags and adds some of the global attributes to various tags.

@ryan2 years ago

Introduce wp_kses_allowed_html()

comment:11 @ryan2 years ago

  • Introduce wp_kses_allowed_htm() which accepts a context string and returns an array of allowed tags.
  • Removes explicit declarations of class, id, style, and title from $allowedposttags
  • wp_kses_allowed_htm() dynamically adds the global attributes to every tag for the 'post' context
  • No longer calls wp_kses_array_lc() every time wp_kses() runs. Instead it runs once if CUSTOM_TAGS is true. Plugins directly passinga custom allowed_html array would no longer get the lc treatment. We need to see if that would be a problem.
  • wp_kses_data() and wp_filter_kses() pass current_filter() for the $allowed_html argument to wp_kses().
  • wp_kses_allowed_htm() handles being passed a filter name for a context. If the filter is not a recognized one it defaults to using $allowedtags as was done before for wp_kses_data() and wp_filter_kses().
  • wp_kses_allowed_htm() recognizes user_description and pre_user_description out of the box. For these it takes $allowedtags and inserts rel attribute support.
  • wp_kses_allowed_htm() allows plugins to override the return values for the default contexts and support arbitrary context via a wp_kses_allowed_tags filter.
  • Maybe @todo wp_kses_hook() can now pass a string context for $allowed_html to the pre_kses filter. We might have to pass the result of wp_kses_allowed_htm() instead if it turns out that plugins are digging in $allowed_html.
  • @todo There are several spots in $allowedposttags where array() can be replaced with true. If we go with this I'll tidy that up.

So, basically, $allowed_html can now be passed around as a string context that wp_kses_allowed_htm() will turn into the proper array when needed. This reduces a lot of array passing and avoids making new globals for every possible context. Some of the kses filter functions now pass current_filter() as the $allowed_html context allowing us to support custom allowed html for any field in any filter context. wp_kses_allowed_html() is sort of like default-filters.php for kses.

Version 1, edited 2 years ago by ryan (previous) (next) (diff)

@ryan2 years ago

@ryan2 years ago

comment:12 @ryan2 years ago

BTW, this passes unit tests but is otherwise untested.

comment:13 @ryan2 years ago

Also, instead of calling _wp_add_global_attributes against $allowedposttags every time wp_kses_allowed_html() runs it can be run once against $allowedposttags inside the ! CUSTOM_TAGS block. @todo

@ryan2 years ago

Run _wp_add_global_attributes() one time only. Handle an array being passed to wp_kses_allowed_html().

@ryan2 years ago

comment:14 @ryan2 years ago

In [21790]:

  • Introduce wp_kses_allowed_html() which accepts a context string and returns an array of allowed tags.
  • Remove explicit declarations of class, id, style, and title from $allowedposttags
  • Dynamicallly add global attributes to every tag for the 'post' context
  • No longer calls wp_kses_array_lc() every time wp_kses() runs. Instead it runs once if CUSTOM_TAGS is true. Plugins directly passing a custom allowed_html array will no longer get the lc treatment. Keep an eye out for problems with this.
  • wp_kses_data() and wp_filter_kses() pass current_filter() for the $allowed_html argument to wp_kses().
  • wp_kses_allowed_html() handles being passed a filter name for a context. If the filter is not a recognized one it defaults to using $allowedtags as was done before for wp_kses_data() and wp_filter_kses().
  • wp_kses_allowed_html() recognizes user_description and pre_user_description out of the box. For these it takes $allowedtags and inserts rel attribute support.
  • wp_kses_allowed_html() allows plugins to override the return values for the default contexts and support arbitrary contexts via a wp_kses_allowed_html filter.
  • wp_kses_hook() can now pass a string context for $allowed_html to the pre_kses filter. We might have to pass the result of wp_kses_allowed_html() instead if it turns out that plugins are digging in $allowed_html.

fixes #17977
see #20210

comment:16 @ryan2 years ago

  • Milestone changed from Future Release to 3.5

comment:17 @nacin2 years ago

In [21796]:

Recommend the wp_kses_allowed_html filter over CUSTOM_TAGS. Soft deprecate CUSTOM_TAGS. see #17977, #20210.

comment:18 @ryan2 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.