Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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: 33 additions & 0 deletions src/wp-includes/kses.php
Original file line number Diff line number Diff line change
Expand Up @@ -2756,6 +2756,27 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
'list-style-image',
);

/*
* CSS attributes that accept rgb(a) color data types.
Comment thread
t-hamano marked this conversation as resolved.
*/
$css_color_data_types = array(
'color',

'border',
'border-color',
'border-right',
'border-right-color',
'border-bottom',
'border-bottom-color',
'border-left',
'border-left-color',
'border-top',
'border-top-color',

'background',
'background-color',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another to add to the list: box-shadow. With some sorting:

Suggested change
'color',
'border',
'border-color',
'border-right',
'border-right-color',
'border-bottom',
'border-bottom-color',
'border-left',
'border-left-color',
'border-top',
'border-top-color',
'background',
'background-color',
'background',
'background-color',
'box-shadow',
'border',
'border-color',
'border-right',
'border-right-color',
'border-bottom',
'border-bottom-color',
'border-left',
'border-left-color',
'border-top',
'border-top-color',
'color',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in c814d87

);

/*
* CSS attributes that accept gradient data types.
*
Expand All @@ -2779,6 +2800,7 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
$css_test_string = $css_item;
$found = false;
$url_attr = false;
$color_attr = false;
$gradient_attr = false;
$is_custom_var = false;

Expand All @@ -2798,6 +2820,7 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
$found = true;
$url_attr = in_array( $css_selector, $css_url_data_types, true );
$gradient_attr = in_array( $css_selector, $css_gradient_data_types, true );
$color_attr = in_array( $css_selector, $css_color_data_types, true );
}

if ( $is_custom_var ) {
Expand Down Expand Up @@ -2840,6 +2863,16 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
}
}

if ( $found && $color_attr ) {
$css_value = trim( $parts[1] );
$comma_syntax = '/^rgba?\(\s*([+-]?\d*\.?\d+)(%)?\s*,\s*([+-]?\d*\.?\d+)(%)?\s*,\s*([+-]?\d*\.?\d+)(%)?\s*(?:,\s*([+-]?\d*\.?\d+)(%)?\s*)?\)$/i';
$space_syntax = '/^rgba?\(\s*([+-]?\d*\.?\d+)(%)?\s+([+-]?\d*\.?\d+)(%)?\s+([+-]?\d*\.?\d+)(%)?\s*(?:\/\s*([+-]?\d*\.?\d+)(%)?\s*)?\)$/i';

if ( preg_match( $comma_syntax, $css_value ) || preg_match( $space_syntax, $css_value ) ) {
$css_test_string = str_replace( $css_value, '', $css_test_string );
}
}

if ( $found ) {
/*
* Allow CSS functions like var(), calc(), etc. by removing them from the test string.
Comment thread
t-hamano marked this conversation as resolved.
Expand Down
92 changes: 86 additions & 6 deletions tests/phpunit/tests/kses.php
Original file line number Diff line number Diff line change
Expand Up @@ -1203,15 +1203,30 @@ public function data_safecss_filter_attr() {
'css' => 'height: expression( body.scrollTop + 50 + "px" )',
'expected' => '',
),
// RGB color values are not allowed.
// RGBA background color are allowed.
Comment thread
t-hamano marked this conversation as resolved.
Outdated
array(
'css' => 'color: rgb( 100, 100, 100 )',
'expected' => '',
'css' => 'background-color: rgba(0,0,0,0)',
'expected' => 'background-color: rgba(0,0,0,0)',
),
// RGBA color values are not allowed.
array(
'css' => 'color: rgb( 100, 100, 100, .4 )',
'expected' => '',
'css' => 'background-color: rgba(0, 0, 0, 0)',
'expected' => 'background-color: rgba(0, 0, 0, 0)',
),
array(
'css' => 'background-color: rgba(100, 100, 100, 0)',
'expected' => 'background-color: rgba(100, 100, 100, 0)',
),
array(
'css' => 'background-color: rgba(10%, 10%, 10%, 0)',
'expected' => 'background-color: rgba(10%, 10%, 10%, 0)',
),
array(
'css' => 'background-color: rgba(0, 0, 0, 0.1)',
'expected' => 'background-color: rgba(0, 0, 0, 0.1)',
),
array(
'css' => 'background-color: rgba(0, 0, 0, .1)',
'expected' => 'background-color: rgba(0, 0, 0, .1)',
),
// Allow min().
array(
Expand Down Expand Up @@ -1333,6 +1348,71 @@ public function data_safecss_filter_attr() {
'css' => 'gap: 10px;column-gap: 5px;row-gap: 20px',
'expected' => 'gap: 10px;column-gap: 5px;row-gap: 20px',
),
// RGB color.
array(
'css' => 'color: rgb(255, 0, 0)',
'expected' => 'color: rgb(255, 0, 0)',
),
array(
'css' => 'color: rgb(255 0 0)',
'expected' => 'color: rgb(255 0 0)',
),
array(
'css' => 'color: rgb(100%, 0%, 50%)',
'expected' => 'color: rgb(100%, 0%, 50%)',
),
array(
'css' => 'color: rgb(255, 50%, 0)',
'expected' => 'color: rgb(255, 50%, 0)',
),
Comment on lines +1364 to +1367
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mixing integers with a percentage here isn't valid:

body {
   background: rgb(255, 50%, 0);
}
Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for noticing, I have corrected the invalid values in 129c006

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think it's fixed? The problem is the 50%. Apparently CSS doesn't like mixing percentages and non-percentages. The above is still marked as invalid CSS by the CSS Validator.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When I try validating:

body {
   color: rgb(255 50% 0);
}

I get:

Image

Is the CSS Validator out of date?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Chrome seems fine with it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, and PhpStorm is fine with the comma-less version:

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This format is the Modern Syntax defined in CSS Color Module Level 4.

https://www.w3.org/TR/css-color-4/#rgb-functions

It should be supported by all browsers.

https://codepen.io/editor/Tetsuaki-Hamno/pen/019df1b7-20f5-77a8-b2f1-27811658e5bc

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, specifically the legacy format uses the commas and which doesn't allow mixing percentage and non-percentage values:

image

The modern syntax without the commas allows mixing, and 129c006 looks right.

// RGBA color.
array(
'css' => 'color: rgba(255, 128, 0, 0.5)',
'expected' => 'color: rgba(255, 128, 0, 0.5)',
),
array(
'css' => 'color: rgb(255 128 0 / 50%)',
'expected' => 'color: rgb(255 128 0 / 50%)',
),
// RGB color with extra whitespace.
array(
'css' => 'color: rgb( 255 , 128 , 0 )',
'expected' => 'color: rgb( 255 , 128 , 0 )',
),
// RGB background color.
array(
'css' => 'background-color: rgb(200, 100, 50)',
'expected' => 'background-color: rgb(200, 100, 50)',
),
// RGBA border color.
array(
'css' => 'border-color: rgba(100, 200, 300, 0.8)',
'expected' => 'border-color: rgba(100, 200, 300, 0.8)',
),
// Malformed RGB color, invalid number of values.
array(
'css' => 'color: rgb(255, 128, 0, 0.5, 100)',
'expected' => '',
),
array(
'css' => 'color: rgb(255, 128)',
'expected' => '',
),
// Malformed RGB color, non-numeric values.
array(
'css' => 'color: rgb(red, green, blue)',
'expected' => '',
),
// Malformed RGB color, unmatched parentheses.
array(
'css' => 'color: rgb(255, 128, 0',
'expected' => '',
),
// Malformed RGB color, empty values.
array(
'css' => 'color: rgb(, , )',
'expected' => '',
),
// Margin and padding logical properties introduced in 6.1.
array(
'css' => 'margin-block-start: 1px;margin-block-end: 2px;margin-inline-start: 3px;margin-inline-end: 4px;',
Expand Down
Loading