[wp-trac] [WordPress Trac] #56504: `sanitize_html_class()` is both too restrictive, and too permissive so it may return an invalid class name

WordPress Trac noreply at wordpress.org
Mon Sep 5 01:14:52 UTC 2022


#56504: `sanitize_html_class()` is both too restrictive, and too permissive so it
may return an invalid class name
-------------------------------------------------+-------------------------
 Reporter:  anrghg                               |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  Awaiting
                                                 |  Review
Component:  General                              |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  close changes-requested needs-       |     Focuses:
  testing needs-dev-note needs-I18N-review       |
-------------------------------------------------+-------------------------
Changes (by anrghg):

 * keywords:  close => close changes-requested needs-testing needs-dev-note
     needs-I18N-review


Comment:

 Replying to [comment:3 peterwilsoncc]:
 > It is possible to use a number within CSS by properly escaping it, for
 example `class="300"` can be styled by using `.\33 00 {}`.

 Good point, thank you. `.\00003300 {}` would be an alternative working
 better in code highlighters. But both have the downside of getting class
 names look different in HTML than in CSS.

 > As classes are case sensitive, it would be problematic to convert them
 to lower case in this function.

 Indeed I missed this point. The argument is likely to be already lowercase
 anyway.

 > To cover the side note about title, `sanitize_title()` is applied to
 slugs and converts characters to lowercase:

 I know at least one website (Quora) that uses titlecase in slugs. It looks
 much better. But it seems to require support from the server; I got
 randomly 404 errors (no error for ‘Titlecase’, but errors for other
 slugs). As I’m filtering `sanitize_title()` to allow titlecase in fragment
 identifiers, I’ll need to add an opt-in to apply it to slugs.

 > The purpose of the sanitization and escaping functions in WordPress is
 to make code safe rather than validate it.

 WordPress is preferring a non-conformant way, throwing the baby with the
 bathwater.

 Since page slugs are used as class names, all scripts should be equal:
 Latin, Greek, Cyrillic, all 160 (number growing) Non-Latin scripts already
 supported by Unicode.

 > My inclination is to close this issue as `sanitize_html_class()` is
 achieving it's goal of making the class names safe.

 At the expense of usability, equity, internationalization and
 localization.

 Please do not close this issue without fixing it, e.g. by positive
 deletion like so ([Per spec](https://www.w3.org/International/questions
 /qa-escapes#css_other) we can even use all these symbols and punctuation
 provided they are backslash-escaped. This too prevents malicious code from
 running.)
 {{{
 $class = preg_replace( '/(?<!\\)[%^{}~@`\'"&#$()+[\]|\/*<>=?;:!,.]/', '',
 $class );
 }}}

 Don’t you think that this achieves `sanitize_html_class()`’s goal in a
 better way?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/56504#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list