Merge pull request #696 from Automattic/revert-693-downgrade_htmlAttrNotByEscHTML

Revert "Downgrade htmlAttrNotByEscHTML to a warning"

Author: rebeccahum

WordPress-VIP-Go/ruleset-test.inc

@@ -253,10 +253,10 @@ $test = @in_array( $array, $needle, true ); // Error.
 
 // WordPressVIPMinimum.Security.ProperEscapingFunction.htmlAttrNotByEscHTML
 echo '<a href="' . esc_attr( $some_var ) . '"></a>'; // Error.
-echo '<a title="' . esc_html( $some_var ) . '"></a>'; // Warning.
+echo '<a title="' . esc_html( $some_var ) . '"></a>'; // Error.
 echo '<a href="' . esc_url( $some_var ) . '"></a>'; // OK.
 ?><a href="<?php echo esc_attr( $some_var ); ?>">Hello</a> <!-- Error. -->
-<a href="" class="<?php echo esc_html( $some_var); ?>">Hey</a> <!-- Warning. -->
+<a href="" class="<?php echo esc_html( $some_var); ?>">Hey</a> <!-- Error. -->
 <a href="<?php esc_url( $url );?>"></a> <!-- Ok. -->
 <a title="<?php esc_attr( $url );?>"></a> <?php // Ok.
 

WordPress-VIP-Go/ruleset-test.php

@@ -27,7 +27,9 @@
 		188 => 1,
 		252 => 1,
 		255 => 1,
+		256 => 1,
 		258 => 1,
+		259 => 1,
 		318 => 1,
 		329 => 1,
 		334 => 1,
@@ -191,8 +193,6 @@
 		245 => 1,
 		246 => 1,
 		247 => 1,
-		256 => 1,
-		259 => 1,
 		265 => 1,
 		269 => 1,
 		273 => 1,

WordPress-VIP-Go/ruleset.xml

@@ -229,6 +229,10 @@
 	<rule ref="Generic.PHP.NoSilencedErrors">
 		<severity>1</severity>
 	</rule>
+	<rule ref="WordPressVIPMinimum.Security.ProperEscapingFunction.htmlAttrNotByEscHTML">
+		<!-- This is still safe, just sub-optimal-->
+		<severity>3</severity>
+	</rule>
 	<rule ref="WordPressVIPMinimum.Functions.RestrictedFunctions.is_multi_author_is_multi_author">
 		<severity>1</severity>
 	</rule>

WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php

@@ -205,8 +205,8 @@ public function process_token( $stackPtr ) {
 
 		if ( $escaping_type === 'html' ) {
 			$message = 'Wrong escaping function. HTML attributes should be escaped by `esc_attr()`, not by `%s()`.';
-			$this->phpcsFile->addWarning( $message, $stackPtr, 'htmlAttrNotByEscHTML', $data );
-			return; // Warning level because sub-optimal due to different filters, but still OK.
+			$this->phpcsFile->addError( $message, $stackPtr, 'htmlAttrNotByEscHTML', $data );
+			return;
 		}
 	}
 

WordPressVIPMinimum/Tests/Security/ProperEscapingFunctionUnitTest.inc

@@ -12,15 +12,15 @@ echo '<a title="' . esc_attr( $some_var ) . '"></a>'; // OK.
 
 echo "<a title='" . \esc_attr( $some_var ) . "'></a>"; // OK.
 
-echo '<a title="' . esc_html_x( $some_var ) . '"></a>'; // Warning.
+echo '<a title="' . esc_html_x( $some_var ) . '"></a>'; // Error.
 
-echo "<a title='" . \esc_html( $some_var ) . "'></a>"; // Warning.
+echo "<a title='" . \esc_html( $some_var ) . "'></a>"; // Error.
 
 ?>
 
 <a href="<?php echo esc_attr( $some_var ); ?>">Hello</a> <!-- Error. -->
 
-<a href="" class="<?php esc_html_e( $some_var); ?>">Hey</a> <!-- Warning. -->
+<a href="" class="<?php esc_html_e( $some_var); ?>">Hey</a> <!-- Error. -->
 
 <a href="<?php esc_url( $url );?>"></a> <!-- OK. -->
 
@@ -71,9 +71,9 @@ echo "<$tag>  " , esc_attr( $test ) , "</$tag>"; // Error.
 <?php echo "<div>" . $test . "</div>"; // OK.
 echo "<{$tag}>" . esc_attr( $tag_content ) . "</{$tag}>"; // Error.
 echo "<$tag" . '   >' . esc_attr( $tag_content ) . "</$tag>"; // Error.
-echo '<div class=\'' . esc_html($class) . '\'>'; // Warning.
-echo "<div class=\"" . \esc_html__($class) . '">'; // Warning.
-echo "<div $someAttribute class=\"" . esc_html($class) . '">'; // Warning.
+echo '<div class=\'' . esc_html($class) . '\'>'; // Error.
+echo "<div class=\"" . \esc_html__($class) . '">'; // Error.
+echo "<div $someAttribute class=\"" . esc_html($class) . '">'; // Error.
 echo '<a href=\'' . esc_html($url) . '\'>'; // Error.
 echo "<img src=\"" . esc_html($src) . '"/>'; // Error.
 echo "<div $someAttributeName-url=\"" . esc_html($url) . '">'; // Error.

WordPressVIPMinimum/Tests/Security/ProperEscapingFunctionUnitTest.php

@@ -27,7 +27,10 @@ public function getErrorList() {
 		return [
 			3   => 1,
 			5   => 1,
+			15  => 1,
+			17  => 1,
 			21  => 1,
+			23  => 1,
 			33  => 1,
 			37  => 1,
 			41  => 1,
@@ -42,6 +45,9 @@ public function getErrorList() {
 			69  => 1,
 			72  => 1,
 			73  => 1,
+			74  => 1,
+			75  => 1,
+			76  => 1,
 			77  => 1,
 			78  => 1,
 			79  => 1,
@@ -60,14 +66,7 @@ public function getErrorList() {
 	 * @return array <int line number> => <int number of warnings>
 	 */
 	public function getWarningList() {
-		return [
-			15 => 1,
-			17 => 1,
-			23 => 1,
-			74 => 1,
-			75 => 1,
-			76 => 1,
-		];
+		return [];
 	}
 
 }

WordPressVIPMinimum/ruleset-test.inc

@@ -548,7 +548,7 @@ echo '<a href="{{href}}">{{{data}}}</div></a>'; // Warning.
 
 // WordPressVIPMinimum.Security.ProperEscapingFunction
 echo '<a href="' . esc_attr( $some_var ) . '"></a>'; // Error.
-echo '<a title="' . esc_html( $some_var ) . '"></a>'; // Warning.
+echo '<a title="' . esc_html( $some_var ) . '"></a>'; // Error.
 
 // WordPressVIPMinimum.Security.StaticStrreplace
 str_replace( 'foo', array( 'bar', 'foo' ), 'foobar' ); // Error.

WordPressVIPMinimum/ruleset-test.php

@@ -179,6 +179,7 @@
 		523 => 1,
 		525 => 1,
 		550 => 1,
+		551 => 1,
 		554 => 1,
 		569 => 1,
 		570 => 1,
@@ -289,7 +290,6 @@
 		535 => 1,
 		538 => 1,
 		545 => 1,
-		551 => 1,
 		559 => 1,
 		565 => 1,
 		589 => 1,