Skip to content

KSES: Add support for rgb(a) color. #3097

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

Closed
t-hamano wants to merge 8 commits into from
Closed

KSES: Add support for rgb(a) color. #3097

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5d2801f
KSES: Add support for rgba background-color.
t-hamano Aug 16, 2022
692f37a
Fix lint error
t-hamano Aug 16, 2022
c9b2fb5
support rgb color
t-hamano Aug 17, 2022
08e1e31
Add missing supports
t-hamano Aug 18, 2022
8d49e8f
Relaxed rules for regular expression
t-hamano Aug 18, 2022
af88ca5
Update tests
t-hamano Aug 18, 2022
b1dc05a
Merge branch 'trunk' into fix/kses-allow-background-color
t-hamano Jul 31, 2025
97fe09e
Refactoring regular expressions and tests
t-hamano Jul 31, 2025
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
25 changes: 25 additions & 0 deletions src/wp-includes/kses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2395,6 +2395,22 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
'list-style-image',
);

/*
* CSS attributes that accept rgb(a) color data types.
*
*/
$css_color_data_types = array(
'color',

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

'background-color',
Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc Aug 18, 2022

Choose a reason for hiding this comment

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

Also don't forget the shorthand

  • background
  • border
  • border-(left|right|top|bottom)

That's assuming support :)

);

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

if ( strpos( $css_item, ':' ) === false ) {
Expand All @@ -2430,6 +2447,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 );
}
}

Expand Down Expand Up @@ -2466,6 +2484,13 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
}
}

if ( $found && $color_attr ) {
$css_value = trim( $parts[1] );
if ( preg_match( '/^rgb[a]?\((\d{1,3}%?),\s?(\d{1,3}%?),\s?(\d{1,3}%?)(,\s?|\s\/\s)(\d*(?:\.\d+)?)\)$/', $css_value ) ) {
Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc Aug 18, 2022

Choose a reason for hiding this comment

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

It might be possible to simplify the regex a lot here.

The goal of kses is to make the HTML (and CSS it contains) safe rather than valid. It's nice if it's valid but that's a problem for other parts of the code base.

You might be able to use /^rgba?\([\s\d%\/.,]+\)$/ to limit the user to the allowed characters

$css_test_string = str_replace( $css_value, '', $css_test_string );
}
}

if ( $found ) {
// Allow CSS calc().
$css_test_string = preg_replace( '/calc\(((?:\([^()]*\)?|[^()])*)\)/', '', $css_test_string );
Expand Down
80 changes: 76 additions & 4 deletions tests/phpunit/tests/kses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,14 +1110,86 @@ public function data_test_safecss_filter_attr() {
'css' => 'height: expression( body.scrollTop + 50 + "px" )',
'expected' => '',
),
// RGB color values are not allowed.
// Allowed RGBA color.
array(
'css' => 'color: rgb( 100, 100, 100 )',
'css' => 'color: rgb(0,0,0,0)',
'expected' => 'color: rgb(0,0,0,0)',
),
array(
'css' => 'border-color: rgba(0,0,0,0)',
'expected' => 'border-color: rgba(0,0,0,0)',
),
array(
'css' => 'border-right-color: rgba(0, 0, 0, 0)',
'expected' => 'border-right-color: rgba(0, 0, 0, 0)',
),
array(
'css' => 'border-bottom-color: rgba(100, 100, 100, 0)',
'expected' => 'border-bottom-color: rgba(100, 100, 100, 0)',
),
array(
'css' => 'border-left-color: rgba(10%, 10%, 10%, 0)',
'expected' => 'border-left-color: rgba(10%, 10%, 10%, 0)',
),
array(
'css' => 'border-top-color: rgba(0, 0, 0, 0.1)',
'expected' => 'border-top-color: rgba(0, 0, 0, 0.1)',
),
array(
'css' => 'background-color: rgba(0, 0, 0, .1)',
'expected' => 'background-color: rgba(0, 0, 0, .1)',
),
Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc Aug 17, 2022

Choose a reason for hiding this comment

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

There's an test below under the comment "RGBA color values are not allowed." that will now fail.

It would be good to test for some malformed values too.

Copy link
Copy Markdown
Contributor Author

@t-hamano t-hamano Aug 17, 2022

Choose a reason for hiding this comment

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

I am thinking about regular expression and test cases, am I correct in this perception?

✅ color: rgba(0, 0, 0 / 0.1)
❌ color: rgba(0, 0, 0/ 0.1) (don't allow no space before slash)
❌ color: rgba(0, 0, 0 /0.1) (don't allow no space after slash)

✅ color: rgba(0, 0, 0, 0)
✅ color: rgba(0,0,0,0) (allow no space after comma)
❌ color: rgba(0 , 0, 0, 0) (don't allow space before comma)
❌ color: rgba(0, 0, 0, 0) (don't allow multiple spaces after comma *it would appear as single space on the comments)

✅ color: rgba(100%, 0, 0, 0)
❌ color: rgba(100px, 0, 0, 0) (don't allow units other than %)
❌ color: rgba(red, 0, 0, 0) (don't allow string value)

According to MDN, I believe the same rules can be applied to both rgb() and rgba().

Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc Aug 17, 2022

Choose a reason for hiding this comment

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

I am thinking about regular expression and test cases, am I correct in this perception?

Yes, you are. These are the sort of tests I was suggesting.

You may also wish to include something containing either a " or a ' as an attempt to close the style tag. The expectation is that kses would return an empty string.

Thanks so much.

array(
'css' => 'color: rgba(0, 0, 0,.1)',
'expected' => 'color: rgba(0, 0, 0,.1)',
),
array(
'css' => 'border-color: rgba(0, 0, 0 / 0.1)',
'expected' => 'border-color: rgba(0, 0, 0 / 0.1)',
),
// Invalid RGBA color.
array(
'css' => 'color: rg(0, 0, 0, 0)',
'expected' => '',
),
// RGBA color values are not allowed.
array(
'css' => 'color: rgb( 100, 100, 100, .4 )',
'css' => 'text-decoration-color : rgba(0, 0, 0, 0)',
'expected' => '',
),
array(
'css' => 'border-color: rgba(0 , 0, 0, 0)',
'expected' => '',
),
array(
'css' => 'border-right-color: rgba(0, 0, 0, 0)',
Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc Aug 18, 2022

Choose a reason for hiding this comment

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

I'd expect this to pass and be retained. The format is a bit ugly but it's valid https://jsbin.com/viripuz/1/edit?css,output

'expected' => '',
),
array(
'css' => 'border-bottom-color: rgba(0, 0, 0/ 0.1 )',
'expected' => '',
),
array(
'css' => 'border-left-color: rgba(0, 0, 0 /0.1 )',
'expected' => '',
),
array(
'css' => 'border-top-color: rgba(0, 0, 0 / 0.1 )',
'expected' => '',
),
array(
'css' => 'background-color: rgba(red, 0, 0, 0)',
'expected' => '',
),
array(
'css' => 'color: rgba(100px, 0, 0, 0)',
'expected' => '',
),
array(
'css' => 'border-right-color: "rgba(0, 0, 0, 0)',
'expected' => '',
),
array(
'css' => "border-bottom-color: 'rgba(0, 0, 0, 0)",
'expected' => '',
),
);
Expand Down