Skip to content

Commit

Permalink
Defer mustache tag replacements to right before serialization and onl…
Browse files Browse the repository at this point in the history
…y target template elements
  • Loading branch information
westonruter committed Mar 14, 2018
1 parent 3612e8f commit 4a10ee4
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 29 deletions.
62 changes: 38 additions & 24 deletions includes/utils/class-amp-dom-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,6 @@ public static function get_dom( $document ) {
// @todo In the future consider an AMP_DOMDocument subclass that does this automatically. See <https://github.com/Automattic/amp-wp/pull/895/files#r163825513>.
$document = self::convert_amp_bind_attributes( $document );

/*
* Prevent amp-mustache syntax from getting URL-encoded in attributes when saveHTML is done.
* While this is applying to the entire document, it only really matters inside of <template>
* elements, since URL-encoding of curly braces in href attributes would not normally matter.
* But when this is done inside of a <template> then it breaks Mustache. Since Mustache
* is logic-less and curly braces are not unsafe for HTML, we can do a global replacement.
* The replacement is done on the entire HTML document instead of just inside of the <template>
* elements since it is faster and wouldn't change the outcome.
*/
$placeholders = self::get_mustache_tag_placeholders();
$document = str_replace(
array_keys( $placeholders ),
array_values( $placeholders ),
$document
);

// Force all self-closing tags to have closing tags since DOMDocument isn't fully aware.
$document = preg_replace(
'#<(' . implode( '|', self::$self_closing_tags ) . ')[^>]*>(?!</\1>)#',
Expand Down Expand Up @@ -365,13 +349,51 @@ public static function get_content_from_dom_node( $dom, $node ) {
$self_closing_tags_regex = "#</({$self_closing_tags})>#i";
}

/*
* Prevent amp-mustache syntax from getting URL-encoded in attributes when saveHTML is done.
* While this is applying to the entire document, it only really matters inside of <template>
* elements, since URL-encoding of curly braces in href attributes would not normally matter.
* But when this is done inside of a <template> then it breaks Mustache. Since Mustache
* is logic-less and curly braces are not unsafe for HTML, we can do a global replacement.
* The replacement is done on the entire HTML document instead of just inside of the <template>
* elements since it is faster and wouldn't change the outcome.
*/
$mustache_tag_placeholders = self::get_mustache_tag_placeholders();
$mustache_tags_replaced = false;
$xpath = new DOMXPath( $dom );
$templates = $dom->getElementsByTagName( 'template' );
foreach ( $templates as $template ) {

// These attributes are the only ones that saveHTML() will URL-encode.
foreach ( $xpath->query( './/*/@src|.//*/@href|.//*/@action', $template ) as $attribute ) {
$attribute->nodeValue = str_replace(
array_keys( $mustache_tag_placeholders ),
array_values( $mustache_tag_placeholders ),
$attribute->nodeValue,
$count
);
if ( $count ) {
$mustache_tags_replaced = true;
}
}
}

$html = $dom->saveHTML( $node );

// Whitespace just causes unit tests to fail... so whitespace begone.
if ( '' === trim( $html ) ) {
return '';
}

// Restore amp-mustache placeholders which were replaced to prevent URL-encoded corruption by saveHTML.
if ( $mustache_tags_replaced ) {
$html = str_replace(
array_values( $mustache_tag_placeholders ),
array_keys( $mustache_tag_placeholders ),
$html
);
}

// Restore noscript elements which were temporarily removed to prevent libxml<2.8 parsing problems.
if ( version_compare( LIBXML_DOTTED_VERSION, '2.8', '<' ) ) {
$html = str_replace(
Expand All @@ -383,14 +405,6 @@ public static function get_content_from_dom_node( $dom, $node ) {

$html = self::restore_amp_bind_attributes( $html );

// Restore amp-mustache placeholders which were replaced to prevent URL-encoded corruption by saveHTML.
$placeholders = self::get_mustache_tag_placeholders();
$html = str_replace(
array_values( $placeholders ),
array_keys( $placeholders ),
$html
);

/*
* Travis w/PHP 7.1 generates <br></br> and <hr></hr> vs. <br/> and <hr/>, respectively.
* Travis w/PHP 7.x generates <source ...></source> vs. <source ... />. Etc.
Expand Down
65 changes: 65 additions & 0 deletions tests/test-class-amp-dom-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ public function test_amp_bind_conversion() {
* Test handling of empty elements.
*
* @covers \AMP_DOM_Utils::get_dom()
* @covers \AMP_DOM_Utils::get_content_from_dom_node()
*/
public function test_html5_empty_elements() {
$original = '<amp-video width="432" height="987">';
Expand All @@ -146,6 +147,70 @@ public function test_html5_empty_elements() {
$this->assertEquals( 'span', $video->childNodes->item( 5 )->nodeName );
}

/**
* Test parsing DOM with Mustache or Mustache-like templates.
*
* @covers \AMP_DOM_Utils::get_dom()
* @covers \AMP_DOM_Utils::get_content_from_dom_node()
*/
public function test_mustache_replacements() {

$data = array(
'foo' => array(
'bar' => array(
'baz' => array(),
),
),
);

$html = implode( "\n", array(
'<!--amp-source-stack {"block_name":"core\/columns"}-->',
'<div class="wp-block-columns has-2-columns">',
'<!--amp-source-stack {"block_name":"core\/quote","block_attrs":{"layout":"column-1"}}-->',
'<blockquote class="wp-block-quote layout-column-1"><p>Quote</p><cite>Famous</cite></blockquote>',
'<!--/amp-source-stack {"block_name":"core\/quote","block_attrs":{"layout":"column-1"}}-->',
'<!-- wp:paragraph -->',
'<p><a href="https://example.com/"><img src="https://example.com/img.jpg"></a></p>',
'<!-- /wp:paragraph -->',
'</div>',
'<!--/amp-source-stack {"block_name":"core\/columns"}-->',
'<!-- wp:html {} -->',
'<script type="application/json">' . wp_json_encode( $data ) . '</script>',
'<template type="amp-mustache">Hello {{world}}! <a href="{{href}}" title="Hello {{name}}"><img src="{{src}}"></a><blockquote cite="{{cite}}">{{quote}}</blockquote></template>',
'<!-- /wp:html -->',
) );

$dom = AMP_DOM_Utils::get_dom_from_content( $html );
$xpath = new DOMXPath( $dom );

// Ensure that JSON in scripts are left intact.
$script = $xpath->query( '//script' )->item( 0 );
$this->assertEquals(
$data,
json_decode( $script->nodeValue, true )
);

// Ensure that mustache var in a[href] attribute is intact.
$template_link = $xpath->query( '//template/a' )->item( 0 );
$this->assertSame( '{{href}}', $template_link->getAttribute( 'href' ) );
$this->assertEquals( 'Hello {{name}}', $template_link->getAttribute( 'title' ) );

// Ensure that mustache var in img[src] attribute is intact.
$template_img = $xpath->query( '//template/a/img' )->item( 0 );
$this->assertEquals( '{{src}}', $template_img->getAttribute( 'src' ) );

// Ensure that mustache var in blockquote[cite] is not changed.
$template_blockquote = $xpath->query( '//template/blockquote' )->item( 0 );
$this->assertEquals( '{{cite}}', $template_blockquote->getAttribute( 'cite' ) );

$serialized_html = AMP_DOM_Utils::get_content_from_dom_node( $dom, $dom->documentElement );

$this->assertContains( '<a href="{{href}}" title="Hello {{name}}">', $serialized_html );
$this->assertContains( '<img src="{{src}}">', $serialized_html );
$this->assertContains( '<blockquote cite="{{cite}}">', $serialized_html );
$this->assertContains( '"block_attrs":{"layout":"column-1"}}', $serialized_html );
}

/**
* Get Table Row Iterations
*
Expand Down
34 changes: 29 additions & 5 deletions tests/test-class-amp-validation-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,28 +157,44 @@ public function get_block_data() {
'paragraph' => array(
"<!-- wp:paragraph -->\n<p>Latest posts:</p>\n<!-- /wp:paragraph -->",
"<!--amp-source-stack {\"block_name\":\"core\/paragraph\"}-->\n<p>Latest posts:</p>\n<!--/amp-source-stack {\"block_name\":\"core\/paragraph\"}-->",
array(
'element' => 'p',
'blocks' => array( 'core/paragraph' ),
),
),
'latest_posts' => array(
'<!-- wp:latest-posts /-->',
'<!--amp-source-stack {"block_name":"core\/latest-posts","type":"plugin","name":"gutenberg","function":"render_block_core_latest_posts"}--><ul class="wp-block-latest-posts aligncenter"></ul><!--/amp-source-stack {"block_name":"core\/latest-posts","type":"plugin","name":"gutenberg","function":"render_block_core_latest_posts"}-->',
array(
'element' => 'ul',
'blocks' => array( 'core/latest-posts' ),
),
),
'columns' => array(
"<!-- wp:columns -->\n<div class=\"wp-block-columns has-2-columns\">\n <!-- wp:quote {\"layout\":\"column-1\"} -->\n <blockquote class=\"wp-block-quote layout-column-1\">\n <p>A quotation!</p><cite>Famous</cite></blockquote>\n <!-- /wp:quote -->\n\n <!-- wp:html {\"layout\":\"column-2\"} -->\n <div class=\"layout-column-2\">\n <script>\n document.write('Not allowed!');\n </script>\n </div>\n <!-- /wp:html -->\n</div>\n<!-- /wp:columns -->",
"<!--amp-source-stack {\"block_name\":\"core\/columns\"}-->\n<div class=\"wp-block-columns has-2-columns\">\n\n\n\n<!--amp-source-stack {\"block_name\":\"core\/quote\",\"block_attrs\":{\"layout\":\"column-1\"}}-->\n <blockquote class=\"wp-block-quote layout-column-1\">\n <p>A quotation!</p><cite>Famous</cite></blockquote>\n <!--/amp-source-stack {\"block_name\":\"core\/quote\",\"block_attrs\":{\"layout\":\"column-1\"}}--><!--amp-source-stack {\"block_name\":\"core\/html\",\"block_attrs\":{\"layout\":\"column-2\"}}-->\n <div class=\"layout-column-2\">\n <script>\n document.write('Not allowed!');\n </script>\n </div>\n <!--/amp-source-stack {\"block_name\":\"core\/html\",\"block_attrs\":{\"layout\":\"column-2\"}}--></div>\n<!--/amp-source-stack {\"block_name\":\"core\/columns\"}-->",
"<!-- wp:columns -->\n<div class=\"wp-block-columns has-2-columns\">\n <!-- wp:quote {\"layout\":\"column-1\",\"foo\":{\"bar\":1}} -->\n <blockquote class=\"wp-block-quote layout-column-1\">\n <p>A quotation!</p><cite>Famous</cite></blockquote>\n <!-- /wp:quote -->\n\n <!-- wp:html {\"layout\":\"column-2\"} -->\n <div class=\"layout-column-2\">\n <script>\n document.write('Not allowed!');\n </script>\n </div>\n <!-- /wp:html -->\n</div>\n<!-- /wp:columns -->",
"<!--amp-source-stack {\"block_name\":\"core\/columns\"}-->\n<div class=\"wp-block-columns has-2-columns\">\n\n\n\n<!--amp-source-stack {\"block_name\":\"core\/quote\",\"block_attrs\":{\"layout\":\"column-1\",\"foo\":{\"bar\":1}}}-->\n <blockquote class=\"wp-block-quote layout-column-1\">\n <p>A quotation!</p><cite>Famous</cite></blockquote>\n <!--/amp-source-stack {\"block_name\":\"core\/quote\",\"block_attrs\":{\"layout\":\"column-1\",\"foo\":{\"bar\":1}}}--><!--amp-source-stack {\"block_name\":\"core\/html\",\"block_attrs\":{\"layout\":\"column-2\"}}-->\n <div class=\"layout-column-2\">\n <script>\n document.write('Not allowed!');\n </script>\n </div>\n <!--/amp-source-stack {\"block_name\":\"core\/html\",\"block_attrs\":{\"layout\":\"column-2\"}}--></div>\n<!--/amp-source-stack {\"block_name\":\"core\/columns\"}-->",
array(
'element' => 'blockquote',
'blocks' => array(
'core/columns',
'core/quote',
),
),
),
);
}

/**
* Test do_blocks.
*
* @param string $content Content.
* @param string $content Content.
* @param string $expected Expected content.
* @param array $query Query.
* @dataProvider get_block_data
* @covers AMP_Validation_Utils::do_blocks()
* @covers AMP_Validation_Utils::render_blocks
* @covers AMP_Validation_Utils::render_blocks()
*/
public function test_do_blocks( $content, $expected ) {
public function test_do_blocks( $content, $expected, $query ) {
if ( ! function_exists( 'do_blocks' ) ) {
$this->markTestSkipped( 'Gutenberg not active.' );
}
Expand All @@ -188,6 +204,14 @@ public function test_do_blocks( $content, $expected ) {
preg_replace( '/\s+/', ' ', $expected ),
preg_replace( '/\s+/', ' ', $rendered_block )
);

$dom = AMP_DOM_Utils::get_dom_from_content( $rendered_block );
$el = $dom->getElementsByTagName( $query['element'] )->item( 0 );

$this->assertEquals(
$query['blocks'],
wp_list_pluck( AMP_Validation_Utils::locate_sources( $el ), 'block_name' )
);
}

/**
Expand Down

0 comments on commit 4a10ee4

Please sign in to comment.