WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 18 months 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 20 months ago.
Parity with wordpress.com
20210.2.diff (12.9 KB) - added by ryan 20 months ago.
Introduce wp_kses_allowed_html()
20210-allowed-html-ut.diff (1.1 KB) - added by ryan 20 months ago.
20210.3.diff (12.9 KB) - added by ryan 20 months ago.
20210.4.diff (13.1 KB) - added by ryan 20 months 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 20 months ago.

Download all attachments as: .zip

Change History (24)

comment:1 westi2 years ago

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

comment:2 westi2 years ago

  • Description modified (diff)

comment:3 in reply to: ↑ description duck_2 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: westi2 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 nacin2 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 solarissmoke2 years ago

Related: #16713

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

comment:8 in reply to: ↑ 4 scribu20 months 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

ryan20 months ago

Parity with wordpress.com

comment:9 ryan20 months 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.

ryan20 months ago

Introduce wp_kses_allowed_html()

comment:11 ryan20 months ago

  • Introduce wp_kses_allowed_html() 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_html() dynamicallly 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 passing a 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_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.
  • @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_html() 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.

Last edited 20 months ago by ryan (previous) (diff)

ryan20 months ago

comment:12 ryan20 months ago

BTW, this passes unit tests but is otherwise untested.

comment:13 ryan20 months 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

ryan20 months ago

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

ryan20 months ago

comment:14 ryan20 months 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 ryan20 months ago

  • Milestone changed from Future Release to 3.5

comment:17 nacin20 months ago

In [21796]:

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

comment:18 ryan18 months ago

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