Merge pull request #693 from Automattic/downgrade_htmlAttrNotByEscHTML

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>'; // Error.
+echo '<a title="' . esc_html( $some_var ) . '"></a>'; // Warning.
 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> <!-- Error. -->
+<a href="" class="<?php echo esc_html( $some_var); ?>">Hey</a> <!-- Warning. -->
 <a href="<?php esc_url( $url );?>"></a> <!-- Ok. -->
 <a title="<?php esc_attr( $url );?>"></a> <?php // Ok.
 

WordPress-VIP-Go/ruleset-test.php

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

WordPress-VIP-Go/ruleset.xml

@@ -229,10 +229,6 @@
 	<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->addError( $message, $stackPtr, 'htmlAttrNotByEscHTML', $data );
-			return;
+			$this->phpcsFile->addWarning( $message, $stackPtr, 'htmlAttrNotByEscHTML', $data );
+			return; // Warning level because sub-optimal due to different filters, but still OK.
 		}
 	}
 

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>'; // Error.
+echo '<a title="' . esc_html_x( $some_var ) . '"></a>'; // Warning.
 
-echo "<a title='" . \esc_html( $some_var ) . "'></a>"; // Error.
+echo "<a title='" . \esc_html( $some_var ) . "'></a>"; // Warning.
 
 ?>
 
 <a href="<?php echo esc_attr( $some_var ); ?>">Hello</a> <!-- Error. -->
 
-<a href="" class="<?php esc_html_e( $some_var); ?>">Hey</a> <!-- Error. -->
+<a href="" class="<?php esc_html_e( $some_var); ?>">Hey</a> <!-- Warning. -->
 
 <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) . '\'>'; // Error.
-echo "<div class=\"" . \esc_html__($class) . '">'; // Error.
-echo "<div $someAttribute class=\"" . esc_html($class) . '">'; // Error.
+echo '<div class=\'' . esc_html($class) . '\'>'; // Warning.
+echo "<div class=\"" . \esc_html__($class) . '">'; // Warning.
+echo "<div $someAttribute class=\"" . esc_html($class) . '">'; // Warning.
 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,10 +27,7 @@ public function getErrorList() {
 		return [
 			3   => 1,
 			5   => 1,
-			15  => 1,
-			17  => 1,
 			21  => 1,
-			23  => 1,
 			33  => 1,
 			37  => 1,
 			41  => 1,
@@ -45,9 +42,6 @@ public function getErrorList() {
 			69  => 1,
 			72  => 1,
 			73  => 1,
-			74  => 1,
-			75  => 1,
-			76  => 1,
 			77  => 1,
 			78  => 1,
 			79  => 1,
@@ -66,7 +60,14 @@ public function getErrorList() {
 	 * @return array <int line number> => <int number of warnings>
 	 */
 	public function getWarningList() {
-		return [];
+		return [
+			15 => 1,
+			17 => 1,
+			23 => 1,
+			74 => 1,
+			75 => 1,
+			76 => 1,
+		];
 	}
 
 }

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>'; // Error.
+echo '<a title="' . esc_html( $some_var ) . '"></a>'; // Warning.
 
 // WordPressVIPMinimum.Security.StaticStrreplace
 str_replace( 'foo', array( 'bar', 'foo' ), 'foobar' ); // Error.

WordPressVIPMinimum/ruleset-test.php

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