From 6becc253f6201082239f6e172ea015e06cdef981 Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Fri, 2 Feb 2024 17:57:00 -0300 Subject: [PATCH 1/9] sanitize font collection data --- .../fonts/class-wp-font-collection.php | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php index 154621ead50fb..f1ababf44d0bc 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php @@ -44,6 +44,54 @@ final class WP_Font_Collection { */ private $src; + /** + * Font collection sanitization schema. + * + * @since 6.5.0 + * + * @var array + */ + private const COLLECTION_SANITIZATION_SCHEMA = array( + 'name' => 'sanitize_text_field', + 'description' => 'sanitize_text_field', + 'font_families' => array( + array( + 'font_family_settings' => array( + 'name' => 'sanitize_text_field', + 'slug' => 'sanitize_title', + 'fontFamily' => 'sanitize_text_field', + 'preview' => 'sanitize_url', + 'fontFace' => array( + array( + 'fontFamily' => 'sanitize_text_field', + 'fontStyle' => 'sanitize_text_field', + 'fontWeight' => 'sanitize_text_field', + 'src' => 'sanitize_url', + 'preview' => 'sanitize_url', + 'fontDisplay' => 'sanitize_text_field', + 'fontStretch' => 'sanitize_text_field', + 'ascentOverride' => 'sanitize_text_field', + 'descentOverride' => 'sanitize_text_field', + 'fontVariant' => 'sanitize_text_field', + 'fontFeatureSettings' => 'sanitize_text_field', + 'fontVariationSettings' => 'sanitize_text_field', + 'lineGapOverride' => 'sanitize_text_field', + 'sizeAdjust' => 'sanitize_text_field', + 'unicodeRange' => 'sanitize_text_field', + ), + ), + ), + 'categories' => array( 'sanitize_title' ) + ), + ), + 'categories' => array( + array( + 'name' => 'sanitize_text_field', + 'slug' => 'sanitize_title', + ), + ), + ); + /** * WP_Font_Collection constructor. * @@ -201,6 +249,7 @@ private function load_from_url( $url ) { * @return array|WP_Error Array of data if valid, otherwise a WP_Error instance. */ private function validate_data( $data ) { + $data = WP_Font_Utils::sanitize_from_schema( $data, self::COLLECTION_SANITIZATION_SCHEMA ); $required_properties = array( 'name', 'font_families' ); foreach ( $required_properties as $property ) { if ( empty( $data[ $property ] ) ) { From 148bcb08afb6fe9892271ccf19de032a14776dcf Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Fri, 2 Feb 2024 17:57:43 -0300 Subject: [PATCH 2/9] add a test case to be sure that the data returned by get_data is always sanitized following the font collection schema --- .../font-library/wpFontCollection/getData.php | 59 +++++++++++++++++-- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php index 42715907de9c0..4db02c7aed43a 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php @@ -79,13 +79,13 @@ public function data_create_font_collection() { 'slug' => 'my-collection', 'config' => array( 'name' => 'My Collection', - 'font_families' => array( 'mock' ), + 'font_families' => array( array() ), ), 'expected_data' => array( 'description' => '', 'categories' => array(), 'name' => 'My Collection', - 'font_families' => array( 'mock' ), + 'font_families' => array( array() ), ), ), @@ -94,14 +94,61 @@ public function data_create_font_collection() { 'config' => array( 'name' => 'My Collection', 'description' => 'My collection description', - 'font_families' => array( 'mock' ), - 'categories' => array( 'mock' ), + 'font_families' => array( array() ), + 'categories' => array(), ), 'expected_data' => array( 'description' => 'My collection description', - 'categories' => array( 'mock' ), + 'categories' => array(), 'name' => 'My Collection', - 'font_families' => array( 'mock' ), + 'font_families' => array( array() ), + ), + ), + + 'font collection with risky data' => array( + 'slug' => 'my-collection', + 'config' => array( + 'name' => 'My Collection', + 'description' => 'My collection description', + 'font_families' => array( + array( + 'font_family_settings' => array( + 'fontFamily' => 'Open Sans, sans-serif', + 'slug' => 'open-sans', + 'name' => 'Open Sans', + 'unwanted_property'=> 'potentially evil value' + ), + 'categories' => [ 'sans-serif' ] + ) + ), + 'categories' => array( + array ( + 'name' => 'Mock col', + 'slug' => 'mock-col', + 'unwanted_property'=> 'potentially evil value' + ) + ), + 'unwanted_property'=> 'potentially evil value' + ), + 'expected_data' => array( + 'description' => 'My collection description', + 'categories' => array( + array ( + 'name' => 'Mock col', + 'slug' => 'mock-colalertxss' + ) + ), + 'name' => 'My Collection', + 'font_families' => array( + array( + 'font_family_settings' => array( + 'fontFamily' => 'Open Sans, sans-serif', + 'slug' => 'open-sans', + 'name' => 'Open Sans', + ), + 'categories' => [ 'sans-serifalertxss' ] + ) + ), ), ), From ccdf2f9fca25aa43700042958df5c542694f37cc Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Fri, 2 Feb 2024 18:13:47 -0300 Subject: [PATCH 3/9] format php --- .../fonts/class-wp-font-collection.php | 4 +- .../font-library/wpFontCollection/getData.php | 54 +++++++++---------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php index f1ababf44d0bc..b6a2947777f46 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php @@ -81,7 +81,7 @@ final class WP_Font_Collection { ), ), ), - 'categories' => array( 'sanitize_title' ) + 'categories' => array( 'sanitize_title' ), ), ), 'categories' => array( @@ -249,7 +249,7 @@ private function load_from_url( $url ) { * @return array|WP_Error Array of data if valid, otherwise a WP_Error instance. */ private function validate_data( $data ) { - $data = WP_Font_Utils::sanitize_from_schema( $data, self::COLLECTION_SANITIZATION_SCHEMA ); + $data = WP_Font_Utils::sanitize_from_schema( $data, self::COLLECTION_SANITIZATION_SCHEMA ); $required_properties = array( 'name', 'font_families' ); foreach ( $required_properties as $property ) { if ( empty( $data[ $property ] ) ) { diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php index 4db02c7aed43a..712f9bcc717e9 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php @@ -105,50 +105,50 @@ public function data_create_font_collection() { ), ), - 'font collection with risky data' => array( + 'font collection with risky data' => array( 'slug' => 'my-collection', 'config' => array( - 'name' => 'My Collection', - 'description' => 'My collection description', - 'font_families' => array( + 'name' => 'My Collection', + 'description' => 'My collection description', + 'font_families' => array( array( 'font_family_settings' => array( - 'fontFamily' => 'Open Sans, sans-serif', - 'slug' => 'open-sans', - 'name' => 'Open Sans', - 'unwanted_property'=> 'potentially evil value' + 'fontFamily' => 'Open Sans, sans-serif', + 'slug' => 'open-sans', + 'name' => 'Open Sans', + 'unwanted_property' => 'potentially evil value', ), - 'categories' => [ 'sans-serif' ] - ) - ), - 'categories' => array( - array ( - 'name' => 'Mock col', - 'slug' => 'mock-col', - 'unwanted_property'=> 'potentially evil value' - ) + 'categories' => array( 'sans-serif' ), + ), + ), + 'categories' => array( + array( + 'name' => 'Mock col', + 'slug' => 'mock-col', + 'unwanted_property' => 'potentially evil value', + ), ), - 'unwanted_property'=> 'potentially evil value' + 'unwanted_property' => 'potentially evil value', ), 'expected_data' => array( 'description' => 'My collection description', 'categories' => array( - array ( + array( 'name' => 'Mock col', - 'slug' => 'mock-colalertxss' - ) + 'slug' => 'mock-colalertxss', + ), ), 'name' => 'My Collection', - 'font_families' => array( + 'font_families' => array( array( 'font_family_settings' => array( 'fontFamily' => 'Open Sans, sans-serif', - 'slug' => 'open-sans', - 'name' => 'Open Sans', + 'slug' => 'open-sans', + 'name' => 'Open Sans', ), - 'categories' => [ 'sans-serifalertxss' ] - ) - ), + 'categories' => array( 'sans-serifalertxss' ), + ), + ), ), ), From 9e52287e11933ce211da827e97933985a8909fff Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Fri, 2 Feb 2024 18:14:26 -0300 Subject: [PATCH 4/9] lint --- lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php index b6a2947777f46..9dd6bfdb616f7 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php @@ -51,7 +51,7 @@ final class WP_Font_Collection { * * @var array */ - private const COLLECTION_SANITIZATION_SCHEMA = array( + const COLLECTION_SANITIZATION_SCHEMA = array( 'name' => 'sanitize_text_field', 'description' => 'sanitize_text_field', 'font_families' => array( From 6b88f9be6d0f0e2ae058d6fa98e99da8341daf8d Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Fri, 2 Feb 2024 20:04:10 -0300 Subject: [PATCH 5/9] custom function to validate src properties --- .../fonts/class-wp-font-collection.php | 25 +++++++++++++- .../font-library/wpFontCollection/getData.php | 34 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php index 9dd6bfdb616f7..0850f8bd9b070 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php @@ -66,7 +66,7 @@ final class WP_Font_Collection { 'fontFamily' => 'sanitize_text_field', 'fontStyle' => 'sanitize_text_field', 'fontWeight' => 'sanitize_text_field', - 'src' => 'sanitize_url', + 'src' => 'WP_Font_Collection::sanitize_src_property', 'preview' => 'sanitize_url', 'fontDisplay' => 'sanitize_text_field', 'fontStretch' => 'sanitize_text_field', @@ -266,5 +266,28 @@ private function validate_data( $data ) { return $data; } + + /** + * Sanitizes a src property. + * + * Font faces can have a src property consisting on a string or an array of strings. + * This method sanitizes the src property value. + * + * @since 6.5.0 + * + * @param string|array $value src property value to sanitize. + * + * @return string|array Sanitized URL value. + */ + public static function sanitize_src_property( $value ) { + if( is_array( $value ) ) { + foreach( $value as $key => $val ) { + $value[$key] = sanitize_url( $val ); + } + return $value; + } + + return sanitize_url( $value ); + } } } diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php index 712f9bcc717e9..3be87cf879018 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php @@ -116,6 +116,23 @@ public function data_create_font_collection() { 'fontFamily' => 'Open Sans, sans-serif', 'slug' => 'open-sans', 'name' => 'Open Sans', + 'fontFace' => array( + array( + 'fontFamily' => 'Open Sans', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => 'https://example.com/src-as-string.ttf?a=', + ), + array( + 'fontFamily' => 'Open Sans', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => array( + 'https://example.com/src-as-array.woff2?a=', + 'https://example.com/src-as-array.ttf', + ) + ), + ), 'unwanted_property' => 'potentially evil value', ), 'categories' => array( 'sans-serif' ), @@ -145,6 +162,23 @@ public function data_create_font_collection() { 'fontFamily' => 'Open Sans, sans-serif', 'slug' => 'open-sans', 'name' => 'Open Sans', + 'fontFace' => array( + array( + 'fontFamily' => 'Open Sans', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => 'https://example.com/src-as-string.ttf?a=scriptalert(xss)/script', + ), + array( + 'fontFamily' => 'Open Sans', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => array( + 'https://example.com/src-as-array.woff2?a=scriptalert(xss)/script', + 'https://example.com/src-as-array.ttf', + ) + ), + ) ), 'categories' => array( 'sans-serifalertxss' ), ), From 98e829526d420cc0a530f245b2fdcaad45ef3b85 Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Fri, 2 Feb 2024 20:08:59 -0300 Subject: [PATCH 6/9] rename function --- lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php | 6 +++--- .../tests/fonts/font-library/wpFontCollection/getData.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php index 0850f8bd9b070..f614d7faf3a2d 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php @@ -146,7 +146,7 @@ public function get_data() { } // Validate required properties are not empty. - $data = $this->validate_data( $this->data ); + $data = $this->validate_andd_sanitize( $this->data ); if ( is_wp_error( $data ) ) { return $data; } @@ -229,7 +229,7 @@ private function load_from_url( $url ) { } // Make sure the data is valid before caching it. - $data = $this->validate_data( $data ); + $data = $this->validate_andd_sanitize( $data ); if ( is_wp_error( $data ) ) { return $data; } @@ -248,7 +248,7 @@ private function load_from_url( $url ) { * @param array $data Font collection configuration to validate. * @return array|WP_Error Array of data if valid, otherwise a WP_Error instance. */ - private function validate_data( $data ) { + private function validate_andd_sanitize( $data ) { $data = WP_Font_Utils::sanitize_from_schema( $data, self::COLLECTION_SANITIZATION_SCHEMA ); $required_properties = array( 'name', 'font_families' ); foreach ( $required_properties as $property ) { diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php index 3be87cf879018..7fdfb329fd846 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php @@ -195,7 +195,7 @@ public function data_create_font_collection() { * @param array $config Font collection config. */ public function test_should_error_when_missing_properties( $config ) { - $this->setExpectedIncorrectUsage( 'WP_Font_Collection::validate_data' ); + $this->setExpectedIncorrectUsage( 'WP_Font_Collection::validate_andd_sanitize' ); $collection = new WP_Font_Collection( 'my-collection', $config ); $data = $collection->get_data(); From cd5b3386d4a720d33e821ffee7e996ee2261fb2c Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Fri, 2 Feb 2024 20:17:43 -0300 Subject: [PATCH 7/9] php format --- .../fonts/class-wp-font-collection.php | 14 +++++++------- .../font-library/wpFontCollection/getData.php | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php index f614d7faf3a2d..f628b90d31b00 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php @@ -269,20 +269,20 @@ private function validate_andd_sanitize( $data ) { /** * Sanitizes a src property. - * + * * Font faces can have a src property consisting on a string or an array of strings. * This method sanitizes the src property value. - * + * * @since 6.5.0 - * + * * @param string|array $value src property value to sanitize. - * + * * @return string|array Sanitized URL value. */ public static function sanitize_src_property( $value ) { - if( is_array( $value ) ) { - foreach( $value as $key => $val ) { - $value[$key] = sanitize_url( $val ); + if ( is_array( $value ) ) { + foreach ( $value as $key => $val ) { + $value[ $key ] = sanitize_url( $val ); } return $value; } diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php index 7fdfb329fd846..c77456dde1405 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php @@ -116,7 +116,7 @@ public function data_create_font_collection() { 'fontFamily' => 'Open Sans, sans-serif', 'slug' => 'open-sans', 'name' => 'Open Sans', - 'fontFace' => array( + 'fontFace' => array( array( 'fontFamily' => 'Open Sans', 'fontStyle' => 'normal', @@ -130,7 +130,7 @@ public function data_create_font_collection() { 'src' => array( 'https://example.com/src-as-array.woff2?a=', 'https://example.com/src-as-array.ttf', - ) + ), ), ), 'unwanted_property' => 'potentially evil value', @@ -162,7 +162,7 @@ public function data_create_font_collection() { 'fontFamily' => 'Open Sans, sans-serif', 'slug' => 'open-sans', 'name' => 'Open Sans', - 'fontFace' => array( + 'fontFace' => array( array( 'fontFamily' => 'Open Sans', 'fontStyle' => 'normal', @@ -176,9 +176,9 @@ public function data_create_font_collection() { 'src' => array( 'https://example.com/src-as-array.woff2?a=scriptalert(xss)/script', 'https://example.com/src-as-array.ttf', - ) + ), ), - ) + ), ), 'categories' => array( 'sans-serifalertxss' ), ), From f3079723fe6aab401437a95dbe4f4adb57cda006 Mon Sep 17 00:00:00 2001 From: Grant Kinney Date: Fri, 2 Feb 2024 19:15:22 -0600 Subject: [PATCH 8/9] Updates how collection data is sanitized and validated - Sanitize and validate as early as possible, so that appropriate notices are logged right away - Use static method for sanitization schema so that we can use a closure rather than a public method for src sanitization - Adds a check for WP_Font_Utils::sanitize_from_schema so that class callables like array( $this, 'sanitization_method' ) can be used - Updates method name to indicate that sanitization is done first, as this might remove invalid data and affect the validation result --- .../fonts/class-wp-font-collection.php | 171 ++++++++---------- .../fonts/class-wp-font-utils.php | 3 +- .../wpFontCollection/__construct.php | 6 +- .../font-library/wpFontCollection/getData.php | 6 +- .../wpFontLibrary/getFontCollection.php | 7 +- .../wpFontLibrary/registerFontCollection.php | 10 +- .../unregisterFontCollection.php | 11 +- 7 files changed, 108 insertions(+), 106 deletions(-) diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php index f628b90d31b00..9f5946fbe5dcc 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php @@ -31,7 +31,7 @@ final class WP_Font_Collection { * * @since 6.5.0 * - * @var array + * @var array|WP_Error|null */ private $data; @@ -40,58 +40,10 @@ final class WP_Font_Collection { * * @since 6.5.0 * - * @var string + * @var string|null */ private $src; - /** - * Font collection sanitization schema. - * - * @since 6.5.0 - * - * @var array - */ - const COLLECTION_SANITIZATION_SCHEMA = array( - 'name' => 'sanitize_text_field', - 'description' => 'sanitize_text_field', - 'font_families' => array( - array( - 'font_family_settings' => array( - 'name' => 'sanitize_text_field', - 'slug' => 'sanitize_title', - 'fontFamily' => 'sanitize_text_field', - 'preview' => 'sanitize_url', - 'fontFace' => array( - array( - 'fontFamily' => 'sanitize_text_field', - 'fontStyle' => 'sanitize_text_field', - 'fontWeight' => 'sanitize_text_field', - 'src' => 'WP_Font_Collection::sanitize_src_property', - 'preview' => 'sanitize_url', - 'fontDisplay' => 'sanitize_text_field', - 'fontStretch' => 'sanitize_text_field', - 'ascentOverride' => 'sanitize_text_field', - 'descentOverride' => 'sanitize_text_field', - 'fontVariant' => 'sanitize_text_field', - 'fontFeatureSettings' => 'sanitize_text_field', - 'fontVariationSettings' => 'sanitize_text_field', - 'lineGapOverride' => 'sanitize_text_field', - 'sizeAdjust' => 'sanitize_text_field', - 'unicodeRange' => 'sanitize_text_field', - ), - ), - ), - 'categories' => array( 'sanitize_title' ), - ), - ), - 'categories' => array( - array( - 'name' => 'sanitize_text_field', - 'slug' => 'sanitize_title', - ), - ), - ); - /** * WP_Font_Collection constructor. * @@ -110,10 +62,10 @@ final class WP_Font_Collection { public function __construct( $slug, $data_or_file ) { $this->slug = sanitize_title( $slug ); - // Data or json are lazy loaded and validated in get_data(). if ( is_array( $data_or_file ) ) { - $this->data = $data_or_file; + $this->data = $this->sanitize_and_validate_data( $data_or_file ); } else { + // JSON data is lazy loaded by ::get_data(). $this->src = $data_or_file; } @@ -135,30 +87,25 @@ public function __construct( $slug, $data_or_file ) { * @return array|WP_Error An array containing the font collection data, or a WP_Error on failure. */ public function get_data() { - // If we have a JSON config, load it and cache the data if it's valid. + // If the collection uses JSON data, load it and cache the data/error. if ( $this->src && empty( $this->data ) ) { - $data = $this->load_from_json( $this->src ); - if ( is_wp_error( $data ) ) { - return $data; - } - - $this->data = $data; + $this->data = $this->load_from_json( $this->src ); } - // Validate required properties are not empty. - $data = $this->validate_andd_sanitize( $this->data ); - if ( is_wp_error( $data ) ) { - return $data; + $data_or_error = $this->data; + + if ( ! is_wp_error( $data_or_error ) ) { + // Set defaults for optional properties. + $data_or_error = wp_parse_args( + $this->data, + array( + 'description' => '', + 'categories' => array(), + ) + ); } - // Set defaults for optional properties. - return wp_parse_args( - $data, - array( - 'description' => '', - 'categories' => array(), - ) - ); + return $data_or_error; } /** @@ -199,7 +146,7 @@ private function load_from_file( $file ) { return new WP_Error( 'font_collection_decode_error', __( 'Error decoding the font collection JSON file contents.', 'gutenberg' ) ); } - return $data; + return $this->sanitize_and_validate_data( $data ); } /** @@ -228,8 +175,8 @@ private function load_from_url( $url ) { return new WP_Error( 'font_collection_decode_error', __( 'Error decoding the font collection data from the REST response JSON.', 'gutenberg' ) ); } - // Make sure the data is valid before caching it. - $data = $this->validate_andd_sanitize( $data ); + // Make sure the data is valid before storing it in a transient. + $data = $this->sanitize_and_validate_data( $data ); if ( is_wp_error( $data ) ) { return $data; } @@ -241,15 +188,18 @@ private function load_from_url( $url ) { } /** - * Validates the font collection configuration. + * Sanitizes and validates the font collection data. * * @since 6.5.0 * - * @param array $data Font collection configuration to validate. - * @return array|WP_Error Array of data if valid, otherwise a WP_Error instance. + * @param array $data Font collection data. + * + * @return array|WP_Error Sanitized data if valid, otherwise a WP_Error instance. */ - private function validate_andd_sanitize( $data ) { - $data = WP_Font_Utils::sanitize_from_schema( $data, self::COLLECTION_SANITIZATION_SCHEMA ); + private function sanitize_and_validate_data( $data ) { + $schema = self::get_sanitization_schema(); + $data = WP_Font_Utils::sanitize_from_schema( $data, $schema ); + $required_properties = array( 'name', 'font_families' ); foreach ( $required_properties as $property ) { if ( empty( $data[ $property ] ) ) { @@ -268,26 +218,57 @@ private function validate_andd_sanitize( $data ) { } /** - * Sanitizes a src property. - * - * Font faces can have a src property consisting on a string or an array of strings. - * This method sanitizes the src property value. + * Retrieves the font collection sanitization schema. * * @since 6.5.0 * - * @param string|array $value src property value to sanitize. - * - * @return string|array Sanitized URL value. + * @return array Font collection sanitization schema. */ - public static function sanitize_src_property( $value ) { - if ( is_array( $value ) ) { - foreach ( $value as $key => $val ) { - $value[ $key ] = sanitize_url( $val ); - } - return $value; - } - - return sanitize_url( $value ); + private static function get_sanitization_schema() { + return array( + 'name' => 'sanitize_text_field', + 'description' => 'sanitize_text_field', + 'font_families' => array( + array( + 'font_family_settings' => array( + 'name' => 'sanitize_text_field', + 'slug' => 'sanitize_title', + 'fontFamily' => 'sanitize_text_field', + 'preview' => 'sanitize_url', + 'fontFace' => array( + array( + 'fontFamily' => 'sanitize_text_field', + 'fontStyle' => 'sanitize_text_field', + 'fontWeight' => 'sanitize_text_field', + 'src' => function ( $value ) { + return is_array( $value ) + ? array_map( 'sanitize_text_field', $value ) + : sanitize_text_field( $value ); + }, + 'preview' => 'sanitize_url', + 'fontDisplay' => 'sanitize_text_field', + 'fontStretch' => 'sanitize_text_field', + 'ascentOverride' => 'sanitize_text_field', + 'descentOverride' => 'sanitize_text_field', + 'fontVariant' => 'sanitize_text_field', + 'fontFeatureSettings' => 'sanitize_text_field', + 'fontVariationSettings' => 'sanitize_text_field', + 'lineGapOverride' => 'sanitize_text_field', + 'sizeAdjust' => 'sanitize_text_field', + 'unicodeRange' => 'sanitize_text_field', + ), + ), + ), + 'categories' => array( 'sanitize_title' ), + ), + ), + 'categories' => array( + array( + 'name' => 'sanitize_text_field', + 'slug' => 'sanitize_title', + ), + ), + ); } } } diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-utils.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-utils.php index 792a5aaa80eef..415e9e7f1f350 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-utils.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-utils.php @@ -158,7 +158,7 @@ public static function sanitize_from_schema( $tree, $schema ) { } $is_value_array = is_array( $value ); - $is_schema_array = is_array( $schema[ $key ] ); + $is_schema_array = is_array( $schema[ $key ] ) && ! is_callable( $schema[ $key ] ); if ( $is_value_array && $is_schema_array ) { if ( wp_is_numeric_array( $value ) ) { @@ -193,6 +193,7 @@ public static function sanitize_from_schema( $tree, $schema ) { * Apply the sanitizer to the value. * * @since 6.5.0 + * * @param mixed $value The value to sanitize. * @param mixed $sanitizer The sanitizer to apply. * diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php b/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php index 53a1ce0a5482b..a0693ce341456 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php @@ -14,8 +14,12 @@ class Tests_Fonts_WpFontCollection_Construct extends WP_UnitTestCase { public function test_should_do_it_wrong_with_invalid_slug() { $this->setExpectedIncorrectUsage( 'WP_Font_Collection::__construct' ); + $mock_collection_data = array( + 'name' => 'Test Collection', + 'font_families' => array( 'mock ' ), + ); - $collection = new WP_Font_Collection( 'slug with spaces', array() ); + $collection = new WP_Font_Collection( 'slug with spaces', $mock_collection_data ); $this->assertSame( 'slug-with-spaces', $collection->slug, 'Slug is not sanitized.' ); } diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php index c77456dde1405..1cb16e271db61 100644 --- a/phpunit/tests/fonts/font-library/wpFontCollection/getData.php +++ b/phpunit/tests/fonts/font-library/wpFontCollection/getData.php @@ -167,14 +167,14 @@ public function data_create_font_collection() { 'fontFamily' => 'Open Sans', 'fontStyle' => 'normal', 'fontWeight' => '400', - 'src' => 'https://example.com/src-as-string.ttf?a=scriptalert(xss)/script', + 'src' => 'https://example.com/src-as-string.ttf?a=', ), array( 'fontFamily' => 'Open Sans', 'fontStyle' => 'normal', 'fontWeight' => '400', 'src' => array( - 'https://example.com/src-as-array.woff2?a=scriptalert(xss)/script', + 'https://example.com/src-as-array.woff2?a=', 'https://example.com/src-as-array.ttf', ), ), @@ -195,7 +195,7 @@ public function data_create_font_collection() { * @param array $config Font collection config. */ public function test_should_error_when_missing_properties( $config ) { - $this->setExpectedIncorrectUsage( 'WP_Font_Collection::validate_andd_sanitize' ); + $this->setExpectedIncorrectUsage( 'WP_Font_Collection::sanitize_and_validate_data' ); $collection = new WP_Font_Collection( 'my-collection', $config ); $data = $collection->get_data(); diff --git a/phpunit/tests/fonts/font-library/wpFontLibrary/getFontCollection.php b/phpunit/tests/fonts/font-library/wpFontLibrary/getFontCollection.php index bc6b6d6005d1c..2c246de80b8b9 100644 --- a/phpunit/tests/fonts/font-library/wpFontLibrary/getFontCollection.php +++ b/phpunit/tests/fonts/font-library/wpFontLibrary/getFontCollection.php @@ -13,7 +13,12 @@ class Tests_Fonts_WpFontLibrary_GetFontCollection extends WP_Font_Library_UnitTestCase { public function test_should_get_font_collection() { - wp_register_font_collection( 'my-font-collection', array( 'font_families' => array( 'mock' ) ) ); + $mock_collection_data = array( + 'name' => 'Test Collection', + 'font_families' => array( 'mock' ), + ); + + wp_register_font_collection( 'my-font-collection', $mock_collection_data ); $font_collection = WP_Font_Library::get_font_collection( 'my-font-collection' ); $this->assertInstanceOf( 'WP_Font_Collection', $font_collection ); } diff --git a/phpunit/tests/fonts/font-library/wpFontLibrary/registerFontCollection.php b/phpunit/tests/fonts/font-library/wpFontLibrary/registerFontCollection.php index 1ba9ab1d32fb4..24529d0dd090b 100644 --- a/phpunit/tests/fonts/font-library/wpFontLibrary/registerFontCollection.php +++ b/phpunit/tests/fonts/font-library/wpFontLibrary/registerFontCollection.php @@ -13,6 +13,7 @@ class Tests_Fonts_WpFontLibrary_RegisterFontCollection extends WP_Font_Library_UnitTestCase { public function test_should_register_font_collection() { $config = array( + 'name' => 'My Collection', 'font_families' => array( 'mock' ), ); @@ -21,14 +22,19 @@ public function test_should_register_font_collection() { } public function test_should_return_error_if_slug_is_repeated() { + $mock_collection_data = array( + 'name' => 'Test Collection', + 'font_families' => array( 'mock' ), + ); + // Register first collection. - $collection1 = WP_Font_Library::register_font_collection( 'my-collection-1', array( 'font_families' => array( 'mock' ) ) ); + $collection1 = WP_Font_Library::register_font_collection( 'my-collection-1', $mock_collection_data ); $this->assertInstanceOf( 'WP_Font_Collection', $collection1, 'A collection should be registered.' ); // Expects a _doing_it_wrong notice. $this->setExpectedIncorrectUsage( 'WP_Font_Library::register_font_collection' ); // Try to register a second collection with same slug. - WP_Font_Library::register_font_collection( 'my-collection-1', array( 'font_families' => array( 'mock' ) ) ); + WP_Font_Library::register_font_collection( 'my-collection-1', $mock_collection_data ); } } diff --git a/phpunit/tests/fonts/font-library/wpFontLibrary/unregisterFontCollection.php b/phpunit/tests/fonts/font-library/wpFontLibrary/unregisterFontCollection.php index 9546d0082eff9..657d3bb07aaaf 100644 --- a/phpunit/tests/fonts/font-library/wpFontLibrary/unregisterFontCollection.php +++ b/phpunit/tests/fonts/font-library/wpFontLibrary/unregisterFontCollection.php @@ -13,9 +13,14 @@ class Tests_Fonts_WpFontLibrary_UnregisterFontCollection extends WP_Font_Library_UnitTestCase { public function test_should_unregister_font_collection() { + $mock_collection_data = array( + 'name' => 'Test Collection', + 'font_families' => array( 'mock' ), + ); + // Registers two mock font collections. - WP_Font_Library::register_font_collection( 'mock-font-collection-1', array( 'font_families' => array( 'mock' ) ) ); - WP_Font_Library::register_font_collection( 'mock-font-collection-2', array( 'font_families' => array( 'mock' ) ) ); + WP_Font_Library::register_font_collection( 'mock-font-collection-1', $mock_collection_data ); + WP_Font_Library::register_font_collection( 'mock-font-collection-2', $mock_collection_data ); // Unregister mock font collection. WP_Font_Library::unregister_font_collection( 'mock-font-collection-1' ); @@ -36,6 +41,6 @@ public function unregister_non_existing_collection() { // Unregisters non-existing font collection. WP_Font_Library::unregister_font_collection( 'non-existing-collection' ); $collections = WP_Font_Library::get_font_collections(); - $this->assertEmpty( $collections, 'Should not be registered collections.' ); + $this->assertEmpty( $collections, 'No collections should be registered.' ); } } From 37e899ad3f2b2fd92c951ab1ab84fa1fce8b5c7d Mon Sep 17 00:00:00 2001 From: Grant Kinney Date: Sun, 4 Feb 2024 13:46:48 -0600 Subject: [PATCH 9/9] Fix potential nitpicks --- .../fonts/class-wp-font-collection.php | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php index 9f5946fbe5dcc..f89ec63386f5a 100644 --- a/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php +++ b/lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php @@ -92,20 +92,16 @@ public function get_data() { $this->data = $this->load_from_json( $this->src ); } - $data_or_error = $this->data; - - if ( ! is_wp_error( $data_or_error ) ) { - // Set defaults for optional properties. - $data_or_error = wp_parse_args( - $this->data, - array( - 'description' => '', - 'categories' => array(), - ) - ); + if ( is_wp_error( $this->data ) ) { + return $this->data; } - return $data_or_error; + // Set defaults for optional properties. + $defaults = array( + 'description' => '', + 'categories' => array(), + ); + return wp_parse_args( $this->data, $defaults ); } /** @@ -192,8 +188,7 @@ private function load_from_url( $url ) { * * @since 6.5.0 * - * @param array $data Font collection data. - * + * @param array $data Font collection data to sanitize and validate. * @return array|WP_Error Sanitized data if valid, otherwise a WP_Error instance. */ private function sanitize_and_validate_data( $data ) {