From 5321f2dfc47481628736634634aa17c4da15ebd5 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 9 Aug 2022 11:43:28 +1000 Subject: [PATCH 1/5] Allow a single custom properties with which we can test incoming `--wp--*` custom properties --- ...-wp-style-engine-css-declarations-test.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php b/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php index 795933da9efe9b..0c4ddf919b6fe6 100644 --- a/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php +++ b/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php @@ -215,6 +215,26 @@ public function data_should_compile_css_declarations_to_css_declarations_string( ); } + /** + * Tests allows --wp--* CSS custom properties. + */ + public function test_should_allow_wp_custom_properties() { + $input_declarations = array( + '--wp--love-your-work' => 'url("https://wordpress.org")', + '--wp--style--unstable-gallery-gap' => '12em', + '--wp-yeah-nah' => '100%', + '--wp--preset--here-is-a-potato' => '88px', + '--wp--style--/::target-[]' => '2000.75em', + '--wp--style--block-gap' => '2em', + ); + $css_declarations = new WP_Style_Engine_CSS_Declarations( $input_declarations ); + + $this->assertSame( + '--wp--style--unstable-gallery-gap:12em;', + $css_declarations->get_declarations_string() + ); + } + /** * Tests removing a single declaration. * From ba273ace08df250520b202bd0bd9c1168bcdeb2f Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 9 Aug 2022 15:56:50 +1000 Subject: [PATCH 2/5] Whoa horsey! Let's simplify this and check for explicit Gutenberg CSS custom properties. --- ...class-wp-style-engine-css-declarations.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/style-engine/class-wp-style-engine-css-declarations.php b/packages/style-engine/class-wp-style-engine-css-declarations.php index 360d992c6a3b38..de76f89d569248 100644 --- a/packages/style-engine/class-wp-style-engine-css-declarations.php +++ b/packages/style-engine/class-wp-style-engine-css-declarations.php @@ -17,6 +17,21 @@ * @access private */ class WP_Style_Engine_CSS_Declarations { + /** + * An array of valid CSS custom properties. + * CSS custom properties are permitted by safecss_filter_attr() + * since WordPress 6.1. See: https://core.trac.wordpress.org/ticket/56353. + * + * This whitelist exists so that the Gutenberg plugin maintains + * backwards compatibility with versions of WordPress < 6.1. + * + * It does not need to be backported to future versions of WordPress. + * + * @var array + */ + const VALID_CUSTOM_PROPERTIES = array( + '--wp--style--unstable-gallery-gap', + ); /** * An array of CSS declarations (property => value pairs). @@ -125,6 +140,22 @@ public function get_declarations() { */ protected static function filter_declaration( $property, $value, $spacer = '' ) { $filtered_value = wp_strip_all_tags( $value, true ); + + /** + * Allows CSS custom properties starting with `--wp--`. + * + * CSS custom properties are permitted by safecss_filter_attr() + * since WordPress 6.1. See: https://core.trac.wordpress.org/ticket/56353. + * + * This condition exists so that the Gutenberg plugin maintains + * backwards compatibility with versions of WordPress < 6.1. + * + * It does not need to be backported to future versions of WordPress. + */ + if ( in_array( $property, static::VALID_CUSTOM_PROPERTIES, true ) ) { + return "{$property}:{$spacer}{$filtered_value}"; + } + if ( '' !== $filtered_value ) { return safecss_filter_attr( "{$property}:{$spacer}{$filtered_value}" ); } From da9f89fb97ce249f2215275089be3c318acc6d0e Mon Sep 17 00:00:00 2001 From: ramonjd Date: Thu, 11 Aug 2022 12:40:54 +1000 Subject: [PATCH 3/5] Using a CSS property to run the value of a CSS custom property through safecss_filter_attr --- .../class-wp-style-engine-css-declarations.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/style-engine/class-wp-style-engine-css-declarations.php b/packages/style-engine/class-wp-style-engine-css-declarations.php index de76f89d569248..a5ea30bf046ad7 100644 --- a/packages/style-engine/class-wp-style-engine-css-declarations.php +++ b/packages/style-engine/class-wp-style-engine-css-declarations.php @@ -29,8 +29,8 @@ class WP_Style_Engine_CSS_Declarations { * * @var array */ - const VALID_CUSTOM_PROPERTIES = array( - '--wp--style--unstable-gallery-gap', + protected static $valid_custom_declarations = array( + '--wp--style--unstable-gallery-gap' => 'gap', ); /** @@ -142,7 +142,7 @@ protected static function filter_declaration( $property, $value, $spacer = '' ) $filtered_value = wp_strip_all_tags( $value, true ); /** - * Allows CSS custom properties starting with `--wp--`. + * Allows a specific list of CSS custom properties starting with `--wp--`. * * CSS custom properties are permitted by safecss_filter_attr() * since WordPress 6.1. See: https://core.trac.wordpress.org/ticket/56353. @@ -152,8 +152,9 @@ protected static function filter_declaration( $property, $value, $spacer = '' ) * * It does not need to be backported to future versions of WordPress. */ - if ( in_array( $property, static::VALID_CUSTOM_PROPERTIES, true ) ) { - return "{$property}:{$spacer}{$filtered_value}"; + if ( isset( static::$valid_custom_declarations[ $property ] ) ) { + return safecss_filter_attr( static::$valid_custom_declarations[ $property ] . ":{$spacer}{$value}" ) ? + "{$property}:{$spacer}{$value}" : ''; } if ( '' !== $filtered_value ) { From 160762bb3888deb970db7faef78c4e0fcaefaea7 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 27 Sep 2022 15:34:48 +1000 Subject: [PATCH 4/5] Updated tests --- ...class-wp-style-engine-css-declarations.php | 2 +- ...-wp-style-engine-css-declarations-test.php | 46 ++++++------------- 2 files changed, 15 insertions(+), 33 deletions(-) diff --git a/packages/style-engine/class-wp-style-engine-css-declarations.php b/packages/style-engine/class-wp-style-engine-css-declarations.php index a5ea30bf046ad7..6e7fdfc58e08ff 100644 --- a/packages/style-engine/class-wp-style-engine-css-declarations.php +++ b/packages/style-engine/class-wp-style-engine-css-declarations.php @@ -152,7 +152,7 @@ protected static function filter_declaration( $property, $value, $spacer = '' ) * * It does not need to be backported to future versions of WordPress. */ - if ( isset( static::$valid_custom_declarations[ $property ] ) ) { + if ( '' !== $filtered_value && isset( static::$valid_custom_declarations[ $property ] ) ) { return safecss_filter_attr( static::$valid_custom_declarations[ $property ] . ":{$spacer}{$value}" ) ? "{$property}:{$spacer}{$value}" : ''; } diff --git a/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php b/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php index 0c4ddf919b6fe6..e94daf697a7bff 100644 --- a/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php +++ b/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php @@ -121,22 +121,24 @@ public function test_should_strip_html_tags_and_remove_unsafe_css_properties() { /** - * Tests that calc, clamp, min, max, and minmax CSS functions are allowed. + * Tests that calc, clamp, min, max, minmax CSS functions and --wp--* CSS custom properties are allowed. * * @covers ::get_declarations_string * @covers ::filter_declaration */ public function test_should_allow_css_functions_and_strip_unsafe_css_values() { $input_declarations = array( - 'background' => 'var(--wp--preset--color--primary, 10px)', // Simple var(). - 'font-size' => 'clamp(36.00rem, calc(32.00rem + 10.00vw), 40.00rem)', // Nested clamp(). - 'width' => 'min(150vw, 100px)', - 'min-width' => 'max(150vw, 100px)', - 'max-width' => 'minmax(400px, 50%)', - 'padding' => 'calc(80px * -1)', - 'background-image' => 'url("https://wordpress.org")', - 'line-height' => 'url("https://wordpress.org")', - 'margin' => 'illegalfunction(30px)', + 'background' => 'var(--wp--preset--color--primary, 10px)', // Simple var(). + 'font-size' => 'clamp(36.00rem, calc(32.00rem + 10.00vw), 40.00rem)', // Nested clamp(). + 'width' => 'min(150vw, 100px)', + 'min-width' => 'max(150vw, 100px)', + 'max-width' => 'minmax(400px, 50%)', + 'padding' => 'calc(80px * -1)', + 'background-image' => 'url("https://wordpress.org")', + 'line-height' => 'url("https://wordpress.org")', + 'margin' => 'illegalfunction(30px)', + '--wp--style--unstable-gallery-gap' => '12em', + '/::border-[]' => '2000.75em', ); $css_declarations = new WP_Style_Engine_CSS_Declarations_Gutenberg( $input_declarations ); $safecss_filter_attr_allow_css_mock_action = new MockAction(); @@ -146,13 +148,13 @@ public function test_should_allow_css_functions_and_strip_unsafe_css_values() { $css_declarations_string = $css_declarations->get_declarations_string(); $this->assertSame( - 9, + 11, $safecss_filter_attr_allow_css_mock_action->get_call_count(), '"safecss_filter_attr_allow_css" filters were not applied to CSS declaration values.' ); $this->assertSame( - 'background:var(--wp--preset--color--primary, 10px);font-size:clamp(36.00rem, calc(32.00rem + 10.00vw), 40.00rem);width:min(150vw, 100px);min-width:max(150vw, 100px);max-width:minmax(400px, 50%);padding:calc(80px * -1);background-image:url("https://wordpress.org");', + 'background:var(--wp--preset--color--primary, 10px);font-size:clamp(36.00rem, calc(32.00rem + 10.00vw), 40.00rem);width:min(150vw, 100px);min-width:max(150vw, 100px);max-width:minmax(400px, 50%);padding:calc(80px * -1);background-image:url("https://wordpress.org");--wp--style--unstable-gallery-gap:12em;border-width:2000.75em;', $css_declarations_string, 'Unsafe values were not removed' ); @@ -215,26 +217,6 @@ public function data_should_compile_css_declarations_to_css_declarations_string( ); } - /** - * Tests allows --wp--* CSS custom properties. - */ - public function test_should_allow_wp_custom_properties() { - $input_declarations = array( - '--wp--love-your-work' => 'url("https://wordpress.org")', - '--wp--style--unstable-gallery-gap' => '12em', - '--wp-yeah-nah' => '100%', - '--wp--preset--here-is-a-potato' => '88px', - '--wp--style--/::target-[]' => '2000.75em', - '--wp--style--block-gap' => '2em', - ); - $css_declarations = new WP_Style_Engine_CSS_Declarations( $input_declarations ); - - $this->assertSame( - '--wp--style--unstable-gallery-gap:12em;', - $css_declarations->get_declarations_string() - ); - } - /** * Tests removing a single declaration. * From 4bfdd3cd5f4b5c28f290b975c9e97428563f8e6f Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 14 Oct 2022 12:40:59 +1100 Subject: [PATCH 5/5] Updated comment and function name to reflect contents of test. --- .../class-wp-style-engine-css-declarations-test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php b/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php index e94daf697a7bff..5380cc81d58314 100644 --- a/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php +++ b/phpunit/style-engine/class-wp-style-engine-css-declarations-test.php @@ -121,12 +121,12 @@ public function test_should_strip_html_tags_and_remove_unsafe_css_properties() { /** - * Tests that calc, clamp, min, max, minmax CSS functions and --wp--* CSS custom properties are allowed. + * Tests that calc, clamp, min, max, minmax CSS functions and CSS custom properties are allowed. * * @covers ::get_declarations_string * @covers ::filter_declaration */ - public function test_should_allow_css_functions_and_strip_unsafe_css_values() { + public function test_should_allow_css_functions_and_custom_properties_and_strip_unsafe_css_values() { $input_declarations = array( 'background' => 'var(--wp--preset--color--primary, 10px)', // Simple var(). 'font-size' => 'clamp(36.00rem, calc(32.00rem + 10.00vw), 40.00rem)', // Nested clamp().