Issue #3259493 by Wim Leers, lauriii, larowlan: [GHS] Unable to limit attribute values: ::allowedElementsStringToHtmlSupportConfig() does not generate configuration that CKEditor 5 expects

merge-requests/1877/head
bnjmnm 2022-02-23 05:53:44 -05:00
parent 11fc6f006e
commit 910b34ac7a
4 changed files with 247 additions and 7 deletions

View File

@ -807,8 +807,26 @@ final class HTMLRestrictions {
if (is_array($value)) {
$value = array_keys($value);
}
// Drupal never allows style attributes due to security concerns.
// @see \Drupal\Component\Utility\Xss
if ($name === 'style') {
continue;
}
assert($value === TRUE || Inspector::assertAllStrings($value));
$to_allow['attributes'][$name] = $value;
if ($name === 'class') {
$to_allow['classes'] = $value;
continue;
}
// If a single attribute value is allowed, it must be TRUE (see the
// assertion above). Otherwise, it must be an array of strings (see
// the assertion above), which lists all allowed attribute values. To
// be able to configure GHS to a range of values, we need to use a
// regular expression.
// @todo Expand to support partial wildcards in
// https://www.drupal.org/project/drupal/issues/3260853.
$to_allow['attributes'][$name] = is_array($value)
? ['regexp' => ['pattern' => '/^(' . implode('|', $value) . ')$/']]
: $value;
}
}
$allowed[] = $to_allow;

View File

@ -0,0 +1,205 @@
<?php
namespace Drupal\Tests\ckeditor5\FunctionalJavascript;
use Drupal\ckeditor5\HTMLRestrictions;
use Drupal\editor\Entity\Editor;
use Drupal\filter\Entity\FilterFormat;
use Drupal\Tests\ckeditor5\Traits\CKEditor5TestTrait;
use Drupal\ckeditor5\Plugin\Editor\CKEditor5;
use Symfony\Component\Validator\ConstraintViolation;
/**
* @coversDefaultClass \Drupal\ckeditor5\Plugin\CKEditor5Plugin\SourceEditing
* @group ckeditor5
* @internal
*/
class SourceEditingTest extends CKEditor5TestBase {
use CKEditor5TestTrait;
/**
* The user to use during testing.
*
* @var \Drupal\user\UserInterface
*/
protected $adminUser;
/**
* A host entity with a body field whose text to edit with CKEditor 5.
*
* @var \Drupal\node\NodeInterface
*/
protected $host;
/**
* {@inheritdoc}
*/
protected static $modules = [
'ckeditor5',
'node',
'text',
];
/**
* {@inheritdoc}
*/
protected $defaultTheme = 'stark';
/**
* {@inheritdoc}
*/
protected function setUp(): void {
parent::setUp();
FilterFormat::create([
'format' => 'test_format',
'name' => 'Test format',
'filters' => [
'filter_html' => [
'status' => TRUE,
'settings' => [
'allowed_html' => '<p> <br> <a href>',
],
],
'filter_align' => ['status' => TRUE],
'filter_caption' => ['status' => TRUE],
],
])->save();
Editor::create([
'editor' => 'ckeditor5',
'format' => 'test_format',
'settings' => [
'toolbar' => [
'items' => [
'sourceEditing',
'link',
],
],
'plugins' => [
'ckeditor5_sourceEditing' => [
'allowed_tags' => [],
],
],
],
'image_upload' => [
'status' => FALSE,
],
])->save();
$this->assertSame([], array_map(
function (ConstraintViolation $v) {
return (string) $v->getMessage();
},
iterator_to_array(CKEditor5::validatePair(
Editor::load('test_format'),
FilterFormat::load('test_format')
))
));
$this->adminUser = $this->drupalCreateUser([
'use text format test_format',
'bypass node access',
]);
// Create a sample host entity to test CKEditor 5.
$this->host = $this->createNode([
'type' => 'page',
'title' => 'Animals with strange names',
'body' => [
'value' => '<p>The <a href="https://example.com/pirate" class="button" data-grammar="subject">pirate</a> is <a href="https://example.com/irate" class="use-ajax" data-grammar="adjective">irate</a>.</p>',
'format' => 'test_format',
],
]);
$this->host->save();
$this->drupalLogin($this->adminUser);
}
/**
* Tests allowing extra attributes on already supported tags using GHS.
*
* @dataProvider providerAllowingExtraAttributes
*/
public function testAllowingExtraAttributes(string $expected_markup, ?string $allowed_elements_string = NULL) {
if ($allowed_elements_string) {
// Allow creating additional HTML using SourceEditing.
$text_editor = Editor::load('test_format');
$settings = $text_editor->getSettings();
$settings['plugins']['ckeditor5_sourceEditing']['allowed_tags'][] = $allowed_elements_string;
$text_editor->setSettings($settings);
// Keep the allowed HTML tags in sync.
$text_format = FilterFormat::load('test_format');
$allowed_elements = HTMLRestrictions::fromTextFormat($text_format);
$updated_allowed_tags = $allowed_elements->merge(HTMLRestrictions::fromString($allowed_elements_string));
$filter_html_config = $text_format->filters('filter_html')
->getConfiguration();
$filter_html_config['settings']['allowed_html'] = $updated_allowed_tags->toFilterHtmlAllowedTagsString();
$text_format->setFilterConfig('filter_html', $filter_html_config);
// Verify the text format and editor are still a valid pair.
$this->assertSame([], array_map(
function (ConstraintViolation $v) {
return (string) $v->getMessage();
},
iterator_to_array(CKEditor5::validatePair(
$text_editor,
$text_format
))
));
// If valid, save both.
$text_format->save();
$text_editor->save();
}
$this->drupalGet($this->host->toUrl('edit-form'));
$this->waitForEditor();
$this->assertSame($expected_markup, $this->getEditorDataAsHtmlString());
}
/**
* Data provider for ::testAllowingExtraAttributes().
*
* @return array
* The test cases.
*/
public function providerAllowingExtraAttributes(): array {
return [
'no extra attributes allowed' => [
'<p>The <a href="https://example.com/pirate">pirate</a> is <a href="https://example.com/irate">irate</a>.</p>',
],
// Common case: any attribute that is not `style` or `class`.
'<a data-grammar="subject">' => [
'<p>The <a href="https://example.com/pirate" data-grammar="subject">pirate</a> is <a href="https://example.com/irate">irate</a>.</p>',
'<a data-grammar="subject">',
],
'<a data-grammar="adjective">' => [
'<p>The <a href="https://example.com/pirate">pirate</a> is <a href="https://example.com/irate" data-grammar="adjective">irate</a>.</p>',
'<a data-grammar="adjective">',
],
'<a data-grammar>' => [
'<p>The <a href="https://example.com/pirate" data-grammar="subject">pirate</a> is <a href="https://example.com/irate" data-grammar="adjective">irate</a>.</p>',
'<a data-grammar>',
],
// Edge case: `class`.
'<a class="button">' => [
'<p>The <a class="button" href="https://example.com/pirate">pirate</a> is <a href="https://example.com/irate">irate</a>.</p>',
'<a class="button">',
],
'<a class="use-ajax">' => [
'<p>The <a href="https://example.com/pirate">pirate</a> is <a class="use-ajax" href="https://example.com/irate">irate</a>.</p>',
'<a class="use-ajax">',
],
'<a class>' => [
'<p>The <a class="button" href="https://example.com/pirate">pirate</a> is <a class="use-ajax" href="https://example.com/irate">irate</a>.</p>',
'<a class>',
],
// Edge case: `style`.
// @todo https://www.drupal.org/project/drupal/issues/3260857
];
}
}

View File

@ -19,10 +19,20 @@ trait CKEditor5TestTrait {
*
* @return \DOMDocument
* The result of parsing CKEditor 5's data into a PHP DOMDocument.
*/
protected function getEditorDataAsDom(): \DOMDocument {
return Html::load($this->getEditorDataAsHtmlString());
}
/**
* Gets CKEditor 5 instance data as a HTML string.
*
* @return string
* The result of retrieving CKEditor 5's data.
*
* @see https://ckeditor.com/docs/ckeditor5/latest/api/module_editor-classic_classiceditor-ClassicEditor.html#function-getData
*/
protected function getEditorDataAsDom(): \DOMDocument {
protected function getEditorDataAsHtmlString(): string {
// We cannot trust on CKEditor updating the textarea every time model
// changes. Therefore, the most reliable way to get downcasted data is to
// use the CKEditor API.
@ -31,7 +41,7 @@ trait CKEditor5TestTrait {
return Drupal.CKEditor5Instances.get(Drupal.CKEditor5Instances.keys().next().value).getData();
})();
JS;
return Html::load($this->getSession()->evaluateScript($javascript));
return $this->getSession()->evaluateScript($javascript);
}
/**

View File

@ -442,15 +442,19 @@ class HTMLRestrictionsTest extends UnitTestCase {
];
yield 'realistic' => [
new HTMLRestrictions(['a' => ['href' => TRUE, 'hreflang' => ['en' => TRUE, 'fr' => TRUE]], 'p' => ['data-*' => TRUE], 'br' => FALSE]),
['<a href hreflang="en fr">', '<p data-*>', '<br>'],
'<a href hreflang="en fr"> <p data-*> <br>',
new HTMLRestrictions(['a' => ['href' => TRUE, 'hreflang' => ['en' => TRUE, 'fr' => TRUE]], 'p' => ['data-*' => TRUE, 'class' => ['block' => TRUE]], 'br' => FALSE]),
['<a href hreflang="en fr">', '<p data-* class="block">', '<br>'],
'<a href hreflang="en fr"> <p data-* class="block"> <br>',
[
[
'name' => 'a',
'attributes' => [
'href' => TRUE,
'hreflang' => ['en', 'fr'],
'hreflang' => [
'regexp' => [
'pattern' => '/^(en|fr)$/',
],
],
],
],
[
@ -458,6 +462,9 @@ class HTMLRestrictionsTest extends UnitTestCase {
'attributes' => [
'data-*' => TRUE,
],
'classes' => [
'block',
],
],
['name' => 'br'],
],