Skip to content

Commit ded7952

Browse files
committed
Icons: Skip sanitizing the icons bundled with WordPress
Icons that ship with WordPress in the core icon library are no longer sanitized when they are loaded, because they are reviewed and versioned like the rest of core. This cuts the server time it takes to render the admin bar and admin menu icons on every admin screen. Icons registered by plugins and themes are still sanitized. Developed in: #14492 Props fushar, softglaze, wildworks. Fixes #66338. git-svn-id: https://develop.svn.wordpress.org/trunk@64800 602fd350-edb4-49c9-b593-d223f7449a82
1 parent d9062a3 commit ded7952

3 files changed

Lines changed: 66 additions & 3 deletions

File tree

‎src/wp-includes/class-wp-icons-registry.php‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ class WP_Icons_Registry {
2020
*/
2121
protected $registered_icons = array();
2222

23+
/**
24+
* Resolved path of the core icon library directory.
25+
*
26+
* @since 7.2.0
27+
* @var string|false|null
28+
*/
29+
private $core_icon_library_directory = null;
30+
2331
/**
2432
* Container for the main instance of the class.
2533
*
@@ -367,6 +375,7 @@ protected function sanitize_icon_content( $icon_content ) {
367375
* Retrieves the content of a registered icon.
368376
*
369377
* @since 7.0.0
378+
* @since 7.2.0 Core icons skip sanitization.
370379
*
371380
* @param string $icon_name Icon name including namespace.
372381
* @return string|null The content of the icon, if found.
@@ -390,7 +399,12 @@ protected function get_content( $icon_name ) {
390399
return null;
391400
}
392401

393-
$content = $this->sanitize_icon_content( file_get_contents( $icon_path ) );
402+
$content = file_get_contents( $icon_path );
403+
404+
// Core icons are trusted, so skip the costly sanitization.
405+
if ( ! $this->is_core_icon_file( $icon_path ) ) {
406+
$content = $this->sanitize_icon_content( $content );
407+
}
394408

395409
if ( empty( $content ) ) {
396410
wp_trigger_error(
@@ -405,6 +419,22 @@ protected function get_content( $icon_name ) {
405419
return $this->registered_icons[ $icon_name ]['content'];
406420
}
407421

422+
/**
423+
* Determines whether an icon file is a core icon.
424+
*
425+
* @since 7.2.0
426+
*
427+
* @param string $icon_path Icon file path, resolved with `realpath()`.
428+
* @return bool Whether the file is in the core icon library.
429+
*/
430+
private function is_core_icon_file( $icon_path ) {
431+
if ( null === $this->core_icon_library_directory ) {
432+
$this->core_icon_library_directory = realpath( ABSPATH . WPINC . '/images/icon-library' );
433+
}
434+
435+
return is_string( $this->core_icon_library_directory ) && str_starts_with( $icon_path, $this->core_icon_library_directory . DIRECTORY_SEPARATOR );
436+
}
437+
408438
/**
409439
* Retrieves an array containing the properties of a registered icon.
410440
*

‎tests/phpunit/tests/icons/wpGetIcon.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ public function test_wp_get_icon_returns_empty_string_for_unknown_icon() {
4747

4848
/**
4949
* @ticket 64847
50+
* @ticket 66338
5051
*/
5152
public function test_wp_get_icon_default_attributes() {
5253
$output = wp_get_icon( 'core/plus' );
53-
// WP_HTML_Tag_Processor lowercases attribute names.
54-
$this->assertStringContainsString( 'viewbox="0 0 24 24"', $output );
54+
$this->assertStringContainsStringIgnoringCase( 'viewbox="0 0 24 24"', $output );
5555
$this->assertStringContainsString( 'width="24"', $output );
5656
$this->assertStringContainsString( 'height="24"', $output );
5757
$this->assertStringContainsString( 'aria-hidden="true"', $output );

‎tests/phpunit/tests/icons/wpIconsRegistry.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,39 @@ public function test_get_content_reads_from_valid_file_path() {
499499
$this->assertStringContainsString( '<path', $icon['content'] );
500500
}
501501

502+
/**
503+
* @ticket 66338
504+
*
505+
* @covers ::get_content
506+
*/
507+
public function test_get_content_sanitizes_non_core_icon_file() {
508+
$path = $this->create_temp_icon_file( '<svg><script>alert( 1 )</script><path d="M0 0"/></svg>' );
509+
510+
$this->registry->register(
511+
'test-collection/from-file',
512+
array(
513+
'label' => 'From File',
514+
'file_path' => $path,
515+
)
516+
);
517+
518+
$icon = $this->registry->get_registered_icon( 'test-collection/from-file' );
519+
$this->assertStringContainsString( '<path', $icon['content'] );
520+
$this->assertStringNotContainsString( '<script', $icon['content'] );
521+
}
522+
523+
/**
524+
* Should serve a core icon unsanitized (sanitizing would lowercase `viewBox`, which would fail this test).
525+
*
526+
* @ticket 66338
527+
*
528+
* @covers ::get_content
529+
*/
530+
public function test_get_content_does_not_sanitize_core_icon() {
531+
$icon = $this->original_registry->get_registered_icon( 'core/pencil' );
532+
$this->assertSame( file_get_contents( ABSPATH . WPINC . '/images/icon-library/pencil.svg' ), $icon['content'] );
533+
}
534+
502535
/**
503536
* Provides icon files that cannot yield valid content.
504537
*

0 commit comments

Comments
 (0)