Skip to content

Commit f96b45e

Browse files
magento-engcom-teammagento-engcom-team
committed
ENGCOM-8312: Fix missing escape Url method #30008
- Merge Pull Request #30008 from mrtuvn/magento2:fix-missing-escape-url-method - Merged commits: 1. 777c7c7 2. d2f3466 3. 5f69036
2 parents 8b1d118 + 5f69036 commit f96b45e

File tree

2 files changed

+121
-83
lines changed
  • app/code/Magento/Widget/view/adminhtml/templates/instance/edit
  • dev/tests/integration/testsuite/Magento/Widget/Block/Adminhtml/Widget/Instance/Edit/Tab/Main

2 files changed

+121
-83
lines changed

‎app/code/Magento/Widget/view/adminhtml/templates/instance/edit/layout.phtml

Lines changed: 58 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55
*/
66

77
/** @var \Magento\Widget\Block\Adminhtml\Widget\Instance\Edit\Tab\Main\Layout $block */
8+
/** @var \Magento\Framework\Escaper $escaper */
89
/** @var \Magento\Framework\View\Helper\SecureHtmlRenderer $secureRenderer */
910

1011
?>
1112
<fieldset class="fieldset">
12-
<legend class="legend"><span><?= $block->escapeHtml(__('Layout Updates')) ?></span></legend>
13+
<legend class="legend"><span><?= $escaper->escapeHtml(__('Layout Updates')) ?></span></legend>
1314
<br />
1415
<div class="widget-layout-updates">
1516
<div id="page_group_container"></div>
@@ -45,56 +46,56 @@ var pageGroupTemplate = '<div class="fieldset-wrapper page_group_container" id="
4546
script;
4647
foreach ($block->getDisplayOnContainers() as $container):
4748
$scriptString .= <<<script
48-
'<div class="no-display {$block->escapeJs($container['code'])} group_container" '+
49-
'id="{$block->escapeJs($container['name'])}_<%- data.id %>">'+
49+
'<div class="no-display {$escaper->escapeJs($container['code'])} group_container" '+
50+
'id="{$escaper->escapeJs($container['name'])}_<%- data.id %>">'+
5051
'<input disabled="disabled" type="hidden" class="container_name" name="__[container_name]" '+
51-
'value="widget_instance[<%- data.id %>][{$block->escapeJs($container['name'])}]" />'+
52+
'value="widget_instance[<%- data.id %>][{$escaper->escapeJs($container['name'])}]" />'+
5253
'<input disabled="disabled" type="hidden" '+
53-
'name="widget_instance[<%- data.id %>][{$block->escapeJs($container['name'])}][page_id]" '+
54+
'name="widget_instance[<%- data.id %>][{$escaper->escapeJs($container['name'])}][page_id]" '+
5455
'value="<%- data.page_id %>" />'+
5556
'<input disabled="disabled" type="hidden" class="layout_handle_pattern" '+
56-
'name="widget_instance[<%- data.id %>][{$block->escapeJs($container['name'])}][layout_handle]" '+
57-
'value="{$block->escapeJs($container['layout_handle'])}" />'+
57+
'name="widget_instance[<%- data.id %>][{$escaper->escapeJs($container['name'])}][layout_handle]" '+
58+
'value="{$escaper->escapeJs($container['layout_handle'])}" />'+
5859
'<table class="data-table">'+
5960
'<col width="200" />'+
6061
'<thead>'+
6162
'<tr>'+
62-
'<th><label>{$block->escapeJs(__('%1', $container['label']))}</label></th>'+
63-
'<th><label>{$block->escapeJs(__('Container'))} <span class="required">*</span></label></th>'+
64-
'<th><label>{$block->escapeJs(__('Template'))}</label></th>'+
63+
'<th><label>{$escaper->escapeJs(__('%1', $container['label']))}</label></th>'+
64+
'<th><label>{$escaper->escapeJs(__('Container'))} <span class="required">*</span></label></th>'+
65+
'<th><label>{$escaper->escapeJs(__('Template'))}</label></th>'+
6566
'</tr>'+
6667
'</thead>'+
6768
'<tbody>'+
6869
'<tr>'+
6970
'<td>'+
7071
'<input disabled="disabled" type="radio" class="radio for_all" '+
71-
'id="all_{$block->escapeJs($container['name'])}_<%- data.id %>" '+
72-
'name="widget_instance[<%- data.id %>][{$block->escapeJs($container['name'])}][for]" '+
72+
'id="all_{$escaper->escapeJs($container['name'])}_<%- data.id %>" '+
73+
'name="widget_instance[<%- data.id %>][{$escaper->escapeJs($container['name'])}][for]" '+
7374
'value="all" checked="checked" />&nbsp;'+
74-
'<label for="all_{$block->escapeJs($container['name'])}_<%- data.id %>">'+
75-
'{$block->escapeJs(__('All'))}</label><br />'+
75+
'<label for="all_{$escaper->escapeJs($container['name'])}_<%- data.id %>">'+
76+
'{$escaper->escapeJs(__('All'))}</label><br />'+
7677
'<input disabled="disabled" type="radio" class="radio for_specific" '+
77-
'id="specific_{$block->escapeJs($container['name'])}_<%- data.id %>" '+
78-
'name="widget_instance[<%- data.id %>][{$block->escapeJs($container['name'])}][for]" '+
78+
'id="specific_{$escaper->escapeJs($container['name'])}_<%- data.id %>" '+
79+
'name="widget_instance[<%- data.id %>][{$escaper->escapeJs($container['name'])}][for]" '+
7980
'value="specific" />&nbsp;'+
80-
'<label for="specific_{$block->escapeJs($container['name'])}_<%- data.id %>">'+
81-
'{$block->escapeJs(__('Specific %1', $container['label']))}</label>'+
81+
'<label for="specific_{$escaper->escapeJs($container['name'])}_<%- data.id %>">'+
82+
'{$escaper->escapeJs(__('Specific %1', $container['label']))}</label>'+
8283
8384
script;
8485

8586
$scriptString1 = $secureRenderer->renderEventListenerAsTag(
8687
"onclick",
8788
"WidgetInstance.togglePageGroupChooser(this)",
88-
"all_" . $block->escapeJs($container['name']) . "_<%- data.id %>"
89+
"all_" . $escaper->escapeJs($container['name']) . "_<%- data.id %>"
8990
);
90-
$scriptString .= "'" . $block->escapeJs($scriptString1) . "'+" . PHP_EOL;
91+
$scriptString .= "'" . $escaper->escapeJs($scriptString1) . "'+" . PHP_EOL;
9192

9293
$scriptString1 = $secureRenderer->renderEventListenerAsTag(
9394
"onclick",
9495
"WidgetInstance.togglePageGroupChooser(this)",
95-
"specific_" . $block->escapeJs($container['name']) . "_<%- data.id %>"
96+
"specific_" . $escaper->escapeJs($container['name']) . "_<%- data.id %>"
9697
);
97-
$scriptString .= "'" . $block->escapeJs($scriptString1) . "'+" . PHP_EOL;
98+
$scriptString .= "'" . $escaper->escapeJs($scriptString1) . "'+" . PHP_EOL;
9899

99100
$scriptString .= <<<script
100101
'</td>'+
@@ -111,26 +112,30 @@ script;
111112
'</tr>'+
112113
'</tbody>'+
113114
'</table>'+
114-
'<div class="no-display chooser_container" id="{$block->escapeJs($container['name'])}_ids_<%- data.id %>">'+
115+
'<div class="no-display chooser_container" id="{$escaper->escapeJs($container['name'])}_ids_<%- data.id %>">'+
115116
'<input disabled="disabled" type="hidden" class="is_anchor_only" '+
116-
'name="widget_instance[<%- data.id %>][{$block->escapeJs($container['name'])}][is_anchor_only]" '+
117-
'value="{$block->escapeJs($container['is_anchor_only'])}" />'+
117+
'name="widget_instance[<%- data.id %>][{$escaper->escapeJs($container['name'])}][is_anchor_only]" '+
118+
'value="{$escaper->escapeJs($container['is_anchor_only'])}" />'+
118119
'<input disabled="disabled" type="hidden" class="product_type_id" '+
119-
'name="widget_instance[<%- data.id %>][{$block->escapeJs($container['name'])}][product_type_id]" '+
120-
'value="{$block->escapeJs($container['product_type_id'])}" />'+
120+
'name="widget_instance[<%- data.id %>][{$escaper->escapeJs($container['name'])}][product_type_id]" '+
121+
'value="{$escaper->escapeJs($container['product_type_id'])}" />'+
121122
'<p>' +
122123
'<input disabled="disabled" type="text" class="input-text entities" '+
123-
'name="widget_instance[<%- data.id %>][{$block->escapeJs($container['name'])}][entities]" '+
124-
'value="<%- data.{$block->escapeJs($container['name'])}_entities %>" readonly="readonly" />&nbsp;' +
124+
'name="widget_instance[<%- data.id %>][{$escaper->escapeJs($container['name'])}][entities]" '+
125+
'value="<%- data.{$escaper->escapeJs($container['name'])}_entities %>" readonly="readonly" />&nbsp;' +
125126
'<a class="widget-option-chooser" href="#" '+
126-
'title="{$block->escapeJs(__('Open Chooser'))}">' +
127-
'<img src="{$block->escapeJs($block->getViewFileUrl('images/rule_chooser_trigger.gif'))}" '+
128-
'alt="{$block->escapeJs(__('Open Chooser'))}" />' +
127+
'title="{$escaper->escapeJs(__('Open Chooser'))}">' +
128+
'<img src="{$escaper->escapeJs(
129+
$escaper->escapeUrl($block->getViewFileUrl('images/rule_chooser_trigger.gif'))
130+
)}" '+
131+
'alt="{$escaper->escapeJs(__('Open Chooser'))}" />' +
129132
'</a>&nbsp;' +
130133
'<a id="widget-apply-<%- data.id %>" href="#" '+
131-
'title="{$block->escapeJs(__('Apply'))}">' +
132-
'<img src="{$block->escapeJs($block->getViewFileUrl('images/rule_component_apply.gif'))}" '+
133-
'alt="{$block->escapeJs(__('Apply'))}" />' +
134+
'title="{$escaper->escapeJs(__('Apply'))}">' +
135+
'<img src="{$escaper->escapeJs(
136+
$escaper->escapeUrl($block->getViewFileUrl('images/rule_component_apply.gif'))
137+
)}" '+
138+
'alt="{$escaper->escapeJs(__('Apply'))}" />' +
134139
'</a>' +
135140
'</p>'+
136141
'<div class="chooser"></div>'+
@@ -141,19 +146,19 @@ script;
141146
$scriptString1 = $secureRenderer->renderEventListenerAsTag(
142147
"onclick",
143148
"event.preventDefault();
144-
WidgetInstance.displayEntityChooser('" .$block->escapeJs($container['code']) .
145-
"', '" . $block->escapeJs($container['name']) . "_ids_<%- data.id %>')",
146-
"div#" . $block->escapeJs($container['name']) . "_ids_<%- data.id %> a.widget-option-chooser"
149+
WidgetInstance.displayEntityChooser('" .$escaper->escapeJs($container['code']) .
150+
"', '" . $escaper->escapeJs($container['name']) . "_ids_<%- data.id %>')",
151+
"div#" . $escaper->escapeJs($container['name']) . "_ids_<%- data.id %> a.widget-option-chooser"
147152
);
148-
$scriptString .= "'" . $block->escapeJs($scriptString1) . "'+" . PHP_EOL;
153+
$scriptString .= "'" . $escaper->escapeJs($scriptString1) . "'+" . PHP_EOL;
149154

150155
$scriptString1 = $secureRenderer->renderEventListenerAsTag(
151156
'onclick',
152157
"event.preventDefault();
153-
WidgetInstance.hideEntityChooser('" . $block->escapeJs($container['name']) . "_ids_<%- data.id %>')",
158+
WidgetInstance.hideEntityChooser('" . $escaper->escapeJs($container['name']) . "_ids_<%- data.id %>')",
154159
"a#widget-apply-<%- data.id %>"
155160
);
156-
$scriptString .= "'" . $block->escapeJs($scriptString1) . "'+" . PHP_EOL;
161+
$scriptString .= "'" . $escaper->escapeJs($scriptString1) . "'+" . PHP_EOL;
157162
$scriptString .= <<<script
158163
159164
'</div>'+
@@ -175,8 +180,8 @@ $scriptString .= <<<script
175180
'<col width="200" />'+
176181
'<thead>'+
177182
'<tr>'+
178-
'<th><label>{$block->escapeJs(__('Container'))} <span class="required">*</span></label></th>'+
179-
'<th><label>{$block->escapeJs(__('Template'))}</label></th>'+
183+
'<th><label>{$escaper->escapeJs(__('Container'))} <span class="required">*</span></label></th>'+
184+
'<th><label>{$escaper->escapeJs(__('Template'))}</label></th>'+
180185
'<th>&nbsp;</th>'+
181186
'</tr>'+
182187
'</thead>'+
@@ -208,9 +213,9 @@ $scriptString .= <<<script
208213
'<col width="200" />'+
209214
'<thead>'+
210215
'<tr>'+
211-
'<th><label>{$block->escapeJs(__('Page'))} <span class="required">*</span></label></th>'+
212-
'<th><label>{$block->escapeJs(__('Container'))} <span class="required">*</span></label></th>'+
213-
'<th><label>{$block->escapeJs(__('Template'))}</label></th>'+
216+
'<th><label>{$escaper->escapeJs(__('Page'))} <span class="required">*</span></label></th>'+
217+
'<th><label>{$escaper->escapeJs(__('Container'))} <span class="required">*</span></label></th>'+
218+
'<th><label>{$escaper->escapeJs(__('Template'))}</label></th>'+
214219
'</tr>'+
215220
'</thead>'+
216221
'<tbody>'+
@@ -242,9 +247,9 @@ $scriptString .= <<<script
242247
'<col width="200" />'+
243248
'<thead>'+
244249
'<tr>'+
245-
'<th><label>{$block->escapeJs(__('Page'))} <span class="required">*</span></label></th>'+
246-
'<th><label>{$block->escapeJs(__('Container'))} <span class="required">*</span></label></th>'+
247-
'<th><label>{$block->escapeJs(__('Template'))}</label></th>'+
250+
'<th><label>{$escaper->escapeJs(__('Page'))} <span class="required">*</span></label></th>'+
251+
'<th><label>{$escaper->escapeJs(__('Container'))} <span class="required">*</span></label></th>'+
252+
'<th><label>{$escaper->escapeJs(__('Template'))}</label></th>'+
248253
'</tr>'+
249254
'</thead>'+
250255
'<tbody>'+
@@ -412,10 +417,10 @@ var WidgetInstance = {
412417
additional = {};
413418
}
414419
if (type == 'categories') {
415-
additional.url = '{$block->escapeJs($block->getCategoriesChooserUrl())}';
420+
additional.url = '{$escaper->escapeJs($escaper->escapeUrl($block->getCategoriesChooserUrl()))}';
416421
additional.post_parameters = \$H({'is_anchor_only':$(chooser).down('input.is_anchor_only').value});
417422
} else if (type == 'products') {
418-
additional.url = '{$block->escapeUrl($block->getProductsChooserUrl())}';
423+
additional.url = '{$escaper->escapeJs($escaper->escapeUrl($block->getProductsChooserUrl()))}';
419424
additional.post_parameters = \$H({'product_type_id':$(chooser).down('input.product_type_id').value});
420425
}
421426
if (chooser && additional) {
@@ -521,13 +526,13 @@ var WidgetInstance = {
521526
selected = '';
522527
parameters = {};
523528
if (type == 'block_reference') {
524-
url = '{$block->escapeJs($block->getBlockChooserUrl())}';
529+
url = '{$escaper->escapeJs($escaper->escapeUrl($block->getBlockChooserUrl()))}';
525530
if (additional.selectedBlock) {
526531
selected = additional.selectedBlock;
527532
}
528533
parameters.layout = value;
529534
} else if (type == 'block_template') {
530-
url = '{$block->escapeJs($block->getTemplateChooserUrl())}';
535+
url = '{$escaper->escapeJs($escaper->escapeUrl($block->getTemplateChooserUrl()))}';
531536
if (additional.selectedTemplate) {
532537
selected = additional.selectedTemplate;
533538
}

‎dev/tests/integration/testsuite/Magento/Widget/Block/Adminhtml/Widget/Instance/Edit/Tab/Main/LayoutTest.php

Lines changed: 63 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,62 +3,95 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
7+
68
namespace Magento\Widget\Block\Adminhtml\Widget\Instance\Edit\Tab\Main;
79

10+
use Magento\Framework\App\Area;
11+
use Magento\Framework\App\State;
12+
use Magento\Framework\Escaper;
13+
use Magento\Framework\ObjectManagerInterface;
14+
use Magento\Framework\View\DesignInterface;
15+
use Magento\Framework\View\LayoutInterface;
16+
use Magento\TestFramework\Helper\Bootstrap;
17+
use Magento\Widget\Model\Widget\Instance;
18+
use PHPUnit\Framework\TestCase;
19+
820
/**
921
* @magentoAppArea adminhtml
1022
*/
11-
class LayoutTest extends \PHPUnit\Framework\TestCase
23+
class LayoutTest extends TestCase
1224
{
1325
/**
14-
* @var \Magento\Widget\Block\Adminhtml\Widget\Instance\Edit\Tab\Main\Layout
26+
* @var ObjectManagerInterface
27+
*/
28+
private $objectManager;
29+
30+
/**
31+
* @var Layout
1532
*/
16-
protected $_block;
33+
private $block;
1734

35+
/**
36+
* @inheritDoc
37+
*/
1838
protected function setUp(): void
1939
{
20-
parent::setUp();
40+
$this->objectManager = Bootstrap::getObjectManager();
2141

22-
$this->_block = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
23-
\Magento\Framework\View\LayoutInterface::class
24-
)->createBlock(
25-
\Magento\Widget\Block\Adminhtml\Widget\Instance\Edit\Tab\Main\Layout::class,
26-
'',
27-
[
28-
'data' => [
29-
'widget_instance' => \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
30-
\Magento\Widget\Model\Widget\Instance::class
31-
),
42+
$this->block = $this->objectManager->get(LayoutInterface::class)
43+
->createBlock(
44+
Layout::class,
45+
'',
46+
[
47+
'data' => [
48+
'widget_instance' => $this->objectManager->create(Instance::class),
49+
],
3250
]
33-
]
34-
);
35-
$this->_block->setLayout(
36-
\Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
37-
\Magento\Framework\View\LayoutInterface::class
38-
)
39-
);
51+
);
52+
$this->block->setLayout($this->objectManager->get(LayoutInterface::class));
4053
}
4154

4255
/**
4356
* @magentoAppIsolation enabled
4457
*/
4558
public function testGetLayoutsChooser()
4659
{
47-
\Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
48-
\Magento\Framework\App\State::class
49-
)->setAreaCode(
50-
\Magento\Framework\App\Area::AREA_FRONTEND
51-
);
52-
\Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
53-
\Magento\Framework\View\DesignInterface::class
54-
)->setDefaultDesignTheme();
60+
$this->objectManager->get(State::class)
61+
->setAreaCode(Area::AREA_FRONTEND);
62+
$this->objectManager->get(DesignInterface::class)
63+
->setDefaultDesignTheme();
5564

56-
$actualHtml = $this->_block->getLayoutsChooser();
65+
$actualHtml = $this->block->getLayoutsChooser();
5766
$this->assertStringStartsWith('<select ', $actualHtml);
5867
$this->assertStringEndsWith('</select>', $actualHtml);
5968
$this->assertStringContainsString('id="layout_handle"', $actualHtml);
6069
$optionCount = substr_count($actualHtml, '<option ');
6170
$this->assertGreaterThan(1, $optionCount, 'HTML select tag must provide options to choose from.');
6271
$this->assertEquals($optionCount, substr_count($actualHtml, '</option>'));
6372
}
73+
74+
/**
75+
* Check that escapeUrl called from template
76+
*
77+
* @return void
78+
*/
79+
public function testToHtml(): void
80+
{
81+
$escaperMock = $this->createMock(Escaper::class);
82+
$this->objectManager->addSharedInstance($escaperMock, Escaper::class);
83+
84+
$escaperMock->expects($this->atLeast(6))
85+
->method('escapeUrl');
86+
87+
$this->block->toHtml();
88+
}
89+
90+
/**
91+
* @inheritDoc
92+
*/
93+
protected function tearDown(): void
94+
{
95+
$this->objectManager->removeSharedInstance(Escaper::class);
96+
}
6497
}

0 commit comments

Comments
 (0)