Skip to content

[Support] Page builder content is broken when html element contains script tags with html template #521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
omiroshnichenko merged 3 commits into from
Jun 11, 2020
Merged

[Support] Page builder content is broken when html element contains script tags with html template #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1369bad
MC-34846: [Page Builder] “Order by SKU” Widget is displayed incorrect…
bubasuma Jun 5, 2020
161d3c4
MC-34846: [Page Builder] “Order by SKU” Widget is displayed incorrect…
bubasuma Jun 9, 2020
8c4d192
Merge branch '1.3-develop' into magento-mpi-MC-34846
omiroshnichenko Jun 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 98 additions & 33 deletions app/code/Magento/PageBuilder/Model/Filter/Template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,47 +7,65 @@

namespace Magento\PageBuilder\Model\Filter;

use DOMDocument;
use DOMElement;
use DOMException;
use DOMNode;
use DOMXPath;
use Exception;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Math\Random;
use Magento\Framework\Serialize\Serializer\Json;
use Magento\Framework\View\ConfigInterface;
use Magento\PageBuilder\Plugin\Filter\TemplatePlugin;
use Psr\Log\LoggerInterface;

/**
* Specific template filters for Page Builder content
*/
class Template
{
/**
* @var \Magento\Framework\View\ConfigInterface
* @var ConfigInterface
*/
private $viewConfig;

/**
* @var \Psr\Log\LoggerInterface
* @var LoggerInterface
*/
private $logger;

/**
* @var \DOMDocument
* @var DOMDocument
*/
private $domDocument;

/**
* @var \Magento\Framework\Math\Random
* @var Random
*/
private $mathRandom;

/**
* @var \Magento\Framework\Serialize\Serializer\Json
* @var Json
*/
private $json;

/**
* @param \Psr\Log\LoggerInterface $logger
* @param \Magento\Framework\View\ConfigInterface $viewConfig
* @param \Magento\Framework\Math\Random $mathRandom
* @param \Magento\Framework\Serialize\Serializer\Json $json
* @var array
*/
private $scripts;

/**
* @param LoggerInterface $logger
* @param ConfigInterface $viewConfig
* @param Random $mathRandom
* @param Json $json
*/
public function __construct(
\Psr\Log\LoggerInterface $logger,
\Magento\Framework\View\ConfigInterface $viewConfig,
\Magento\Framework\Math\Random $mathRandom,
\Magento\Framework\Serialize\Serializer\Json $json
LoggerInterface $logger,
ConfigInterface $viewConfig,
Random $mathRandom,
Json $json
) {
$this->logger = $logger;
$this->viewConfig = $viewConfig;
Expand All @@ -59,22 +77,22 @@ public function __construct(
* After filter of template data apply transformations
*
* @param string $result
*
* @return string
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function filter(string $result) : string
{
$this->domDocument = false;
$this->scripts = [];

// Validate if the filtered result requires background image processing
if (preg_match(\Magento\PageBuilder\Plugin\Filter\TemplatePlugin::BACKGROUND_IMAGE_PATTERN, $result)) {
if (preg_match(TemplatePlugin::BACKGROUND_IMAGE_PATTERN, $result)) {
$document = $this->getDomDocument($result);
$this->generateBackgroundImageStyles($document);
}

// Process any HTML content types, they need to be decoded on the front-end
if (preg_match(\Magento\PageBuilder\Plugin\Filter\TemplatePlugin::HTML_CONTENT_TYPE_PATTERN, $result)) {
if (preg_match(TemplatePlugin::HTML_CONTENT_TYPE_PATTERN, $result)) {
$document = $this->getDomDocument($result);
$uniqueNodeNameToDecodedOuterHtmlMap = $this->generateDecodedHtmlPlaceholderMappingInDocument($document);
}
Expand Down Expand Up @@ -112,6 +130,8 @@ function ($matches) {

$result = $docHtml;
}

$result = $this->unmaskScriptTags($result);
}

return $result;
Expand All @@ -122,9 +142,9 @@ function ($matches) {
*
* @param string $html
*
* @return \DOMDocument
* @return DOMDocument
*/
private function getDomDocument(string $html) : \DOMDocument
private function getDomDocument(string $html) : DOMDocument
{
if (!$this->domDocument) {
$this->domDocument = $this->createDomDocument($html);
Expand All @@ -138,14 +158,16 @@ private function getDomDocument(string $html) : \DOMDocument
*
* @param string $html
*
* @return \DOMDocument
* @return DOMDocument
*/
private function createDomDocument(string $html) : \DOMDocument
private function createDomDocument(string $html) : DOMDocument
{
$domDocument = new \DOMDocument('1.0', 'UTF-8');
$html = $this->maskScriptTags($html);

$domDocument = new DOMDocument('1.0', 'UTF-8');
set_error_handler(
function ($errorNumber, $errorString) {
throw new \DOMException($errorString, $errorNumber);
throw new DOMException($errorString, $errorNumber);
}
);
$string = mb_convert_encoding($html, 'HTML-ENTITIES', 'UTF-8');
Expand All @@ -155,7 +177,7 @@ function ($errorNumber, $errorString) {
'<html><body>' . $string . '</body></html>'
);
libxml_clear_errors();
} catch (\Exception $e) {
} catch (Exception $e) {
restore_error_handler();
$this->logger->critical($e);
}
Expand All @@ -167,16 +189,16 @@ function ($errorNumber, $errorString) {
/**
* Convert encoded HTML content types to placeholders and generate decoded outer html map for future replacement
*
* @param \DOMDocument $document
* @param DOMDocument $document
* @return array
* @throws \Magento\Framework\Exception\LocalizedException
* @throws LocalizedException
*/
private function generateDecodedHtmlPlaceholderMappingInDocument(\DOMDocument $document): array
private function generateDecodedHtmlPlaceholderMappingInDocument(DOMDocument $document): array
{
$xpath = new \DOMXPath($document);
$xpath = new DOMXPath($document);

// construct xpath query to fetch top-level ancestor html content type nodes
/** @var $htmlContentTypeNodes \DOMNode[] */
/** @var $htmlContentTypeNodes DOMNode[] */
$htmlContentTypeNodes = $xpath->query(
'//*[@data-content-type="html" and not(@data-decoded="true")]' .
'[not(ancestor::*[@data-content-type="html"])]'
Expand Down Expand Up @@ -221,7 +243,7 @@ private function generateDecodedHtmlPlaceholderMappingInDocument(\DOMDocument $d
// by the dom library
$uniqueNodeName = $this->mathRandom->getRandomString(32, $this->mathRandom::CHARS_LOWERS);

$uniqueNode = new \DOMElement($uniqueNodeName);
$uniqueNode = new DOMElement($uniqueNodeName);
$htmlContentTypeNode->parentNode->replaceChild($uniqueNode, $htmlContentTypeNode);

$uniqueNodeNameToDecodedOuterHtmlMap[$uniqueNodeName] = $decodedOuterHtml;
Expand All @@ -233,14 +255,14 @@ private function generateDecodedHtmlPlaceholderMappingInDocument(\DOMDocument $d
/**
* Generate the CSS for any background images on the page
*
* @param \DOMDocument $document
* @param DOMDocument $document
*/
private function generateBackgroundImageStyles(\DOMDocument $document) : void
private function generateBackgroundImageStyles(DOMDocument $document) : void
{
$xpath = new \DOMXPath($document);
$xpath = new DOMXPath($document);
$nodes = $xpath->query('//*[@data-background-images]');
foreach ($nodes as $node) {
/* @var \DOMElement $node */
/* @var DOMElement $node */
$backgroundImages = $node->attributes->getNamedItem('data-background-images');
if ($backgroundImages->nodeValue !== '') {
$elementClass = uniqid('background-image-');
Expand Down Expand Up @@ -337,4 +359,47 @@ private function getMediaQuery(string $view) : ?string
}
return null;
}

/**
* Masks "x-magento-template" script tags in html content before loading it into DOM parser
*
* DOMDocument::loadHTML() will remove any closing tag inside script tag and will result in broken html template
*
* @param string $content
* @return string
* @see https://bugs.php.net/bug.php?id=52012
*/
private function maskScriptTags(string $content): string
{
$tag = 'script';
$content = preg_replace_callback(
sprintf('#<%1$s[^>]*type="text/x-magento-template\"[^>]*>.*?</%1$s>#is', $tag),
function ($matches) {
$key = $this->mathRandom->getRandomString(32, $this->mathRandom::CHARS_LOWERS);
$this->scripts[$key] = $matches[0];
return '<' . $key . '>' . '</' . $key . '>';
},
$content
);
return $content;
}

/**
* Replaces masked "x-magento-template" script tags with their corresponding content
*
* @param string $content
* @return string
* @see maskScriptTags()
*/
private function unmaskScriptTags(string $content): string
{
foreach ($this->scripts as $key => $script) {
$content = str_replace(
'<' . $key . '>' . '</' . $key . '>',
$script,
$content
);
}
return $content;
}
}
137 changes: 137 additions & 0 deletions app/code/Magento/PageBuilder/Test/Unit/Model/Filter/TemplateTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\PageBuilder\Test\Unit\Model\Filter;

use Magento\Framework\Math\Random;
use Magento\Framework\Serialize\Serializer\Json;
use Magento\Framework\View\ConfigInterface;
use Magento\PageBuilder\Model\Filter\Template;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

class TemplateTest extends TestCase
{
/**
* @var MockObject|LoggerInterface
*/
private $logger;

/**
* @var ConfigInterface|MockObject
*/
private $viewConfig;

/**
* @var Template
*/
private $model;

/**
* @inheritDoc
*/
protected function setUp()
{
parent::setUp();
$this->logger = $this->createMock(LoggerInterface::class);
$this->viewConfig = $this->createMock(ConfigInterface::class);
$this->model = new Template(
$this->logger,
$this->viewConfig,
new Random(),
new Json()
);
}

/**
* @dataProvider filterProvider
* @param string $input
* @param string $output
*/
public function testFilter(string $input, string $output): void
{
$this->assertEquals($output, $this->model->filter($input));
}

/**
* @return array
*/
public function filterProvider(): array
{
return [
[
'',
''
],
[
'<div class="messages">' .
'<span class="message alert-success">success</span>' .
'</div>',
'<div class="messages">' .
'<span class="message alert-success">success</span>' .
'</div>'
],
[
'<div data-content-type="html">' .
'<div class="messages">' .
'<span class="message alert-success">success</span>' .
'</div>' .
'</div>',
'<div data-content-type="html" data-decoded="true">' .
'<div class="messages">' .
'<span class="message alert-success">success</span>' .
'</div>' .
'</div>'
],
[
'<div data-content-type="html">' .
'&lt;div class="messages"&gt;' .
'&lt;span class="message alert-success"&gt;success&lt;/span&gt;' .
'&lt;/div&gt;' .
'</div>',
'<div data-content-type="html" data-decoded="true">' .
'<div class="messages">' .
'<span class="message alert-success">success</span>' .
'</div>' .
'</div>'
],
[
'<div class="widget">' .
'<div>smart widget</div>' .
'<script type="text/x-magento-template">' .
'<span>smart template</span>' .
'</script>' .
'</div>',
'<div class="widget">' .
'<div>smart widget</div>' .
'<script type="text/x-magento-template">' .
'<span>smart template</span>' .
'</script>' .
'</div>'
],
[
'<div data-content-type="html">' .
'<div class="widget">' .
'<div>smart widget</div>' .
'<script type="text/x-magento-template">' .
'<span>smart template</span>' .
'</script>' .
'</div>' .
'</div>',
'<div data-content-type="html" data-decoded="true">' .
'<div class="widget">' .
'<div>smart widget</div>' .
'<script type="text/x-magento-template">' .
'<span>smart template</span>' .
'</script>' .
'</div>' .
'</div>',
],
];
}
}