Skip to content

Commit 116d9c9

Browse files
author
Cristian Partica
committed
FearlessKiwis-MAGETWO-8436-Tax-class-name-corrupted-if-contain-char
- all fixes for this bug into one squashed into 1 commit
1 parent db7e503 commit 116d9c9

File tree

5 files changed

+301
-14
lines changed

5 files changed

+301
-14
lines changed

‎app/code/Magento/Tax/Controller/Adminhtml/Tax.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function __construct(
4949
*/
5050
protected function _processClassName($className)
5151
{
52-
$className = trim($this->_objectManager->get('Magento\Framework\Escaper')->escapeHtml($className));
52+
$className = trim($className);
5353
if ($className == '') {
5454
throw new InputException(__('Invalid name of tax class specified.'));
5555
}

‎app/code/Magento/Tax/Test/Unit/Model/TaxClass/Source/CustomerTest.php

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,4 +181,115 @@ public function dataProviderGetAllOptions()
181181
['isEmpty' => true, 'expected' => []]
182182
];
183183
}
184+
185+
/**
186+
* Run test getAllOptions method for names integrity
187+
*
188+
* @param array $value
189+
* @dataProvider dataProviderGetAllOptionsNameIntegrity
190+
*/
191+
public function testGetAllOptionsNameIntegrity(array $value)
192+
{
193+
$filterMock = $this->getMock(
194+
'Magento\Framework\Api\Filter',
195+
[],
196+
[],
197+
'',
198+
false
199+
);
200+
$searchCriteriaMock = $this->getMock(
201+
'Magento\Framework\Api\SearchCriteria',
202+
[],
203+
[],
204+
'',
205+
false
206+
);
207+
$searchResultsMock = $this->getMockForAbstractClass(
208+
'Magento\Tax\Api\Data\TaxClassSearchResultsInterface',
209+
[],
210+
'',
211+
false,
212+
true,
213+
true,
214+
['getItems']
215+
);
216+
$taxClassMock = $this->getMockForAbstractClass(
217+
'Magento\Tax\Api\Data\TaxClassInterface',
218+
['getClassId', 'getClassName'],
219+
'',
220+
false,
221+
true,
222+
true
223+
);
224+
225+
$this->filterBuilderMock->expects($this->once())
226+
->method('setField')
227+
->with(\Magento\Tax\Model\ClassModel::KEY_TYPE)
228+
->willReturnSelf();
229+
$this->filterBuilderMock->expects($this->once())
230+
->method('setValue')
231+
->with(\Magento\Tax\Api\TaxClassManagementInterface::TYPE_CUSTOMER)
232+
->willReturnSelf();
233+
$this->filterBuilderMock->expects($this->once())
234+
->method('create')
235+
->willReturn($filterMock);
236+
$this->searchCriteriaBuilderMock->expects($this->once())
237+
->method('addFilter')
238+
->with([$filterMock])
239+
->willReturnSelf();
240+
$this->searchCriteriaBuilderMock->expects($this->once())
241+
->method('create')
242+
->willReturn($searchCriteriaMock);
243+
$this->taxClassRepositoryMock->expects($this->once())
244+
->method('getList')
245+
->with($searchCriteriaMock)
246+
->willReturn($searchResultsMock);
247+
248+
249+
$taxClassMock->expects($this->once())
250+
->method('getClassId')
251+
->willReturn($value['value']);
252+
$taxClassMock->expects($this->once())
253+
->method('getClassName')
254+
->willReturn($value['label']);
255+
256+
$items = [$taxClassMock];
257+
$searchResultsMock->expects($this->once())
258+
->method('getItems')
259+
->willReturn($items);
260+
261+
$result=($this->customer->getAllOptions());
262+
$expected=$value;
263+
$this->assertEquals([$expected], $result);
264+
}
265+
266+
/**
267+
* Data provider for testGetAllOptionsNameIntegrity
268+
*
269+
* @return array
270+
*/
271+
public function dataProviderGetAllOptionsNameIntegrity()
272+
{
273+
return [
274+
[
275+
'value' => ['value' => 1, 'label' => 'unescaped name'],
276+
],
277+
[
278+
'value' => ['value' => 2, 'label' => 'tax < 50%'],
279+
],
280+
[
281+
'value' => ['value' => 3, 'label' => 'rule for 1 & 2'],
282+
],
283+
[
284+
'value' => ['value' => 4, 'label' => 'html <tag>'],
285+
],
286+
[
287+
'value' => ['value' => 5, 'label' => 'comment <!-- comment -->'],
288+
],
289+
[
290+
'value' => ['value' => 6, 'label' => 'php tag <?php echo "2"; ?>'],
291+
],
292+
293+
];
294+
}
184295
}

‎app/code/Magento/Tax/Test/Unit/Model/TaxClass/Source/ProductTest.php

Lines changed: 170 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,32 +9,84 @@
99

1010
class ProductTest extends \PHPUnit_Framework_TestCase
1111
{
12+
/**
13+
* @var \Magento\Tax\Api\TaxClassRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject
14+
*/
15+
protected $taxClassRepositoryMock;
16+
17+
/**
18+
* @var \Magento\Framework\Api\SearchCriteriaBuilder|\PHPUnit_Framework_MockObject_MockObject
19+
*/
20+
protected $searchCriteriaBuilderMock;
21+
22+
/**
23+
* @var \Magento\Framework\Api\FilterBuilder|\PHPUnit_Framework_MockObject_MockObject
24+
*/
25+
protected $filterBuilderMock;
26+
27+
/**
28+
* @var ObjectManager
29+
*/
30+
protected $objectManager;
31+
1232
/**
1333
* @var \Magento\Tax\Model\TaxClass\Source\Product
1434
*/
15-
protected $_model;
35+
protected $product;
1636

1737
public function setUp()
1838
{
19-
$objectManager = new ObjectManager($this);
20-
$this->_model = $objectManager->getObject('Magento\Tax\Model\TaxClass\Source\Product');
39+
$this->objectManager = new ObjectManager($this);
40+
41+
$this->taxClassRepositoryMock = $this->getMockForAbstractClass(
42+
'Magento\Tax\Api\TaxClassRepositoryInterface',
43+
['getList'],
44+
'',
45+
false,
46+
true,
47+
true,
48+
[]
49+
);
50+
$this->searchCriteriaBuilderMock = $this->getMock(
51+
'Magento\Framework\Api\SearchCriteriaBuilder',
52+
['addFilter', 'create'],
53+
[],
54+
'',
55+
false
56+
);
57+
$this->filterBuilderMock = $this->getMock(
58+
'Magento\Framework\Api\FilterBuilder',
59+
['setField', 'setValue', 'create'],
60+
[],
61+
'',
62+
false
63+
);
64+
65+
$this->product = $this->objectManager->getObject(
66+
'Magento\Tax\Model\TaxClass\Source\Product',
67+
[
68+
'taxClassRepository' => $this->taxClassRepositoryMock,
69+
'searchCriteriaBuilder' => $this->searchCriteriaBuilderMock,
70+
'filterBuilder' => $this->filterBuilderMock
71+
]
72+
);
2173
}
2274

2375
public function testGetFlatColumns()
2476
{
25-
$abstractAttributeMock = $this->getMock(
77+
$abstractAttrMock = $this->getMock(
2678
'\Magento\Eav\Model\Entity\Attribute\AbstractAttribute',
2779
['getAttributeCode', '__wakeup'],
2880
[],
2981
'',
3082
false
3183
);
3284

33-
$abstractAttributeMock->expects($this->any())->method('getAttributeCode')->will($this->returnValue('code'));
85+
$abstractAttrMock->expects($this->any())->method('getAttributeCode')->will($this->returnValue('code'));
3486

35-
$this->_model->setAttribute($abstractAttributeMock);
87+
$this->product->setAttribute($abstractAttrMock);
3688

37-
$flatColumns = $this->_model->getFlatColumns();
89+
$flatColumns = $this->product->getFlatColumns();
3890

3991
$this->assertTrue(is_array($flatColumns), 'FlatColumns must be an array value');
4092
$this->assertTrue(!empty($flatColumns), 'FlatColumns must be not empty');
@@ -47,4 +99,115 @@ public function testGetFlatColumns()
4799
$this->assertArrayHasKey('comment', $result, 'FlatColumns must have "comment" column');
48100
}
49101
}
102+
103+
/**
104+
* Run test getAllOptions method for names integrity
105+
*
106+
* @param array $value
107+
* @dataProvider dataProviderGetAllOptionsNameIntegrity
108+
*/
109+
public function testGetAllOptionsNameIntegrity(array $value)
110+
{
111+
$filterMock = $this->getMock(
112+
'Magento\Framework\Api\Filter',
113+
[],
114+
[],
115+
'',
116+
false
117+
);
118+
$searchCriteriaMock = $this->getMock(
119+
'Magento\Framework\Api\SearchCriteria',
120+
[],
121+
[],
122+
'',
123+
false
124+
);
125+
$searchResultsMock = $this->getMockForAbstractClass(
126+
'Magento\Tax\Api\Data\TaxClassSearchResultsInterface',
127+
[],
128+
'',
129+
false,
130+
true,
131+
true,
132+
['getItems']
133+
);
134+
$taxClassMock = $this->getMockForAbstractClass(
135+
'Magento\Tax\Api\Data\TaxClassInterface',
136+
['getClassId', 'getClassName'],
137+
'',
138+
false,
139+
true,
140+
true
141+
);
142+
143+
$this->filterBuilderMock->expects($this->once())
144+
->method('setField')
145+
->with(\Magento\Tax\Model\ClassModel::KEY_TYPE)
146+
->willReturnSelf();
147+
$this->filterBuilderMock->expects($this->once())
148+
->method('setValue')
149+
->with(\Magento\Tax\Api\TaxClassManagementInterface::TYPE_PRODUCT)
150+
->willReturnSelf();
151+
$this->filterBuilderMock->expects($this->once())
152+
->method('create')
153+
->willReturn($filterMock);
154+
$this->searchCriteriaBuilderMock->expects($this->once())
155+
->method('addFilter')
156+
->with([$filterMock])
157+
->willReturnSelf();
158+
$this->searchCriteriaBuilderMock->expects($this->once())
159+
->method('create')
160+
->willReturn($searchCriteriaMock);
161+
$this->taxClassRepositoryMock->expects($this->once())
162+
->method('getList')
163+
->with($searchCriteriaMock)
164+
->willReturn($searchResultsMock);
165+
166+
167+
$taxClassMock->expects($this->once())
168+
->method('getClassId')
169+
->willReturn($value['value']);
170+
$taxClassMock->expects($this->once())
171+
->method('getClassName')
172+
->willReturn($value['label']);
173+
174+
$items = [$taxClassMock];
175+
$searchResultsMock->expects($this->once())
176+
->method('getItems')
177+
->willReturn($items);
178+
179+
$result=($this->product->getAllOptions(false));
180+
$expected=$value;
181+
$this->assertEquals([$expected], $result);
182+
}
183+
184+
/**
185+
* Data provider for testGetAllOptionsNameIntegrity
186+
*
187+
* @return array
188+
*/
189+
public function dataProviderGetAllOptionsNameIntegrity()
190+
{
191+
return [
192+
[
193+
'value' => ['value' => 1, 'label' => 'unescaped name'],
194+
],
195+
[
196+
'value' => ['value' => 2, 'label' => 'tax < 50%'],
197+
],
198+
[
199+
'value' => ['value' => 3, 'label' => 'rule for 1 & 2'],
200+
],
201+
[
202+
'value' => ['value' => 4, 'label' => 'html <tag>'],
203+
],
204+
[
205+
'value' => ['value' => 5, 'label' => 'comment <!-- comment -->'],
206+
],
207+
[
208+
'value' => ['value' => 6, 'label' => 'php tag <?php echo "2"; ?>'],
209+
],
210+
211+
];
212+
}
50213
}

‎dev/tests/integration/testsuite/Magento/Tax/Controller/Adminhtml/TaxTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,11 @@ public function ajaxActionDataProvider()
9494
],
9595
[
9696
['class_type' => 'PRODUCT', 'class_name' => '11111<22222'],
97-
['class_name' => '11111&lt;22222']
97+
['class_name' => '11111<22222']
9898
],
9999
[
100100
['class_type' => 'CUSTOMER', 'class_name' => ' 12<>sa&df '],
101-
['class_name' => '12&lt;&gt;sa&amp;df']
101+
['class_name' => '12<>sa&df']
102102
]
103103
];
104104
}

‎lib/web/mage/backend/editablemultiselect.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ define([
129129
},
130130
data: function(value, settings) {
131131
settings.isChecked.apply(this, [settings]);
132+
if (typeof value === 'string') {
133+
var retval = value.unescapeHTML();
134+
return (retval);
135+
}
132136
return value;
133137
},
134138
submitdata: this.submitData,
@@ -153,8 +157,10 @@ define([
153157
var select = $(this).closest('.mselect-list').prev(),
154158
current = $(this).closest('.mselect-list-item').index();
155159
if (result.success) {
156-
select.find('option').eq(current).val(result[entityIdName]).text(result[entityValueName]);
157-
$(this).html(result[entityValueName]);
160+
if (typeof result[entityValueName] === 'string') {
161+
select.find('option').eq(current).val(result[entityIdName]).text(result[entityValueName]);
162+
$(this).html(result[entityValueName].escapeHTML());
163+
}
158164
} else {
159165
alert(result.error_message);
160166
}
@@ -188,11 +194,18 @@ define([
188194
url: this.newUrl,
189195
success: function(result, status) {
190196
if (result.success) {
197+
var resultEntityValueName="";
198+
if (typeof result[entityValueName] === 'string') {
199+
resultEntityValueName=result[entityValueName].escapeHTML();
200+
}
201+
else {
202+
resultEntityValueName=result[entityValueName];
203+
}
191204
// Add item to initial select element
192205
select.append('<option value="' + result[entityIdName] + '" selected="selected">' +
193-
result[entityValueName] + '</option>');
206+
resultEntityValueName + '</option>');
194207
// Add editable multiselect item
195-
var mselectItemHtml = $(options.item.replace(/%value%|%label%/gi, result[entityValueName])
208+
var mselectItemHtml = $(options.item.replace(/%value%|%label%/gi, resultEntityValueName)
196209
.replace(/%mselectDisabledClass%|%iseditable%|%isremovable%/gi, '')
197210
.replace(/%mselectListItemClass%/gi, options.mselectListItemClass))
198211
.find('[type=checkbox]')

0 commit comments

Comments
 (0)