Skip to content

Commit

Permalink
Merge pull request #4333 from ampproject/fix/mustache-template-workar…
Browse files Browse the repository at this point in the history 
…ound

Prevent removal of closing `table` and `td` tags in `script[template="amp-mustache"]`
  • Loading branch information
@westonruter
westonruter authored Mar 10, 2020
2 parents c14c1ce + b74d853 commit 62634ed
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 4 deletions.
6 changes: 3 additions & 3 deletions Gruntfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ module.exports = function( grunt ) {
command: [
'if [ ! -e build ]; then echo "Run grunt build first."; exit 1; fi',
'cd build',
'if [ -d vendor/ampproject/common/vendor ]; then rm -r vendor/ampproject/common/vendor; fi',
'if [ -d vendor/ampproject/optimizer/vendor ]; then rm -r vendor/ampproject/optimizer/vendor; fi',
'composer install --no-dev -o',
'for symlinksource in $(find vendor/ampproject -type l); do symlinktarget=$(readlink "$symlinksource") && rm "$symlinksource" && cp -r "vendor/ampproject/$symlinktarget" "$symlinksource"; done',
'composer remove cweagans/composer-patches --update-no-dev -o',
'rm -r ' + productionVendorExcludedFilePatterns.join( ' ' )
'rm -r ' + productionVendorExcludedFilePatterns.join( ' ' ),
'if [ -d vendor/ampproject/common/vendor ]; then rm -r vendor/ampproject/common/vendor; fi',
'if [ -d vendor/ampproject/optimizer/vendor ]; then rm -r vendor/ampproject/optimizer/vendor; fi'
].join( ' && ' ),
},
create_build_zip: {
Expand Down
43 changes: 43 additions & 0 deletions lib/common/src/Dom/Document.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ public function loadHTMLFragment($source, $options = 0)
$source = $this->convertAmpBindAttributes($source);
$source = $this->replaceSelfClosingTags($source);
$source = $this->maybeReplaceNoscriptElements($source);
$source = $this->secureMustacheScriptTemplates($source);
$source = $this->secureDoctypeNode($source);
$source = $this->convertAmpEmojiAttribute($source);

Expand All @@ -384,6 +385,7 @@ public function loadHTMLFragment($source, $options = 0)

if ($success) {
$this->normalizeHtmlAttributes();
$this->restoreMustacheScriptTemplates();

// Remove http-equiv charset again.
$meta = $this->head->firstChild;
Expand Down Expand Up @@ -850,6 +852,47 @@ private function maybeRestoreNoscriptElements($html)
);
}

/**
* Secures instances of script[template="amp-mustache"] by renaming element to tmp-script, as a workaround to a libxml parsing issue.
*
* This script can have closing tags of its children table and td stripped.
* So this changes its name from script to tmp-script to avoid this.
*
* @link https://github.com/ampproject/amp-wp/issues/4254
* @see restoreMustacheScriptTemplates() Reciprocal function.
*
* @param string $html To replace the tag name that contains the mustache templates.
* @return string The HTML, with the tag name of the mustache templates replaced.
*/
private function secureMustacheScriptTemplates($html)
{
return preg_replace(
'#<script(\s[^>]*?template=(["\']?)amp-mustache\2[^>]*)>(.*?)</script\s*?>#i',
'<tmp-script$1>$3</tmp-script>',
$html
);
}

/**
* Restores the tag names of script[template="amp-mustache"] elements that were replaced earlier.
*
* @see secureMustacheScriptTemplates() Reciprocal function.
*/
private function restoreMustacheScriptTemplates()
{
$tmp_script_elements = iterator_to_array($this->getElementsByTagName('tmp-script'));
foreach ($tmp_script_elements as $tmp_script_element) {
$script = $this->createElement(Tag::SCRIPT);
foreach ($tmp_script_element->attributes as $attr) {
$script->setAttribute($attr->nodeName, $attr->nodeValue);
}
while ($tmp_script_element->firstChild) {
$script->appendChild($tmp_script_element->firstChild);
}
$tmp_script_element->parentNode->replaceChild($script, $tmp_script_element);
}
}

/**
* Replace AMP binding attributes with something that libxml can parse (as HTML5 data-* attributes).
*
Expand Down
35 changes: 35 additions & 0 deletions lib/common/tests/Dom/DocumentTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,41 @@ public function dataDomDocument()
'<!DOCTYPE html><html><head profile=""></head><body></body></html>',
'<!DOCTYPE html><html><head><meta charset="utf-8"></head><body></body></html>',
],
'amp_mustache_template_with_table' => [
'utf-8',
'<!DOCTYPE html><html>' . $head . '<body><template type="amp-mustache"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></template></body></html>',
'<!DOCTYPE html><html>' . $head . '<body><template type="amp-mustache"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></template></body></html>',
],
'amp_mustache_template_script_with_table' => [
'utf-8',
'<!DOCTYPE html><html>' . $head . '<body><script id="baz" type="text/plain" template="amp-mustache"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
'<!DOCTYPE html><html>' . $head . '<body><script id="baz" type="text/plain" template="amp-mustache"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
],
'amp_mustache_spaces_in_closing_script' => [
'utf-8',
'<!DOCTYPE html><html>' . $head . '<body><script id="baz" type="text/plain" template="amp-mustache"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script ></body></html>',
'<!DOCTYPE html><html>' . $head . '<body><script id="baz" type="text/plain" template="amp-mustache"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
],
'amp_mustache_template_single_quotes' => [
'utf-8',
'<!DOCTYPE html><html>' . $head . '<body><script type=\'text/plain\' template=\'amp-mustache\'><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
'<!DOCTYPE html><html>' . $head . '<body><script type="text/plain" template="amp-mustache"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
],
'amp_mustache_no_quote' => [
'utf-8',
'<!DOCTYPE html><html>' . $head . '<body><script type=\'text/plain\' template=amp-mustache><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
'<!DOCTYPE html><html>' . $head . '<body><script type="text/plain" template="amp-mustache"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
],
'amp_mustache_script_multiple_children' => [
'utf-8',
'<!DOCTYPE html><html>' . $head . '<body><script type="text/plain" template="amp-mustache"><h1>{{heading}}</h1><p>{{content}}</p><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
'<!DOCTYPE html><html>' . $head . '<body><script type="text/plain" template="amp-mustache"><h1>{{heading}}</h1><p>{{content}}</p><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script></body></html>',
],
'multiple_mustache_templates_still_appear' => [
'utf-8',
'<!DOCTYPE html><html>' . $head . '<body><script template="amp-mustache" type="text/plain" id="foo"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script><script type="text/plain" template="amp-mustache" id="example"><p>{{#baz}}This is inside a template{{/baz}}</p></script></body></html>',
'<!DOCTYPE html><html>' . $head . '<body><script template="amp-mustache" type="text/plain" id="foo"><table><tr>{{#example}}<td></td>{{/example}}</tr></table></script><script type="text/plain" template="amp-mustache" id="example"><p>{{#baz}}This is inside a template{{/baz}}</p></script></body></html>',
],
];
}

Expand Down
2 changes: 1 addition & 1 deletion lib/optimizer/tests/src/TestMarkup.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ final class TestMarkup
const SCRIPT_AMPMRAID = '<script async host-service="amp-mraid" src="https://cdn.ampproject.org/v0/amp-mraid-0.1.js"></script>';

// ScriptAMPMustache is the script for amp-mustache.
const SCRIPT_AMPMUSTACHE = '<script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-0.1.js"></script>';
const SCRIPT_AMPMUSTACHE = '<script async="" custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-0.1.js"></script>';

// ScriptAMPRuntime is the AMP script tag.
const SCRIPT_AMPRUNTIME = '<script async src="https://cdn.ampproject.org/v0.js"></script>';
Expand Down

0 comments on commit 62634ed

Please sign in to comment.