Issue #3274648 by nod_, Wim Leers: HTMLRestrictions::merge() and... (fd25d636) · Commits · project / drupal

core/modules/ckeditor5/src/HTMLRestrictions.php

+66 −98

Original line number	Diff line number	Diff line
		@@ -693,6 +693,67 @@ public function doIntersect(HTMLRestrictions $other): HTMLRestrictions {
		return new self($intersection);
		}

		/**
		* Merge arrays of allowed elements according to HTMLRestrictions rules.
		*
		* @param array $array1
		* The first array of allowed elements.
		* @param array $array2
		* The second array of allowed elements.
		*
		* @return array
		* Merged array of allowed elements.
		*/
		private static function mergeAllowedElementsLevel(array $array1, array $array2): array {
		$union = [];
		$array1_keys = array_keys($array1);
		$array2_keys = array_keys($array2);
		$common_keys = array_intersect($array1_keys, $array2_keys);
		if (count($common_keys) === 0) {
		// There are no keys in common, simply append the arrays.
		$union = $array1 + $array2;
		}
		else {
		// For all the distinct keys, append them to the result.
		$filter_keys = array_flip($common_keys);
		// Add all unique keys from $array1.
		$union += array_diff_key($array1, $filter_keys);
		// Add all unique keys from $array2.
		$union += array_diff_key($array2, $filter_keys);

		// There are some keys in common that need to be merged.
		foreach ($common_keys as $key) {
		$value1 = $array1[$key];
		$value2 = $array2[$key];
		$value1_is_bool = is_bool($value1);
		$value2_is_bool = is_bool($value2);

		// When both values are boolean, combine the two.
		if ($value1_is_bool && $value2_is_bool) {
		$union[$key] = $value1 \|\| $value2;
		}
		// When only one value is a boolean, take the most permissive result:
		// - when the value it TRUE, keep TRUE as it is the most permissive
		// - when the value is FALSE, take the other value.
		elseif ($value1_is_bool) {
		$union[$key] = $value1 ?: $value2;
		}
		elseif ($value2_is_bool) {
		$union[$key] = $value2 ?: $value1;
		}
		// Process nested arrays, in this case it correspond to tag attributes
		// configuration.
		elseif (is_array($value1) && is_array($value2)) {
		$union[$key] = self::mergeAllowedElementsLevel($value1, $value2);
		}
		}
		}
		// Make sure the order of the union array matches the order of the keys in
		// the arrays provided.
		$keys_order = array_merge($array1_keys, $array2_keys);
		return array_merge(array_flip($keys_order), $union);
		}

		/**
		* Computes set union of two HTML restrictions, with wildcard support.
		*
		@@ -704,103 +765,7 @@ public function doIntersect(HTMLRestrictions $other): HTMLRestrictions {
		* are either allowed in $this or in $other.
		*/
		public function merge(HTMLRestrictions $other): HTMLRestrictions {
		$union = array_merge_recursive($this->elements, $other->elements);
		// When recursively merging elements arrays, unkeyed boolean values can
		// appear in attribute config arrays. This removes them.
		foreach ($union as $tag => $tag_config) {
		if (is_array($tag_config)) {
		// If the HTML tag restrictions for both operands were both booleans,
		// then the result of array_merge_recursive() is an array containing two
		// booleans (because it is designed for arrays, not for also merging
		// booleans) under the first two numeric keys: 0 and 1. This does not
		// match the structure expected of HTML restrictions. Combine the two
		// booleans.
		if (array_key_exists(0, $tag_config) && array_key_exists(1, $tag_config) && is_bool($tag_config[0]) && is_bool($tag_config[1])) {
		// Twice FALSE.
		if ($tag_config === [FALSE, FALSE]) {
		$union[$tag] = FALSE;
		}
		// Once or twice TRUE.
		else {
		$union[$tag] = TRUE;
		}
		continue;
		}

		// If the HTML tag restrictions for only one of the two operands was a
		// boolean, then the result of array_merge_recursive() is an array
		// containing the complete contents of the non-boolean operand plus an
		// additional key-value pair with the first numeric key: 0.
		if (array_key_exists(0, $tag_config)) {
		// If the boolean was FALSE (meaning: "no attributes allowed"), then
		// the other operand's values should be used in an union: this yields
		// the most permissive result.
		if ($tag_config[0] === FALSE) {
		unset($union[$tag][0]);
		}
		// If the boolean was TRUE (meaning: "all attributes allowed"), then
		// the other operand's values should be ignored in an union: this
		// yields the most permissive result.
		elseif ($tag_config[0] === TRUE) {
		$union[$tag] = TRUE;
		}
		continue;
		}

		// If the HTML tag restrictions are arrays for both operands, similar
		// logic needs to be applied to the attribute-level restrictions.
		foreach ($tag_config as $html_tag_attribute_name => $html_tag_attribute_restrictions) {
		if (is_bool($html_tag_attribute_restrictions)) {
		continue;
		}

		if (array_key_exists(0, $html_tag_attribute_restrictions)) {
		// Special case: the global attribute `*` HTML tag.
		// @see https://html.spec.whatwg.org/multipage/dom.html#global-attributes
		// @see validateAllowedRestrictionsPhase2()
		// @see validateAllowedRestrictionsPhase4()
		if ($tag === '*') {
		assert(is_bool($html_tag_attribute_restrictions[0]) \|\| is_bool($html_tag_attribute_restrictions[1]));
		// When both are boolean, pick the most permissive value.
		if (is_bool($html_tag_attribute_restrictions[0]) && isset($html_tag_attribute_restrictions[1]) && is_bool($html_tag_attribute_restrictions[1])) {
		$value = $html_tag_attribute_restrictions[0] \|\| $html_tag_attribute_restrictions[1];
		}
		else {
		$value = is_bool($html_tag_attribute_restrictions[0])
		? $html_tag_attribute_restrictions[0]
		: $html_tag_attribute_restrictions[1];
		}
		$union[$tag][$html_tag_attribute_name] = $value;
		continue;
		}

		// The "twice FALSE" case cannot occur for attributes, because
		// attribute restrictions either have "TRUE" (to indicate any value
		// is allowed for the attribute) or a list of allowed attribute
		// values. If there is a numeric key, then one of the two operands
		// must allow all attribute values (the "TRUE" case). Otherwise, an
		// array merge would have happened, and no numeric key would exist.
		// Therefore, this is always once or twice TRUE.
		// e.g.: <foo bar> and <foo bar>, or <foo bar> and <foo bar="baz">
		assert($html_tag_attribute_restrictions[0] === TRUE \|\| $html_tag_attribute_restrictions[1] === TRUE);
		$union[$tag][$html_tag_attribute_name] = TRUE;
		}
		else {
		// Finally, when both operands list the same allowed attribute
		// values, then the result provided by array_merge_recursive() for
		// those allowed attribute values is an array containing two times
		// `TRUE` (because it is designed for arrays, not for also merging
		// booleans) under the first two numeric keys: 0 and 1.
		// e.g.: <foo bar="baz qux"> merged with <foo bar="baz quux">.
		foreach ($html_tag_attribute_restrictions as $allowed_attribute_value => $merged_result) {
		if ($merged_result === [0 => TRUE, 1 => TRUE]) {
		$union[$tag][$html_tag_attribute_name][$allowed_attribute_value] = TRUE;
		}
		}
		}
		}
		}
		}
		$union = self::mergeAllowedElementsLevel($this->elements, $other->elements);

		// Special case: wildcard attributes, and the ability to define restrictions
		// for all concrete attributes matching them using:
		@@ -1109,7 +1074,10 @@ public function toGeneralHtmlSupportConfig(): array {
		// that this class expects to the `['en', 'fr']` structure that the
		// GHS functionality in CKEditor 5 expects.
		if (is_array($value)) {
		$value = array_keys($value);
		// Ensure that all values are strings, this is necessary since PHP
		// transforms the "1" string into 1 the number when it is used as
		// an array key.
		$value = array_map('strval', array_keys($value));
		}
		// Drupal never allows style attributes due to security concerns.
		// @see \Drupal\Component\Utility\Xss

core/modules/ckeditor5/tests/src/Unit/HTMLRestrictionsTest.php

+57 −0

Original line number	Diff line number	Diff line
		@@ -294,6 +294,14 @@ public function providerConvenienceConstructors(): \Generator {
		'<a target class>',
		['a' => ['target' => TRUE, 'class' => TRUE]],
		];
		yield 'tag with allowed attribute value that happen to be numbers' => [
		'<ol type="1 A I">',
		['ol' => ['type' => [1 => TRUE, 'A' => TRUE, 'I' => TRUE]]],
		];
		yield 'tag with allowed attribute value that happen to be numbers (reversed)' => [
		'<ol type="I A 1">',
		['ol' => ['type' => ['I' => TRUE, 'A' => TRUE, 1 => TRUE]]],
		];

		// Multiple tag cases.
		yield 'two tags' => [
		@@ -682,6 +690,27 @@ public function providerRepresentations(): \Generator {
		],
		],
		];

		yield '<ol type="1 A">' => [
		new HTMLRestrictions(['ol' => ['type' => ['1' => TRUE, 'A' => TRUE]]]),
		['<ol type="1 A">'],
		'<ol type="1 A">',
		[
		[
		'name' => 'ol',
		'attributes' => [
		[
		'key' => 'type',
		'value' => [
		'regexp' => [
		'pattern' => '/^(1\|A)$/',
		],
		],
		],
		],
		],
		],
		];
		}

		/**
		@@ -906,6 +935,34 @@ public function providerOperands(): \Generator {
		'intersection' => 'a',
		'union' => 'b',
		];
		yield 'attribute restrictions are different: <ol type=*> vs <ol type="A">' => [
		'a' => new HTMLRestrictions(['ol' => ['type' => TRUE]]),
		'b' => new HTMLRestrictions(['ol' => ['type' => ['A' => TRUE]]]),
		'diff' => 'a',
		'intersection' => 'b',
		'union' => 'a',
		];
		yield 'attribute restrictions are different: <ol type=*> vs <ol type="A"> — vice versa' => [
		'b' => new HTMLRestrictions(['ol' => ['type' => ['A' => TRUE]]]),
		'a' => new HTMLRestrictions(['ol' => ['type' => TRUE]]),
		'diff' => HTMLRestrictions::emptySet(),
		'intersection' => 'a',
		'union' => 'b',
		];
		yield 'attribute restrictions are different: <ol type=*> vs <ol type="1">' => [
		'a' => new HTMLRestrictions(['ol' => ['type' => TRUE]]),
		'b' => new HTMLRestrictions(['ol' => ['type' => ['1' => TRUE]]]),
		'diff' => 'a',
		'intersection' => 'b',
		'union' => 'a',
		];
		yield 'attribute restrictions are different: <ol type=*> vs <ol type="1"> — vice versa' => [
		'b' => new HTMLRestrictions(['ol' => ['type' => ['1' => TRUE]]]),
		'a' => new HTMLRestrictions(['ol' => ['type' => TRUE]]),
		'diff' => HTMLRestrictions::emptySet(),
		'intersection' => 'a',
		'union' => 'b',
		];

		// Complex cases.
		yield 'attribute restrictions are different: <a hreflang="en"> vs <strong>' => [