Prevent skipping elements that don't explicitly match an attribute sp…

…ec list when missing any optional attributes

includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php

@@ -494,13 +494,20 @@ private function validate_tag_spec_for_node( $node, $tag_spec ) {
 	 * @param DOMNode $node           Node.
 	 * @param array[] $attr_spec_list Attribute Spec list.
 	 *
-	 * @return int Number of times the attribute spec list matched. If there was a mismatch, then 0 is returned.
+	 * @return float Number of times the attribute spec list matched. If there was a mismatch, then 0 is returned. 0.5 is returned if there is an implicit match.
 	 */
 	private function validate_attr_spec_list_for_node( $node, $attr_spec_list ) {
-
-		// If node has no attributes there is no point in continuing.
+		/*
+		 * If node has no attributes there is no point in continuing, but if none of attributes
+		 * in the spec list are mandatory, then we give this a score.
+		 */
 		if ( ! $node->hasAttributes() ) {
-			return 0;
+			foreach ( $attr_spec_list as $attr_name => $attr_spec_rule ) {
+				if ( isset( $attr_spec_rule[ AMP_Rule_Spec::MANDATORY ] ) ) {
+					return 0;
+				}
+			}
+			return 0.5;
 		}
 
 		foreach ( $node->attributes as $attr_name => $attr_node ) {
@@ -522,6 +529,13 @@ private function validate_attr_spec_list_for_node( $node, $attr_spec_list ) {
 
 		$score = 0;
 
+		/*
+		 * Keep track of how many of the attribute spec rules are mandatory,
+		 * because if none are mandatory, then we will let this rule have a
+		 * score since all the invalid attributes can just be removed.
+		 */
+		$mandatory_count = 0;
+
 		/*
 		 * Iterate through each attribute rule in this attr spec list and run
 		 * the series of tests. Each filter is given a `$node`, an `$attr_name`,
@@ -538,6 +552,7 @@ private function validate_attr_spec_list_for_node( $node, $attr_spec_list ) {
 
 			// If a mandatory attribute is required, and attribute exists, pass.
 			if ( isset( $attr_spec_rule[ AMP_Rule_Spec::MANDATORY ] ) ) {
+				$mandatory_count++;
 				$result = $this->check_attr_spec_rule_mandatory( $node, $attr_name, $attr_spec_rule );
 				if ( AMP_Rule_Spec::PASS === $result ) {
 					$score++;
@@ -677,6 +692,11 @@ private function validate_attr_spec_list_for_node( $node, $attr_spec_list ) {
 			}
 		}
 
+		// Give the spec a score if it doesn't have any mandatory attributes.
+		if ( 0 === $mandatory_count && 0 === $score ) {
+			$score = 0.5;
+		}
+
 		return $score;
 	}
 

tests/amp-tag-and-attribute-sanitizer-private-methods-tests.php

@@ -881,15 +881,15 @@ public function get_validate_attr_spec_list_for_node_data() {
 					'node_tag_name' => 'div',
 					'attr_spec_list' => array(),
 				),
-				0,
+				0.5, // Because there are no mandatory attributes.
 			),
 			'attributes_no_spec' => array(
 				array(
 					'source' => '<div attribute1 attribute2 attribute3></div>',
 					'node_tag_name' => 'div',
 					'attr_spec_list' => array(),
 				),
-				0,
+				0.5, // Because there are no mandatory attributes.
 			),
 			'attributes_alternative_names' => array(
 				array(
@@ -905,7 +905,7 @@ public function get_validate_attr_spec_list_for_node_data() {
 						),
 					),
 				),
-				0,
+				0.5, // Because there are no mandatory attributes.
 			),
 			'attributes_mandatory' => array(
 				array(

tests/test-amp-form-sanitizer.php

@@ -60,6 +60,10 @@ public function get_data() {
 				'<form method="post" action="https://example.org/" target="some_other_target"></form>',
 				'<form method="post" target="_blank" action-xhr="https://example.org/?_wp_amp_action_xhr_converted=1"><div submit-error=""><template type="amp-mustache">{{{error}}}</template></div></form>',
 			),
+			'jetpack_contact_form' => array(
+				'<form action="https://src.wordpress-develop.test/contact/#contact-form-9" method="post" class="contact-form commentsblock"><div class="element-has-attributes">hello</div><div><label for="g9-favoritenumber" class="grunion-field-label text">Favorite number</label><input type="text" name="g9-favoritenumber" id="g9-favoritenumber" value="" class="text"></div><p class="contact-submit"><input type="submit" value="Submit" class="pushbutton-wide"><input type="hidden" id="_wpnonce" name="_wpnonce" value="640996fb1e"><input type="hidden" name="_wp_http_referer" value="/contact/"><input type="hidden" name="contact-form-id" value="9"><input type="hidden" name="action" value="grunion-contact-form"><input type="hidden" name="contact-form-hash" value="df9f9136763f5eb819f433e4fe4af3447534e8cc"></p></form>',
+				'<form method="post" class="contact-form commentsblock" action-xhr="https://src.wordpress-develop.test/contact/?_wp_amp_action_xhr_converted=1#contact-form-9" target="_top"><div class="element-has-attributes">hello</div><div><label for="g9-favoritenumber" class="grunion-field-label text">Favorite number</label><input type="text" name="g9-favoritenumber" id="g9-favoritenumber" value="" class="text"></div><p class="contact-submit"><input type="submit" value="Submit" class="pushbutton-wide"><input type="hidden" id="_wpnonce" name="_wpnonce" value="640996fb1e"><input type="hidden" name="_wp_http_referer" value="/contact/"><input type="hidden" name="contact-form-id" value="9"><input type="hidden" name="action" value="grunion-contact-form"><input type="hidden" name="contact-form-hash" value="df9f9136763f5eb819f433e4fe4af3447534e8cc"></p><div submit-error=""><template type="amp-mustache">{{{error}}}</template></div></form>',
+			),
 		);
 	}
 
@@ -83,6 +87,8 @@ public function test_converter( $source, $expected = null ) {
 		$whitelist_sanitizer->sanitize();
 
 		$content = AMP_DOM_Utils::get_content_from_dom( $dom );
+		$content = preg_replace( '/(?<=>)\s+(?=<)/', '', $content );
+
 		$this->assertEquals( $expected, $content );
 	}
 

tests/test-tag-and-attribute-sanitizer.php

@@ -759,7 +759,7 @@ public function get_html_data() {
 			),
 			'bad_meta_charset' => array(
 				'<html amp><head><meta charset="latin-1"><title>Mojibake?</title></head><body></body></html>',
-				'<html amp><head><title>Mojibake?</title></head><body></body></html>',
+				'<html amp><head><meta><title>Mojibake?</title></head><body></body></html>', // Note the charset attribute is removed because it violates the attribute spec, but the entire element is not removed because charset is not mandatory.
 			),
 			'bad_meta_viewport' => array(
 				'<html amp><head><meta charset="utf-8"><meta name="viewport" content="width=device-width"></head><body></body></html>',