minor #44814 [HtmlSanitizer] Some minor changes in the config API (javiereguiluz)

fabpot · fabpot · commit 098ff6277a39 · 2022-01-07T19:32:06.000+01:00
This PR was squashed before being merged into the 6.1 branch. Discussion ---------- [HtmlSanitizer] Some minor changes in the config API | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - First of all, thanks to @tgalopin for this superb contribution 🙇 This PR makes 3 little changes: (1) Fix two minor typos (2) Rename `allowAllStaticElements()` as `allowStaticElements()` to be consistent with the rest of methods, which don't include the `All` word. (3) A proposal to change this default value: ```diff -public function allowElement(string $element, array|string $allowedAttributes = []): static +public function allowElement(string $element, array|string $allowedAttributes = '*'): static ``` In my opinion, when you want to allow some element, most of the times you want to allow the standard attributes on them too. So, the following should allow `<div>` and their standard attributes: ```php ->allowElement('div') ``` Forcing to write it as `->allowElement('div', '*')` seems cumbersome. The previous behavior (forbid all attributes) would now be like this: ```php ->allowElement('div', []) ``` Commits ------- 84470ef [HtmlSanitizer] Some minor changes in the config API
diff --git a/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php b/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php
@@ -105,7 +105,7 @@ public function __construct()
      * All scripts will be removed but the output may still contain other dangerous
      * behaviors like CSS injection (click-jacking), CSS expressions, ...
      */
-    public function allowAllStaticElements(): static
+    public function allowStaticElements(): static
     {
         $elements = array_merge(
             array_keys(W3CReference::HEAD_ELEMENTS),
diff --git a/src/Symfony/Component/HtmlSanitizer/README.md b/src/Symfony/Component/HtmlSanitizer/README.md
@@ -22,7 +22,7 @@ $config = (new HtmlSanitizerConfig())
     // standard. All scripts will be removed but the output may still contain
     // other dangerous behaviors like CSS injection (click-jacking), CSS
     // expressions, ...
-    ->allowAllStaticElements()
+    ->allowStaticElements()
 
     // Allow the "div" element and no attribute can be on it
     ->allowElement('div')
diff --git a/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerAllTest.php b/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerAllTest.php
@@ -21,7 +21,7 @@ private function createSanitizer(): HtmlSanitizer
     {
         return new HtmlSanitizer(
             (new HtmlSanitizerConfig())
-                ->allowAllStaticElements()
+                ->allowStaticElements()
                 ->allowLinkHosts(['trusted.com', 'external.com'])
                 ->allowMediaHosts(['trusted.com', 'external.com'])
                 ->allowRelativeLinks()
diff --git a/src/Symfony/Component/HtmlSanitizer/TextSanitizer/StringSanitizer.php b/src/Symfony/Component/HtmlSanitizer/TextSanitizer/StringSanitizer.php
@@ -26,7 +26,7 @@ final class StringSanitizer
             // "&#34;" is shorter than "&quot;"
             '&quot;',
 
-            // Fix several potential issues in how browsers intepret attributes values
+            // Fix several potential issues in how browsers interpret attributes values
             '+',
             '=',
             '@',
diff --git a/src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php b/src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php
@@ -47,7 +47,7 @@ final class DomVisitor
     private array $elementsConfig;
 
     /**
-     * Registry of attributes to forcefuly set on nodes, index by element and attribute.
+     * Registry of attributes to forcefully set on nodes, index by element and attribute.
      *
      * @var array<string, array<string, string>>
      */

Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ public function __construct()`
`105`	`105`	`* All scripts will be removed but the output may still contain other dangerous`
`106`	`106`	`* behaviors like CSS injection (click-jacking), CSS expressions, ...`
`107`	`107`	`*/`
`108`		`- public function allowAllStaticElements(): static`
	`108`	`+ public function allowStaticElements(): static`
`109`	`109`	`{`
`110`	`110`	`$elements = array_merge(`
`111`	`111`	`array_keys(W3CReference::HEAD_ELEMENTS),`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ private function createSanitizer(): HtmlSanitizer`
`21`	`21`	`{`
`22`	`22`	`return new HtmlSanitizer(`
`23`	`23`	`(new HtmlSanitizerConfig())`
`24`		`- ->allowAllStaticElements()`
	`24`	`+ ->allowStaticElements()`
`25`	`25`	`->allowLinkHosts(['trusted.com', 'external.com'])`
`26`	`26`	`->allowMediaHosts(['trusted.com', 'external.com'])`
`27`	`27`	`->allowRelativeLinks()`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ final class DomVisitor`
`47`	`47`	`private array $elementsConfig;`
`48`	`48`
`49`	`49`	`/**`
`50`		`- * Registry of attributes to forcefuly set on nodes, index by element and attribute.`
	`50`	`+ * Registry of attributes to forcefully set on nodes, index by element and attribute.`
`51`	`51`	`*`
`52`	`52`	`* @var array<string, array<string, string>>`
`53`	`53`	`*/`