From 4e73e600ae30c85a3853b6ab12d6e667d0566fd8 Mon Sep 17 00:00:00 2001 From: Sven Reichel Date: Fri, 6 Sep 2024 17:30:42 +0200 Subject: [PATCH] Added method to make use of `Mage_Core_Model_Security_HtmlEscapedString` easier (#4123) * Rector: CQ - UnusedForeachValueToArrayKeysRector (#1) * Rector: CQ - UnusedForeachValueToArrayKeysRector See Rector\CodeQuality\Rector\Foreach_\UnusedForeachValueToArrayKeysRector * fixes + phpstan See fix at rector: https://github.com/rectorphp/rector-src/pull/6164 * Revert "Rector: CQ - UnusedForeachValueToArrayKeysRector (#1)" This reverts commit 3d7eaf63f2211a9e1a8defe8c29e8f6da889ab2e. * 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 * Renamed methods * Reverted renaming, updated docblocks --------- Co-authored-by: Ng Kiat Siong --- app/Mage.php | 2 +- .../Block/Sales/Order/Comments/View.php | 6 +-- .../Block/Sales/Order/View/History.php | 6 +-- app/code/core/Mage/Adminhtml/Helper/Sales.php | 6 +-- app/code/core/Mage/Core/Block/Abstract.php | 45 +++++++++++++++++-- app/code/core/Mage/Core/Helper/Abstract.php | 19 ++++---- app/code/core/Mage/Core/Model/Layout.php | 2 +- .../Core/Model/Security/HtmlEscapedString.php | 37 ++++++++++++++- app/code/core/Mage/Page/Block/Html/Header.php | 16 ++----- .../core/Mage/Page/Block/Html/Welcome.php | 4 +- 10 files changed, 102 insertions(+), 41 deletions(-) diff --git a/app/Mage.php b/app/Mage.php index e52278f171b..29a0ce52bcb 100644 --- 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 index 42536309125..6e43ea89528 100644 --- 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 index 372a968706e..cadd04f47d8 100644 --- 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 index 87a2ea7da46..8e6f84457df 100644 --- 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 index a164fea34fc..39f49084c8e 100644 --- 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 index fc0580855c7..81a2d5832bd 100644 --- 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 index 33243ff947f..0c645edf50c 100644 --- 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 index dd3c3f47632..afda0b6e9a0 100644 --- 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 index dab05f44c1f..1b251001f59 100644 --- 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 index 2f1e1f98238..b8d4308488e 100644 --- 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')); } }