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 2 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
17 changes: 17 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,14 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
'list-style-image',
);

/*
* CSS attributes that accept rgba color data types.
*
*/
$css_rgba_color_data_types = array(
'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 :)

);
Comment thread
peterwilsoncc marked this conversation as resolved.
Outdated

/*
* CSS attributes that accept gradient data types.
*
Expand All @@ -2418,6 +2426,7 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
$css_test_string = $css_item;
$found = false;
$url_attr = false;
$rgba_attr = false;
Comment thread
peterwilsoncc marked this conversation as resolved.
Outdated
$gradient_attr = false;

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

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

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

@peterwilsoncc peterwilsoncc Aug 16, 2022

Choose a reason for hiding this comment

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

Is it possible to accept both rgb and rgba? It will be super confusing to people as to why one is accepted but not the other. rgb now accepts a forth argument for transparency so they're essentially the same function. See the MDN color docs.

This will require some changes to the test_safecss_filter_attr_filtered data provider to promote rgb to be always allowed.

$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
25 changes: 25 additions & 0 deletions tests/phpunit/tests/kses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,31 @@ public function data_test_safecss_filter_attr() {
'css' => 'height: expression( body.scrollTop + 50 + "px" )',
'expected' => '',
),
// RGBA background color are allowed.
array(
'css' => 'background-color: rgba(0,0,0,0)',
'expected' => 'background-color: rgba(0,0,0,0)',
),
array(
'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)',
),
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.

// RGB color values are not allowed.
array(
'css' => 'color: rgb( 100, 100, 100 )',
Expand Down