Skip to content
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

Jump to bottom

Font Library: sanitize font collection data #58636

Merged
merged 9 commits into from
Feb 5, 2024
Merged

Font Library: sanitize font collection data #58636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6becc25
sanitize font collection data
matiasbenedetto Feb 2, 2024
148bcb0
add a test case to be sure that the data returned by get_data is alwa…
matiasbenedetto Feb 2, 2024
ccdf2f9
format php
matiasbenedetto Feb 2, 2024
9e52287
lint
matiasbenedetto Feb 2, 2024
6b88f9b
custom function to validate src properties
matiasbenedetto Feb 2, 2024
98e8295
rename function
matiasbenedetto Feb 2, 2024
cd5b338
php format
matiasbenedetto Feb 2, 2024
f307972
Updates how collection data is sanitized and validated
creativecoder Feb 3, 2024
37e899a
Fix potential nitpicks
creativecoder Feb 4, 2024
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
104 changes: 76 additions & 28 deletions lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ final class WP_Font_Collection {
*
* @since 6.5.0
*
* @var array
* @var array|WP_Error|null
*/
private $data;

Expand All @@ -40,7 +40,7 @@ final class WP_Font_Collection {
*
* @since 6.5.0
*
* @var string
* @var string|null
*/
private $src;

Expand All @@ -62,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;
}

Expand All @@ -87,30 +87,21 @@ 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_data( $this->data );
if ( is_wp_error( $data ) ) {
return $data;
if ( is_wp_error( $this->data ) ) {
return $this->data;
}

// Set defaults for optional properties.
return wp_parse_args(
$data,
array(
'description' => '',
'categories' => array(),
)
$defaults = array(
'description' => '',
'categories' => array(),
);
return wp_parse_args( $this->data, $defaults );
}

/**
Expand Down Expand Up @@ -151,7 +142,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 );
}

/**
Expand Down Expand Up @@ -180,8 +171,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_data( $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;
}
Expand All @@ -193,14 +184,17 @@ 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 to sanitize and validate.
* @return array|WP_Error Sanitized data if valid, otherwise a WP_Error instance.
*/
private function validate_data( $data ) {
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 ] ) ) {
Expand All @@ -217,5 +211,59 @@ private function validate_data( $data ) {

return $data;
}

/**
* Retrieves the font collection sanitization schema.
*
* @since 6.5.0
*
* @return array Font collection sanitization schema.
*/
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',
),
),
);
}
}
}
3 changes: 2 additions & 1 deletion lib/compat/wordpress-6.5/fonts/class-wp-font-utils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ] );
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding this check allows using a class instance method, like array( $this, 'sanitization_method' ).


if ( $is_value_array && $is_schema_array ) {
if ( wp_is_numeric_array( $value ) ) {
Expand Down Expand Up @@ -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.
*
Expand Down
6 changes: 5 additions & 1 deletion phpunit/tests/fonts/font-library/wpFontCollection/__construct.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.' );
}
Expand Down
95 changes: 88 additions & 7 deletions phpunit/tests/fonts/font-library/wpFontCollection/getData.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() ),
),
),

Expand All @@ -94,14 +94,95 @@ 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<script>alert("xss")</script>',
'description' => 'My collection description<script>alert("xss")</script>',
'font_families' => array(
array(
'font_family_settings' => array(
'fontFamily' => 'Open Sans, sans-serif<script>alert("xss")</script>',
'slug' => 'open-sans',
'name' => 'Open Sans<script>alert("xss")</script>',
'fontFace' => array(
array(
'fontFamily' => 'Open Sans',
'fontStyle' => 'normal',
'fontWeight' => '400',
'src' => 'https://example.com/src-as-string.ttf?a=<script>alert("xss")</script>',
),
array(
'fontFamily' => 'Open Sans',
'fontStyle' => 'normal',
'fontWeight' => '400',
'src' => array(
'https://example.com/src-as-array.woff2?a=<script>alert("xss")</script>',
'https://example.com/src-as-array.ttf',
),
),
),
'unwanted_property' => 'potentially evil value',
),
'categories' => array( 'sans-serif<script>alert("xss")</script>' ),
),
),
'categories' => array(
array(
'name' => 'Mock col<script>alert("xss")</script>',
'slug' => 'mock-col<script>alert("xss")</script>',
'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',
'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',
),
),
),
),
'categories' => array( 'sans-serifalertxss' ),
),
),
),
),

Expand All @@ -114,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::sanitize_and_validate_data' );

$collection = new WP_Font_Collection( 'my-collection', $config );
$data = $collection->get_data();
Expand Down
7 changes: 6 additions & 1 deletion phpunit/tests/fonts/font-library/wpFontLibrary/getFontCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
Expand Down
10 changes: 8 additions & 2 deletions phpunit/tests/fonts/font-library/wpFontLibrary/registerFontCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' ),
);

Expand All @@ -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 );
}
}
11 changes: 8 additions & 3 deletions phpunit/tests/fonts/font-library/wpFontLibrary/unregisterFontCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand All @@ -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.' );
}
}
Loading