Problem/Motivation
HTML5 custom data attribute names containing numerals are getting mangled by \Drupal\Component\Utility\Xss::attributes(). This occurs in Xss::attributes() because the regex pattern in $mode 0 does not match numerals.
Reproduce by running <a data-a2a-url="foo"></a> through Xss::filter:
print_r(\Drupal\Component\Utility\Xss::filter('<a data-a2a-url="foo"></a>'));
The data attribute name is partly stripped from the first numeral:
<a a-url="foo"></a>
Proposed resolution
In attribute names, permit numerals after the first character, because the HTML5 specification permits numerals in custom data attribute names, such as data-j2-range (an example from the spec).
Remaining tasks
Patch needed
Review needed
User interface changes
None.
API changes
None.
Comments
Comment #2
micropat commentedComment #3
micropat commentedComment #4
dawehner@micropat It would be great if you could expand the test coverage on
\Drupal\Tests\Component\Utility\XssTestfor that.Comment #5
star-szrYes we should have some tests. Thanks!
Comment #6
cilefen commentedComment #8
star-szrWhat's the upstream bugfix needed? Patch is ready to go I'd say but not sure when it can get in…
Comment #10
micropat commentedFixed my tag (oops). This issue is at least blocking the AddToAny contrib module. Anyone else think the patch qualifies for rc target triage?
Comment #11
chx commentedI am sorry but this is a classic example of a regex where the backtracking will break down horribly because there are so many ways this can match. You wanted
[-a-zA-Z][-a-zA-Z0-9]*.Comment #12
dawehnerhttp://www.regular-expressions.info/catastrophic.html explains this a bit better, thank you @chx for the review + pointing to the URL.
Comment #13
chx commentedWell it's not that bad because this is only O(n^2) as there's only one loop but still: the simpler regexp is just O(n).
Comment #14
star-szrExcellent point, I agree!
Comment #15
micropat commentedThanks @chx!
Comment #16
chx commentedGreat, thanks!
Comment #18
star-szrBot
Comment #20
alexpottBased on @xjm's post release triage document, committed 3e28af5 and pushed to 8.0.x and 8.1.x. Thanks!
Comment #23
David_Rothstein commentedThis seems like it should be backported to Drupal 7 - I created #2847553: XSS attribute handling mangles valid attribute names containing numbers (D7 backport) to do that.