Skip to content

Commit 3f6d8c4

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

File tree

7 files changed

+533
-0
lines changed

7 files changed

+533
-0
lines changed

‎app/code/Magento/Catalog/Model/Attribute/Backend/DefaultBackend.php

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
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\Catalog\Model\Attribute\Backend;
10+
11+
use Magento\Catalog\Model\AbstractModel;
12+
use Magento\Catalog\Model\ResourceModel\Eav\Attribute;
13+
use Magento\Eav\Model\Entity\Attribute\Backend\DefaultBackend as ParentBackend;
14+
use Magento\Eav\Model\Entity\Attribute\Exception;
15+
use Magento\Framework\DataObject;
16+
use Magento\Framework\Exception\LocalizedException;
17+
use Magento\Framework\Validation\ValidationException;
18+
use Magento\Framework\Validator\HTML\WYSIWYGValidatorInterface;
19+
20+
/**
21+
* Default backend model for catalog attributes.
22+
*/
23+
class DefaultBackend extends ParentBackend
24+
{
25+
/**
26+
* @var WYSIWYGValidatorInterface
27+
*/
28+
private $wysiwygValidator;
29+
30+
/**
31+
* @param WYSIWYGValidatorInterface $wysiwygValidator
32+
*/
33+
public function __construct(WYSIWYGValidatorInterface $wysiwygValidator)
34+
{
35+
$this->wysiwygValidator = $wysiwygValidator;
36+
}
37+
38+
/**
39+
* Validate user HTML value.
40+
*
41+
* @param DataObject $object
42+
* @return void
43+
* @throws LocalizedException
44+
*/
45+
private function validateHtml(DataObject $object): void
46+
{
47+
$attribute = $this->getAttribute();
48+
$code = $attribute->getAttributeCode();
49+
if ($attribute instanceof Attribute && $attribute->getIsHtmlAllowedOnFront()) {
50+
if ($object->getData($code)
51+
&& (!($object instanceof AbstractModel) || $object->getData($code) !== $object->getOrigData($code))
52+
) {
53+
try {
54+
$this->wysiwygValidator->validate($object->getData($code));
55+
} catch (ValidationException $exception) {
56+
$attributeException = new Exception(
57+
__(
58+
'Using restricted HTML elements for "%1". %2',
59+
$attribute->getName(),
60+
$exception->getMessage()
61+
),
62+
$exception
63+
);
64+
$attributeException->setAttributeCode($code)->setPart('backend');
65+
throw $attributeException;
66+
}
67+
}
68+
}
69+
}
70+
71+
/**
72+
* @inheritDoc
73+
*/
74+
public function beforeSave($object)
75+
{
76+
parent::beforeSave($object);
77+
$this->validateHtml($object);
78+
79+
return $this;
80+
}
81+
82+
/**
83+
* @inheritDoc
84+
*/
85+
public function validate($object)
86+
{
87+
$isValid = parent::validate($object);
88+
if ($isValid) {
89+
$this->validateHtml($object);
90+
}
91+
92+
return $isValid;
93+
}
94+
}

‎app/code/Magento/Catalog/Model/ResourceModel/Eav/Attribute.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
namespace Magento\Catalog\Model\ResourceModel\Eav;
88

9+
use Magento\Catalog\Model\Attribute\Backend\DefaultBackend;
910
use Magento\Catalog\Model\Attribute\LockValidatorInterface;
11+
use Magento\Eav\Model\Entity;
1012
use Magento\Framework\Api\AttributeValueFactory;
1113
use Magento\Framework\Stdlib\DateTime\DateTimeFormatterInterface;
1214

@@ -901,4 +903,17 @@ public function setIsFilterableInGrid($isFilterableInGrid)
901903
$this->setData(self::IS_FILTERABLE_IN_GRID, $isFilterableInGrid);
902904
return $this;
903905
}
906+
907+
/**
908+
* @inheritDoc
909+
*/
910+
protected function _getDefaultBackendModel()
911+
{
912+
$backend = parent::_getDefaultBackendModel();
913+
if ($backend === Entity::DEFAULT_BACKEND_MODEL) {
914+
$backend = DefaultBackend::class;
915+
}
916+
917+
return $backend;
918+
}
904919
}

‎app/code/Magento/Catalog/Test/Unit/Model/Attribute/Backend/DefaultBackendTest.php

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
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\Catalog\Test\Unit\Model\Attribute\Backend;
10+
11+
use Magento\Catalog\Model\AbstractModel;
12+
use Magento\Catalog\Model\Attribute\Backend\DefaultBackend;
13+
use Magento\Framework\DataObject;
14+
use Magento\Framework\Validation\ValidationException;
15+
use Magento\Framework\Validator\HTML\WYSIWYGValidatorInterface;
16+
use PHPUnit\Framework\TestCase;
17+
use Magento\Eav\Model\Entity\Attribute as BasicAttribute;
18+
use Magento\Catalog\Model\ResourceModel\Eav\Attribute;
19+
use Magento\Eav\Model\Entity\Attribute\Exception as AttributeException;
20+
21+
class DefaultBackendTest extends TestCase
22+
{
23+
/**
24+
* Different cases for attribute validation.
25+
*
26+
* @return array
27+
*/
28+
public function getAttributeConfigurations(): array
29+
{
30+
return [
31+
'basic-attribute' => [true, false, true, 'basic', 'value', false, true, false],
32+
'non-html-attribute' => [false, false, false, 'non-html', 'value', false, false, false],
33+
'empty-html-attribute' => [false, false, true, 'html', null, false, true, false],
34+
'invalid-html-attribute' => [false, false, false, 'html', 'value', false, true, true],
35+
'valid-html-attribute' => [false, true, false, 'html', 'value', false, true, false],
36+
'changed-invalid-html-attribute' => [false, false, true, 'html', 'value', true, true, true],
37+
'changed-valid-html-attribute' => [false, true, true, 'html', 'value', true, true, false]
38+
];
39+
}
40+
41+
/**
42+
* Test attribute validation.
43+
*
44+
* @param bool $isBasic
45+
* @param bool $isValidated
46+
* @param bool $isCatalogEntity
47+
* @param string $code
48+
* @param mixed $value
49+
* @param bool $isChanged
50+
* @param bool $isHtmlAttribute
51+
* @param bool $exceptionThrown
52+
* @dataProvider getAttributeConfigurations
53+
*/
54+
public function testValidate(
55+
bool $isBasic,
56+
bool $isValidated,
57+
bool $isCatalogEntity,
58+
string $code,
59+
$value,
60+
bool $isChanged,
61+
bool $isHtmlAttribute,
62+
bool $exceptionThrown
63+
): void {
64+
if ($isBasic) {
65+
$attributeMock = $this->createMock(BasicAttribute::class);
66+
} else {
67+
$attributeMock = $this->createMock(Attribute::class);
68+
$attributeMock->expects($this->any())
69+
->method('getIsHtmlAllowedOnFront')
70+
->willReturn($isHtmlAttribute);
71+
}
72+
$attributeMock->expects($this->any())->method('getAttributeCode')->willReturn($code);
73+
74+
$validatorMock = $this->getMockForAbstractClass(WYSIWYGValidatorInterface::class);
75+
if (!$isValidated) {
76+
$validatorMock->expects($this->any())
77+
->method('validate')
78+
->willThrowException(new ValidationException(__('HTML is invalid')));
79+
} else {
80+
$validatorMock->expects($this->any())->method('validate');
81+
}
82+
83+
if ($isCatalogEntity) {
84+
$objectMock = $this->createMock(AbstractModel::class);
85+
$objectMock->expects($this->any())
86+
->method('getOrigData')
87+
->willReturn($isChanged ? $value .'-OLD' : $value);
88+
} else {
89+
$objectMock = $this->createMock(DataObject::class);
90+
}
91+
$objectMock->expects($this->any())->method('getData')->with($code)->willReturn($value);
92+
93+
$model = new DefaultBackend($validatorMock);
94+
$model->setAttribute($attributeMock);
95+
96+
$actuallyThrownForSave = false;
97+
try {
98+
$model->beforeSave($objectMock);
99+
} catch (AttributeException $exception) {
100+
$actuallyThrownForSave = true;
101+
}
102+
$actuallyThrownForValidate = false;
103+
try {
104+
$model->validate($objectMock);
105+
} catch (AttributeException $exception) {
106+
$actuallyThrownForValidate = true;
107+
}
108+
$this->assertEquals($actuallyThrownForSave, $actuallyThrownForValidate);
109+
$this->assertEquals($actuallyThrownForSave, $exceptionThrown);
110+
}
111+
}

‎app/etc/di.xml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1832,4 +1832,21 @@
18321832
</argument>
18331833
</arguments>
18341834
</type>
1835+
<virtualType name="DefaultWYSIWYGValidator" type="Magento\Framework\Validator\HTML\ConfigurableWYSIWYGValidator">
1836+
<arguments>
1837+
<argument name="allowedTags" xsi:type="array">
1838+
<item name="div" xsi:type="string">div</item>
1839+
<item name="a" xsi:type="string">a</item>
1840+
</argument>
1841+
<argument name="allowedAttributes" xsi:type="array">
1842+
<item name="class" xsi:type="string">class</item>
1843+
</argument>
1844+
<argument name="attributesAllowedByTags" xsi:type="array">
1845+
<item name="a" xsi:type="array">
1846+
<item name="href" xsi:type="string">href</item>
1847+
</item>
1848+
</argument>
1849+
</arguments>
1850+
</virtualType>
1851+
<preference for="Magento\Framework\Validator\HTML\WYSIWYGValidatorInterface" type="DefaultWYSIWYGValidator" />
18351852
</config>

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

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
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\ConfigurableWYSIWYGValidator;
13+
use PHPUnit\Framework\TestCase;
14+
15+
class ConfigurableWYSIWYGValidatorTest extends TestCase
16+
{
17+
/**
18+
* Configurations to test.
19+
*
20+
* @return array
21+
*/
22+
public function getConfigurations(): array
23+
{
24+
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],
31+
'tags-with-attrs' => [
32+
['div', 'p'],
33+
['class', 'style'],
34+
[],
35+
'text and <div class="fake-class">a div</div> and <p style="color: blue">a p</p>',
36+
true
37+
],
38+
'tags-with-restricted-attrs' => [
39+
['div', 'p'],
40+
['class', 'align'],
41+
[],
42+
'text and <div class="fake-class">a div</div> and <p style="color: blue">a p</p>',
43+
false
44+
],
45+
'tags-with-specific-attrs' => [
46+
['div', 'a', 'p'],
47+
['class'],
48+
['a' => ['href'], 'div' => ['style']],
49+
'<div class="fake-class" style="color: blue">a div</div>, <a href="/some-path" class="a">an a</a>'
50+
.', <p class="p-class">a p</p>',
51+
true
52+
],
53+
'tags-with-specific-restricted-attrs' => [
54+
['div', 'a'],
55+
['class'],
56+
['a' => ['href']],
57+
'text and <div class="fake-class" href="what">a div</div> and <a href="/some-path" class="a">an a</a>',
58+
false
59+
],
60+
'invalid-tag-with-full-config' => [
61+
['div', 'a', 'p'],
62+
['class', 'src'],
63+
['a' => ['href'], 'div' => ['style']],
64+
'<div class="fake-class" style="color: blue">a div</div>, <a href="/some-path" class="a">an a</a>'
65+
.', <p class="p-class">a p</p>, <img src="test.jpg" />',
66+
false
67+
],
68+
'invalid-html' => [
69+
['div', 'a', 'p'],
70+
['class', 'src'],
71+
['a' => ['href'], 'div' => ['style']],
72+
'some </,none-> </html>',
73+
true
74+
],
75+
'invalid-html-with-violations' => [
76+
['div', 'a', 'p'],
77+
['class', 'src'],
78+
['a' => ['href'], 'div' => ['style']],
79+
'some </,none-> </html> <tr>some trs</tr>',
80+
false
81+
]
82+
];
83+
}
84+
85+
/**
86+
* Test different configurations and content.
87+
*
88+
* @param array $allowedTags
89+
* @param array $allowedAttr
90+
* @param array $allowedTagAttrs
91+
* @param string $html
92+
* @param bool $isValid
93+
* @return void
94+
* @dataProvider getConfigurations
95+
*/
96+
public function testConfigurations(
97+
array $allowedTags,
98+
array $allowedAttr,
99+
array $allowedTagAttrs,
100+
string $html,
101+
bool $isValid
102+
): void {
103+
$validator = new ConfigurableWYSIWYGValidator($allowedTags, $allowedAttr, $allowedTagAttrs);
104+
$valid = true;
105+
try {
106+
$validator->validate($html);
107+
} catch (ValidationException $exception) {
108+
$valid = false;
109+
}
110+
111+
self::assertEquals($isValid, $valid);
112+
}
113+
}

0 commit comments

Comments
 (0)