ENGCOM-8132: Unable to insert widget with text value which contains }} string #29006

gabrieldagama · web-flow · commit 1f51e88232b9 · 2020-09-10T16:41:09.000+01:00
diff --git a/app/code/Magento/CatalogWidget/Test/Mftf/ActionGroup/AdminFillCatalogProductsListWidgetTitleActionGroup.xml b/app/code/Magento/CatalogWidget/Test/Mftf/ActionGroup/AdminFillCatalogProductsListWidgetTitleActionGroup.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ /**
+  * Copyright © Magento, Inc. All rights reserved.
+  * See COPYING.txt for license details.
+  */
+-->
+
+<actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+              xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
+    <actionGroup name="AdminFillCatalogProductsListWidgetTitleActionGroup">
+        <annotations>
+            <description>Fill catalog products list title field.</description>
+        </annotations>
+
+        <arguments>
+            <argument name="title" type="string" defaultValue=""/>
+        </arguments>
+        <waitForElementVisible selector="{{InsertWidgetSection.title}}" stepKey="waitForField"/>
+        <fillField selector="{{InsertWidgetSection.title}}" userInput="{{title}}" stepKey="fillTitleField"/>
+    </actionGroup>
+</actionGroups>
diff --git a/app/code/Magento/CatalogWidget/Test/Mftf/ActionGroup/StorefrontAssertWidgetTitleActionGroup.xml b/app/code/Magento/CatalogWidget/Test/Mftf/ActionGroup/StorefrontAssertWidgetTitleActionGroup.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ /**
+  * Copyright © Magento, Inc. All rights reserved.
+  * See COPYING.txt for license details.
+  */
+-->
+
+<actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+              xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
+    <actionGroup name="StorefrontAssertWidgetTitleActionGroup">
+        <annotations>
+            <description>Assert widget title on storefront.</description>
+        </annotations>
+        <arguments>
+            <argument name="title" type="string"/>
+        </arguments>
+
+        <grabTextFrom selector="{{StorefrontWidgetsSection.widgetProductsGrid}} {{StorefrontWidgetsSection.widgetTitle}}"
+                      stepKey="grabWidgetTitle"/>
+        <assertEquals stepKey="assertWidgetTitle">
+            <actualResult type="string">$grabWidgetTitle</actualResult>
+            <expectedResult type="string">{{title}}</expectedResult>
+        </assertEquals>
+    </actionGroup>
+</actionGroups>
diff --git a/app/code/Magento/CatalogWidget/Test/Mftf/Section/CatalogWidgetSection/InsertWidgetSection.xml b/app/code/Magento/CatalogWidget/Test/Mftf/Section/CatalogWidgetSection/InsertWidgetSection.xml
@@ -19,5 +19,6 @@
         <element name="checkElementStorefrontByPrice" type="button" selector="//*[@class='product-items widget-product-grid']//*[contains(text(),'${{arg4}}.00')]" parameterized="true"/>
         <element name="checkElementStorefrontByName" type="button" selector="//*[@class='product-items widget-product-grid']//*[@class='product-item'][{{productPosition}}]//a[contains(text(), '{{productName}}')]" parameterized="true"/>
         <element name="categoryTreeWrapper" type="text" selector=".rule-chooser .tree.x-tree"/>
+        <element name="title" type="text" selector="input[name='parameters[title]']"/>
     </section>
 </sections>
diff --git a/app/code/Magento/Cms/Test/Mftf/Test/StoreFrontWidgetTitleWithReservedCharsTest.xml b/app/code/Magento/Cms/Test/Mftf/Test/StoreFrontWidgetTitleWithReservedCharsTest.xml
@@ -0,0 +1,53 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ /**
+  * Copyright © Magento, Inc. All rights reserved.
+  * See COPYING.txt for license details.
+  */
+-->
+<tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/testSchema.xsd">
+    <test name="StoreFrontWidgetTitleWithReservedCharsTest">
+        <annotations>
+            <features value="Cms"/>
+            <stories value="Create a CMS Page via the Admin when widget title contains reserved chairs"/>
+            <title value="Create CMS Page via the Admin when widget title contains reserved chairs"/>
+            <description value="See CMS Page title on store front page if titled widget with reserved chairs added"/>
+            <severity value="MAJOR"/>
+            <testCaseId value="MC-37419"/>
+            <group value="Cms"/>
+            <group value="WYSIWYGDisabled"/>
+        </annotations>
+        <before>
+            <actionGroup ref="AdminLoginActionGroup" stepKey="loginAsAdmin"/>
+            <createData entity="simpleProductWithoutCategory" stepKey="createSimpleProductWithoutCategory"/>
+            <createData entity="_defaultCmsPage" stepKey="createCmsPage"/>
+        </before>
+        <after>
+            <deleteData createDataKey="createSimpleProductWithoutCategory" stepKey="deleteProduct"/>
+            <deleteData createDataKey="createCmsPage" stepKey="deleteCmsPage" />
+            <actionGroup ref="AdminLogoutActionGroup" stepKey="logout"/>
+        </after>
+        <!--Navigate to Page in Admin-->
+        <actionGroup ref="NavigateToCreatedCMSPageActionGroup" stepKey="navigateToCreatedCMSPage">
+            <argument name="CMSPage" value="$createCmsPage$"/>
+        </actionGroup>
+        <!--Insert widget-->
+        <actionGroup ref="AdminInsertWidgetToCmsPageContentActionGroup" stepKey="insertWidgetToCmsPageContent">
+            <argument name="widgetType" value="Catalog Products List"/>
+        </actionGroup>
+        <!--Fill widget title and save-->
+        <actionGroup ref="AdminFillCatalogProductsListWidgetTitleActionGroup" stepKey="fillWidgetTitle">
+            <argument name="title" value="Tittle }}"/>
+        </actionGroup>
+        <actionGroup ref="AdminClickInsertWidgetActionGroup" stepKey="clickInsertWidgetButton"/>
+        <actionGroup ref="SaveCmsPageActionGroup" stepKey="saveOpenedPage"/>
+        <!--Verify data on frontend-->
+        <actionGroup ref="StorefrontGoToCMSPageActionGroup" stepKey="navigateToPageOnStorefront">
+            <argument name="identifier" value="$createCmsPage.identifier$"/>
+        </actionGroup>
+        <actionGroup ref="StorefrontAssertWidgetTitleActionGroup" stepKey="verifyPageDataOnFrontend">
+            <argument name="title" value="Tittle }}"/>
+        </actionGroup>
+    </test>
+</tests>
diff --git a/app/code/Magento/Widget/Model/Widget.php b/app/code/Magento/Widget/Model/Widget.php
@@ -5,6 +5,16 @@
  */
 namespace Magento\Widget\Model;
 
+use Magento\Framework\App\Cache\Type\Config;
+use Magento\Framework\DataObject;
+use Magento\Framework\Escaper;
+use Magento\Framework\Math\Random;
+use Magento\Framework\View\Asset\Repository;
+use Magento\Framework\View\Asset\Source;
+use Magento\Framework\View\FileSystem;
+use Magento\Widget\Helper\Conditions;
+use Magento\Widget\Model\Config\Data;
+
 /**
  * Widget model for different purposes
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -15,32 +25,32 @@
 class Widget
 {
     /**
-     * @var \Magento\Widget\Model\Config\Data
+     * @var Data
      */
     protected $dataStorage;
 
     /**
-     * @var \Magento\Framework\App\Cache\Type\Config
+     * @var Config
      */
     protected $configCacheType;
 
     /**
-     * @var \Magento\Framework\View\Asset\Repository
+     * @var Repository
      */
     protected $assetRepo;
 
     /**
-     * @var \Magento\Framework\View\Asset\Source
+     * @var Source
      */
     protected $assetSource;
 
     /**
-     * @var \Magento\Framework\View\FileSystem
+     * @var FileSystem
      */
     protected $viewFileSystem;
 
     /**
-     * @var \Magento\Framework\Escaper
+     * @var Escaper
      */
     protected $escaper;
 
@@ -50,30 +60,35 @@ class Widget
     protected $widgetsArray = [];
 
     /**
-     * @var \Magento\Widget\Helper\Conditions
+     * @var Conditions
      */
     protected $conditionsHelper;
 
     /**
-     * @var \Magento\Framework\Math\Random
+     * @var Random
      */
     private $mathRandom;
 
     /**
-     * @param \Magento\Framework\Escaper $escaper
-     * @param \Magento\Widget\Model\Config\Data $dataStorage
-     * @param \Magento\Framework\View\Asset\Repository $assetRepo
-     * @param \Magento\Framework\View\Asset\Source $assetSource
-     * @param \Magento\Framework\View\FileSystem $viewFileSystem
-     * @param \Magento\Widget\Helper\Conditions $conditionsHelper
+     * @var string[]
+     */
+    private $reservedChars = ['}', '{'];
+
+    /**
+     * @param Escaper $escaper
+     * @param Data $dataStorage
+     * @param Repository $assetRepo
+     * @param Source $assetSource
+     * @param FileSystem $viewFileSystem
+     * @param Conditions $conditionsHelper
      */
     public function __construct(
-        \Magento\Framework\Escaper $escaper,
-        \Magento\Widget\Model\Config\Data $dataStorage,
-        \Magento\Framework\View\Asset\Repository $assetRepo,
-        \Magento\Framework\View\Asset\Source $assetSource,
-        \Magento\Framework\View\FileSystem $viewFileSystem,
-        \Magento\Widget\Helper\Conditions $conditionsHelper
+        Escaper $escaper,
+        Data $dataStorage,
+        Repository $assetRepo,
+        Source $assetSource,
+        FileSystem $viewFileSystem,
+        Conditions $conditionsHelper
     ) {
         $this->escaper = $escaper;
         $this->dataStorage = $dataStorage;
@@ -110,14 +125,11 @@ public function getWidgetByClassType($type)
         $widgets = $this->getWidgets();
         /** @var array $widget */
         foreach ($widgets as $widget) {
-            if (isset($widget['@'])) {
-                if (isset($widget['@']['type'])) {
-                    if ($type === $widget['@']['type']) {
-                        return $widget;
-                    }
-                }
+            if (isset($widget['@']['type']) && $type === $widget['@']['type']) {
+                return $widget;
             }
         }
+
         return null;
     }
 
@@ -131,6 +143,7 @@ public function getWidgetByClassType($type)
      */
     public function getConfigAsXml($type)
     {
+        // phpstan:ignore
         return $this->getXmlElementByType($type);
     }
 
@@ -296,42 +309,70 @@ public function getWidgetsArray($filters = [])
      */
     public function getWidgetDeclaration($type, $params = [], $asIs = true)
     {
-        $directive = '{{widget type="' . $type . '"';
         $widget = $this->getConfigAsObject($type);
 
+        $params = array_filter($params, function ($value) {
+            return $value !== null && $value !== '';
+        });
+
+        $directiveParams = '';
         foreach ($params as $name => $value) {
             // Retrieve default option value if pre-configured
-            if ($name == 'conditions') {
-                $name = 'conditions_encoded';
-                $value = $this->conditionsHelper->encode($value);
-            } elseif (is_array($value)) {
-                $value = implode(',', $value);
-            } elseif (trim($value) == '') {
-                $parameters = $widget->getParameters();
-                if (isset($parameters[$name]) && is_object($parameters[$name])) {
-                    $value = $parameters[$name]->getValue();
-                }
-            }
-            if (isset($value)) {
-                $directive .= sprintf(' %s="%s"', $name, $this->escaper->escapeHtmlAttr($value, false));
-            }
+            $directiveParams .= $this->getDirectiveParam($widget, $name, $value);
         }
 
-        $directive .= $this->getWidgetPageVarName($params);
-
-        $directive .= '}}';
+        $directive = sprintf('{{widget type="%s"%s%s}}', $type, $directiveParams, $this->getWidgetPageVarName($params));
 
         if ($asIs) {
             return $directive;
         }
 
-        $html = sprintf(
+        return sprintf(
             '<img id="%s" src="%s" title="%s">',
             $this->idEncode($directive),
             $this->getPlaceholderImageUrl($type),
             $this->escaper->escapeUrl($directive)
         );
-        return $html;
+    }
+
+    /**
+     * Returns directive param with prepared value
+     *
+     * @param DataObject $widget
+     * @param string $name
+     * @param string|array $value
+     * @return string
+     */
+    private function getDirectiveParam(DataObject $widget, string $name, $value): string
+    {
+        if ($name === 'conditions') {
+            $name = 'conditions_encoded';
+            $value = $this->conditionsHelper->encode($value);
+        } elseif (is_array($value)) {
+            $value = implode(',', $value);
+        } elseif (trim($value) === '') {
+            $parameters = $widget->getParameters();
+            if (isset($parameters[$name]) && is_object($parameters[$name])) {
+                $value = $parameters[$name]->getValue();
+            }
+        } else {
+            $value = $this->getPreparedValue($value);
+        }
+
+        return sprintf(' %s="%s"', $name, $this->escaper->escapeHtmlAttr($value, false));
+    }
+
+    /**
+     * Returns encoded value if it contains reserved chars
+     *
+     * @param string $value
+     * @return string
+     */
+    private function getPreparedValue(string $value): string
+    {
+        $pattern = sprintf('/%s/', implode('|', $this->reservedChars));
+
+        return preg_match($pattern, $value) ? rawurlencode($value) : $value;
     }
 
     /**
