core/modules/ckeditor/js/ckeditor.js | 77 ++++++++++- .../ckeditor/Plugin/CKEditorPlugin/Internal.php | 137 ++++++++++++++++++- .../lib/Drupal/ckeditor/Tests/CKEditorTest.php | 30 ++++- core/modules/filter/filter.module | 142 ++++++++++++++++++++ .../lib/Drupal/filter/Plugin/Filter/FilterHtml.php | 15 +++ .../filter/Plugin/Filter/FilterHtmlEscape.php | 8 ++ .../filter/lib/Drupal/filter/Plugin/FilterBase.php | 7 + .../lib/Drupal/filter/Plugin/FilterInterface.php | 68 ++++++++++ .../lib/Drupal/filter/Tests/FilterAPITest.php | 99 ++++++++++++-- .../Filter/FilterTestRestrictTagsAndAttributes.php | 41 ++++++ 10 files changed, 609 insertions(+), 15 deletions(-) diff --git a/core/modules/ckeditor/js/ckeditor.js b/core/modules/ckeditor/js/ckeditor.js index 447f916..895a5d1 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 blacklist 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 4516762..a0d0642 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php @@ -56,7 +56,11 @@ public function getConfig(Editor $editor) { ), ); - // Next, add the format_tags setting, if its button is enabled. + // Add the allowedContent setting, which ensures CKEditor only allows tags + // and attributes that are allowed by the text format for this text editor. + $config['allowedContent'] = $this->generateAllowedContentSetting($editor); + + // Add the format_tags setting, if its button is enabled. $toolbar_buttons = array_unique(NestedArray::mergeDeepArray($editor->settings['toolbar']['buttons'])); if (in_array('Format', $toolbar_buttons)) { $config['format_tags'] = $this->generateFormatTagsSetting($editor); @@ -243,6 +247,7 @@ public function getButtons() { * * @param \Drupal\editor\Plugin\Core\Entity\Editor $editor * A configured text editor object. + * * @return array * An array containing the "format_tags" configuration. */ @@ -265,4 +270,134 @@ protected function generateFormatTagsSetting(Editor $editor) { return implode(';', $format_tags); } + /** + * Builds the "allowedContent" configuration part of the CKEditor JS settings. + * + * This ensures that CKEditor obeys the HTML restrictions defined by Drupal's + * filter system, by enabling CKEditor's Advanced Content Filter (ACF) + * functionality: http://ckeditor.com/blog/CKEditor-4.1-RC-Released. + * + * @see getConfig() + * + * @param \Drupal\editor\Plugin\Core\Entity\Editor $editor + * A configured text editor object. + * + * @return string|TRUE + * The "allowedContent" configuration: a well-formatted string or TRUE. The + * latter indicates that anything is allowed. + */ + protected function generateAllowedContentSetting(Editor $editor) { + // When nothing is disallowed, set allowedContent to true. + $filter_types = filter_get_filter_types_by_format($editor->format); + if (!in_array(FILTER_TYPE_HTML_RESTRICTOR, $filter_types)) { + return TRUE; + } + // Generate setting that accurately reflects allowed tags and attributes. + else { + $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 ($html_restrictions === FALSE) { + return TRUE; + } + $setting = array(); + foreach ($html_restrictions['allowedTags'] as $tag => $attributes) { + if ($attributes === TRUE) { + // 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 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 $setting; + } + } + } diff --git a/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php b/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php index 6f71077..70640db 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorTest.php @@ -53,6 +53,9 @@ function setUp() { 'filters' => array( 'filter_html' => array( 'status' => 1, + 'settings' => array( + 'allowed_html' => '