Skip to content

Commit 14e1888

Browse files
OlgaVasyltsunOlgaVasyltsun
committed
MAGETWO-70735: [UX][Security] Missing validation and hint for "Compatible File Extensions" in Custom Option of Type=File
1 parent 28916b5 commit 14e1888

File tree

9 files changed

+542
-185
lines changed

9 files changed

+542
-185
lines changed

‎app/code/Magento/Catalog/Model/Product/Option.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,10 @@ public function beforeSave()
377377
}
378378
}
379379
}
380+
if ($this->getGroupByType($this->getData('type')) === self::OPTION_GROUP_FILE) {
381+
$this->cleanFileExtensions();
382+
}
383+
380384
return $this;
381385
}
382386

@@ -902,4 +906,20 @@ private function getMetadataPool()
902906
}
903907

904908
//@codeCoverageIgnoreEnd
909+
910+
/**
911+
* Clears all non-accepted characters from file_extension field.
912+
*
913+
* @return void
914+
*/
915+
private function cleanFileExtensions()
916+
{
917+
$rawExtensions = $this->getFileExtension();
918+
$matches = [];
919+
preg_match_all('/(?<extensions>[a-z0-9]+)/i', strtolower($rawExtensions), $matches);
920+
if (!empty($matches)) {
921+
$extensions = implode(', ', array_unique($matches['extensions']));
922+
}
923+
$this->setFileExtension($extensions);
924+
}
905925
}

‎app/code/Magento/Catalog/Test/Unit/Model/Product/OptionTest.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,41 @@ public function testGetRegularPrice()
6868
$this->model->setPriceType(null);
6969
$this->assertEquals(50, $this->model->getRegularPrice());
7070
}
71+
72+
/**
73+
* Tests removing ineligible characters from file_extension.
74+
*
75+
* @param string $rawExtensions
76+
* @param string $expectedExtensions
77+
* @dataProvider cleanFileExtensionsDataProvider
78+
*/
79+
public function testCleanFileExtensions(string $rawExtensions, string $expectedExtensions)
80+
{
81+
$this->model->setType(Option::OPTION_GROUP_FILE);
82+
$this->model->setFileExtension($rawExtensions);
83+
$this->model->beforeSave();
84+
$actualExtensions = $this->model->getFileExtension();
85+
$this->assertEquals(
86+
$expectedExtensions,
87+
$actualExtensions
88+
);
89+
}
90+
91+
/**
92+
* Data provider for testCleanFileExtensions.
93+
*
94+
* @return array
95+
*/
96+
public function cleanFileExtensionsDataProvider()
97+
{
98+
return [
99+
['JPG, PNG, GIF', 'jpg, png, gif'],
100+
['jpg, jpg, jpg', 'jpg'],
101+
['jpg, png, gif', 'jpg, png, gif'],
102+
['jpg png gif', 'jpg, png, gif'],
103+
['!jpg@png#gif%', 'jpg, png, gif'],
104+
['jpg, png, 123', 'jpg, png, 123'],
105+
['', '']
106+
];
107+
}
71108
}

‎app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/CustomOptions.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,7 @@ protected function getFileExtensionFieldConfig($sortOrder)
10461046
'data' => [
10471047
'config' => [
10481048
'label' => __('Compatible File Extensions'),
1049+
'notice' => __('Enter separated extensions, like: png, jpg, gif. Leave blank to allow any.'),
10491050
'componentType' => Field::NAME,
10501051
'formElement' => Input::NAME,
10511052
'dataScope' => static::FIELD_FILE_EXTENSION_NAME,

‎dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Adminhtml/Product/Edit/Section/Options.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,13 @@ class Options extends Section
7575
*/
7676
protected $staticDataRow = '[data-index="container_type_static"] div:nth-child(%d)';
7777

78+
/**
79+
* Locator for file_extension field.
80+
*
81+
* @var string
82+
*/
83+
private $hintMessage = "div[data-index='file_extension'] div[id^='notice']";
84+
7885
/**
7986
* Sort rows data.
8087
*
@@ -386,4 +393,14 @@ public function getValuesDataForOption(array $options, string $optionType, strin
386393

387394
return $formDataItem;
388395
}
396+
397+
/**
398+
* Returns notice-message elements for 'file_extension' fields.
399+
*
400+
* @return ElementInterface[]
401+
*/
402+
public function getFileOptionElements()
403+
{
404+
return $this->_rootElement->getElements($this->hintMessage);
405+
}
389406
}

‎dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertCustomOptions.php

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Catalog\Test\Constraint;
8+
9+
use Magento\Catalog\Test\Page\Adminhtml\CatalogProductEdit;
10+
use Magento\Catalog\Test\Page\Adminhtml\CatalogProductIndex;
11+
use Magento\Mtf\Fixture\FixtureInterface;
12+
13+
/**
14+
* Asserts what custom option values are same as expected.
15+
*/
16+
class AssertCustomOptions extends AssertProductForm
17+
{
18+
/**
19+
* Assert form data equals fixture data
20+
*
21+
* @param FixtureInterface $product
22+
* @param CatalogProductIndex $productGrid
23+
* @param CatalogProductEdit $productPage
24+
* @return void
25+
*/
26+
public function processAssert(
27+
FixtureInterface $product,
28+
CatalogProductIndex $productGrid,
29+
CatalogProductEdit $productPage
30+
) {
31+
$expectedCustomOptions = $this->arguments['expectedCustomOptions'];
32+
$filter = ['sku' => $product->getSku()];
33+
$productGrid->open();
34+
$productGrid->getProductGrid()->searchAndOpen($filter);
35+
$productData = [];
36+
if ($product->hasData('custom_options')) {
37+
$productData = $this->addExpectedOptionValues($product, $expectedCustomOptions);
38+
}
39+
$fixtureData = $this->prepareFixtureData($productData, $this->sortFields);
40+
$formData = $this->prepareFormData($productPage->getProductForm()->getData($product), $this->sortFields);
41+
$error = $this->verifyData($fixtureData, $formData);
42+
\PHPUnit\Framework\Assert::assertTrue(empty($error), $error);
43+
}
44+
45+
/**
46+
* Adds expected value of Custom Options.
47+
*
48+
* @param FixtureInterface $product
49+
* @param array $expectedCustomOptions
50+
* @return array
51+
*/
52+
private function addExpectedOptionValues(FixtureInterface $product, array $expectedCustomOptions)
53+
{
54+
/** @var array $customOptionsSource */
55+
$customOptionsSource = $product->getDataFieldConfig('custom_options')['source']->getCustomOptions();
56+
foreach (array_keys($customOptionsSource) as $optionKey) {
57+
foreach ($expectedCustomOptions as $expectedCustomOption) {
58+
if ($customOptionsSource[$optionKey]['type'] === $expectedCustomOption['optionType']) {
59+
$options = array_keys($customOptionsSource[$optionKey]['options']);
60+
$optionField = $expectedCustomOption['optionField'];
61+
$optionValue = $expectedCustomOption['optionValue'];
62+
foreach ($options as $optionsKey) {
63+
$customOptionsSource[$optionKey]['options'][$optionsKey][$optionField] = $optionValue;
64+
}
65+
}
66+
}
67+
}
68+
69+
return ['custom_options' => $customOptionsSource];
70+
}
71+
72+
/**
73+
* Returns a string representation of the object.
74+
*
75+
* @return string
76+
*/
77+
public function toString()
78+
{
79+
return 'Custom option values are same as expected.';
80+
}
81+
}

‎dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertFileExtensionHints.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+
namespace Magento\Catalog\Test\Constraint;
8+
9+
use Magento\Catalog\Test\Block\Adminhtml\Product\ProductForm;
10+
use Magento\Catalog\Test\Page\Adminhtml\CatalogProductEdit;
11+
use Magento\Mtf\Constraint\AbstractAssertForm;
12+
13+
/**
14+
* Checks file_extension field hint on the product page custom options section.
15+
*/
16+
class AssertFileExtensionHints extends AbstractAssertForm
17+
{
18+
/**
19+
* Expected file_extension field hint.
20+
*
21+
* @var string
22+
*/
23+
const EXPECTED_MESSAGE = 'Enter separated extensions, like: png, jpg, gif. Leave blank to allow any.';
24+
25+
/**
26+
* Assert that file extension message is showed.
27+
*
28+
* @param CatalogProductEdit $productPage
29+
* @return void
30+
*/
31+
public function processAssert(CatalogProductEdit $productPage)
32+
{
33+
/** @var ProductForm $productForm */
34+
$productForm = $productPage->getProductForm();
35+
$productForm->openSection('customer-options');
36+
/** @var \Magento\Catalog\Test\Block\Adminhtml\Product\Edit\Section\Options $options */
37+
$options = $productForm->getSection('customer-options');
38+
$fileOptionElements = $options->getFileOptionElements();
39+
foreach ($fileOptionElements as $fileOptionElement) {
40+
\PHPUnit\Framework\Assert::assertEquals(
41+
self::EXPECTED_MESSAGE,
42+
$fileOptionElement->getText(),
43+
'Actual message differ from expected.'
44+
);
45+
}
46+
}
47+
48+
/**
49+
* Returns a string representation of the object.
50+
*
51+
* @return string
52+
*/
53+
public function toString()
54+
{
55+
return 'Assert correct file extensions hint is showed.';
56+
}
57+
}

‎dev/tests/functional/tests/app/Magento/Catalog/Test/Repository/Product/CustomOptions.xml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,5 +726,21 @@
726726
</item>
727727
</field>
728728
</dataset>
729+
730+
<dataset name="file_option_type">
731+
<field name="0" xsi:type="array">
732+
<item name="title" xsi:type="string">custom option file %isolation%</item>
733+
<item name="is_require" xsi:type="string">Yes</item>
734+
<item name="type" xsi:type="string">File/File</item>
735+
<item name="options" xsi:type="array">
736+
<item name="0" xsi:type="array">
737+
<item name="price" xsi:type="string">40</item>
738+
<item name="price_type" xsi:type="string">Percent</item>
739+
<item name="sku" xsi:type="string">file_option</item>
740+
<item name="file_extension" xsi:type="string">jpg, jpg gif .png</item>
741+
</item>
742+
</item>
743+
</field>
744+
</dataset>
729745
</repository>
730746
</config>

0 commit comments

Comments
 (0)