From 6160dcfe90d2d1bfca98d8de4f846bcbe2df0097 Mon Sep 17 00:00:00 2001 From: Edouard Courty Date: Wed, 6 Dec 2023 10:37:53 +0100 Subject: [PATCH 1/2] [HtmlSanitizer] Add functions to handle operations on multiple attributes or elements at the same time --- .../Component/HtmlSanitizer/CHANGELOG.md | 5 + .../HtmlSanitizer/HtmlSanitizerConfig.php | 113 ++++++++++++ .../Tests/HtmlSanitizerConfigTest.php | 173 +++++++++++++++++- 3 files changed, 290 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/HtmlSanitizer/CHANGELOG.md b/src/Symfony/Component/HtmlSanitizer/CHANGELOG.md index c5d32f929a689..3ebacd53c6b82 100644 --- a/src/Symfony/Component/HtmlSanitizer/CHANGELOG.md +++ b/src/Symfony/Component/HtmlSanitizer/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +7.1 +--- + + * Add functions to allow operations on arrays of attributes and elements at a time + 6.4 --- diff --git a/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php b/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php index f46ffff61b192..88420172f39ab 100644 --- a/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php +++ b/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php @@ -274,6 +274,29 @@ public function allowElement(string $element, array|string $allowedAttributes = return $clone; } + /** + * Configures the given elements as allowed. + * + * Allowed elements are elements the sanitizer should retain from the input. + * + * A list of allowed attributes for this element can be passed as a second argument. + * Passing "*" will allow all standard attributes on this element. By default, no + * attributes are allowed on the element. + * + * @param list $elements + * @param list|string $allowedAttributes + */ + public function allowElements(array $elements, array|string $allowedAttributes = []): static + { + $clone = clone $this; + + foreach ($elements as $element) { + $clone = $clone->allowElement($element, $allowedAttributes); + } + + return $clone; + } + /** * Configures the given element as blocked. * @@ -292,6 +315,23 @@ public function blockElement(string $element): static return $clone; } + /** + * Configures the given elements as blocked. + * + * Blocked elements are elements the sanitizer should remove from the input, but retain + * their children. + */ + public function blockElements(array $elements): static + { + $clone = clone $this; + + foreach ($elements as $element) { + $clone = $clone->blockElement($element); + } + + return $clone; + } + /** * Configures the given element as dropped. * @@ -310,6 +350,29 @@ public function dropElement(string $element): static return $clone; } + /** + * Configures the given elements as dropped. + * + * Dropped elements are elements the sanitizer should remove from the input, including + * their children. + * + * Note: when using an empty configuration, all unknown elements are dropped + * automatically. This method let you drop elements that were allowed earlier + * in the configuration. + * + * @param list $elements + */ + public function dropElements(array $elements): static + { + $clone = clone $this; + + foreach ($elements as $element) { + $clone = $clone->dropElement($element); + } + + return $clone; + } + /** * Configures the given attribute as allowed. * @@ -339,6 +402,30 @@ public function allowAttribute(string $attribute, array|string $allowedElements) return $clone; } + /** + * Configures the given attributes as allowed. + * + * Allowed attributes are attributes the sanitizer should retain from the input. + * + * A list of allowed elements for these attributes can be passed as a second argument. + * Passing "*" will allow all currently allowed elements to use this attribute. + * + * To configure each attribute for a specific element, please use the allowAttribute method instead. + * + * @param list $attributes + * @param list|string $allowedElements + */ + public function allowAttributes(array $attributes, array|string $allowedElements): static + { + $clone = clone $this; + + foreach ($attributes as $attribute) { + $clone = $clone->allowAttribute($attribute, $allowedElements); + } + + return $clone; + } + /** * Configures the given attribute as dropped. * @@ -367,6 +454,32 @@ public function dropAttribute(string $attribute, array|string $droppedElements): return $clone; } + /** + * Configures the given attributes as dropped. + * + * Dropped attributes are attributes the sanitizer should remove from the input. + * + * A list of elements on which to drop these attributes can be passed as a second argument. + * Passing "*" will drop this attribute from all currently allowed elements. + * + * Note: when using an empty configuration, all unknown attributes are dropped + * automatically. This method let you drop attributes that were allowed earlier + * in the configuration. + * + * @param list $attributes + * @param list|string $droppedElements + */ + public function dropAttributes(array $attributes, array|string $droppedElements): static + { + $clone = clone $this; + + foreach ($attributes as $attribute) { + $clone = $clone->dropAttribute($attribute, $droppedElements); + } + + return $clone; + } + /** * Forcefully set the value of a given attribute on a given element. * diff --git a/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerConfigTest.php b/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerConfigTest.php index b98af74d02818..82fd3599d35de 100644 --- a/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerConfigTest.php +++ b/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerConfigTest.php @@ -68,6 +68,14 @@ public function testAllowElement() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowElements() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section'], ['style']); + $this->assertSame(['div' => ['style' => true], 'section' => ['style' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testAllowElementTwiceOverridesIt() { $config = new HtmlSanitizerConfig(); @@ -104,6 +112,14 @@ public function testAllowElementNoAttributes() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowElementsNoAttributes() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'script'], []); + $this->assertSame(['div' => [], 'script' => []], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testAllowElementStandardAttributes() { $config = new HtmlSanitizerConfig(); @@ -113,6 +129,16 @@ public function testAllowElementStandardAttributes() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowElementsStandardAttributes() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'script'], '*'); + $this->assertSame(['div', 'script'], array_keys($config->getAllowedElements())); + $this->assertCount(211, $config->getAllowedElements()['div']); + $this->assertCount(211, $config->getAllowedElements()['script']); + $this->assertSame([], $config->getBlockedElements()); + } + public function testAllowElementStringAttribute() { $config = new HtmlSanitizerConfig(); @@ -121,6 +147,14 @@ public function testAllowElementStringAttribute() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowElementsStringAttribute() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'script'], 'width'); + $this->assertSame(['div' => ['width' => true], 'script' => ['width' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testBlockElement() { $config = new HtmlSanitizerConfig(); @@ -128,6 +162,13 @@ public function testBlockElement() $this->assertSame(['div' => true], $config->getBlockedElements()); } + public function testBlockElements() + { + $config = new HtmlSanitizerConfig(); + $config = $config->blockElements(['iframe', 'script']); + $this->assertSame(['iframe' => true, 'script' => true], $config->getBlockedElements()); + } + public function testBlockElementDisallowsIt() { $config = new HtmlSanitizerConfig(); @@ -140,6 +181,18 @@ public function testBlockElementDisallowsIt() $this->assertSame(['div' => true], $config->getBlockedElements()); } + public function testBlockElementsDisallowsIt() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['iframe', 'script'], 'width'); + $this->assertSame(['iframe' => ['width' => true], 'script' => ['width' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + + $config = $config->blockElement('iframe'); + $this->assertSame(['script' => ['width' => true]], $config->getAllowedElements()); + $this->assertSame(['iframe' => true], $config->getBlockedElements()); + } + public function testDropAllowedElement() { $config = new HtmlSanitizerConfig(); @@ -152,6 +205,18 @@ public function testDropAllowedElement() $this->assertSame([], $config->getBlockedElements()); } + public function testDropAllowedElements() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section'], 'width'); + $this->assertSame(['div' => ['width' => true], 'section' => ['width' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + + $config = $config->dropElements(['div', 'section']); + $this->assertSame([], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testDropBlockedElement() { $config = new HtmlSanitizerConfig(); @@ -164,6 +229,18 @@ public function testDropBlockedElement() $this->assertSame([], $config->getBlockedElements()); } + public function testDropBlockedElements() + { + $config = new HtmlSanitizerConfig(); + $config = $config->blockElements(['div', 'section']); + $this->assertSame([], $config->getAllowedElements()); + $this->assertSame(['div' => true, 'section' => true], $config->getBlockedElements()); + + $config = $config->dropElements(['div', 'section']); + $this->assertSame([], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testAllowAttributeNoElement() { $config = new HtmlSanitizerConfig(); @@ -172,6 +249,14 @@ public function testAllowAttributeNoElement() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowAttributesNoElement() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowAttributes(['width', 'height'], 'div'); + $this->assertSame([], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testAllowAttributeAllowedElement() { $config = new HtmlSanitizerConfig(); @@ -181,6 +266,15 @@ public function testAllowAttributeAllowedElement() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowAttributesAllowedElement() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElement('div'); + $config = $config->allowAttributes(['width', 'height'], 'div'); + $this->assertSame(['div' => ['width' => true, 'height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testAllowAttributeAllElements() { $config = new HtmlSanitizerConfig(); @@ -191,6 +285,15 @@ public function testAllowAttributeAllElements() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowAttributesAllElements() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section']); + $config = $config->allowAttributes(['width', 'height'], '*'); + $this->assertSame(['div' => ['width' => true, 'height' => true], 'section' => ['width' => true, 'height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testAllowAttributeElementsArray() { $config = new HtmlSanitizerConfig(); @@ -201,6 +304,15 @@ public function testAllowAttributeElementsArray() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowAttributesElementsArray() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section']); + $config = $config->allowAttributes(['width', 'height'], ['section']); + $this->assertSame(['div' => [], 'section' => ['width' => true, 'height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testAllowAttributeElementsString() { $config = new HtmlSanitizerConfig(); @@ -211,7 +323,16 @@ public function testAllowAttributeElementsString() $this->assertSame([], $config->getBlockedElements()); } - public function testAllowAttributeOverridesIt() + public function testAllowAttributesElementsString() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section']); + $config = $config->allowAttributes(['width', 'height'], 'section'); + $this->assertSame(['div' => [], 'section' => ['width' => true, 'height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + + public function testAllowAttributesOverridesIt() { $config = new HtmlSanitizerConfig(); $config = $config->allowElement('div'); @@ -226,6 +347,20 @@ public function testAllowAttributeOverridesIt() $this->assertSame([], $config->getBlockedElements()); } + public function testAllowAttributeArraysOverridesIt() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section']); + + $config = $config->allowAttributes(['width', 'height'], 'div'); + $this->assertSame(['div' => ['width' => true, 'height' => true], 'section' => []], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + + $config = $config->allowAttributes(['width', 'height'], 'section'); + $this->assertSame(['div' => [], 'section' => ['width' => true, 'height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testDropAllowedAttributeAllowedElementsArray() { $config = new HtmlSanitizerConfig(); @@ -239,6 +374,18 @@ public function testDropAllowedAttributeAllowedElementsArray() $this->assertSame([], $config->getBlockedElements()); } + public function testDropAllowedAttributeArrayAllowedElementsArray() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section', 'main'], 'width'); + $this->assertSame(['div' => ['width' => true], 'section' => ['width' => true], 'main' => ['width' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + + $config = $config->dropAttribute('width', ['div', 'section']); + $this->assertSame(['div' => [], 'section' => [], 'main' => ['width' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testDropAllowedAttributeAllowedElementString() { $config = new HtmlSanitizerConfig(); @@ -252,6 +399,18 @@ public function testDropAllowedAttributeAllowedElementString() $this->assertSame([], $config->getBlockedElements()); } + public function testDropAllowedAttributesAllowedElementString() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section'], ['width', 'height']); + $this->assertSame(['div' => ['width' => true, 'height' => true], 'section' => ['width' => true, 'height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + + $config = $config->dropAttributes(['width'], 'section'); + $this->assertSame(['div' => ['width' => true, 'height' => true], 'section' => ['height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testDropAllowedAttributeAllElements() { $config = new HtmlSanitizerConfig(); @@ -265,6 +424,18 @@ public function testDropAllowedAttributeAllElements() $this->assertSame([], $config->getBlockedElements()); } + public function testDropAllowedAttributesAllElements() + { + $config = new HtmlSanitizerConfig(); + $config = $config->allowElements(['div', 'section'], ['width', 'height']); + $this->assertSame(['div' => ['width' => true, 'height' => true], 'section' => ['width' => true, 'height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + + $config = $config->dropAttribute('width', '*'); + $this->assertSame(['div' => ['height' => true], 'section' => ['height' => true]], $config->getAllowedElements()); + $this->assertSame([], $config->getBlockedElements()); + } + public function testWithWithoutAttributeSanitizer() { $config = new HtmlSanitizerConfig(); From eebd0503c644fbd1ce8efc2ea10f6b46f9b4c9e0 Mon Sep 17 00:00:00 2001 From: Edouard Courty Date: Thu, 7 Dec 2023 15:23:00 +0100 Subject: [PATCH 2/2] [HtmlSanitizer] Add functions to handle operations on multiple attributes or elements at the same time - Wrapped some logic in private functions to stay DRY - Fixed PHPDoc comments - Fixed typo --- .../HtmlSanitizer/HtmlSanitizerConfig.php | 122 +++++++++++------- 1 file changed, 77 insertions(+), 45 deletions(-) diff --git a/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php b/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php index 88420172f39ab..ca06a9dcaffc1 100644 --- a/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php +++ b/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php @@ -260,16 +260,7 @@ public function forceHttpsUrls(bool $forceHttpsUrls = true): static public function allowElement(string $element, array|string $allowedAttributes = []): static { $clone = clone $this; - - // Unblock the element is necessary - unset($clone->blockedElements[$element]); - - $clone->allowedElements[$element] = []; - - $attrs = ('*' === $allowedAttributes) ? array_keys(W3CReference::ATTRIBUTES) : (array) $allowedAttributes; - foreach ($attrs as $allowedAttr) { - $clone->allowedElements[$element][$allowedAttr] = true; - } + $this->handleAllowElement($clone, $element, $allowedAttributes); return $clone; } @@ -279,11 +270,11 @@ public function allowElement(string $element, array|string $allowedAttributes = * * Allowed elements are elements the sanitizer should retain from the input. * - * A list of allowed attributes for this element can be passed as a second argument. + * A list of allowed attributes for these elements can be passed as a second argument. * Passing "*" will allow all standard attributes on this element. By default, no * attributes are allowed on the element. * - * @param list $elements + * @param string[] $elements * @param list|string $allowedAttributes */ public function allowElements(array $elements, array|string $allowedAttributes = []): static @@ -291,7 +282,7 @@ public function allowElements(array $elements, array|string $allowedAttributes = $clone = clone $this; foreach ($elements as $element) { - $clone = $clone->allowElement($element, $allowedAttributes); + $this->handleAllowElement($clone, $element, $allowedAttributes); } return $clone; @@ -306,11 +297,7 @@ public function allowElements(array $elements, array|string $allowedAttributes = public function blockElement(string $element): static { $clone = clone $this; - - // Disallow the element is necessary - unset($clone->allowedElements[$element]); - - $clone->blockedElements[$element] = true; + $this->handleBlockElement($clone, $element); return $clone; } @@ -326,7 +313,7 @@ public function blockElements(array $elements): static $clone = clone $this; foreach ($elements as $element) { - $clone = $clone->blockElement($element); + $this->handleBlockElement($clone, $element); } return $clone; @@ -345,7 +332,7 @@ public function blockElements(array $elements): static public function dropElement(string $element): static { $clone = clone $this; - unset($clone->allowedElements[$element], $clone->blockedElements[$element]); + $this->handleDropElement($clone, $element); return $clone; } @@ -360,14 +347,14 @@ public function dropElement(string $element): static * automatically. This method let you drop elements that were allowed earlier * in the configuration. * - * @param list $elements + * @param string[] $elements */ public function dropElements(array $elements): static { $clone = clone $this; foreach ($elements as $element) { - $clone = $clone->dropElement($element); + $this->handleDropElement($clone, $element); } return $clone; @@ -386,18 +373,7 @@ public function dropElements(array $elements): static public function allowAttribute(string $attribute, array|string $allowedElements): static { $clone = clone $this; - $allowedElements = ('*' === $allowedElements) ? array_keys($clone->allowedElements) : (array) $allowedElements; - - // For each configured element ... - foreach ($clone->allowedElements as $element => $attrs) { - if (\in_array($element, $allowedElements, true)) { - // ... if the attribute should be allowed, add it - $clone->allowedElements[$element][$attribute] = true; - } else { - // ... if the attribute should not be allowed, remove it - unset($clone->allowedElements[$element][$attribute]); - } - } + $this->handleAllowAttribute($clone, $allowedElements, $attribute); return $clone; } @@ -412,7 +388,7 @@ public function allowAttribute(string $attribute, array|string $allowedElements) * * To configure each attribute for a specific element, please use the allowAttribute method instead. * - * @param list $attributes + * @param string[] $attributes * @param list|string $allowedElements */ public function allowAttributes(array $attributes, array|string $allowedElements): static @@ -420,7 +396,7 @@ public function allowAttributes(array $attributes, array|string $allowedElements $clone = clone $this; foreach ($attributes as $attribute) { - $clone = $clone->allowAttribute($attribute, $allowedElements); + $this->handleAllowAttribute($clone, $allowedElements, $attribute); } return $clone; @@ -443,13 +419,7 @@ public function allowAttributes(array $attributes, array|string $allowedElements public function dropAttribute(string $attribute, array|string $droppedElements): static { $clone = clone $this; - $droppedElements = ('*' === $droppedElements) ? array_keys($clone->allowedElements) : (array) $droppedElements; - - foreach ($droppedElements as $element) { - if (isset($clone->allowedElements[$element][$attribute])) { - unset($clone->allowedElements[$element][$attribute]); - } - } + $this->handleDropAttribute($clone, $droppedElements, $attribute); return $clone; } @@ -466,7 +436,7 @@ public function dropAttribute(string $attribute, array|string $droppedElements): * automatically. This method let you drop attributes that were allowed earlier * in the configuration. * - * @param list $attributes + * @param string[] $attributes * @param list|string $droppedElements */ public function dropAttributes(array $attributes, array|string $droppedElements): static @@ -474,7 +444,7 @@ public function dropAttributes(array $attributes, array|string $droppedElements) $clone = clone $this; foreach ($attributes as $attribute) { - $clone = $clone->dropAttribute($attribute, $droppedElements); + $this->handleDropAttribute($clone, $droppedElements, $attribute); } return $clone; @@ -617,4 +587,66 @@ public function getAttributeSanitizers(): array { return $this->attributeSanitizers; } + + /** + * @param string[]|string $allowedElements + */ + public function handleAllowAttribute(self $clone, array|string $allowedElements, string $attribute): void + { + $allowedElements = ('*' === $allowedElements) ? array_keys($clone->allowedElements) : (array) $allowedElements; + + // For each configured element ... + foreach ($clone->allowedElements as $element => $attrs) { + if (\in_array($element, $allowedElements, true)) { + // ... if the attribute should be allowed, add it + $clone->allowedElements[$element][$attribute] = true; + } else { + // ... if the attribute should not be allowed, remove it + unset($clone->allowedElements[$element][$attribute]); + } + } + } + + /** + * @param string[]|string $allowedAttributes + */ + private function handleAllowElement(self $clone, string $element, array|string $allowedAttributes): void + { + // Unblock the element is necessary + unset($clone->blockedElements[$element]); + + $clone->allowedElements[$element] = []; + + $attrs = ('*' === $allowedAttributes) ? array_keys(W3CReference::ATTRIBUTES) : (array) $allowedAttributes; + foreach ($attrs as $allowedAttr) { + $clone->allowedElements[$element][$allowedAttr] = true; + } + } + + private function handleBlockElement(self $clone, string $element): void + { + // Disallow the element is necessary + unset($clone->allowedElements[$element]); + + $clone->blockedElements[$element] = true; + } + + /** + * @param string[]|string $droppedElements + */ + private function handleDropAttribute(self $clone, array|string $droppedElements, string $attribute): void + { + $droppedElements = ('*' === $droppedElements) ? array_keys($clone->allowedElements) : (array) $droppedElements; + + foreach ($droppedElements as $element) { + if (isset($clone->allowedElements[$element][$attribute])) { + unset($clone->allowedElements[$element][$attribute]); + } + } + } + + private function handleDropElement(self $clone, string $element): void + { + unset($clone->allowedElements[$element], $clone->blockedElements[$element]); + } }