.../ckeditor/Plugin/CKEditorPlugin/Internal.php | 26 +++++--- .../lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php | 2 + .../lib/Drupal/ckeditor/Tests/CKEditorTest.php | 66 ++++++++++++++++++-- core/modules/filter/filter.module | 8 ++- .../lib/Drupal/filter/Plugin/FilterInterface.php | 5 +- .../lib/Drupal/filter/Tests/FilterAPITest.php | 16 ++--- .../Filter/FilterTestRestrictTagsAndAttributes.php | 38 ++++++++++- 7 files changed, 133 insertions(+), 28 deletions(-) diff --git a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php index 0a5e99d..d206597 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php @@ -293,11 +293,10 @@ protected function generateAllowedContentSetting(Editor $editor) { } // Generate setting that accurately reflects allowed tags and attributes. else { - $is_not_forbidden = function($value) { - return $value !== FALSE; - }; $get_allowed_attribute_values = function($attribute_values) { - $values = array_keys(array_filter($attribute_values, $is_not_forbidden)); + $values = array_keys(array_filter($attribute_values, function($value) { + return $value !== FALSE; + })); if (count($values)) { return implode(',', $values); } @@ -313,8 +312,16 @@ protected function generateAllowedContentSetting(Editor $editor) { } $setting = array(); foreach ($html_restrictions['allowedTags'] as $tag => $attributes) { - if ($attributes === TRUE) { - // Tell CKEditor anything is allowed! + // Tell CKEditor the tag is allowed, but no attributes. + if ($attributes === FALSE) { + $setting[$tag] = array( + 'attributes' => FALSE, + 'styles' => FALSE, + 'classes' => FALSE, + ); + } + // Tell CKEditor the tag is allowed, as well as any attribute on it. + else if ($attributes === TRUE) { $setting[$tag] = array( 'attributes' => TRUE, 'styles' => TRUE, @@ -364,10 +371,13 @@ protected function generateAllowedContentSetting(Editor $editor) { } } } + // Tell CKEditor the tag is allowed, along with some tags. else if (is_array($attributes)) { // CKEditor does not yet support blacklisting, so ignore those. // @todo Update this once http://dev.ckeditor.com/ticket/10276 lands. - $attributes = array_filter($attributes, $is_not_forbidden); + $attributes = array_filter($attributes, function($value) { + return $value !== FALSE; + }); // Configure allowed attributes, allowed "style" attribute values and // allowed "class" attribute values. @@ -388,7 +398,7 @@ protected function generateAllowedContentSetting(Editor $editor) { } } if (isset($attributes['class']) && is_array($attributes['class'])) { - $allowed_classes = $get_allowed_attribute_values($attributes['style']); + $allowed_classes = $get_allowed_attribute_values($attributes['class']); if (isset($allowed_classes)) { $setting[$tag]['classes'] = $allowed_classes; } diff --git a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php index c52636d..577ab74 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php @@ -143,6 +143,8 @@ public function getJSSettings(EditorEntity $editor) { 'drupalExternalPlugins' => array_map('file_create_url', $external_plugins), ); + ksort($settings); + return $settings; } diff --git a/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php b/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php index d0d005d..f8fd5b7 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php @@ -21,7 +21,7 @@ class CKEditorTest extends DrupalUnitTestBase { * * @var array */ - public static $modules = array('system', 'editor', 'ckeditor'); + public static $modules = array('system', 'editor', 'ckeditor', 'filter_test'); /** * An instance of the "CKEditor" text editor plugin. @@ -87,6 +87,7 @@ function testGetJSSettings() { 'stylesSet' => FALSE, 'drupalExternalPlugins' => array(), ); + ksort($expected_config); $this->assertIdentical($expected_config, $this->ckeditor->getJSSettings($editor), 'Generated JS settings are correct for default configuration.'); // Customize the configuration: add button, have two contextually enabled @@ -106,7 +107,8 @@ function testGetJSSettings() { $expected_config['drupalExternalPlugins']['llama_contextual_and_button'] = file_create_url('core/modules/ckeditor/tests/modules/js/llama_contextual_and_button.js'); $expected_config['contentsCss'][] = file_create_url('core/modules/ckeditor/tests/modules/ckeditor_test.css'); $expected_config['keystrokes'] = array(array(1114187, 'link'), array(1114188, NULL)); - $this->assertEqual($expected_config, $this->ckeditor->getJSSettings($editor), 'Generated JS settings are correct for customized configuration.'); + ksort($expected_config); + $this->assertIdentical($expected_config, $this->ckeditor->getJSSettings($editor), 'Generated JS settings are correct for customized configuration.'); // Change the allowed HTML tags; the "allowedContent" and "format_tags" // settings for CKEditor should automatically be updated as well. @@ -116,14 +118,68 @@ function testGetJSSettings() { $expected_config['allowedContent']['pre'] = array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE); $expected_config['allowedContent']['h3'] = array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE); $expected_config['format_tags'] = 'p;h3;h4;h5;h6;pre'; - $this->assertEqual($expected_config, $this->ckeditor->getJSSettings($editor), 'Generated JS settings are correct for customized configuration.'); + $this->assertIdentical($expected_config, $this->ckeditor->getJSSettings($editor), 'Generated JS settings are correct for customized configuration.'); - // Remove the filtered_html filter: allow *all *tags. + // Disable the filter_html filter: allow *all *tags. $format->setFilterConfig('filter_html', array('status' => 0)); $format->save(); $expected_config['allowedContent'] = TRUE; $expected_config['format_tags'] = 'p;h1;h2;h3;h4;h5;h6;pre'; - $this->assertEqual($expected_config, $this->ckeditor->getJSSettings($editor), 'Generated JS settings are correct for customized configuration.'); + $this->assertIdentical($expected_config, $this->ckeditor->getJSSettings($editor), 'Generated JS settings are correct for customized configuration.'); + + // Enable the filter_test_restrict_tags_and_attributes filter. + $format->setFilterConfig('filter_test_restrict_tags_and_attributes', array( + 'status' => 1, + 'settings' => array( + 'restrictions' => array( + 'allowedTags' => array( + 'p' => TRUE, + 'a' => array( + 'href' => TRUE, + 'rel' => array('nofollow' => TRUE), + 'class' => array('external' => TRUE), + 'target' => array('_blank' => FALSE), + ), + 'span' => array( + 'class' => array('dodo' => FALSE), + 'property' => array('dc:*' => TRUE), + 'rel' => array('foaf:*' => FALSE), + ), + '*' => array( + 'style' => FALSE, + 'data-*' => TRUE, + ), + 'del' => FALSE, + ) + ), + ), + )); + $format->save(); + $expected_config['allowedContent'] = array( + 'p' => array( + 'attributes' => TRUE, + 'styles' => FALSE, + 'classes' => TRUE, + ), + 'a' => array( + 'attributes' => 'href,rel,class,target', + 'classes' => 'external', + ), + 'span' => array( + 'attributes' => 'class,property,rel', + ), + '*' => array( + 'attributes' => 'data-*', + ), + 'del' => array( + 'attributes' => FALSE, + 'styles' => FALSE, + 'classes' => FALSE, + ), + ); + $expected_config['format_tags'] = 'p'; + ksort($expected_config); + $this->assertIdentical($expected_config, $this->ckeditor->getJSSettings($editor), 'Generated JS settings are correct for customized configuration.'); } /** diff --git a/core/modules/filter/filter.module b/core/modules/filter/filter.module index 59164b6..007509e 100644 --- a/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -418,6 +418,12 @@ function filter_get_filter_types_by_format($format_id) { /** * Retrieve all HTML restrictions (tags and attributes) for a given text format. * + * Note that restrictions applied to the "*" tag (the wildcard tag, i.e. all + * tags) are treated just like any other HTML tag. That means that any + * restrictions applied to it are not automatically applied to all other tags. + * It is up to the caller to handle this in whatever way it sees fit; this way + * no information granularity is lost. + * * @param string $format_id * A text format ID. * @@ -544,7 +550,7 @@ function filter_get_html_restrictions_by_format($format_id) { unset($restrictions['forbiddenTags']); } - // Simplification: if the only remaining allowed tag is the asterisk (which) + // Simplification: if the only remaining allowed tag is the asterisk (which // contains attribute restrictions that apply to all tags), and only // whitelisting filters were used, then effectively nothing is allowed. if (isset($restrictions['allowedTags'])) { diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php b/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php index 5ae56b6..fd93ced 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php @@ -213,7 +213,7 @@ public function process($text, $langcode, $cache, $cache_id); * 'src' => TRUE, * 'alt' => TRUE, * ), - * // Allow RDFa on tags, using only the dc, foaf,xsd and sioc + * // Allow RDFa on tags, using only the dc, foaf, xsd and sioc * // vocabularies/namespaces. * 'span' => array( * 'property' => array('dc:*' => TRUE, 'foaf:*' => TRUE), @@ -221,8 +221,7 @@ public function process($text, $langcode, $cache, $cache_id); * 'rel' => array('sioc:*' => TRUE), * ), * // Forbid the 'style' and 'on*' ('onClick' etc.) attributes on any - * // tag. Only when *all* tags have the value FALSE, blacklisting - * // will occur. + * // tag. * '*' => array( * 'style' => FALSE, * 'on*' => FALSE, diff --git a/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php b/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php index 89d386d..a742c5d 100644 --- a/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php +++ b/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php @@ -96,24 +96,24 @@ function testCheckMarkup() { */ function testFilterFormatAPI() { // Test on filtered_html. - $this->assertEqual( + $this->assertIdentical( filter_get_html_restrictions_by_format('filtered_html'), array('allowedTags' => array('p' => TRUE, 'br' => TRUE, 'strong' => TRUE, 'a' => TRUE, '*' => array('style' => FALSE, 'on*' => FALSE))), 'filter_get_html_restrictions_by_format() works as expected for the filtered_html format.' ); - $this->assertEqual( + $this->assertIdentical( filter_get_filter_types_by_format('filtered_html'), array(FILTER_TYPE_HTML_RESTRICTOR, FILTER_TYPE_MARKUP_LANGUAGE), 'filter_get_filter_types_by_format() works as expected for the filtered_html format.' ); // Test on full_html. - $this->assertEqual( + $this->assertIdentical( filter_get_html_restrictions_by_format('full_html'), FALSE, // Every tag is allowed. 'filter_get_html_restrictions_by_format() works as expected for the full_html format.' ); - $this->assertEqual( + $this->assertIdentical( filter_get_filter_types_by_format('full_html'), array(), 'filter_get_filter_types_by_format() works as expected for the full_html format.' @@ -133,12 +133,12 @@ function testFilterFormatAPI() { ), )); $stupid_filtered_html_format->save(); - $this->assertEqual( + $this->assertIdentical( filter_get_html_restrictions_by_format('stupid_filtered_html'), array('allowedTags' => array()), // No tag is allowed. 'filter_get_html_restrictions_by_format() works as expected for the stupid_filtered_html format.' ); - $this->assertEqual( + $this->assertIdentical( filter_get_filter_types_by_format('stupid_filtered_html'), array(FILTER_TYPE_HTML_RESTRICTOR), 'filter_get_filter_types_by_format() works as expected for the stupid_filtered_html format.' @@ -172,12 +172,12 @@ function testFilterFormatAPI() { ) )); $very_restricted_html->save(); - $this->assertEqual( + $this->assertIdentical( filter_get_html_restrictions_by_format('very_restricted_html'), array('allowedTags' => array('p' => TRUE, 'br' => FALSE, 'a' => array('href' => TRUE), '*' => array('style' => FALSE, 'on*' => FALSE))), 'filter_get_html_restrictions_by_format() works as expected for the very_restricted_html format.' ); - $this->assertEqual( + $this->assertIdentical( filter_get_filter_types_by_format('very_restricted_html'), array(FILTER_TYPE_HTML_RESTRICTOR), 'filter_get_filter_types_by_format() works as expected for the very_restricted_html format.' diff --git a/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestRestrictTagsAndAttributes.php b/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestRestrictTagsAndAttributes.php index 16e556f..b80ea92 100644 --- a/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestRestrictTagsAndAttributes.php +++ b/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestRestrictTagsAndAttributes.php @@ -10,6 +10,7 @@ use Drupal\filter\Annotation\Filter; use Drupal\Core\Annotation\Translation; use Drupal\filter\Plugin\FilterBase; +use Drupal\Component\Utility\Xss; /** * Provides a test filter to restirct HTML tags and attributes. @@ -18,7 +19,7 @@ * id = "filter_test_restrict_tags_and_attributes", * module = "filter_test", * title = @Translation("Tag and attribute restricting filter"), - * description = @Translation("Does nothing, used for testing filter_get_html_restrictions_by_format()."), + * description = @Translation("Used for testing filter_get_html_restrictions_by_format()."), * type = FILTER_TYPE_HTML_RESTRICTOR * ) */ @@ -28,14 +29,45 @@ class FilterTestRestrictTagsAndAttributes extends FilterBase { * {@inheritdoc} */ public function process($text, $langcode, $cache, $cache_id) { - return $text; + $allowed_tags = array_filter($this->settings['restrictions']['allowedTags'], function($value) { + return is_array($value) || (bool) $value !== FALSE; + }); + return Xss::filter($text, array_keys($allowed_tags)); } /** * {@inheritdoc} */ public function getHTMLRestrictions() { - return $this->settings['restrictions']; + $restrictions = $this->settings['restrictions']; + + // The configuration system stores FALSE as '0' and TRUE as '1'. Fix that. + if (isset($restrictions['allowedTags'])) { + foreach ($restrictions['allowedTags'] as $tag => $attrs_or_bool) { + if (!is_array($attrs_or_bool)) { + $restrictions['allowedTags'][$tag] = (bool) $attrs_or_bool; + } + else { + foreach ($attrs_or_bool as $attr => $attrvals_or_bool) { + if (!is_array($attrvals_or_bool)) { + $restrictions['allowedTags'][$tag][$attr] = (bool) $attrvals_or_bool; + } + else { + foreach ($attrvals_or_bool as $attrval => $bool) { + $restrictions['allowedTags'][$tag][$attr][$attrval] = (bool) $bool; + } + } + } + } + } + } + if (isset($restrictions['forbiddenTags'])) { + foreach ($restrictions['forbiddenTags'] as $tag => $bool) { + $restrictions['forbiddenTags'][$tag] = (bool) $bool; + } + } + + return $restrictions; } }