Skip to content

Commit

Permalink
minor symfony#44814 [HtmlSanitizer] Some minor changes in the config …
Browse files Browse the repository at this point in the history
…API (javiereguiluz)

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
  • Loading branch information
fabpot committed Jan 7, 2022
2 parents 06f4bd7 + 84470ef commit 098ff62
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 5 deletions.
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HtmlSanitizer/README.md
Expand Up @@ -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')
Expand Down
Expand Up @@ -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()
Expand Down
Expand Up @@ -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
'+',
'=',
'@',
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php
Expand Up @@ -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>>
*/
Expand Down

0 comments on commit 098ff62

Please sign in to comment.