MAGETWO-54243: [Github] Directive values are not quote-escaped #3860

Author: Bohdan Korablov

app/code/Magento/Widget/Controller/Adminhtml/Widget/LoadOptions.php

@@ -25,7 +25,8 @@ public function execute()
         try {
             $this->_view->loadLayout();
             if ($paramsJson = $this->getRequest()->getParam('widget')) {
-                $request = $this->_objectManager->get('Magento\Framework\Json\Helper\Data')->jsonDecode($paramsJson);
+                $request = $this->_objectManager->get(\Magento\Framework\Json\Helper\Data::class)
+                    ->jsonDecode($paramsJson);
                 if (is_array($request)) {
                     $optionsBlock = $this->_view->getLayout()->getBlock('wysiwyg_widget.options');
                     if (isset($request['widget_type'])) {
@@ -45,7 +46,7 @@ public function execute()
         } catch (\Magento\Framework\Exception\LocalizedException $e) {
             $result = ['error' => true, 'message' => $e->getMessage()];
             $this->getResponse()->representJson(
-                $this->_objectManager->get('Magento\Framework\Json\Helper\Data')->jsonEncode($result)
+                $this->_objectManager->get(\Magento\Framework\Json\Helper\Data::class)->jsonEncode($result)
             );
         }
     }
@@ -57,7 +58,7 @@ public function execute()
     private function getConditionsHelper()
     {
         if (!$this->conditionsHelper) {
-            $this->conditionsHelper = ObjectManager::getInstance()->get('\Magento\Widget\Helper\Conditions');
+            $this->conditionsHelper = ObjectManager::getInstance()->get(\Magento\Widget\Helper\Conditions::class);
         }
 
         return $this->conditionsHelper;

app/code/Magento/Widget/Model/Widget.php

@@ -314,7 +314,7 @@ public function getWidgetDeclaration($type, $params = [], $asIs = true)
                 }
             }
             if ($value) {
-                $directive .= sprintf(' %s="%s"', $name, htmlspecialchars($value));
+                $directive .= sprintf(' %s="%s"', $name, $this->escaper->escapeQuote($value));
             }
         }
 

app/code/Magento/Widget/Test/Unit/Model/WidgetTest.php

@@ -15,6 +15,11 @@ class WidgetTest extends \PHPUnit_Framework_TestCase
      */
     protected $dataStorageMock;
 
+    /**
+     * @var \Magento\Framework\Escaper|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $escaperMock;
+
     /**
      * @var \Magento\Widget\Model\Widget
      */
@@ -27,17 +32,24 @@ class WidgetTest extends \PHPUnit_Framework_TestCase
 
     protected function setUp()
     {
-        $this->dataStorageMock = $this->getMockBuilder('Magento\Widget\Model\Config\Data')
+        $this->dataStorageMock = $this->getMockBuilder(\Magento\Widget\Model\Config\Data::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->conditionsHelper = $this->getMockBuilder('\Magento\Widget\Helper\Conditions')
+        $this->conditionsHelper = $this->getMockBuilder(\Magento\Widget\Helper\Conditions::class)
             ->setMethods(['encode'])
             ->disableOriginalConstructor()
             ->getMock();
+        $this->escaperMock = $this->getMockBuilder(\Magento\Framework\Escaper::class)
+            ->disableOriginalConstructor()
+            ->getMock();
         $objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
         $this->widget = $objectManagerHelper->getObject(
-            'Magento\Widget\Model\Widget',
-            ['dataStorage' => $this->dataStorageMock, 'conditionsHelper' => $this->conditionsHelper]
+            \Magento\Widget\Model\Widget::class,
+            [
+                'dataStorage' => $this->dataStorageMock,
+                'conditionsHelper' => $this->conditionsHelper,
+                'escaper' => $this->escaperMock,
+            ]
         );
     }
 
@@ -160,6 +172,17 @@ public function testGetWidgetDeclaration()
 
         $this->conditionsHelper->expects($this->once())->method('encode')->with($conditions)
             ->willReturn('encoded-conditions-string');
+        $this->escaperMock->expects($this->atLeastOnce())
+            ->method('escapeQuote')
+            ->willReturnMap([
+                ['my "widget"', false, 'my "widget"'],
+                ['1', false, '1'],
+                ['5', false, '5'],
+                ['10', false, '10'],
+                ['product/widget/content/grid.phtml', false, 'product/widget/content/grid.phtml'],
+                ['encoded-conditions-string', false, 'encoded-conditions-string'],
+            ]);
+
         $result = $this->widget->getWidgetDeclaration('Magento\CatalogWidget\Block\Product\ProductsList', $params);
         $this->assertContains('{{widget type="Magento\CatalogWidget\Block\Product\ProductsList"', $result);
         $this->assertContains('title="my "widget""', $result);