WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#3420 closed defect (bug) (fixed)

Should not filter class attribute on abbr used in hreview microformat

Reported by: conoro Owned by: rob1n
Milestone: 2.3 Priority: normal
Severity: normal Version: 2.0.5
Component: Administration Keywords:
Focuses: Cc:

Description

I posted this on the "Requests and Feedback" forum but was told it might be more appropriate as a trac ticket. Apologies if it is not.

There are two things currently breaking the posting of hreview (and possibly other microformats) via XMLRPC to wordpress.org/wordpress.com/wpmu blogs. I'll post them as separate tickets.

The first is that kses.php is stripping harmless class attributes which are the core of microformat markup. To allow hreviews to be posted, the following trivial change is required in kses.php.

In allowedposttags, abbr is currently listed as follows:

'abbr' => array ('title' => array ())

this just needs to change to:

'abbr' => array ('title' => array (), 'class' => array ())

Would you consider doing this fix to provide further support for microformats as you already do for XFN? I'll continue to test the other microformats like hcard, hcalendar etc to see if they run into any problems. Thanks.

Attachments (1)

3420.diff (7.8 KB) - added by johnbillion 8 years ago.
Allow the class attribute on an abbr tag

Download all attachments as: .zip

Change History (21)

comment:1 follow-up: @foolswisdom8 years ago

Is the problem described here specific to xml-rpc? Does it relate to the user's role?

My limited understanding is that class is not always harmless, and some CMS systems have a blacklist of values --- though I doubt that is relevant to the abbr attribute.

comment:2 in reply to: ↑ 1 @westi8 years ago

Replying to foolswisdom:

Is the problem described here specific to xml-rpc? Does it relate to the user's role?

I would expect so - if the user has the unfilterd_html cap then the attributes won't be stripped AFAIK.

comment:3 @conoro8 years ago

The fix I suggested applies to all of wordpress.com, wordpress.org and wordpress-mu and would still allow for unfiltered_html to remain disabled for most users.

comment:4 @foolswisdom8 years ago

  • Milestone set to 2.2
  • Summary changed from Posting via xmlrpc breaks microformat markup to Should not filter class attribute on abbr used in hreview microformat

conoro, the reason I asked is because the bug Summary is then not very good for two reasons: (a) it suggests it is specific to a scenario that it is not, "posting via xmlrpc" and (b) it presents a general symptom, "breaks microformats markup".

comment:5 @foolswisdom8 years ago

  • Component changed from XML-RPC to Administration

comment:6 @conoro8 years ago

Sorry about that, was my first bug post. I hadn't realised that entering the markup via the editor causes the same result.

comment:7 @foolswisdom8 years ago

conoro, of course, absolutely no problem. Thank you for participating in WordPress!

@johnbillion8 years ago

Allow the class attribute on an abbr tag

comment:8 follow-up: @johnbillion8 years ago

  • Keywords has-patch added

+1 for allowing the class attribute on abbr in order to allow the hReview microformat. Patch attached.

If someone can provide details of when the class attribute can be dangerous then please post here. I found nothing after 15 minutes of Googling.

comment:9 in reply to: ↑ 8 @foolswisdom8 years ago

  • Milestone changed from 2.2 to 2.3

Replying to johnbillion:

If someone can provide details of when the class attribute can be dangerous then please post here.

Plone's filtering "blacklist for the class attribute: by default it is empty but there is a sample kupu configuration which adds in some class names used by Microsoft Word."
http://plone.org/documentation/how-to/filteringhtml

comment:10 follow-up: @johnbillion8 years ago

It's interesting to see that many other blogging platforms and CMSs disallow the class attribute, but it still doens't explain why the class attribute can be dangerous.

comment:11 in reply to: ↑ 10 ; follow-up: @foolswisdom8 years ago

Replying to johnbillion:

It's interesting to see that many other blogging platforms and CMSs disallow the class attribute,
but it still doens't explain why the class attribute can be dangerous.

I guess the concern is that a user could use an existing CSS class for the web site to fool a visitor to a site.

comment:12 in reply to: ↑ 11 ; follow-up: @johnbillion8 years ago

Replying to foolswisdom:

I guess the concern is that a user could use an existing CSS class for the web site to fool a visitor to a site.

Fools, good point there. I can now see how this may present an opportunity to mislead visitors to a site. I would be confident in saying that a class attribute applied to an abbr element wouldn't present such a risk though. If it did, it would be an edge case.

I think the positive aspects of supporting more microformats outweigh the potential negative aspects mentioned above.

comment:13 in reply to: ↑ 12 ; follow-up: @foolswisdom8 years ago

Replying to johnbillion:

I think the positive aspects of supporting more microformats outweigh the potential negative aspects mentioned above.

If someone is a trusted user of the blog then kses.php is not run, so it seems like there should be nothing currently preventing "supporting more microformats".

comment:14 in reply to: ↑ 13 @johnbillion8 years ago

Replying to foolswisdom:

If someone is a trusted user of the blog then kses.php is not run

Kses is run for users with the role of Author.

comment:15 @johnbillion8 years ago

  • Keywords dev-fedback 2nd-opinion added

How about it then.

comment:16 @johnbillion8 years ago

  • Keywords dev-feedback added; dev-fedback removed

comment:17 @foolswisdom8 years ago

-1 I don't think this is worthwhile, without having someone that understands the possible class attribute security to eliminate those concerns. Seems likely that a blacklisting functionality would then be necessary.

comment:18 @andy8 years ago

  • Keywords commit added; dev-feedback 2nd-opinion removed

There is no security-related reason not to allow the class attribute on abbr. I believe it was an unintended omission. The patch is okay.

comment:19 @rob1n8 years ago

  • Keywords has-patch commit removed
  • Owner changed from anonymous to rob1n
  • Status changed from new to assigned

Patch needs refreshing.

comment:20 @rob1n8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [5383]) Don't take out class attribute for <abbr />, for hReview. fixes #3420

Note: See TracTickets for help on using tickets.