diff -Nuar a/vendor/magento/module-sales/Helper/Admin.php b/vendor/magento/module-sales/Helper/Admin.php --- a/vendor/magento/module-sales/Helper/Admin.php +++ b/vendor/magento/module-sales/Helper/Admin.php @@ -7,6 +7,8 @@ namespace Magento\Sales\Helper; +use Magento\Framework\App\ObjectManager; + /** * Sales admin helper. */ @@ -32,24 +34,33 @@ */ protected $escaper; + /** + * @var \DOMDocumentFactory + */ + private $domDocumentFactory; + /** * @param \Magento\Framework\App\Helper\Context $context * @param \Magento\Store\Model\StoreManagerInterface $storeManager * @param \Magento\Sales\Model\Config $salesConfig * @param \Magento\Framework\Pricing\PriceCurrencyInterface $priceCurrency * @param \Magento\Framework\Escaper $escaper + * @param \DOMDocumentFactory|null $domDocumentFactory */ public function __construct( \Magento\Framework\App\Helper\Context $context, \Magento\Store\Model\StoreManagerInterface $storeManager, \Magento\Sales\Model\Config $salesConfig, \Magento\Framework\Pricing\PriceCurrencyInterface $priceCurrency, - \Magento\Framework\Escaper $escaper + \Magento\Framework\Escaper $escaper, + \DOMDocumentFactory $domDocumentFactory = null ) { $this->priceCurrency = $priceCurrency; $this->_storeManager = $storeManager; $this->_salesConfig = $salesConfig; $this->escaper = $escaper; + $this->domDocumentFactory = $domDocumentFactory + ?: ObjectManager::getInstance()->get(\DOMDocumentFactory::class); parent::__construct($context); } @@ -150,30 +161,41 @@ public function escapeHtmlWithLinks($data, $allowedTags = null) { if (!empty($data) && is_array($allowedTags) && in_array('a', $allowedTags)) { - $links = []; - $i = 1; - $data = str_replace('%', '%%', $data); - $regexp = "#(?J).*?)\\1\s*)|(?:\S+\s*=\s*(['\"])(.*?)\\3)\s*)*)|>)" - .">?(?:(?:(?.*?)(?:<\/a\s*>?|(?=<\w))|(?.*)))#si"; - while (preg_match($regexp, $data, $matches)) { - $text = ''; - if (!empty($matches['text'])) { - $text = str_replace('%%', '%', $matches['text']); - } - $url = $this->filterUrl($matches['link'] ?? ''); - //Recreate a minimalistic secure a tag - $links[] = sprintf( - '%s', - htmlspecialchars($url, ENT_QUOTES, 'UTF-8', false), - $this->escaper->escapeHtml($text) - ); - $data = str_replace($matches[0], '%' . $i . '$s', $data); - ++$i; + $wrapperElementId = uniqid(); + $domDocument = $this->domDocumentFactory->create(); + + $internalErrors = libxml_use_internal_errors(true); + + $domDocument->loadHTML( + '' . $data . '' + ); + + libxml_use_internal_errors($internalErrors); + + $linkTags = $domDocument->getElementsByTagName('a'); + + foreach ($linkTags as $linkNode) { + $linkAttributes = []; + foreach ($linkNode->attributes as $attribute) { + $linkAttributes[$attribute->name] = $attribute->value; + } + + foreach ($linkAttributes as $attributeName => $attributeValue) { + if ($attributeName === 'href') { + $url = $this->filterUrl($attributeValue ?? ''); + $url = $this->escaper->escapeUrl($url); + $linkNode->setAttribute('href', $url); + } else { + $linkNode->removeAttribute($attributeName); + } + } } - $data = $this->escaper->escapeHtml($data, $allowedTags); - return vsprintf($data, $links); + + $result = mb_convert_encoding($domDocument->saveHTML(), 'UTF-8', 'HTML-ENTITIES'); + preg_match('/(.+)<\/body><\/html>$/si', $result, $matches); + $data = !empty($matches) ? $matches[1] : ''; } + return $this->escaper->escapeHtml($data, $allowedTags); } @@ -187,7 +209,7 @@ { if ($url) { //Revert the sprintf escaping - $url = str_replace('%%', '%', $url); + // phpcs:ignore Magento2.Functions.DiscouragedFunction $urlScheme = parse_url($url, PHP_URL_SCHEME); $urlScheme = $urlScheme ? strtolower($urlScheme) : ''; if ($urlScheme !== 'http' && $urlScheme !== 'https') { diff -Nuar a/vendor/magento/module-sales/Test/Unit/Helper/AdminTest.php b/vendor/magento/module-sales/Test/Unit/Helper/AdminTest.php --- a/vendor/magento/module-sales/Test/Unit/Helper/AdminTest.php +++ b/vendor/magento/module-sales/Test/Unit/Helper/AdminTest.php @@ -71,7 +71,7 @@ ->disableOriginalConstructor() ->getMock(); - $this->adminHelper = (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))->getObject( + $this->adminHelper = (new ObjectManager($this))->getObject( \Magento\Sales\Helper\Admin::class, [ 'context' => $this->contextMock, @@ -330,72 +330,16 @@ } /** - * @param string $data - * @param string $expected - * @param null|array $allowedTags - * @dataProvider escapeHtmlWithLinksDataProvider + * @return void */ - public function testEscapeHtmlWithLinks($data, $expected, $allowedTags = null) + public function testEscapeHtmlWithLinks(): void { + $expected = '<a>some text in tags</a>'; $this->escaperMock ->expects($this->any()) ->method('escapeHtml') ->will($this->returnValue($expected)); - $actual = $this->adminHelper->escapeHtmlWithLinks($data, $allowedTags); + $actual = $this->adminHelper->escapeHtmlWithLinks('some text in tags'); $this->assertEquals($expected, $actual); } - - /** - * @return array - */ - public function escapeHtmlWithLinksDataProvider() - { - return [ - [ - 'some text in tags', - '<a>some text in tags</a>', - 'allowedTags' => null - ], - [ - 'Transaction ID: "XX123XX"', - 'Transaction ID: "XX123XX"', - 'allowedTags' => ['b', 'br', 'strong', 'i', 'u', 'a'] - ], - [ - 'some text in tags', - 'some text in tags', - 'allowedTags' => ['a'] - ], - 'Not replacement with placeholders' => [ - "", - '<script>alert(1)</script>', - 'allowedTags' => ['a'] - ], - 'Normal usage, url escaped' => [ - 'Foo', - 'Foo', - 'allowedTags' => ['a'] - ], - 'Normal usage, url not escaped' => [ - "Foo", - 'Foo', - 'allowedTags' => ['a'] - ], - 'XSS test' => [ - "Foo", - 'Foo', - 'allowedTags' => ['a'] - ], - 'Additional regex test' => [ - "Foo", - 'Foo', - 'allowedTags' => ['a'] - ], - 'Break of valid urls' => [ - "Foo", - 'Foo', - 'allowedTags' => ['a'] - ], - ]; - } }