Added method to make use of `Mage_Core_Model_Security_HtmlEscapedStri…

…ng` easier (OpenMage#4123) * Rector: CQ - UnusedForeachValueToArrayKeysRector (#1) * Rector: CQ - UnusedForeachValueToArrayKeysRector See Rector\CodeQuality\Rector\Foreach_\UnusedForeachValueToArrayKeysRector * fixes + phpstan See fix at rector: rectorphp/rector-src#6164 * Revert "Rector: CQ - UnusedForeachValueToArrayKeysRector (#1)" This reverts commit 3d7eaf6. * Updates for 20.10.1 release * Re-add possibility to get original value * Changed default value * Moved method to Mage_Core_Block_Abstract * Ignore some phpcs-ecg errors [skip ci] * Added method to work with arrays * Added method to work with arrays (2) * Typo [skip ci] * Update app/code/core/Mage/Core/Model/Security/HtmlEscapedString.php Co-authored-by: Ng Kiat Siong <kiatsiong.ng@gmail.com> * Renamed methods * Reverted renaming, updated docblocks --------- Co-authored-by: Ng Kiat Siong <kiatsiong.ng@gmail.com>
kiatng · Sep 6, 2024 · 4e73e60 · 4e73e60
1 parent 5fd5e04
commit 4e73e60
Show file tree

Hide file tree

Showing 10 changed files with 102 additions and 41 deletions.
diff --git a/app/Mage.php b/app/Mage.php
@@ -433,7 +433,7 @@ public static function getStoreConfigAsInt(string $path, $store = null): int
      * Retrieve config flag for store by path
      *
      * @param string $path
-     * @param mixed $store
+     * @param null|string|bool|int|Mage_Core_Model_Store $store
      * @return bool
      */
     public static function getStoreConfigFlag($path, $store = null)

diff --git a/app/code/core/Mage/Adminhtml/Block/Sales/Order/Comments/View.php b/app/code/core/Mage/Adminhtml/Block/Sales/Order/Comments/View.php
@@ -77,9 +77,9 @@ public function canSendCommentEmail()
     /**
      * Replace links in string
      *
-     * @param array|string $data
-     * @param null|array $allowedTags
-     * @return string
+     * @param string|string[] $data
+     * @param array|null $allowedTags
+     * @return null|string|string[]
      */
     public function escapeHtml($data, $allowedTags = null)
     {

diff --git a/app/code/core/Mage/Adminhtml/Block/Sales/Order/View/History.php b/app/code/core/Mage/Adminhtml/Block/Sales/Order/View/History.php
@@ -80,9 +80,9 @@ public function isCustomerNotificationNotApplicable(Mage_Sales_Model_Order_Statu
     /**
      * Replace links in string
      *
-     * @param array|string $data
-     * @param null|array $allowedTags
-     * @return string
+     * @param string|string[] $data
+     * @param array|null $allowedTags
+     * @return null|string|string[]
      */
     public function escapeHtml($data, $allowedTags = null)
     {

diff --git a/app/code/core/Mage/Adminhtml/Helper/Sales.php b/app/code/core/Mage/Adminhtml/Helper/Sales.php
@@ -109,9 +109,9 @@ public function applySalableProductTypesFilter($collection)
     /**
      * Escape string preserving links
      *
-     * @param array|string $data
-     * @param null|array $allowedTags
-     * @return string
+     * @param string|string[] $data
+     * @param array|null $allowedTags
+     * @return null|string|string[]
      */
     public function escapeHtmlWithLinks($data, $allowedTags = null)
     {

diff --git a/app/code/core/Mage/Core/Block/Abstract.php b/app/code/core/Mage/Core/Block/Abstract.php
@@ -165,6 +165,7 @@ abstract class Mage_Core_Block_Abstract extends Varien_Object
     /**
      * @var Varien_Object
      */
+    // phpcs:ignore Ecg.PHP.PrivateClassMember.PrivateClassMemberError
     private static $_transportObject;
 
     /**
@@ -524,6 +525,7 @@ public function unsetCallChild($alias, $callback, $result, $params)
             }
 
             Mage::helper('core/security')->validateAgainstBlockMethodBlacklist($child, $callback, $params);
+            // phpcs:ignore Ecg.Security.ForbiddenFunction.Found
             if ($result == call_user_func_array([&$child, $callback], $params)) {
                 $this->unsetChild($alias);
             }
@@ -863,7 +865,7 @@ public function getChildGroup($groupName, $callback = null, $skipEmptyResults =
      *
      * @param string $alias
      * @param string $key
-     * @return mixed
+     * @return mixed|void
      */
     public function getChildData($alias, $key = '')
     {
@@ -1167,6 +1169,7 @@ public function getModuleName()
     public function __()
     {
         $args = func_get_args();
+        // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
         $expr = new Mage_Core_Model_Translate_Expr(array_shift($args), $this->getModuleName());
         array_unshift($args, $expr);
         return $this->_getApp()->getTranslator()->translate($args);
@@ -1187,15 +1190,49 @@ public function htmlEscape($data, $allowedTags = null)
     /**
      * Escape html entities
      *
-     * @param   string|array $data
-     * @param   array $allowedTags
-     * @return  string
+     * @param string|string[] $data
+     * @param array|null $allowedTags
+     * @return null|string|string[]
      */
     public function escapeHtml($data, $allowedTags = null)
     {
         return $this->helper('core')->escapeHtml($data, $allowedTags);
     }
 
+    /**
+     * Wrapper for escapeHtml() function with keeping original value
+     *
+     * @param string $data
+     * @param string[]|null $allowedTags
+     * @return Mage_Core_Model_Security_HtmlEscapedString
+     *
+     * @see Mage_Core_Model_Security_HtmlEscapedString::getUnescapedValue()
+     */
+    public function escapeHtmlAsObject(string $data, ?array $allowedTags = null): Mage_Core_Model_Security_HtmlEscapedString
+    {
+        // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
+        return new Mage_Core_Model_Security_HtmlEscapedString($data, $allowedTags);
+    }
+
+    /**
+     * Wrapper for escapeHtml() function with keeping original value
+     *
+     * @param string[] $data
+     * @param string[]|null $allowedTags
+     * @return Mage_Core_Model_Security_HtmlEscapedString[]
+     *
+     *  @see Mage_Core_Model_Security_HtmlEscapedString::getUnescapedValue()
+     */
+    public function escapeHtmlArrayAsObject(array $data, ?array $allowedTags = null): array
+    {
+        $result = [];
+        foreach ($data as $key => $string) {
+            $result[$key] = $this->escapeHtmlAsObject($string, $allowedTags);
+        }
+
+        return $result;
+    }
+
     /**
      * Wrapper for standard strip_tags() function with extra functionality for html entities
      *

diff --git a/app/code/core/Mage/Core/Helper/Abstract.php b/app/code/core/Mage/Core/Helper/Abstract.php
@@ -178,9 +178,10 @@ public function __()
     }
 
     /**
-     * @param array $data
-     * @param array $allowedTags
-     * @return mixed
+     * @param string|string[] $data
+     * @param array|null $allowedTags
+     * @return null|string|string[]
+     *
      * @see self::escapeHtml()
      * @deprecated after 1.4.0.0-rc1
      */
@@ -192,9 +193,9 @@ public function htmlEscape($data, $allowedTags = null)
     /**
      * Escape html entities
      *
-     * @param   string|array $data
-     * @param   array $allowedTags
-     * @return  mixed
+     * @param string|string[] $data
+     * @param array|null $allowedTags
+     * @return null|string|string[]
      */
     public function escapeHtml($data, $allowedTags = null)
     {
@@ -244,7 +245,7 @@ function ($matches) {
      * Wrapper for standard strip_tags() function with extra functionality for html entities
      *
      * @param string $data
-     * @param string $allowableTags
+     * @param null|string|string[] $allowableTags
      * @param bool $escape
      * @return string
      */
@@ -320,9 +321,9 @@ public function escapeScriptIdentifiers($data)
     /**
      * Escape quotes in java script
      *
-     * @param mixed $data
+     * @param string|string[] $data
      * @param string $quote
-     * @return mixed
+     * @return string|string[]
      */
     public function jsQuoteEscape($data, $quote = '\'')
     {

diff --git a/app/code/core/Mage/Core/Model/Layout.php b/app/code/core/Mage/Core/Model/Layout.php
@@ -419,7 +419,7 @@ protected function _translateLayoutNode($node, &$args)
      * Save block in blocks registry
      *
      * @param string $name
-     * @param Mage_Core_Model_Layout $block
+     * @param Mage_Core_Block_Abstract $block
      * @return $this
      */
     public function setBlock($name, $block)

diff --git a/app/code/core/Mage/Core/Model/Security/HtmlEscapedString.php b/app/code/core/Mage/Core/Model/Security/HtmlEscapedString.php
@@ -3,12 +3,35 @@
 declare(strict_types=1);
 
 /**
+ * OpenMage
  *
+ * This source file is subject to the Open Software License (OSL 3.0)
+ * that is bundled with this package in the file LICENSE.txt.
+ * It is also available at https://opensource.org/license/osl-3-0-php
+ *
+ * @category   Mage
+ * @package    Mage_Core
+ * @copyright  Copyright (c) 2024 The OpenMage Contributors (https://www.openmage.org)
+ * @license    https://opensource.org/licenses/osl-3.0.php  Open Software License (OSL 3.0)
+ */
+
+/**
+ * Wrapper to escape a string value with a method to get the original string value
+ *
+ * @category   Mage
+ * @package    Mage_Core
  */
 class Mage_Core_Model_Security_HtmlEscapedString implements Stringable
 {
-    protected $originalValue;
-    protected $allowedTags;
+    /**
+     * @var string
+     */
+    protected string $originalValue;
+
+    /**
+     * @var string[]|null
+     */
+    protected ?array $allowedTags;
 
     /**
      * @param string $originalValue
@@ -20,6 +43,11 @@ public function __construct(string $originalValue, ?array $allowedTags = null)
         $this->allowedTags = $allowedTags;
     }
 
+    /**
+     * Get escaped html entities
+     *
+     * @return string
+     */
     public function __toString(): string
     {
         return (string) Mage::helper('core')->escapeHtml(
@@ -28,6 +56,11 @@ public function __toString(): string
         );
     }
 
+    /**
+     * Get un-escaped html entities
+     *
+     * @return string
+     */
     public function getUnescapedValue(): string
     {
         return $this->originalValue;

diff --git a/app/code/core/Mage/Page/Block/Html/Header.php b/app/code/core/Mage/Page/Block/Html/Header.php
@@ -57,9 +57,7 @@ public function setLogo($logo_src, $logo_alt)
     public function getLogoSrc()
     {
         if (empty($this->_data['logo_src'])) {
-            $this->_data['logo_src'] =  new Mage_Core_Model_Security_HtmlEscapedString(
-                (string) Mage::getStoreConfig('design/header/logo_src')
-            );
+            $this->_data['logo_src'] = $this->escapeHtmlAsObject((string) Mage::getStoreConfig('design/header/logo_src'));
         }
         return $this->getSkinUrl($this->_data['logo_src']);
     }
@@ -70,9 +68,7 @@ public function getLogoSrc()
     public function getLogoSrcSmall()
     {
         if (empty($this->_data['logo_src_small'])) {
-            $this->_data['logo_src_small'] =  new Mage_Core_Model_Security_HtmlEscapedString(
-                (string) Mage::getStoreConfig('design/header/logo_src_small')
-            );
+            $this->_data['logo_src_small'] = $this->escapeHtmlAsObject((string) Mage::getStoreConfig('design/header/logo_src_small'));
         }
         return $this->getSkinUrl($this->_data['logo_src_small']);
     }
@@ -83,9 +79,7 @@ public function getLogoSrcSmall()
     public function getLogoAlt()
     {
         if (empty($this->_data['logo_alt'])) {
-            $this->_data['logo_alt'] = new Mage_Core_Model_Security_HtmlEscapedString(
-                (string) Mage::getStoreConfig('design/header/logo_alt')
-            );
+            $this->_data['logo_alt'] = $this->escapeHtmlAsObject((string) Mage::getStoreConfig('design/header/logo_alt'));
         }
         return $this->_data['logo_alt'];
     }
@@ -103,9 +97,7 @@ public function getWelcome()
             if (Mage::isInstalled() && Mage::getSingleton('customer/session')->isLoggedIn()) {
                 $this->_data['welcome'] = $this->__('Welcome, %s!', $this->escapeHtml(Mage::getSingleton('customer/session')->getCustomer()->getName()));
             } else {
-                $this->_data['welcome'] = new Mage_Core_Model_Security_HtmlEscapedString(
-                    (string) Mage::getStoreConfig('design/header/welcome')
-                );
+                $this->_data['welcome'] = $this->escapeHtmlAsObject((string) Mage::getStoreConfig('design/header/welcome'));
             }
         }
 

diff --git a/app/code/core/Mage/Page/Block/Html/Welcome.php b/app/code/core/Mage/Page/Block/Html/Welcome.php
@@ -44,9 +44,7 @@ protected function _toHtml()
             if (Mage::isInstalled() && $this->_getSession()->isLoggedIn()) {
                 $this->_data['welcome'] = $this->__('Welcome, %s!', $this->escapeHtml($this->_getSession()->getCustomer()->getName()));
             } else {
-                $this->_data['welcome'] = new Mage_Core_Model_Security_HtmlEscapedString(
-                    (string) Mage::getStoreConfig('design/header/welcome')
-                );
+                $this->_data['welcome'] = $this->escapeHtmlAsObject((string) Mage::getStoreConfig('design/header/welcome'));
             }
         }