Skip to content

Commit 3dd1811

Browse files
author
ogorkun
committed
MC-34385: Filter fields allowing HTML
1 parent f087064 commit 3dd1811

File tree

6 files changed

+248
-20
lines changed

6 files changed

+248
-20
lines changed

‎app/etc/di.xml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1837,14 +1837,45 @@
18371837
<argument name="allowedTags" xsi:type="array">
18381838
<item name="div" xsi:type="string">div</item>
18391839
<item name="a" xsi:type="string">a</item>
1840+
<item name="p" xsi:type="string">p</item>
1841+
<item name="span" xsi:type="string">span</item>
1842+
<item name="em" xsi:type="string">em</item>
1843+
<item name="strong" xsi:type="string">strong</item>
1844+
<item name="ul" xsi:type="string">ul</item>
1845+
<item name="li" xsi:type="string">li</item>
1846+
<item name="ol" xsi:type="string">ol</item>
1847+
<item name="h5" xsi:type="string">h5</item>
1848+
<item name="h4" xsi:type="string">h4</item>
1849+
<item name="h3" xsi:type="string">h3</item>
1850+
<item name="h2" xsi:type="string">h2</item>
1851+
<item name="h1" xsi:type="string">h1</item>
1852+
<item name="table" xsi:type="string">table</item>
1853+
<item name="tbody" xsi:type="string">tbody</item>
1854+
<item name="tr" xsi:type="string">tr</item>
1855+
<item name="td" xsi:type="string">td</item>
1856+
<item name="th" xsi:type="string">th</item>
1857+
<item name="tfoot" xsi:type="string">tfoot</item>
1858+
<item name="img" xsi:type="string">img</item>
18401859
</argument>
18411860
<argument name="allowedAttributes" xsi:type="array">
18421861
<item name="class" xsi:type="string">class</item>
1862+
<item name="width" xsi:type="string">width</item>
1863+
<item name="height" xsi:type="string">height</item>
1864+
<item name="style" xsi:type="string">style</item>
1865+
<item name="alt" xsi:type="string">alt</item>
1866+
<item name="title" xsi:type="string">title</item>
1867+
<item name="border" xsi:type="string">border</item>
18431868
</argument>
18441869
<argument name="attributesAllowedByTags" xsi:type="array">
18451870
<item name="a" xsi:type="array">
18461871
<item name="href" xsi:type="string">href</item>
18471872
</item>
1873+
<item name="img" xsi:type="array">
1874+
<item name="src" xsi:type="string">src</item>
1875+
</item>
1876+
</argument>
1877+
<argument name="attributeValidators" xsi:type="array">
1878+
<item name="style" xsi:type="object">Magento\Framework\Validator\HTML\StyleAttributeValidator</item>
18481879
</argument>
18491880
</arguments>
18501881
</virtualType>

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

Lines changed: 74 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use Magento\Framework\Validation\ValidationException;
1212
use Magento\Framework\Validator\HTML\ConfigurableWYSIWYGValidator;
13+
use Magento\Framework\Validator\HTML\AttributeValidatorInterface;
1314
use PHPUnit\Framework\TestCase;
1415

1516
class ConfigurableWYSIWYGValidatorTest extends TestCase
@@ -22,62 +23,100 @@ class ConfigurableWYSIWYGValidatorTest extends TestCase
2223
public function getConfigurations(): array
2324
{
2425
return [
25-
'no-html' => [['div'], [], [], 'just text', true],
26-
'allowed-tag' => [['div'], [], [], 'just text and <div>a div</div>', true],
27-
'restricted-tag' => [['div', 'p'], [], [], 'text and <p>a p</p>, <div>a div</div>, <tr>a tr</tr>', false],
28-
'restricted-tag-wtih-attr' => [['div'], [], [], 'just text and <p class="fake-class">a p</p>', false],
29-
'allowed-tag-with-attr' => [['div'], [], [], 'just text and <div class="fake-class">a div</div>', false],
30-
'multiple-tags' => [['div', 'p'], [], [], 'just text and <div>a div</div> and <p>a p</p>', true],
26+
'no-html' => [['div'], [], [], 'just text', true, []],
27+
'allowed-tag' => [['div'], [], [], 'just text and <div>a div</div>', true, []],
28+
'restricted-tag' => [
29+
['div', 'p'],
30+
[],
31+
[],
32+
'text and <p>a p</p>, <div>a div</div>, <tr>a tr</tr>',
33+
false,
34+
[]
35+
],
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, []],
3139
'tags-with-attrs' => [
3240
['div', 'p'],
3341
['class', 'style'],
3442
[],
3543
'text and <div class="fake-class">a div</div> and <p style="color: blue">a p</p>',
36-
true
44+
true,
45+
[]
3746
],
3847
'tags-with-restricted-attrs' => [
3948
['div', 'p'],
4049
['class', 'align'],
4150
[],
4251
'text and <div class="fake-class">a div</div> and <p style="color: blue">a p</p>',
43-
false
52+
false,
53+
[]
4454
],
4555
'tags-with-specific-attrs' => [
4656
['div', 'a', 'p'],
4757
['class'],
4858
['a' => ['href'], 'div' => ['style']],
4959
'<div class="fake-class" style="color: blue">a div</div>, <a href="/some-path" class="a">an a</a>'
5060
.', <p class="p-class">a p</p>',
51-
true
61+
true,
62+
[]
5263
],
5364
'tags-with-specific-restricted-attrs' => [
5465
['div', 'a'],
5566
['class'],
5667
['a' => ['href']],
5768
'text and <div class="fake-class" href="what">a div</div> and <a href="/some-path" class="a">an a</a>',
58-
false
69+
false,
70+
[]
5971
],
6072
'invalid-tag-with-full-config' => [
6173
['div', 'a', 'p'],
6274
['class', 'src'],
6375
['a' => ['href'], 'div' => ['style']],
6476
'<div class="fake-class" style="color: blue">a div</div>, <a href="/some-path" class="a">an a</a>'
6577
.', <p class="p-class">a p</p>, <img src="test.jpg" />',
66-
false
78+
false,
79+
[]
6780
],
6881
'invalid-html' => [
6982
['div', 'a', 'p'],
7083
['class', 'src'],
7184
['a' => ['href'], 'div' => ['style']],
7285
'some </,none-> </html>',
73-
true
86+
true,
87+
[]
7488
],
7589
'invalid-html-with-violations' => [
7690
['div', 'a', 'p'],
7791
['class', 'src'],
7892
['a' => ['href'], 'div' => ['style']],
7993
'some </,none-> </html> <tr>some trs</tr>',
80-
false
94+
false,
95+
[]
96+
],
97+
'invalid-html-attributes' => [
98+
['div', 'a', 'p'],
99+
['class', 'src'],
100+
[],
101+
'some <div class="value">DIV</div>',
102+
false,
103+
['class' => false]
104+
],
105+
'ignored-html-attributes' => [
106+
['div', 'a', 'p'],
107+
['class', 'src'],
108+
[],
109+
'some <div class="value">DIV</div>',
110+
true,
111+
['src' => false, 'class' => true]
112+
],
113+
'valid-html-attributes' => [
114+
['div', 'a', 'p'],
115+
['class', 'src'],
116+
[],
117+
'some <div class="value">DIV</div>',
118+
true,
119+
['src' => true, 'class' => true]
81120
]
82121
];
83122
}
@@ -90,6 +129,7 @@ public function getConfigurations(): array
90129
* @param array $allowedTagAttrs
91130
* @param string $html
92131
* @param bool $isValid
132+
* @param array $attributeValidityMap
93133
* @return void
94134
* @dataProvider getConfigurations
95135
*/
@@ -98,9 +138,28 @@ public function testConfigurations(
98138
array $allowedAttr,
99139
array $allowedTagAttrs,
100140
string $html,
101-
bool $isValid
141+
bool $isValid,
142+
array $attributeValidityMap
102143
): void {
103-
$validator = new ConfigurableWYSIWYGValidator($allowedTags, $allowedAttr, $allowedTagAttrs);
144+
$attributeValidator = $this->getMockForAbstractClass(AttributeValidatorInterface::class);
145+
$attributeValidator->method('validate')
146+
->willReturnCallback(
147+
function (string $tag, string $attribute, string $content) use ($attributeValidityMap): void {
148+
if (array_key_exists($attribute, $attributeValidityMap) && !$attributeValidityMap[$attribute]) {
149+
throw new ValidationException(__('Invalid attribute for %1', $tag));
150+
}
151+
}
152+
);
153+
$attrValidators = [];
154+
foreach (array_keys($attributeValidityMap) as $attr) {
155+
$attrValidators[$attr] = $attributeValidator;
156+
}
157+
$validator = new ConfigurableWYSIWYGValidator(
158+
$allowedTags,
159+
$allowedAttr,
160+
$allowedTagAttrs,
161+
$attrValidators
162+
);
104163
$valid = true;
105164
try {
106165
$validator->validate($html);

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\Framework\Test\Unit\Validator\HTML;
10+
11+
use Magento\Framework\Validation\ValidationException;
12+
use Magento\Framework\Validator\HTML\StyleAttributeValidator;
13+
use PHPUnit\Framework\TestCase;
14+
15+
class StyleAttributeValidatorTest extends TestCase
16+
{
17+
/**
18+
* Cases for "validate" test.
19+
*
20+
* @return array
21+
*/
22+
public function getAttributes(): array
23+
{
24+
return [
25+
'not a style' => ['class', 'value', true],
26+
'valid style' => ['style', 'color: blue', true],
27+
'invalid position style' => ['style', 'color: blue; position: absolute; width: 100%', false],
28+
'another invalid position style' => ['style', 'position: fixed; width: 100%', false],
29+
'valid position style' => ['style', 'color: blue; position: inherit; width: 100%', true],
30+
'valid background style' => ['style', 'color: blue; background-position: left; width: 100%', true],
31+
'invalid opacity style' => ['style', 'color: blue; width: 100%; opacity: 0.5', false],
32+
'invalid z-index style' => ['style', 'color: blue; width: 100%; z-index: 11', false]
33+
];
34+
}
35+
36+
/**
37+
* Test "validate" method.
38+
*
39+
* @param string $attr
40+
* @param string $value
41+
* @param bool $expectedValid
42+
* @return void
43+
* @dataProvider getAttributes
44+
*/
45+
public function testValidate(string $attr, string $value, bool $expectedValid): void
46+
{
47+
$validator = new StyleAttributeValidator();
48+
49+
try {
50+
$validator->validate('does not matter', $attr, $value);
51+
$actuallyValid = true;
52+
} catch (ValidationException $exception) {
53+
$actuallyValid = false;
54+
}
55+
$this->assertEquals($expectedValid, $actuallyValid);
56+
}
57+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\Framework\Validator\HTML;
10+
11+
use Magento\Framework\Validation\ValidationException;
12+
13+
/**
14+
* Validates HTML attributes content.
15+
*/
16+
interface AttributeValidatorInterface
17+
{
18+
/**
19+
* Validate attribute.
20+
*
21+
* @param string $tag
22+
* @param string $attributeName
23+
* @param string $value
24+
* @return void
25+
* @throws ValidationException
26+
*/
27+
public function validate(string $tag, string $attributeName, string $value): void;
28+
}

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

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,22 @@ class ConfigurableWYSIWYGValidator implements WYSIWYGValidatorInterface
3131
private $attributesAllowedByTags;
3232

3333
/**
34-
* @param array $allowedTags
35-
* @param array $allowedAttributes
36-
* @param array $attributesAllowedByTags
34+
* @var AttributeValidatorInterface[]
3735
*/
38-
public function __construct(array $allowedTags, array $allowedAttributes = [], array $attributesAllowedByTags = [])
39-
{
36+
private $attributeValidators;
37+
38+
/**
39+
* @param string[] $allowedTags
40+
* @param string[] $allowedAttributes
41+
* @param string[] $attributesAllowedByTags
42+
* @param AttributeValidatorInterface[] $attributeValidators
43+
*/
44+
public function __construct(
45+
array $allowedTags,
46+
array $allowedAttributes = [],
47+
array $attributesAllowedByTags = [],
48+
array $attributeValidators = []
49+
) {
4050
if (empty(array_filter($allowedTags))) {
4151
throw new \InvalidArgumentException('List of allowed HTML tags cannot be empty');
4252
}
@@ -49,6 +59,7 @@ function (string $tag) use ($allowedTags): bool {
4959
},
5060
ARRAY_FILTER_USE_KEY
5161
);
62+
$this->attributeValidators = $attributeValidators;
5263
}
5364

5465
/**
@@ -132,6 +143,17 @@ function (string $attribute): string {
132143
);
133144
}
134145
}
146+
147+
//Validating allowed attributes.
148+
if ($this->attributeValidators) {
149+
foreach ($this->attributeValidators as $attr => $validator) {
150+
$found = $xpath->query("//@*[name() = '$attr']");
151+
foreach ($found as $attribute) {
152+
$validator->validate($attribute->parentNode->tagName, $attribute->name, $attribute->value);
153+
}
154+
}
155+
}
156+
135157
}
136158

137159
/**

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\Framework\Validator\HTML;
10+
11+
use Magento\Framework\Validation\ValidationException;
12+
13+
/**
14+
* Validates "style" attribute.
15+
*/
16+
class StyleAttributeValidator implements AttributeValidatorInterface
17+
{
18+
/**
19+
* @inheritDoc
20+
*/
21+
public function validate(string $tag, string $attributeName, string $value): void
22+
{
23+
if ($attributeName !== 'style' || !$value) {
24+
return;
25+
}
26+
27+
if (preg_match('/([^\-]position\s*?\:\s*?[^i\s][^n\s]\w)|(opacity)|(z-index)/ims', " $value")) {
28+
throw new ValidationException(__('HTML attribute "style" contains restricted styles'));
29+
}
30+
}
31+
}

0 commit comments

Comments
 (0)