Skip to content

Commit

Permalink
Remove unrecognized elements and report.
Browse files Browse the repository at this point in the history
Closes #1100.

Fixes the problem of silently removing unrecognized elements.  Uses the same code pattern and methodology as `remove_invalid_child`:

- Prevent double-reporting nodes that are rejected.
- Check through should_sanitize_validation_error().
- Replace if true.
- Else, store in the `should_not_replace_nodes` property.
  • Loading branch information
Tonya Mork committed Jul 25, 2018
1 parent 8c764c3 commit 8476111
Showing 1 changed file with 38 additions and 19 deletions.
57 changes: 38 additions & 19 deletions includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ class AMP_Tag_And_Attribute_Sanitizer extends AMP_Base_Sanitizer {
*/
protected $script_components = array();

/**
* Keep track of nodes that should not be replaced to prevent duplicated validation errors since sanitization is rejected.
*
* @var array
*/
protected $should_not_replace_nodes = array();

/**
* AMP_Tag_And_Attribute_Sanitizer constructor.
*
Expand Down Expand Up @@ -412,7 +419,7 @@ private function process_node( $node ) {
$attr_spec_list = $first_spec[ AMP_Rule_Spec::ATTR_SPEC_LIST ];
}
}
} // End if().
}

if ( ! empty( $attr_spec_list ) && $this->is_missing_mandatory_attribute( $attr_spec_list, $node ) ) {
$this->remove_node( $node );
Expand Down Expand Up @@ -1656,34 +1663,46 @@ private function get_ancestor_with_matching_spec_name( $node, $ancestor_spec_nam
* Also adds them to the stack for processing by the sanitize() function.
*
* @since 0.3.3
* @since 1.0 Fix silently removing unrecognized elements.
* @see https://github.com/Automattic/amp-wp/issues/1100
*
* @param DOMNode $node Node.
*/
private function replace_node_with_children( $node ) {

// If node has no children or no parent, just remove the node.
if ( ! $node->hasChildNodes() || ! $node->parentNode ) {
// If node has no children or no parent, just remove the node.
$this->remove_node( $node );

} else {
/*
* If node has children, replace it with them and push children onto stack
*
* Create a DOM fragment to hold the children
*/
$fragment = $this->dom->createDocumentFragment();

// Add all children to fragment/stack.
$child = $node->firstChild;
while ( $child ) {
$fragment->appendChild( $child );
$this->stack[] = $child;
$child = $node->firstChild;
}
return;
}

// Replace node with fragment.
$node->parentNode->replaceChild( $fragment, $node );
/*
* If node has children, replace it with them and push children onto stack
*
* Create a DOM fragment to hold the children
*/
$fragment = $this->dom->createDocumentFragment();

// Add all children to fragment/stack.
$child = $node->firstChild;
while ( $child ) {
$fragment->appendChild( $child );
$this->stack[] = $child;
$child = $node->firstChild;
}

// Prevent double-reporting nodes that are rejected for sanitization.
if ( isset( $this->should_not_replace_nodes[ $node->nodeName ] ) && in_array( $node, $this->should_not_replace_nodes[ $node->nodeName ], true ) ) {
return;
}

// Replace node with fragment.
$should_replace = $this->should_sanitize_validation_error( array(), compact( 'node' ) );
if ( $should_replace ) {
$node->parentNode->replaceChild( $fragment, $node );
} else {
$this->should_not_replace_nodes[ $node->nodeName ][] = $node;
}
}

Expand Down

0 comments on commit 8476111

Please sign in to comment.