core/modules/ckeditor/js/ckeditor.js | 77 ++++++++++- .../ckeditor/Plugin/CKEditorPlugin/Internal.php | 102 +++++++++++++- .../lib/Drupal/ckeditor/Tests/CKEditorTest.php | 13 +- core/modules/filter/filter.module | 139 ++++++++++++++------ .../lib/Drupal/filter/Plugin/Filter/FilterHtml.php | 11 +- .../filter/Plugin/Filter/FilterHtmlEscape.php | 5 +- .../filter/lib/Drupal/filter/Plugin/FilterBase.php | 2 +- .../lib/Drupal/filter/Plugin/FilterInterface.php | 57 ++++++-- .../lib/Drupal/filter/Tests/FilterAPITest.php | 34 ++--- .../Filter/FilterTestRestrictTagsAndAttributes.php | 2 +- 10 files changed, 359 insertions(+), 83 deletions(-) diff --git a/core/modules/ckeditor/js/ckeditor.js b/core/modules/ckeditor/js/ckeditor.js index 447f916..56a447c 100644 --- a/core/modules/ckeditor/js/ckeditor.js +++ b/core/modules/ckeditor/js/ckeditor.js @@ -6,6 +6,7 @@ Drupal.editors.ckeditor = { attach: function (element, format) { this._loadExternalPlugins(format); + this._ACF_HACK_to_support_blacklisted_attributes(element, format); return !!CKEDITOR.replace(element, format.editorSettings); }, @@ -42,6 +43,7 @@ Drupal.editors.ckeditor = { attachInlineEditor: function (element, format, mainToolbarId, floatedToolbarId) { this._loadExternalPlugins(format); + this._ACF_HACK_to_support_blacklisted_attributes(element, format); var settings = $.extend(true, {}, format.editorSettings); @@ -98,8 +100,81 @@ Drupal.editors.ckeditor = { } delete format.editorSettings.drupalExternalPlugins; } - } + }, + + /** + * This is a huge hack to do ONE thing: to allow Drupal to fully mandate what + * CKEditor should allow by setting CKEditor's allowedContent setting. The + * problem is that allowedContent only allows for whitelisting, whereas + * Drupal's default HTML filtering (the filter_html filter) also blacklists + * the "style" and "on*" ("onClick" etc.) attributes. + * + * So this function hacks in explicit support for Drupal's filter_html's need + * to blacklisting specifically those attributes, until ACF supports + * blacklisting of properties: http://dev.ckeditor.com/ticket/10276. + * + * Limitations: + * - This does not support blacklisting of other attributes, it's only + * intended to implement filter_html's blacklisted attributes. + * - This is only a temporary work-around; it assumes the filter_html + * filter is being used whenever *any* restriction exists. This is a valid + * assumption for the default text formats in Drupal 8 core, but obviously + * won't work for release. + * + * This is the only way we could get https://drupal.org/node/1936392 committed + * before Drupal 8 code freeze on July 1, 2013. CKEditor has committed to + * explicitly supporting this in some way. + * + * @todo D8 remove this once http://dev.ckeditor.com/ticket/10276 is done. + */ + _ACF_HACK_to_support_blacklisted_attributes: function (element, format) { + function override(rule) { + var oldValue = rule.attributes; + function filter_html_override_attributes (attribute) { + // Disallow the "style" and "on*" attributes on any tag. + if (attribute === 'style' || attribute.substr(0, 2) === 'on') { + return false; + } + + // Ensure the original logic still runs, if any. + if (typeof oldValue === 'function') { + return oldValue(attribute); + } + else if (typeof oldValue === 'boolean') { + return oldValue; + } + + // Otherwise, accept this attribute. + return true; + } + rule.attributes = filter_html_override_attributes; + } + CKEDITOR.once('instanceLoaded', function(e) { + if (e.editor.name === element.id) { + // If everything is allowed, everything is allowed. + if (format.editorSettings.allowedContent === true) { + return; + } + // Otherwise, assume Drupal's filter_html filter is being used. + else { + // Get the filter object (ACF). + var filter = e.editor.filter; + // Find the "config" rule (the one caused by the allowedContent + // setting) for each HTML tag, and override its "attributes" value. + for (var el in filter._.rules.elements) { + if (filter._.rules.elements.hasOwnProperty(el)) { + for (var i = 0; i < filter._.rules.elements[el].length; i++) { + if (filter._.rules.elements[el][i].featureName === 'config') { + override(filter._.rules.elements[el][i]); + } + } + } + } + } + } + }); + } }; })(Drupal, CKEDITOR, jQuery); 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 b9c3aff..a0d0642 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php @@ -294,21 +294,109 @@ protected function generateAllowedContentSetting(Editor $editor) { } // Generate setting that accurately reflects allowed tags and attributes. else { - $allowed_html = filter_get_allowed_html_by_format($editor->format); + $is_not_forbidden = function($value) { + return $value !== FALSE; + }; + $get_allowed_attribute_values = function() { + $values = array_keys(array_filter($attribute_values, $is_not_forbidden)); + if (count($values)) { + return implode(',', $values); + } + else { + return NULL; + } + }; + + $html_restrictions = filter_get_html_restrictions_by_format($editor->format); // When all HTML is allowed, also set allowedContent to true. - if ($allowed_html === TRUE) { + if ($html_restrictions === FALSE) { return TRUE; } $setting = array(); - foreach($allowed_html as $tag => $attributes) { + foreach ($html_restrictions['allowedTags'] as $tag => $attributes) { if ($attributes === TRUE) { - $setting[] = $tag . '[*]'; + // Tell CKEditor anything is allowed! + $setting[$tag] = array( + 'attributes' => TRUE, + 'styles' => TRUE, + 'classes' => TRUE, + ); + // We've just marked that any value for the "style" and "class" + // attributes is allowed. However, that may not be the case: the "*" + // tag may still apply restrictions. + // Since CKEditor's ACF follows the following principle: + // Once validated, an element or its property cannot be + // invalidated by another rule. + // That means that the most permissive setting wins. Which means that + // it will still be allowed by CKEditor to e.g. define any style, no + // matter what the "*" tag's restrictions may be. If there's a setting + // for either the "style" or "class" attribute, it cannot possibly be + // more permissive than what was set above. Hence: inherit from the + // "*" tag where possible. + if (isset($html_restrictions['allowedTags']['*'])) { + $wildcard = $html_restrictions['allowedTags']['*']; + if (isset($wildcard['style'])) { + if (!is_array($wildcard['style'])) { + $setting[$tag]['styles'] = $wildcard['style']; + } + else { + $allowed_styles = $get_allowed_attribute_values($attributes['style']); + if (isset($allowed_values)) { + $setting[$tag]['styles'] = $allowed_styles; + } + else { + unset($setting[$tag]['styles']); + } + } + } + if (isset($wildcard['class'])) { + if (!is_array($wildcard['class'])) { + $setting[$tag]['classes'] = $wildcard['class']; + } + else { + $allowed_styles = $get_allowed_attribute_values($attributes['class']); + if (isset($allowed_values)) { + $setting[$tag]['classes'] = $allowed_styles; + } + else { + unset($setting[$tag]['classes']); + } + } + } + } } - else { - $setting[] = $tag . '[' . implode(',', $attributes) . ']'; + 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); + + // Configure allowed attributes, allowed "style" attribute values and + // allowed "class" attribute values. + // CKEditor only allows specific values for the "class" and "style" + // attributes; so ignore any other restrictions. + // NOTE: A Drupal contrib module can subclass this class, override the + // getConfig() method, and override the JavaScript at + // Drupal.editors.ckeditor to somehow make validation of values for + // attributes other than "class" and "style" work. + if (count($attributes)) { + $setting[$tag]['attributes'] = implode(',', array_keys($attributes)); + } + if (isset($attributes['style']) && is_array($attributes['style'])) { + $allowed_styles = $get_allowed_attribute_values($attributes['style']); + if (isset($allowed_values)) { + $setting[$tag]['styles'] = $allowed_styles; + } + } + if (isset($attributes['class']) && is_array($attributes['class'])) { + $allowed_classes = $get_allowed_attribute_values($attributes['style']); + if (isset($allowed_classes)) { + $setting[$tag]['classes'] = $allowed_classes; + } + } } } - return implode(';', $setting); + + return $setting; } } diff --git a/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php b/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php index 9a689ad..70640db 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php @@ -113,7 +113,8 @@ function testGetJSSettings() { $format = entity_load('filter_format', 'filtered_html'); $format->filters('filter_html')->settings['allowed_html'] .= '
'; $format->save(); - $expected_config['allowedContent'] = 'h4[*];h5[*];h6[*];p[*];br[*];strong[*];a[*];pre[*];h3[*]'; + $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.'); @@ -244,7 +245,15 @@ protected function getDefaultInternalConfig() { } protected function getDefaultAllowedContentConfig() { - return 'h4[*];h5[*];h6[*];p[*];br[*];strong[*];a[*]'; + return array( + 'h4' => array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE), + 'h5' => array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE), + 'h6' => array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE), + 'p' => array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE), + 'br' => array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE), + 'strong' => array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE), + 'a' => array('attributes' => TRUE, 'styles' => FALSE, 'classes' => TRUE), + ); } protected function getDefaultToolbarConfig() { diff --git a/core/modules/filter/filter.module b/core/modules/filter/filter.module index b326b8e..c260260 100644 --- a/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -416,83 +416,144 @@ function filter_get_filter_types_by_format($format_id) { } /** - * Retrieve all HTML tags and attributes allowed by a given text format. + * Retrieve all HTML restrictions (tags and attributes) for a given text format. * * @param string $format_id * A text format ID. * - * @return array|TRUE - * An array of HTML tags (in "p", not "
" format) that are allowed by the - * text format. The empty array implies no tags are allowed. TRUE implies all - * tags are allowed. + * @return array|FALSE + * An structured array as returned by FilterInterface::getHTMLRestrictions(), + * but with the intersection of all filters in this text format. + * Will either indicate blacklisting of tags or whitelisting of tags. In the + * latter case, it's possible that restrictions on attributes are also stored. + * FALSE means there are no HTML restrictions. */ -function filter_get_allowed_html_by_format($format_id) { +function filter_get_html_restrictions_by_format($format_id) { $format = filter_format_load($format_id); - // Ignore filters that are disabled or don't have an "allowed html" setting. + // Ignore filters that are disabled or don't have HTML restrictions. $filters = array_filter($format->filters()->getAll(), function($filter) { if (!$filter->status) { return FALSE; } - if ($filter->getType() === FILTER_TYPE_HTML_RESTRICTOR && $filter->getAllowedHTML() !== FALSE) { + if ($filter->getType() === FILTER_TYPE_HTML_RESTRICTOR && $filter->getHTMLRestrictions() !== FALSE) { return TRUE; } return FALSE; }); if (empty($filters)) { - return TRUE; + return FALSE; } else { // From the set of remaining filters (they were filtered by array_filter() // above), collect the list of tags and attributes that are allowed by all // filters, i.e. the intersection of all allowed tags and attributes. - $allowed_html = array_reduce($filters, function($intersection, $filter) { - $new_allowed_html = $filter->getAllowedHTML(); + $restrictions = array_reduce($filters, function($restrictions, $filter) { + $new_restrictions = $filter->getHTMLRestrictions(); - // The first filter with an "allowed html" setting provides the initial - // set. - if (!isset($intersection)) { - return $new_allowed_html; + // The first filter with HTML restrictions provides the initial set. + if (!isset($restrictions)) { + return $new_restrictions; } // Subsequent filters with an "allowed html" setting must be intersected // with the existing set, to ensure we only end up with the tags that are // allowed by *all* filters with an "allowed html" setting. else { - foreach ($intersection as $tag => $attributes) { - // If the current tag is disallowed by the new filter, then it's - // outside of the intersection. - if (!array_key_exists($tag, $new_allowed_html)) { - unset($intersection[$tag]); + // Track the union of forbidden (blacklisted) tags. + if (isset($new_restrictions['forbiddenTags'])) { + if (!isset($restrictions['forbiddenTags'])) { + $restrictions['forbiddenTags'] = $new_restrictions['forbiddenTags']; } - // The tag is in the intersection, but now we most calculate the - // intersection of the allowed attributes. else { - $current_attributes = $intersection[$tag]; - $new_attributes = $new_allowed_html[$tag]; - // The new filter allows less attributes; assign new. - if ($current_attributes == TRUE && is_array($new_attributes)) { - $intersection[$tag] = $new_attributes; - } - // The new filter allows more attributes; retain current. - else if (is_array($current_attributes) && $new_attributes == TRUE) { - continue; - } - // The new filter allows the same attributes; retain current. - else if ($current_attributes == $new_attributes) { - continue; + $restrictions['forbiddenTags'] = array_unique(array_merge($restrictions['forbiddenTags'], $new_restrictions['forbiddenTags'])); + } + } + + // Track the intersection of allowed (whitelisted) tags. + if (isset($restrictions['allowedTags'])) { + $intersection = $restrictions['allowedTags']; + foreach ($intersection as $tag => $attributes) { + // If the current tag is not whitelisted by the new filter, then + // it's outside of the intersection. + if (!array_key_exists($tag, $new_restrictions['allowedTags'])) { + // The exception is the asterisk (which applies to all tags): it + // does not need to be whitelisted by every filter in order to be + // used; not every filter needs attribute restrictions on all tags. + if ($tag === '*') { + continue; + } + unset($intersection[$tag]); } - // Both list an array of allowed attributes; do an intersection. + // The tag is in the intersection, but now we most calculate the + // intersection of the allowed attributes. else { - $intersection[$tag] = array_intersect($intersection[$tag], $new_attributes); + $current_attributes = $intersection[$tag]; + $new_attributes = $new_restrictions['allowedTags'][$tag]; + // The current intersection does not allow any attributes, never + // allow. + if (!is_array($current_attributes) && $current_attributes == FALSE) { + continue; + } + // The new filter allows less attributes (all -> list or none). + else if (!is_array($current_attributes) && $current_attributes == TRUE && ($new_attributes == FALSE || is_array($new_attributes))) { + $intersection[$tag] = $new_attributes; + } + // The new filter allows less attributes (list -> none). + else if (is_array($current_attributes) && $new_attributes == FALSE) { + $intersection[$tag] = $new_attributes; + } + // The new filter allows more attributes; retain current. + else if (is_array($current_attributes) && $new_attributes == TRUE) { + continue; + } + // The new filter allows the same attributes; retain current. + else if ($current_attributes == $new_attributes) { + continue; + } + // Both list an array of attribute values; do an intersection, + // where we take into account that a value of: + // - TRUE means the attribute value is allowed; + // - FALSE means the attribute value is forbidden; + // hence we keep the ANDed result. + else { + $intersection[$tag] = array_intersect_key($intersection[$tag], $new_attributes); + foreach (array_keys($intersection[$tag]) as $attribute_value) { + $intersection[$tag][$attribute_value] = $intersection[$tag][$attribute_value] && $new_attributes[$attribute_value]; + } + } } } + $restrictions['allowedTags'] = $intersection; } - return $intersection; + + return $restrictions; } }, NULL); - return $allowed_html; + // Simplification: if we have both a (intersected) whitelist and a (unioned) + // blacklist, then remove any tags from the whitelist that also exist in the + // blacklist. Now the whitelist alone expresses all tag-level restrictions, + // and we can delete the blacklist. + if (isset($restrictions['allowedTags']) && isset($restrictions['forbiddenTags'])) { + foreach ($restrictions['forbiddenTags'] as $tag) { + if (isset($restrictions['allowedTags'][$tag])) { + unset($restrictions['allowedTags'][$tag]); + } + } + unset($restrictions['forbiddenTags']); + } + + // 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'])) { + if (count($restrictions['allowedTags']) === 1 && array_key_exists('*', $restrictions['allowedTags']) && !isset($restrictions['forbiddenTags'])) { + $restrictions['allowedTags'] = array(); + } + } + + return $restrictions; } } diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php index 4d227c2..d09ddcf 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php @@ -68,13 +68,16 @@ public function process($text, $langcode, $cache, $cache_id) { /** * {@inheritdoc} */ - public function getAllowedHTML() { - $allowed_html = array(); + public function getHTMLRestrictions() { + $restrictions = array('allowedTags' => array()); $tags = preg_split('/\s+|<|>/', $this->settings['allowed_html'], -1, PREG_SPLIT_NO_EMPTY); + // List the allowed HTML tags. foreach ($tags as $tag) { - $allowed_html[$tag] = TRUE; + $restrictions['allowedTags'][$tag] = TRUE; } - return $allowed_html; + // The 'style' and 'on*' ('onClick' etc.) attributes are always forbidden. + $restrictions['allowedTags']['*'] = array('style' => FALSE, 'on*' => FALSE); + return $restrictions; } /** diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlEscape.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlEscape.php index 59f7867..ea05822 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlEscape.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlEscape.php @@ -34,8 +34,9 @@ public function process($text, $langcode, $cache, $cache_id) { /** * {@inheritdoc} */ - public function getAllowedHTML() { - return array(); + public function getHTMLRestrictions() { + // Nothing is allowed. + return array('allowedTags' => array()); } /** diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/FilterBase.php b/core/modules/filter/lib/Drupal/filter/Plugin/FilterBase.php index d6150db..2e80bd0 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/FilterBase.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/FilterBase.php @@ -150,7 +150,7 @@ public function prepare($text, $langcode, $cache, $cache_id) { /** * {@inheritdoc} */ - public function getAllowedHTML() { + public function getHTMLRestrictions() { return FALSE; } diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php b/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php index 7bd3203..92c5ebd 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php @@ -191,20 +191,57 @@ public function process($text, $langcode, $cache, $cache_id); * (keys) the corresponding allowed attributes. An empty array for allowed * attributes means no attributes are allowed, TRUE means all attributes are * allowed. An example: + * + * Here is a concrete example, for a very granular filter: + * array( + * 'allowedTags' => array( + * // Allows any attribute with any value on the
tag. + * 'div' => TRUE, + * // Allows no attributes on the