Skip to content

KSES: Allow Custom Data Attributes having dashes in their dataset name. #6429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dmsnell wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

KSES: Allow Custom Data Attributes having dashes in their dataset name. #6429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions src/wp-includes/kses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1529,24 +1529,27 @@ function wp_kses_attr_check( &$name, &$value, &$whole, $vless, $element, $allowe

$allowed_attr = $allowed_html[ $element_low ];

/*
* Allow Custom Data Attributes (`data-*`).
*
* When specifying `$allowed_html`, the attribute name should be set as
* `data-*` (not to be mixed with the HTML 4.0 `data` attribute, see
* https://www.w3.org/TR/html40/struct/objects.html#adef-data).
*
* Custom data attributes appear on an HTML element in the `dataset`
* property and are available from JavaScript with a transformed name.
*
* @see https://html.spec.whatwg.org/#custom-data-attribute
*/
if ( ! isset( $allowed_attr[ $name_low ] ) || '' === $allowed_attr[ $name_low ] ) {
/*
* Allow `data-*` attributes.
*
* When specifying `$allowed_html`, the attribute name should be set as
* `data-*` (not to be mixed with the HTML 4.0 `data` attribute, see
* https://www.w3.org/TR/html40/struct/objects.html#adef-data).
*
* Note: the attribute name should only contain `A-Za-z0-9_-` chars.
*/
if ( str_starts_with( $name_low, 'data-' ) && ! empty( $allowed_attr['data-*'] )
&& preg_match( '/^data-[a-z0-9_-]+$/', $name_low, $match )
) {
$dataset_name = wp_js_dataset_name( $name );
if ( isset( $dataset_name ) && ! empty( $allowed_attr['data-*'] ) ) {
/*
* Add the whole attribute name to the allowed attributes and set any restrictions
* for the `data-*` attribute values for the current element.
* The attribute name passed in here is the `data-*` name, or the name in
* the raw HTML. Add it to the set of allowed attributes and adopt the
* restrictions applied to all custom data attributes for the element.
*/
$allowed_attr[ $match[0] ] = $allowed_attr['data-*'];
$allowed_attr[ $name_low ] = $allowed_attr['data-*'];
} else {
$name = '';
$value = '';
Expand Down
111 changes: 100 additions & 11 deletions tests/phpunit/tests/kses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1441,25 +1441,114 @@ public function data_safecss_filter_attr() {
/**
* Data attributes are globally accepted.
*
* @ticket 33121
* @ticket 61501
*
* @dataProvider data_data_attributes_and_whether_they_are_allowed
*
* @param string $attribute_name Custom data attribute, e.g. "data-wp-bind--enabled".
* @param bool $is_allowed Whether the given attribute should be allowed.
*/
public function test_wp_kses_attr_data_attribute_is_allowed() {
$test = '<div data-foo="foo" data-bar="bar" datainvalid="gone" data-two-hyphens="remains">Pens and pencils</div>';
$expected = '<div data-foo="foo" data-bar="bar" data-two-hyphens="remains">Pens and pencils</div>';
public function test_wp_kses_attr_boolean_data_attribute_is_allowed( string $attribute_name, bool $is_allowed ) {
$element = "<div {$attribute_name}>Pens and pencils.</div>";
Copy link
Contributor

@peterwilsoncc peterwilsoncc May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also test "<div {$attribute_name}="populated">Pens and pencils.</div>" -- either in this test or another one using the same data provider.

Copy link
Member Author

@dmsnell dmsnell May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do! Out of curiosity, why do this, since kses doesn't make any distinction for whether to allow a data attribute or not based on its value?

Copy link
Contributor

@peterwilsoncc peterwilsoncc May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why do this, since kses doesn't make any distinction for whether to allow a data attribute or not based on its value?

Mainly as a defensive strategy in case a future maintainer (quite possibly us) makes a mistake. The execution path of KSES is a little very confusing.

Copy link
Member Author

@dmsnell dmsnell May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a second test for value-containing attributes in 0498bad


$this->assertEqualHTML( $expected, wp_kses_post( $test ) );
$processor = new WP_HTML_Tag_Processor( $element );
$processor->next_tag();

$this->assertTrue(
$processor->get_attribute( $attribute_name ),
"Failed to find expected attribute '{$attribute_name}' before filtering: check test."
);

$processor = new WP_HTML_Tag_Processor( wp_kses_post( $element ) );
$this->assertTrue(
$processor->next_tag(),
'Failed to find containing tag after filtering: check test.'
);

if ( $is_allowed ) {
$this->assertTrue(
$processor->get_attribute( $attribute_name ),
"Allowed custom data attribute '{$attribute_name}' should not have been removed."
);
} else {
$this->assertNull(
$processor->get_attribute( $attribute_name ),
"Should have removed un-allowed custom data attribute '{$attribute_name}'."
);
}
}

/**
* Data attributes with leading, trailing, and double "-" are globally accepted.
* Ensures that only allowable custom data attributes with values are retained.
*
* @ticket 33121
*
* @dataProvider data_data_attributes_and_whether_they_are_allowed
*
* @ticket 61052
* @param string $attribute_name Custom data attribute, e.g. "dat-wp-bind--enabled".
* @param bool $is_allowed Whether the given attribute should be allowed.
*/
public function test_wp_kses_attr_data_attribute_hypens_allowed() {
$test = '<div data--leading="remains" data-trailing-="remains" data-middle--double="remains">Pens and pencils</div>';
$expected = '<div data--leading="remains" data-trailing-="remains" data-middle--double="remains">Pens and pencils</div>';
public function test_wp_kses_attr_data_attribute_is_allowed( string $attribute_name, bool $is_allowed ) {
$element = "<div {$attribute_name}='shadows and dust'>Pens and pencils.</div>";

$this->assertEqualHTML( $expected, wp_kses_post( $test ) );
$processor = new WP_HTML_Tag_Processor( $element );
$processor->next_tag();

$this->assertIsString(
$processor->get_attribute( $attribute_name ),
"Failed to find expected attribute '{$attribute_name}' before filtering: check test."
);

$processor = new WP_HTML_Tag_Processor( wp_kses_post( $element ) );
$this->assertTrue(
$processor->next_tag(),
'Failed to find containing tag after filtering: check test.'
);

if ( $is_allowed ) {
$this->assertIsString(
$processor->get_attribute( $attribute_name ),
"Allowed custom data attribute '{$attribute_name}' should not have been removed."
);
} else {
$this->assertNull(
$processor->get_attribute( $attribute_name ),
"Should have removed un-allowed custom data attribute '{$attribute_name}'."
);
}
}

/**
* Data provider.
*
* @return array[].
*/
public static function data_data_attributes_and_whether_they_are_allowed() {
return array(
// Allowable custom data attributes.
'Normative attribute' => array( 'data-foo', true ),
'Non-consecutive dashes' => array( 'data-two-hyphens', true ),
'Double-dashes' => array( 'data--double-dash', true ),
'Trailing dash' => array( 'data-trailing-dash-', true ),
'Uppercase alphas' => array( 'data-Post-ID', true ),
'Bind Directive' => array( 'data-wp-bind--enabled', true ),
'Single-dash suffix' => array( 'data-after-', true ),
'Double-dash prefix' => array( 'data--before', true ),
'Double-dash suffix' => array( 'data-after--', true ),
'Double-dashes everywhere' => array( 'data--one--two--', true ),
'Underscore' => array( 'data-over_under', true ),

// Not custom data attributes.
'No data- prefix' => array( 'post-id', false ),
'No dash after prefix' => array( 'datainvalid', false ),

// Un-allowable custom data attributes.
'Nothing after prefix' => array( 'data-', false ),
'Whitespace after prefix' => array( "data-\u{2003}", false ),
'Emoji in name' => array( 'data-🐄', false ),
'Brackets' => array( 'data-[enabled]', false ),
'Colon' => array( 'data-wp:bind', false ),
);
}

/**
Expand Down
Loading