Skip to content

Commit

Permalink
Fix processing of element child sanitization loop when invalid elemen…
Browse files Browse the repository at this point in the history
…ts are replaced with children (#4512)

* Add failing test showing missing amp-form component script

* Fix sanitization loop when invalid node is replaced with children

* Add additional test case for gathering script from invalid parent
  • Loading branch information
westonruter authored Apr 3, 2020
1 parent 4115ea1 commit 4193dbc
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ private function sanitize_element( DOMElement $element ) {
// ancestor nodes in AMP_Tag_And_Attribute_Sanitizer::remove_node().
$this_child = $element->firstChild;
while ( $this_child && $element->parentNode ) {
$prev_child = $this_child->previousSibling;
$next_child = $this_child->nextSibling;
if ( $this_child instanceof DOMElement ) {
$result = $this->sanitize_element( $this_child );
Expand All @@ -350,7 +351,13 @@ private function sanitize_element( DOMElement $element ) {
} elseif ( $this_child instanceof DOMProcessingInstruction ) {
$this->remove_invalid_child( $this_child, [ 'code' => self::DISALLOWED_PROCESSING_INSTRUCTION ] );
}
$this_child = $next_child;

if ( ! $this_child->parentNode ) {
// Handle case where this child is replaced with children.
$this_child = $prev_child ? $prev_child->nextSibling : $element->firstChild;
} else {
$this_child = $next_child;
}
}

// If the element is still in the tree, process it.
Expand Down
27 changes: 27 additions & 0 deletions tests/php/test-tag-and-attribute-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,33 @@ static function () {
[ 'amp-form' ],
],

'scripts-gathered-from-invalid-parents' => [
'
<ancestor-unknown>
<amp-audio src="https://example.com/foo.mp3" width="100" height="200"></amp-audio>
<parent-unknown>
<amp-form>
<form method="GET" id="a_string" class="a_string" action="https://example.com" target="_blank">
<unrecognized>who are you?</unrecognized>
<input type=text value="test" name="hello">
</form>
</amp-form>
</parent-unknown>
<amp-video src="https://example.com/foo.mp4" width="100" height="200"></amp-video>
</ancestor-unknown>
',
'
<amp-audio src="https://example.com/foo.mp3" width="100" height="200"></amp-audio>
<form method="GET" id="a_string" class="a_string" action="https://example.com" target="_blank">
who are you?
<input type="text" value="test" name="hello">
</form>
<amp-video src="https://example.com/foo.mp4" width="100" height="200"></amp-video>
',
[ 'amp-audio', 'amp-form', 'amp-video' ],
array_fill( 0, 4, AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG ),
],

'form-visible-when-invalid' => [
'
<form method="post"
Expand Down

0 comments on commit 4193dbc

Please sign in to comment.