Merge pull request #575 from Automattic/fix/incorrect-message-for-proper-escaping-function

ProperEscaping: Fix message for action attribute

Author: rebeccahum

WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php

@@ -82,8 +82,8 @@ public function process_token( $stackPtr ) {
 
 		$data = [ $function_name ];
 
-		if ( $function_name !== 'esc_url' && $this->is_href_or_src( $this->tokens[ $html ]['content'] ) ) {
-			$message = 'Wrong escaping function. href and src attributes should be escaped by `esc_url()`, not by `%s()`.';
+		if ( $function_name !== 'esc_url' && $this->attr_expects_url( $this->tokens[ $html ]['content'] ) ) {
+			$message = 'Wrong escaping function. href, src, and action attributes should be escaped by `esc_url()`, not by `%s()`.';
 			$this->phpcsFile->addError( $message, $stackPtr, 'hrefSrcEscUrl', $data );
 			return;
 		}
@@ -95,32 +95,32 @@ public function process_token( $stackPtr ) {
 	}
 
 	/**
-	 * Tests whether provided string ends with open src or href attribute.
+	 * Tests whether provided string ends with open attribute which expects a URL value.
 	 *
-	 * @param string $content Haystack in which we look for an open src or href attribute.
+	 * @param string $content Haystack in which we look for an open attribute which exects a URL value.
 	 *
-	 * @return bool True if string ends with open src or href attribute.
+	 * @return bool True if string ends with open attribute which exects a URL value.
 	 */
-	public function is_href_or_src( $content ) {
-		$is_href_or_src = false;
-		foreach ( [ 'href', 'src', 'url' ] as $attr ) {
+	public function attr_expects_url( $content ) {
+		$attr_expects_url = false;
+		foreach ( [ 'href', 'src', 'url', 'action' ] as $attr ) {
 			foreach ( [
 				'="',
 				"='",
 				'=\'"', // The tokenizer does some fun stuff when it comes to mixing double and single quotes.
 				'="\'', // The tokenizer does some fun stuff when it comes to mixing double and single quotes.
 			] as $ending ) {
 				if ( $this->endswith( $content, $attr . $ending ) === true ) {
-					$is_href_or_src = true;
+					$attr_expects_url = true;
 					break;
 				}
 			}
 		}
-		return $is_href_or_src;
+		return $attr_expects_url;
 	}
 
 	/**
-	 * Tests, whether provided string ends with open HMTL attribute.
+	 * Tests whether provided string ends with open HMTL attribute.
 	 *
 	 * @param string $content Haystack in which we look for open HTML attribute.
 	 *

WordPressVIPMinimum/Tests/Security/ProperEscapingFunctionUnitTest.inc

@@ -34,4 +34,8 @@ echo '<media:content url="' . esc_attr( $post_image ) . '" medium="image">'; //
 
 echo 'data-param-url="' . esc_url( $share_url ) . '"'; // OK.
 
-echo 'data-param-url="' . esc_html( $share_url ) . '"'; // NOK.
+echo 'data-param-url="' . esc_html( $share_url ) . '"'; // NOK.
+
+?>
+
+<form method="post" action="<?php echo esc_html(admin_url('admin.php?page='.$base_name.'&amp;mode=logs&amp;id='.$poll_id)); ?>">

WordPressVIPMinimum/Tests/Security/ProperEscapingFunctionUnitTest.php

@@ -33,6 +33,7 @@ public function getErrorList() {
 			23 => 1,
 			33 => 1,
 			37 => 1,
+			41 => 1,
 		];
 	}