Skip to content

Commit 62ccdbe

Browse files
author
ogorkun
committed
MC-34385: Filter fields allowing HTML
1 parent 15f8105 commit 62ccdbe

File tree

5 files changed

+188
-32
lines changed

5 files changed

+188
-32
lines changed

‎app/code/Magento/Cms/etc/di.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,8 @@
282282
<item name="th" xsi:type="string">th</item>
283283
<item name="tfoot" xsi:type="string">tfoot</item>
284284
<item name="img" xsi:type="string">img</item>
285+
<item name="hr" xsi:type="string">hr</item>
286+
<item name="figure" xsi:type="string">figure</item>
285287
</argument>
286288
<argument name="allowedAttributes" xsi:type="array">
287289
<item name="class" xsi:type="string">class</item>

‎app/etc/di.xml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1875,7 +1875,9 @@
18751875
</item>
18761876
</argument>
18771877
<argument name="attributeValidators" xsi:type="array">
1878-
<item name="style" xsi:type="object">Magento\Framework\Validator\HTML\StyleAttributeValidator</item>
1878+
<item name="style" xsi:type="array">
1879+
<item name="style" xsi:type="object">Magento\Framework\Validator\HTML\StyleAttributeValidator</item>
1880+
</item>
18791881
</argument>
18801882
</arguments>
18811883
</virtualType>

‎lib/internal/Magento/Framework/Test/Unit/Validator/HTML/ConfigurableWYSIWYGValidatorTest.php

Lines changed: 82 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Framework\Validation\ValidationException;
1212
use Magento\Framework\Validator\HTML\ConfigurableWYSIWYGValidator;
1313
use Magento\Framework\Validator\HTML\AttributeValidatorInterface;
14+
use Magento\Framework\Validator\HTML\TagValidatorInterface;
1415
use PHPUnit\Framework\TestCase;
1516

1617
class ConfigurableWYSIWYGValidatorTest extends TestCase
@@ -23,25 +24,43 @@ class ConfigurableWYSIWYGValidatorTest extends TestCase
2324
public function getConfigurations(): array
2425
{
2526
return [
26-
'no-html' => [['div'], [], [], 'just text', true, []],
27-
'allowed-tag' => [['div'], [], [], 'just text and <div>a div</div>', true, []],
27+
'no-html' => [['div'], [], [], 'just text', true, [], []],
28+
'allowed-tag' => [['div'], [], [], 'just text and <div>a div</div>', true, [], []],
2829
'restricted-tag' => [
2930
['div', 'p'],
3031
[],
3132
[],
3233
'text and <p>a p</p>, <div>a div</div>, <tr>a tr</tr>',
3334
false,
35+
[],
36+
[]
37+
],
38+
'restricted-tag-wtih-attr' => [
39+
['div'],
40+
[],
41+
[],
42+
'just text and <p class="fake-class">a p</p>',
43+
false,
44+
[],
45+
[]
46+
],
47+
'allowed-tag-with-attr' => [
48+
['div'],
49+
[],
50+
[],
51+
'just text and <div class="fake-class">a div</div>',
52+
false,
53+
[],
3454
[]
3555
],
36-
'restricted-tag-wtih-attr' => [['div'], [], [], 'just text and <p class="fake-class">a p</p>', false, []],
37-
'allowed-tag-with-attr' => [['div'], [], [], 'just text and <div class="fake-class">a div</div>', false, []],
38-
'multiple-tags' => [['div', 'p'], [], [], 'just text and <div>a div</div> and <p>a p</p>', true, []],
56+
'multiple-tags' => [['div', 'p'], [], [], 'just text and <div>a div</div> and <p>a p</p>', true, [], []],
3957
'tags-with-attrs' => [
4058
['div', 'p'],
4159
['class', 'style'],
4260
[],
4361
'text and <div class="fake-class">a div</div> and <p style="color: blue">a p</p>',
4462
true,
63+
[],
4564
[]
4665
],
4766
'tags-with-restricted-attrs' => [
@@ -50,6 +69,7 @@ public function getConfigurations(): array
5069
[],
5170
'text and <div class="fake-class">a div</div> and <p style="color: blue">a p</p>',
5271
false,
72+
[],
5373
[]
5474
],
5575
'tags-with-specific-attrs' => [
@@ -59,6 +79,7 @@ public function getConfigurations(): array
5979
'<div class="fake-class" style="color: blue">a div</div>, <a href="/some-path" class="a">an a</a>'
6080
.', <p class="p-class">a p</p>',
6181
true,
82+
[],
6283
[]
6384
],
6485
'tags-with-specific-restricted-attrs' => [
@@ -67,6 +88,7 @@ public function getConfigurations(): array
6788
['a' => ['href']],
6889
'text and <div class="fake-class" href="what">a div</div> and <a href="/some-path" class="a">an a</a>',
6990
false,
91+
[],
7092
[]
7193
],
7294
'invalid-tag-with-full-config' => [
@@ -76,6 +98,7 @@ public function getConfigurations(): array
7698
'<div class="fake-class" style="color: blue">a div</div>, <a href="/some-path" class="a">an a</a>'
7799
.', <p class="p-class">a p</p>, <img src="test.jpg" />',
78100
false,
101+
[],
79102
[]
80103
],
81104
'invalid-html' => [
@@ -84,6 +107,7 @@ public function getConfigurations(): array
84107
['a' => ['href'], 'div' => ['style']],
85108
'some </,none-> </html>',
86109
true,
110+
[],
87111
[]
88112
],
89113
'invalid-html-with-violations' => [
@@ -92,6 +116,7 @@ public function getConfigurations(): array
92116
['a' => ['href'], 'div' => ['style']],
93117
'some </,none-> </html> <tr>some trs</tr>',
94118
false,
119+
[],
95120
[]
96121
],
97122
'invalid-html-attributes' => [
@@ -100,36 +125,58 @@ public function getConfigurations(): array
100125
[],
101126
'some <div class="value">DIV</div>',
102127
false,
103-
['class' => false]
128+
['class' => false],
129+
[]
104130
],
105131
'ignored-html-attributes' => [
106132
['div', 'a', 'p'],
107133
['class', 'src'],
108134
[],
109135
'some <div class="value">DIV</div>',
110136
true,
111-
['src' => false, 'class' => true]
137+
['src' => false, 'class' => true],
138+
[]
112139
],
113140
'valid-html-attributes' => [
114141
['div', 'a', 'p'],
115142
['class', 'src'],
116143
[],
117144
'some <div class="value">DIV</div>',
118145
true,
119-
['src' => true, 'class' => true]
146+
['src' => true, 'class' => true],
147+
[]
148+
],
149+
'invalid-allowed-tag' => [
150+
['div'],
151+
['class', 'src'],
152+
[],
153+
'<div class="some-class" src="some-src">IS A DIV</div>',
154+
false,
155+
[],
156+
['div' => ['class' => false]]
157+
],
158+
'valid-allowed-tag' => [
159+
['div'],
160+
['class', 'src'],
161+
[],
162+
'<div class="some-class">IS A DIV</div>',
163+
true,
164+
[],
165+
['div' => ['src' => false]]
120166
]
121167
];
122168
}
123169

124170
/**
125171
* Test different configurations and content.
126172
*
127-
* @param array $allowedTags
128-
* @param array $allowedAttr
129-
* @param array $allowedTagAttrs
173+
* @param string[] $allowedTags
174+
* @param string[] $allowedAttr
175+
* @param string[][] $allowedTagAttrs
130176
* @param string $html
131177
* @param bool $isValid
132-
* @param array $attributeValidityMap
178+
* @param bool[] $attributeValidityMap
179+
* @param bool[][] $tagValidators
133180
* @return void
134181
* @dataProvider getConfigurations
135182
*/
@@ -139,7 +186,8 @@ public function testConfigurations(
139186
array $allowedTagAttrs,
140187
string $html,
141188
bool $isValid,
142-
array $attributeValidityMap
189+
array $attributeValidityMap,
190+
array $tagValidators
143191
): void {
144192
$attributeValidator = $this->getMockForAbstractClass(AttributeValidatorInterface::class);
145193
$attributeValidator->method('validate')
@@ -152,13 +200,32 @@ function (string $tag, string $attribute, string $content) use ($attributeValidi
152200
);
153201
$attrValidators = [];
154202
foreach (array_keys($attributeValidityMap) as $attr) {
155-
$attrValidators[$attr] = $attributeValidator;
203+
$attrValidators[$attr] = [$attributeValidator];
204+
}
205+
$tagValidatorsMocks = [];
206+
foreach ($tagValidators as $tag => $allowedAttributes) {
207+
$mock = $this->getMockForAbstractClass(TagValidatorInterface::class);
208+
$mock->method('validate')
209+
->willReturnCallback(
210+
function (string $givenTag, array $attrs, string $value) use($tag, $allowedAttributes): void {
211+
if ($givenTag !== $tag) {
212+
throw new \RuntimeException();
213+
}
214+
foreach (array_keys($attrs) as $attr) {
215+
if (array_key_exists($attr, $allowedAttributes) && !$allowedAttributes[$attr]) {
216+
throw new ValidationException(__('Invalid tag'));
217+
}
218+
}
219+
}
220+
);
221+
$tagValidatorsMocks[$tag] = [$mock];
156222
}
157223
$validator = new ConfigurableWYSIWYGValidator(
158224
$allowedTags,
159225
$allowedAttr,
160226
$allowedTagAttrs,
161-
$attrValidators
227+
$attrValidators,
228+
$tagValidatorsMocks
162229
);
163230
$valid = true;
164231
try {

‎lib/internal/Magento/Framework/Validator/HTML/ConfigurableWYSIWYGValidator.php

Lines changed: 67 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,28 @@ class ConfigurableWYSIWYGValidator implements WYSIWYGValidatorInterface
3131
private $attributesAllowedByTags;
3232

3333
/**
34-
* @var AttributeValidatorInterface[]
34+
* @var AttributeValidatorInterface[][]
3535
*/
3636
private $attributeValidators;
3737

38+
/**
39+
* @var TagValidatorInterface[][]
40+
*/
41+
private $tagValidators;
42+
3843
/**
3944
* @param string[] $allowedTags
4045
* @param string[] $allowedAttributes
41-
* @param string[] $attributesAllowedByTags
42-
* @param AttributeValidatorInterface[] $attributeValidators
46+
* @param string[][] $attributesAllowedByTags
47+
* @param AttributeValidatorInterface[][] $attributeValidators
48+
* @param TagValidatorInterface[][] $tagValidators
4349
*/
4450
public function __construct(
4551
array $allowedTags,
4652
array $allowedAttributes = [],
4753
array $attributesAllowedByTags = [],
48-
array $attributeValidators = []
54+
array $attributeValidators = [],
55+
array $tagValidators = []
4956
) {
5057
if (empty(array_filter($allowedTags))) {
5158
throw new \InvalidArgumentException('List of allowed HTML tags cannot be empty');
@@ -60,6 +67,7 @@ function (string $tag) use ($allowedTags): bool {
6067
ARRAY_FILTER_USE_KEY
6168
);
6269
$this->attributeValidators = $attributeValidators;
70+
$this->tagValidators = $tagValidators;
6371
}
6472

6573
/**
@@ -73,19 +81,32 @@ public function validate(string $content): void
7381
$dom = $this->loadHtml($content);
7482
$xpath = new \DOMXPath($dom);
7583

84+
$this->validateConfigured($xpath);
85+
$this->callDynamicValidators($xpath);
86+
}
87+
88+
/**
89+
* Check declarative restrictions
90+
*
91+
* @param \DOMXPath $xpath
92+
* @return void
93+
* @throws ValidationException
94+
*/
95+
private function validateConfigured(\DOMXPath $xpath): void
96+
{
7697
//Validating tags
7798
$found = $xpath->query(
7899
$query='//*['
79-
. implode(
80-
' and ',
81-
array_map(
82-
function (string $tag): string {
83-
return "name() != '$tag'";
84-
},
85-
array_merge($this->allowedTags, ['body', 'html'])
100+
. implode(
101+
' and ',
102+
array_map(
103+
function (string $tag): string {
104+
return "name() != '$tag'";
105+
},
106+
array_merge($this->allowedTags, ['body', 'html'])
107+
)
86108
)
87-
)
88-
.']'
109+
.']'
89110
);
90111
if (count($found)) {
91112
throw new ValidationException(
@@ -143,17 +164,48 @@ function (string $attribute): string {
143164
);
144165
}
145166
}
167+
}
146168

169+
/**
170+
* Cycle dynamic validators.
171+
*
172+
* @param \DOMXPath $xpath
173+
* @return void
174+
* @throws ValidationException
175+
*/
176+
private function callDynamicValidators(\DOMXPath $xpath): void
177+
{
147178
//Validating allowed attributes.
148179
if ($this->attributeValidators) {
149-
foreach ($this->attributeValidators as $attr => $validator) {
180+
foreach ($this->attributeValidators as $attr => $validators) {
150181
$found = $xpath->query("//@*[name() = '$attr']");
151182
foreach ($found as $attribute) {
152-
$validator->validate($attribute->parentNode->tagName, $attribute->name, $attribute->value);
183+
foreach ($validators as $validator) {
184+
$validator->validate($attribute->parentNode->tagName, $attribute->name, $attribute->value);
185+
}
153186
}
154187
}
155188
}
156189

190+
//Validating allowed tags
191+
if ($this->tagValidators) {
192+
foreach ($this->tagValidators as $tag => $validators) {
193+
$found = $xpath->query("//*[name() = '$tag']");
194+
/** @var \DOMElement $tagNode */
195+
foreach ($found as $tagNode) {
196+
$attributes = [];
197+
if ($tagNode->hasAttributes()) {
198+
/** @var \DOMAttr $attributeNode */
199+
foreach ($tagNode->attributes as $attributeNode) {
200+
$attributes[$attributeNode->name] = $attributeNode->value;
201+
}
202+
}
203+
foreach ($validators as $validator) {
204+
$validator->validate($tagNode->tagName, $attributes, $tagNode->textContent, $this);
205+
}
206+
}
207+
}
208+
}
157209
}
158210

159211
/**
@@ -166,7 +218,6 @@ function (string $attribute): string {
166218
private function loadHtml(string $content): \DOMDocument
167219
{
168220
$dom = new \DOMDocument('1.0', 'UTF-8');
169-
$loaded = true;
170221
set_error_handler(
171222
function () use (&$loaded) {
172223
$loaded = false;

0 commit comments

Comments
 (0)