Prevent case where validation error can be raised on already-removed node

westonruter · westonruter · commit feababcf30d7 · 2020-03-20T22:00:21.000-07:00
This prevents a validation error from having no source stack
diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
@@ -535,10 +535,9 @@ static function ( $validation_error ) {
 				} else {
 					// Otherwise, we have a rare condition where multiple tag specs fail for different reasons.
 					foreach ( $validation_errors as $validation_error ) {
-						$this->remove_invalid_child(
-							$node,
-							$validation_error
-						);
+						if ( true === $this->remove_invalid_child( $node, $validation_error ) ) {
+							break; // Once removed, ignore remaining errors.
+						}
 					}
 				}
 			}
diff --git a/tests/php/test-tag-and-attribute-sanitizer.php b/tests/php/test-tag-and-attribute-sanitizer.php
@@ -96,11 +96,6 @@ public function get_body_data() {
 						'spec_name'            => 'noscript enclosure for boilerplate',
 						'required_parent_name' => 'head',
 					],
-					[
-						'code'                => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG_ANCESTOR,
-						'disallowed_ancestor' => 'noscript',
-						'spec_name'           => 'noscript',
-					],
 				],
 			],
 

Original file line number	Diff line number	Diff line change
`@@ -535,10 +535,9 @@ static function ( $validation_error ) {`
`535`	`535`	`} else {`
`536`	`536`	`// Otherwise, we have a rare condition where multiple tag specs fail for different reasons.`
`537`	`537`	`foreach ( $validation_errors as $validation_error ) {`
`538`		`- $this->remove_invalid_child(`
`539`		`- $node,`
`540`		`- $validation_error`
`541`		`- );`
	`538`	`+ if ( true === $this->remove_invalid_child( $node, $validation_error ) ) {`
	`539`	`+ break; // Once removed, ignore remaining errors.`
	`540`	`+ }`
`542`	`541`	`}`
`543`	`542`	`}`
`544`	`543`	`}`