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.

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

Extend allowed attributes for non-admin users #9954

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
20 changes: 3 additions & 17 deletions gutenberg.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ function gutenberg_pre_init() {
require_once dirname( __FILE__ ) . '/lib/load.php';

add_filter( 'replace_editor', 'gutenberg_init', 10, 2 );

// Include kses fixes so core blocks work for non-admin users.
require_once dirname( __FILE__ ) . '/lib/kses.php';
}

/**
Expand Down Expand Up @@ -481,20 +484,3 @@ function gutenberg_add_admin_body_class( $classes ) {
return "$classes gutenberg-editor-page is-fullscreen-mode";
}
}

/**
* Adds attributes to kses allowed tags that aren't in the default list
* and that Gutenberg needs to save blocks such as the Gallery block.
*
* @param array $tags Allowed HTML.
* @return array (Maybe) modified allowed HTML.
*/
function gutenberg_kses_allowedtags( $tags ) {
if ( isset( $tags['img'] ) ) {
$tags['img']['data-link'] = true;
$tags['img']['data-id'] = true;
}
return $tags;
}

add_filter( 'wp_kses_allowed_html', 'gutenberg_kses_allowedtags', 10, 2 );
24 changes: 24 additions & 0 deletions lib/kses.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php
/**
* KSES related functions for the Gutenberg plugin.
*
* @package gutenberg
*/

/**
* Adds attributes to kses allowed tags that aren't in the default list
* and that Gutenberg needs to save blocks such as the Gallery block.
*
* @param array $tags Allowed HTML.
* @return array (Maybe) modified allowed HTML.
*/
function gutenberg_kses_allowedtags( $tags ) {
$tags['a']['download'] = true;
Copy link
Member

Choose a reason for hiding this comment

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

Something the previous implementation had accounted for which this does not is if a developer calls to unset( $tags['a'] ); in their own filter handler before this one is called. Here, I think an error may occur.

$tags['img']['data-link'] = true;
$tags['img']['data-id'] = true;
$tags['div']['style'] = true;
$tags['div']['aria-hidden'] = true;
return $tags;
}

add_filter( 'wp_kses_allowed_html', 'gutenberg_kses_allowedtags', 10, 2 );
108 changes: 108 additions & 0 deletions phpunit/class-kses-test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?php
/**
* Tests that serialized HTML is not altered by KSES.
*
* @package Gutenberg
*/

require_once dirname( __FILE__ ) . '/../lib/kses.php';

class KSES_Test extends WP_UnitTestCase {
protected static $fixtures_dir;

/**
* Generates a normalized array of tag names and attributes.
* Takes into account how kses alters CSS.
*
* @param string $html HTML.
* @return array Normalized list of tags and attributes.
*/
function get_normalized_dom( $html ) {
$normalized = array();
$document = new DOMDocument();
libxml_use_internal_errors( true ); // Avoid errors with HTML5 elements.
$document->loadHTML( $html );
$node_list = $document->getElementsByTagName( '*' );
foreach ( $node_list as $node ) {
$node_repr = array(
'tag_name' => $node->tagName, // @codingStandardsIgnoreLine WordPress.NamingConventions.ValidVariableName.NotSnakeCaseMemberVar
'attributes' => array(),
);
foreach ( $node->attributes as $attr ) {
$name = $attr->name;
$value = $attr->value;

// Normalise css.
if ( 'style' === $name && ';' === substr( $value, -1 ) ) {
$value = substr( $value, 0, -1 );
}

$node_repr['attributes'][ $name ] = $value;
}
$normalized[] = $node_repr;
}
libxml_clear_errors();
return $normalized;
}

function assert_html_is_equivalent( $expected_html, $actual_html, $message ) {
$expected = $this->get_normalized_dom( $expected_html );
$actual = $this->get_normalized_dom( $actual_html );
$this->assertEquals( $expected, $actual, $message );
}

function kses_test_filenames() {
self::$fixtures_dir = dirname( dirname( __FILE__ ) ) . '/test/integration/full-content/fixtures';

// Exclude fixtures that would require admin to save,
// and so wouldn't go through kses.
$fixture_files = array_filter(
glob( self::$fixtures_dir . '/*.serialized.html' ),
array( $this, 'is_saved_through_kses' )
);

return array_map(
array( $this, 'filename_to_args' ),
$fixture_files
);
}

function is_saved_through_kses( $filename ) {
Copy link
Member

Choose a reason for hiding this comment

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

The function name is not very obvious to its purpose. Doesn't our PHPCS setup force function documentation? Might be good in this instance.

$admin_only = array(
// Freeform HTML.
'core__html.serialized.html',
// Currently broken for non-admin users, https://github.com/WordPress/gutenberg/issues/2539 .
'core__cover-image.serialized.html',
);

foreach ( $admin_only as $excluded ) {
if ( substr( $filename, -strlen( $excluded ) ) === $excluded ) {
return false;
}
}

return true;
}

function filename_to_args( $filename ) {
return array( $filename );
}

function strip_r( $input ) {
return str_replace( "\r", '', $input );
}

/**
* @dataProvider kses_test_filenames
*/
function test_kses_does_not_alter_output( $html_path ) {
$html = self::strip_r( file_get_contents( $html_path ) );
$kses_html = wp_unslash( wp_filter_post_kses( $html ) );

$this->assert_html_is_equivalent(
$html,
$kses_html,
"File '$html_path' was altered by kses"
);
}
}