Issue #3272516 by Wim Leers, yogeshmpawar, bnjmnm, catch: Deprecate FilterInterface::getHTMLRestrictions()' forbidden_tags functionality

merge-requests/1884/merge
catch 2022-05-02 17:20:08 +01:00
parent 13645eaf45
commit 35b8d4f54c
11 changed files with 20 additions and 196 deletions

View File

@ -37,9 +37,6 @@ use Drupal\filter\Plugin\FilterInterface;
*
* @see ::WILDCARD_ELEMENT_METHODS
*
* NOTE: Currently only supports the 'allowed' portion.
* @todo Add support for "forbidden" tags in https://www.drupal.org/project/drupal/issues/3231336
*
* @internal
*/
final class HTMLRestrictions {
@ -341,12 +338,6 @@ final class HTMLRestrictions {
}
$restrictions = $object->getHTMLRestrictions();
if (!isset($restrictions['allowed'])) {
// @todo Handle HTML restrictor filters that only set forbidden_tags
// https://www.drupal.org/project/ckeditor5/issues/3231336.
throw new \DomainException('text formats with only filters that forbid tags rather than allowing tags are not yet supported.');
}
$allowed = $restrictions['allowed'];
return new self($allowed);

View File

@ -25,13 +25,6 @@ class FundamentalCompatibilityConstraint extends Constraint {
*/
public $noMarkupFiltersMessage = 'CKEditor 5 only works with HTML-based text formats. The "%filter_label" (%filter_plugin_id) filter implies this text format is not HTML anymore.';
/**
* The violation message when fundamental HTML elements are forbidden.
*
* @var string
*/
public $forbiddenElementsMessage = 'CKEditor 5 needs at least the <p> and <br> tags to be allowed to be able to function. They are forbidden by the "%filter_label" (%filter_plugin_id) filter.';
/**
* The violation message when fundamental HTML elements are not allowed.
*

View File

@ -114,22 +114,7 @@ class FundamentalCompatibilityConstraintValidator extends ConstraintValidator im
*/
private function checkHtmlRestrictionsAreCompatible(FilterFormatInterface $text_format, FundamentalCompatibilityConstraint $constraint): void {
$fundamental = new HTMLRestrictions($this->pluginManager->getProvidedElements(self::FUNDAMENTAL_CKEDITOR5_PLUGINS));
// @todo Remove in favor of HTMLRestrictions::diff() in https://www.drupal.org/project/drupal/issues/3231336
$html_restrictions = $text_format->getHtmlRestrictions();
$minimum_tags = array_keys($fundamental->getAllowedElements());
$forbidden_minimum_tags = isset($html_restrictions['forbidden_tags'])
? array_diff($minimum_tags, $html_restrictions['forbidden_tags'])
: [];
if (!empty($forbidden_minimum_tags)) {
$offending_filter = static::findHtmlRestrictorFilterForbiddingTags($text_format, $minimum_tags);
$this->context->buildViolation($constraint->forbiddenElementsMessage)
->setParameter('%filter_label', (string) $offending_filter->getLabel())
->setParameter('%filter_plugin_id', $offending_filter->getPluginId())
->addViolation();
}
// @todo Remove early return in https://www.drupal.org/project/drupal/issues/3231336
if (!isset($html_restrictions['allowed'])) {
return;
}
@ -212,47 +197,6 @@ class FundamentalCompatibilityConstraintValidator extends ConstraintValidator im
}
}
/**
* Analyzes a text format to find the filter not allowing required tags.
*
* @param \Drupal\filter\FilterFormatInterface $text_format
* A text format whose filters to check for compatibility.
* @param string[] $required_tags
* A list of HTML tags that are required.
*
* @return \Drupal\filter\Plugin\FilterInterface
* The filter plugin instance not allowing the required tags.
*
* @throws \InvalidArgumentException
*/
private static function findHtmlRestrictorFilterForbiddingTags(FilterFormatInterface $text_format, array $required_tags): FilterInterface {
// Get HTML restrictor filters that actually restrict HTML.
$filters = static::getFiltersInFormatOfType(
$text_format,
FilterInterface::TYPE_HTML_RESTRICTOR,
function (FilterInterface $filter) {
return $filter->getHTMLRestrictions() !== FALSE;
}
);
foreach ($filters as $filter) {
$restrictions = $filter->getHTMLRestrictions();
// @todo Fix
// \Drupal\filter_test\Plugin\Filter\FilterTestRestrictTagsAndAttributes::getHTMLRestrictions(),
// whose computed value for forbidden_tags does not comply with the API
// https://www.drupal.org/project/drupal/issues/3231331.
if (array_keys($restrictions['forbidden_tags']) != range(0, count($restrictions['forbidden_tags']))) {
$restrictions['forbidden_tags'] = array_keys($restrictions['forbidden_tags']);
}
if (isset($restrictions['forbidden_tags']) && !empty(array_intersect($required_tags, $restrictions['forbidden_tags']))) {
return $filter;
}
}
throw new \InvalidArgumentException('This text format does not have a "tags forbidden" restriction that includes the required tags.');
}
/**
* Analyzes a text format to find the filter not allowing required tags.
*

View File

@ -462,33 +462,6 @@ class ValidatorsTest extends KernelTestBase {
],
'violations' => [],
];
$data['INVALID: forbidden tags'] = [
'settings' => [
'toolbar' => [
'items' => [],
],
'plugins' => [],
],
'image_upload' => [
'status' => FALSE,
],
'filters' => [
'filter_test_restrict_tags_and_attributes' => [
'id' => 'filter_test_restrict_tags_and_attributes',
'provider' => 'filter_test',
'status' => TRUE,
'weight' => 0,
'settings' => [
'restrictions' => [
'forbidden_tags' => ['p' => FALSE],
],
],
],
],
'violations' => [
'' => 'CKEditor 5 needs at least the &lt;p&gt; and &lt;br&gt; tags to be allowed to be able to function. They are forbidden by the "<em class="placeholder">Tag and attribute restricting filter</em>" (<em class="placeholder">filter_test_restrict_tags_and_attributes</em>) filter.',
],
];
$restricted_html_format_filters = Yaml::parseFile(__DIR__ . '/../../../../../profiles/standard/config/install/filter.format.restricted_html.yml')['filters'];
$data['INVALID: the default restricted_html text format'] = [
'settings' => [

View File

@ -42,9 +42,9 @@ class Standard extends Xss implements EditorXssFilterInterface {
// <embed>, for example? Then we would strip that tag, even though it is
// allowed, thereby causing data loss!
// Therefore, we want to be smarter still. We want to take into account
// which HTML tags are allowed and forbidden by the text format we're
// filtering for, and if we're switching from another text format, we want
// to take that format's allowed and forbidden tags into account as well.
// which HTML tags are allowed by the text format we're filtering for, and
// if we're switching from another text format, we want to take that
// format's allowed tags into account as well.
// In other words: we only expect markup allowed in both the original and
// the new format to continue to exist.
$format_restrictions = $format->getHtmlRestrictions();
@ -52,17 +52,6 @@ class Standard extends Xss implements EditorXssFilterInterface {
$original_format_restrictions = $original_format->getHtmlRestrictions();
}
// Any tags that are explicitly blacklisted by the text format must be
// appended to the list of default dangerous tags: if they're explicitly
// forbidden, then we must respect that configuration.
// When switching from another text format, we must use the union of
// forbidden tags: if either text format is more restrictive, then the
// safety expectations of *both* text formats apply.
$forbidden_tags = self::getForbiddenTags($format_restrictions);
if ($original_format !== NULL) {
$forbidden_tags = array_merge($forbidden_tags, self::getForbiddenTags($original_format_restrictions));
}
// Any tags that are explicitly whitelisted by the text format must be
// removed from the list of default dangerous tags: if they're explicitly
// allowed, then we must respect that configuration.
@ -78,9 +67,6 @@ class Standard extends Xss implements EditorXssFilterInterface {
// formats.
$blacklisted_tags = array_diff($dangerous_tags, $allowed_tags);
// Also blacklist tags that are explicitly forbidden in either text format.
$blacklisted_tags = array_merge($blacklisted_tags, $forbidden_tags);
$output = static::filter($html, $blacklisted_tags);
// Since data-attributes can contain encoded HTML markup that could be
@ -140,26 +126,6 @@ class Standard extends Xss implements EditorXssFilterInterface {
return $allowed_tags;
}
/**
* Get all forbidden tags from a restrictions data structure.
*
* @param array|false $restrictions
* Restrictions as returned by FilterInterface::getHTMLRestrictions().
*
* @return array
* An array of forbidden HTML tags.
*
* @see \Drupal\filter\Plugin\Filter\FilterInterface::getHTMLRestrictions()
*/
protected static function getForbiddenTags($restrictions) {
if ($restrictions === FALSE || !isset($restrictions['forbidden_tags'])) {
return [];
}
else {
return $restrictions['forbidden_tags'];
}
}
/**
* {@inheritdoc}
*/

View File

@ -305,16 +305,6 @@ class FilterFormat extends ConfigEntityBase implements FilterFormatInterface, En
// 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 {
// Track the union of forbidden tags.
if (isset($new_restrictions['forbidden_tags'])) {
if (!isset($restrictions['forbidden_tags'])) {
$restrictions['forbidden_tags'] = $new_restrictions['forbidden_tags'];
}
else {
$restrictions['forbidden_tags'] = array_unique(array_merge($restrictions['forbidden_tags'], $new_restrictions['forbidden_tags']));
}
}
// Track the intersection of allowed tags.
if (isset($restrictions['allowed'])) {
$intersection = $restrictions['allowed'];
@ -372,32 +362,17 @@ class FilterFormat extends ConfigEntityBase implements FilterFormatInterface, En
$restrictions['allowed'] = $intersection;
}
// Simplification: if the only remaining allowed tag is the asterisk
// (which contains attribute restrictions that apply to all tags),
// then effectively nothing is allowed.
if (count($restrictions['allowed']) === 1 && array_key_exists('*', $restrictions['allowed'])) {
$restrictions['allowed'] = [];
}
return $restrictions;
}
}, NULL);
// Simplification: if we have both allowed (intersected) and forbidden
// (unioned) tags, then remove any allowed tags that are also forbidden.
// Once complete, the list of allowed tags expresses all tag-level
// restrictions, and the list of forbidden tags can be removed.
if (isset($restrictions['allowed']) && isset($restrictions['forbidden_tags'])) {
foreach ($restrictions['forbidden_tags'] as $tag) {
if (isset($restrictions['allowed'][$tag])) {
unset($restrictions['allowed'][$tag]);
}
}
unset($restrictions['forbidden_tags']);
}
// Simplification: if the only remaining allowed tag is the asterisk
// (which contains attribute restrictions that apply to all tags), and
// there are no forbidden tags, then effectively nothing is allowed.
if (isset($restrictions['allowed'])) {
if (count($restrictions['allowed']) === 1 && array_key_exists('*', $restrictions['allowed']) && !isset($restrictions['forbidden_tags'])) {
$restrictions['allowed'] = [];
}
}
return $restrictions;
}
}

View File

@ -72,10 +72,8 @@ interface FilterFormatInterface extends ConfigEntityInterface {
*
* @return array|false
* A structured array as returned by FilterInterface::getHTMLRestrictions(),
* but with the intersection of all filters in this text format. The
* restrictions will either forbid or allow a list of tags. In the latter
* case, it's possible that restrictions on attributes are also stored.
* FALSE means there are no HTML restrictions.
* but with the intersection of all filters in this text format. FALSE means
* there are no HTML restrictions.
*/
public function getHtmlRestrictions();

View File

@ -184,9 +184,9 @@ interface FilterInterface extends ConfigurableInterface, DependentPluginInterfac
* format.
*
* @return array|false
* A nested array with *either* of the following keys:
* - 'allowed': (optional) the allowed tags as keys, and for each of those
* tags (keys) either of the following values:
* A nested array with the following structure:
* - 'allowed': the allowed tags as keys, and for each of those tags
* (keys) either of the following values:
* - TRUE to indicate any attribute is allowed
* - FALSE to indicate no attributes are allowed
* - an array to convey attribute restrictions: the keys must be
@ -198,7 +198,6 @@ interface FilterInterface extends ConfigurableInterface, DependentPluginInterfac
* be attribute values (which may use a wildcard, e.g. "xsd:*"),
* the possible values are TRUE or FALSE: to mark the attribute
* value as allowed or forbidden, respectively
* - 'forbidden_tags': (optional) the forbidden tags
*
* There is one special case: the "wildcard tag", "*": any attribute
* restrictions on that pseudotag apply to all tags.
@ -243,13 +242,6 @@ interface FilterInterface extends ConfigurableInterface, DependentPluginInterfac
* )
* @endcode
*
* A simpler example, for a very coarse filter:
* @code
* array(
* 'forbidden_tags' => array('iframe', 'script')
* )
* @endcode
*
* The simplest example possible: a filter that doesn't allow any HTML:
* @code
* array(

View File

@ -14,9 +14,3 @@ filter_settings.filter_test_restrict_tags_and_attributes:
sequence:
type: ignore
label: 'Tag and optionally list of attributes'
forbidden_tags:
type: sequence
label: 'Forbidden tags'
sequence:
type: boolean
label: 'Tag'

View File

@ -54,11 +54,6 @@ class FilterTestRestrictTagsAndAttributes extends FilterBase {
}
}
}
if (isset($restrictions['forbidden_tags'])) {
foreach ($restrictions['forbidden_tags'] as $tag => $bool) {
$restrictions['forbidden_tags'][$tag] = (bool) $bool;
}
}
return $restrictions;
}

View File

@ -137,8 +137,11 @@ class FilterAPITest extends EntityKernelTestBase {
$stupid_filtered_html_format->save();
$this->assertSame(
$stupid_filtered_html_format->getHtmlRestrictions(),
// No tag is allowed.
['allowed' => []],
[
'allowed' => [
'*' => ['style' => FALSE, 'on*' => FALSE, 'lang' => TRUE, 'dir' => ['ltr' => TRUE, 'rtl' => TRUE]],
],
],
'FilterFormatInterface::getHtmlRestrictions() works as expected for the stupid_filtered_html format.'
);
$this->assertSame(