summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Pott2015-11-02 16:22:24 (GMT)
committerAlex Pott2015-11-02 16:22:24 (GMT)
commit573fab03c7867bcf6800d28d96a9fa611f367ae8 (patch)
tree7cf121412c471536679e2d124b967684465b1fb3
parent468056c3f10f42caa358369d21036825a79bb524 (diff)
Issue #2596083 by Wim Leers, lokapujya, jamin_melville: Allowed attribute 'class' not respected in CKEditor: ACF strips it
-rw-r--r--core/modules/ckeditor/src/Plugin/CKEditorPlugin/Internal.php26
-rw-r--r--core/modules/ckeditor/src/Tests/CKEditorTest.php6
-rw-r--r--core/modules/filter/src/Plugin/Filter/FilterHtml.php19
-rw-r--r--core/modules/filter/src/Tests/FilterAPITest.php35
4 files changed, 72 insertions, 14 deletions
diff --git a/core/modules/ckeditor/src/Plugin/CKEditorPlugin/Internal.php b/core/modules/ckeditor/src/Plugin/CKEditorPlugin/Internal.php
index 7638a17..d1b317e 100644
--- a/core/modules/ckeditor/src/Plugin/CKEditorPlugin/Internal.php
+++ b/core/modules/ckeditor/src/Plugin/CKEditorPlugin/Internal.php
@@ -538,16 +538,26 @@ class Internal extends CKEditorPluginBase implements ContainerFactoryPluginInter
if (count($allowed_attributes)) {
$allowed[$tag]['attributes'] = implode(',', array_keys($allowed_attributes));
}
- if (isset($allowed_attributes['style']) && is_array($allowed_attributes['style'])) {
- $allowed_styles = $get_attribute_values($allowed_attributes['style'], TRUE);
- if (isset($allowed_styles)) {
- $allowed[$tag]['styles'] = $allowed_styles;
+ if (isset($allowed_attributes['style'])) {
+ if (is_bool($allowed_attributes['style'])) {
+ $allowed[$tag]['styles'] = $allowed_attributes['style'];
+ }
+ elseif (is_array($allowed_attributes['style'])) {
+ $allowed_classes = $get_attribute_values($allowed_attributes['style'], TRUE);
+ if (isset($allowed_classes)) {
+ $allowed[$tag]['styles'] = $allowed_classes;
+ }
}
}
- if (isset($allowed_attributes['class']) && is_array($allowed_attributes['class'])) {
- $allowed_classes = $get_attribute_values($allowed_attributes['class'], TRUE);
- if (isset($allowed_classes)) {
- $allowed[$tag]['classes'] = $allowed_classes;
+ if (isset($allowed_attributes['class'])) {
+ if (is_bool($allowed_attributes['class'])) {
+ $allowed[$tag]['classes'] = $allowed_attributes['class'];
+ }
+ elseif (is_array($allowed_attributes['class'])) {
+ $allowed_classes = $get_attribute_values($allowed_attributes['class'], TRUE);
+ if (isset($allowed_classes)) {
+ $allowed[$tag]['classes'] = $allowed_classes;
+ }
}
}
diff --git a/core/modules/ckeditor/src/Tests/CKEditorTest.php b/core/modules/ckeditor/src/Tests/CKEditorTest.php
index dd9a7bf..baef0be 100644
--- a/core/modules/ckeditor/src/Tests/CKEditorTest.php
+++ b/core/modules/ckeditor/src/Tests/CKEditorTest.php
@@ -123,11 +123,13 @@ class CKEditorTest extends KernelTestBase {
// Change the allowed HTML tags; the "allowedContent" and "format_tags"
// settings for CKEditor should automatically be updated as well.
$format = $editor->getFilterFormat();
- $format->filters('filter_html')->settings['allowed_html'] .= '<pre> <h1>';
+ $format->filters('filter_html')->settings['allowed_html'] .= '<pre class> <h1> <blockquote class="*"> <address class="foo bar-* *">';
$format->save();
- $expected_config['allowedContent']['pre'] = array('attributes' => FALSE, 'styles' => FALSE, 'classes' => FALSE);
+ $expected_config['allowedContent']['pre'] = array('attributes' => 'class', 'styles' => FALSE, 'classes' => TRUE);
$expected_config['allowedContent']['h1'] = array('attributes' => FALSE, 'styles' => FALSE, 'classes' => FALSE);
+ $expected_config['allowedContent']['blockquote'] = array('attributes' => 'class', 'styles' => FALSE, 'classes' => TRUE);
+ $expected_config['allowedContent']['address'] = array('attributes' => 'class', 'styles' => FALSE, 'classes' => 'foo,bar-*');
$expected_config['format_tags'] = 'p;h1;h2;h3;h4;h5;h6;pre';
ksort($expected_config['allowedContent']);
$this->assertIdentical($expected_config, $this->castSafeStrings($this->ckeditor->getJSSettings($editor)), 'Generated JS settings are correct for customized configuration.');
diff --git a/core/modules/filter/src/Plugin/Filter/FilterHtml.php b/core/modules/filter/src/Plugin/Filter/FilterHtml.php
index 1baf73c..63c22b3 100644
--- a/core/modules/filter/src/Plugin/Filter/FilterHtml.php
+++ b/core/modules/filter/src/Plugin/Filter/FilterHtml.php
@@ -275,16 +275,27 @@ class FilterHtml extends FilterBase {
foreach ($node->attributes as $name => $attribute) {
// Put back any trailing * on wildcard attribute name.
$name = str_replace($star_protector, '*', $name);
- if ($attribute->value === '') {
+
+ // Put back any trailing * on wildcard attribute value and parse out
+ // the allowed attribute values.
+ $allowed_attribute_values = preg_split('/\s+/', str_replace($star_protector, '*', $attribute->value), -1, PREG_SPLIT_NO_EMPTY);
+
+ // Sanitize the attribute value: it lists the allowed attribute values
+ // but one allowed attribute value that some may be tempted to use
+ // is specifically nonsensical: the asterisk. A prefix is required for
+ // allowed attribute values with a wildcard. A wildcard by itself
+ // would mean whitelisting all possible attribute values. But in that
+ // case, one would not specify an attribute value at all.
+ $allowed_attribute_values = array_filter($allowed_attribute_values, function ($value) use ($star_protector) { return $value !== '*'; });
+
+ if (empty($allowed_attribute_values)) {
// If the value is the empty string all values are allowed.
$restrictions['allowed'][$tag][$name] = TRUE;
}
else {
// A non-empty attribute value is assigned, mark each of the
// specified attribute values as allowed.
- foreach (preg_split('/\s+/', $attribute->value, -1, PREG_SPLIT_NO_EMPTY) as $value) {
- // Put back any trailing * on wildcard attribute value.
- $value = str_replace($star_protector, '*', $value);
+ foreach ($allowed_attribute_values as $value) {
$restrictions['allowed'][$tag][$name][$value] = TRUE;
}
}
diff --git a/core/modules/filter/src/Tests/FilterAPITest.php b/core/modules/filter/src/Tests/FilterAPITest.php
index 08a5e56..5fd9999 100644
--- a/core/modules/filter/src/Tests/FilterAPITest.php
+++ b/core/modules/filter/src/Tests/FilterAPITest.php
@@ -207,6 +207,41 @@ class FilterAPITest extends EntityUnitTestBase {
array(FilterInterface::TYPE_HTML_RESTRICTOR),
'FilterFormatInterface::getFilterTypes() works as expected for the very_restricted_html format.'
);
+
+ // Test on nonsensical_restricted_html, where the allowed attribute values
+ // contain asterisks, which do not have any meaning, but which we also
+ // cannot prevent because configuration can be modified outside of forms.
+ $nonsensical_restricted_html = \Drupal\filter\Entity\FilterFormat::create(array(
+ 'format' => 'nonsensical_restricted_html',
+ 'name' => 'Nonsensical Restricted HTML',
+ 'filters' => array(
+ 'filter_html' => array(
+ 'status' => 1,
+ 'settings' => array(
+ 'allowed_html' => '<a> <b class> <c class="*"> <d class="foo bar-* *">',
+ ),
+ ),
+ )
+ ));
+ $nonsensical_restricted_html->save();
+ $this->assertIdentical(
+ $nonsensical_restricted_html->getHtmlRestrictions(),
+ array(
+ 'allowed' => array(
+ 'a' => FALSE,
+ 'b' => array('class' => TRUE),
+ 'c' => array('class' => TRUE),
+ 'd' => array('class' => array('foo' => TRUE, 'bar-*' => TRUE)),
+ '*' => array('style' => FALSE, 'on*' => FALSE, 'lang' => TRUE, 'dir' => array('ltr' => TRUE, 'rtl' => TRUE)),
+ ),
+ ),
+ 'FilterFormatInterface::getHtmlRestrictions() works as expected for the nonsensical_restricted_html format.'
+ );
+ $this->assertIdentical(
+ $very_restricted_html_format->getFilterTypes(),
+ array(FilterInterface::TYPE_HTML_RESTRICTOR),
+ 'FilterFormatInterface::getFilterTypes() works as expected for the very_restricted_html format.'
+ );
}
/**