From 5794920a3cbe71bdb88d11a14dff0046e0650b92 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Sun, 23 Jul 2023 23:45:51 +0200
Subject: [PATCH 01/44] Security/EscapeOutput: rename test case file

... to allow for adding additional test case files.
---
 ...nitTest.inc => EscapeOutputUnitTest.1.inc} |   0
 .../Tests/Security/EscapeOutputUnitTest.php   | 144 +++++++++---------
 2 files changed, 76 insertions(+), 68 deletions(-)
 rename WordPress/Tests/Security/{EscapeOutputUnitTest.inc => EscapeOutputUnitTest.1.inc} (100%)

diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
similarity index 100%
rename from WordPress/Tests/Security/EscapeOutputUnitTest.inc
rename to WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 4664245d45..8a65bff2c0 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -32,76 +32,84 @@ final class EscapeOutputUnitTest extends AbstractSniffUnitTest {
 	/**
 	 * Returns the lines where errors should occur.
 	 *
+	 * @param string $testFile The name of the file being tested.
+	 *
 	 * @return array <int line number> => <int number of errors>
 	 */
-	public function getErrorList() {
-		return array(
-			17  => 1,
-			19  => 1,
-			36  => 1,
-			39  => 1,
-			40  => 1,
-			41  => 1,
-			43  => 1,
-			46  => 1,
-			53  => 1,
-			59  => 1,
-			60  => 1,
-			65  => 1,
-			68  => 1,
-			71  => 1,
-			73  => 1,
-			75  => 1,
-			101 => 1,
-			103 => 1,
-			111 => 1,
-			112 => 1,
-			113 => 1,
-			114 => 1,
-			125 => 1,
-			126 => 1, // Old-style WPCS ignore comments are no longer supported.
-			127 => 1, // Old-style WPCS ignore comments are no longer supported.
-			128 => 1, // Old-style WPCS ignore comments are no longer supported.
-			131 => 1,
-			135 => 1,
-			138 => 1,
-			145 => 1,
-			147 => 1,
-			149 => 1,
-			152 => 2,
-			159 => 1,
-			162 => 1,
-			169 => 1,
-			172 => 1,
-			173 => 1,
-			182 => 3,
-			190 => 1,
-			191 => 2,
-			205 => 1,
-			206 => 1,
-			207 => 1,
-			223 => 1,
-			225 => 1,
-			226 => 1,
-			241 => 1, // Old-style WPCS ignore comments are no longer supported.
-			245 => 1, // Old-style WPCS ignore comments are no longer supported.
-			249 => 1, // Old-style WPCS ignore comments are no longer supported.
-			252 => 1,
-			253 => 1,
-			263 => 1,
-			264 => 1,
-			266 => 1,
-			282 => 1,
-			286 => 1,
-			289 => 1,
-			294 => 1,
-			297 => 1,
-			307 => 1,
-			313 => 1,
-			314 => 1,
-			315 => 1,
-			319 => 1,
-		);
+	public function getErrorList( $testFile = '' ) {
+		switch ( $testFile ) {
+			case 'EscapeOutputUnitTest.1.inc':
+				return array(
+					17  => 1,
+					19  => 1,
+					36  => 1,
+					39  => 1,
+					40  => 1,
+					41  => 1,
+					43  => 1,
+					46  => 1,
+					53  => 1,
+					59  => 1,
+					60  => 1,
+					65  => 1,
+					68  => 1,
+					71  => 1,
+					73  => 1,
+					75  => 1,
+					101 => 1,
+					103 => 1,
+					111 => 1,
+					112 => 1,
+					113 => 1,
+					114 => 1,
+					125 => 1,
+					126 => 1, // Old-style WPCS ignore comments are no longer supported.
+					127 => 1, // Old-style WPCS ignore comments are no longer supported.
+					128 => 1, // Old-style WPCS ignore comments are no longer supported.
+					131 => 1,
+					135 => 1,
+					138 => 1,
+					145 => 1,
+					147 => 1,
+					149 => 1,
+					152 => 2,
+					159 => 1,
+					162 => 1,
+					169 => 1,
+					172 => 1,
+					173 => 1,
+					182 => 3,
+					190 => 1,
+					191 => 2,
+					205 => 1,
+					206 => 1,
+					207 => 1,
+					223 => 1,
+					225 => 1,
+					226 => 1,
+					241 => 1, // Old-style WPCS ignore comments are no longer supported.
+					245 => 1, // Old-style WPCS ignore comments are no longer supported.
+					249 => 1, // Old-style WPCS ignore comments are no longer supported.
+					252 => 1,
+					253 => 1,
+					263 => 1,
+					264 => 1,
+					266 => 1,
+					282 => 1,
+					286 => 1,
+					289 => 1,
+					294 => 1,
+					297 => 1,
+					307 => 1,
+					313 => 1,
+					314 => 1,
+					315 => 1,
+					319 => 1,
+				);
+
+			default:
+				return array();
+		}
 	}
 
 	/**

From 7d39aad0eeb5f909d43fec1ccb51e808ea158be1 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 01:37:44 +0200
Subject: [PATCH 02/44] Security/EscapeOutput: move parse error related test to
 separate file

... to prevent it interfering with other tests.
---
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc | 2 +-
 WordPress/Tests/Security/EscapeOutputUnitTest.2.inc | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.2.inc

diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index b7a606bfc8..f9f1261667 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -181,7 +181,7 @@ echo '<option name="' . esc_attr( $name ) . '"' .
 _deprecated_hook( 'some_filter', '1.3.0', esc_html__( 'The $arg is deprecated.' ), 'some_other_filter' ); // Ok.
 _deprecated_hook( "filter_{$context}", '1.3.0', __( 'The $arg is deprecated.' ), sprintf( __( 'Some parsed message %s', $variable ) ) ); // Bad.
 
-echo add_filter( get_the_excerpt( get_the_ID() ) ; // Bad, but ignored as code contains a parse error.
+
 
 /*
  * Test using custom properties, setting & unsetting (resetting).
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.2.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.2.inc
new file mode 100644
index 0000000000..6a6c094bf3
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.2.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+echo add_filter( get_the_excerpt( get_the_ID() ) ;

From 2b5fa268adb3dbf516466482e39f0ff27967f0f6 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 30 Jun 2020 09:41:30 +0200
Subject: [PATCH 03/44] Security/EscapeOutput: use Tokens::$magicConstants

The `Tokens::$magicConstants` array was introduced to PHPCS in version 3.5.6, so we can now defer to the upstream array.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 20 +------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 0f00efcf0c..38b6beb097 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -59,24 +59,6 @@ class EscapeOutputSniff extends Sniff {
 		'_ex' => 'echo esc_html_x() or echo esc_attr_x()',
 	);
 
-	/**
-	 * List of names of the tokens representing PHP magic constants.
-	 *
-	 * @since 0.10.0
-	 *
-	 * @var array
-	 */
-	private $magic_constant_tokens = array(
-		'T_CLASS_C'  => true, // __CLASS__
-		'T_DIR'      => true, // __DIR__
-		'T_FILE'     => true, // __FILE__
-		'T_FUNC_C'   => true, // __FUNCTION__
-		'T_LINE'     => true, // __LINE__
-		'T_METHOD_C' => true, // __METHOD__
-		'T_NS_C'     => true, // __NAMESPACE__
-		'T_TRAIT_C'  => true, // __TRAIT__
-	);
-
 	/**
 	 * List of names of the native PHP constants which can be considered safe.
 	 *
@@ -296,7 +278,7 @@ public function process_token( $stackPtr ) {
 			}
 
 			// Handle magic constants for debug functions.
-			if ( isset( $this->magic_constant_tokens[ $this->tokens[ $i ]['type'] ] ) ) {
+			if ( isset( Tokens::$magicConstants[ $this->tokens[ $i ]['code'] ] ) ) {
 				continue;
 			}
 

From 93033c64bd82256bb087579ec975bffe64d391f9 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 01:54:11 +0200
Subject: [PATCH 04/44] Security/EscapeOutput: use PHPCSUtils

---
 WordPress/Sniffs/Security/EscapeOutputSniff.php | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 38b6beb097..6007fea77d 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -10,6 +10,7 @@
 namespace WordPressCS\WordPress\Sniffs\Security;
 
 use PHP_CodeSniffer\Util\Tokens;
+use PHPCSUtils\Tokens\Collections;
 use PHPCSUtils\Utils\PassedParameters;
 use PHPCSUtils\Utils\TextStrings;
 use WordPressCS\WordPress\Helpers\ArrayWalkingFunctionsHelper;
@@ -267,9 +268,7 @@ public function process_token( $stackPtr ) {
 				continue;
 			}
 
-			if ( \T_OPEN_SHORT_ARRAY === $this->tokens[ $i ]['code']
-				|| \T_CLOSE_SHORT_ARRAY === $this->tokens[ $i ]['code']
-			) {
+			if ( isset( Collections::shortArrayTokens()[ $this->tokens[ $i ]['code'] ] ) ) {
 				continue;
 			}
 

From 9af97f433e6204a874368bf8ac1a89b18aacdf0c Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 01:53:09 +0200
Subject: [PATCH 05/44] Security/EscapeOutput: normalize the `$safe_components`
 array

* Use token constants instead of type strings.
* Have the token constant as both the key and the value.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 6007fea77d..564cd5691f 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -86,20 +86,20 @@ class EscapeOutputSniff extends Sniff {
 	 * @var array
 	 */
 	private $safe_components = array(
-		'T_CONSTANT_ENCAPSED_STRING' => true,
-		'T_LNUMBER'                  => true,
-		'T_MINUS'                    => true,
-		'T_PLUS'                     => true,
-		'T_MULTIPLY'                 => true,
-		'T_DIVIDE'                   => true,
-		'T_MODULUS'                  => true,
-		'T_TRUE'                     => true,
-		'T_FALSE'                    => true,
-		'T_NULL'                     => true,
-		'T_DNUMBER'                  => true,
-		'T_START_NOWDOC'             => true,
-		'T_NOWDOC'                   => true,
-		'T_END_NOWDOC'               => true,
+		\T_CONSTANT_ENCAPSED_STRING => \T_CONSTANT_ENCAPSED_STRING,
+		\T_LNUMBER                  => \T_LNUMBER,
+		\T_MINUS                    => \T_MINUS,
+		\T_PLUS                     => \T_PLUS,
+		\T_MULTIPLY                 => \T_MULTIPLY,
+		\T_DIVIDE                   => \T_DIVIDE,
+		\T_MODULUS                  => \T_MODULUS,
+		\T_TRUE                     => \T_TRUE,
+		\T_FALSE                    => \T_FALSE,
+		\T_NULL                     => \T_NULL,
+		\T_DNUMBER                  => \T_DNUMBER,
+		\T_START_NOWDOC             => \T_START_NOWDOC,
+		\T_NOWDOC                   => \T_NOWDOC,
+		\T_END_NOWDOC               => \T_END_NOWDOC,
 	);
 
 	/**
@@ -314,7 +314,7 @@ public function process_token( $stackPtr ) {
 
 			// Allow T_CONSTANT_ENCAPSED_STRING eg: echo 'Some String';
 			// Also T_LNUMBER, e.g.: echo 45; exit -1; and booleans.
-			if ( isset( $this->safe_components[ $this->tokens[ $i ]['type'] ] ) ) {
+			if ( isset( $this->safe_components[ $this->tokens[ $i ]['code'] ] ) ) {
 				continue;
 			}
 

From 26095efafbf5f5f05b43a0fbf0fd646d49ab4e11 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 02:26:20 +0200
Subject: [PATCH 06/44] Security/EscapeOutput: bug fix - expand the "safe
 components" list

Operator tokens in PHP are not insecure, it is the **_operands_** which are potentially insecure.

As things were, we've seen all sorts of messages in bugs reported, where the sniff would report on `&&` or `===` tokens, which is nonsensical.

The "safe components" list already contained a number of operators, most notably the arithmetic operators (though the PHP 5.6+ `**` (`T_POW`) operator was missing).

I'm proposing to expand this list with a much larger list of operators using the predefined token arrays from PHPCS itself + one of the PHPCSUtils token arrays + the `T_BOOLEAN_NOT` (`!`) operator.

With this expansion, we shouldn't be seeing anymore issues reported for the sniff flagging operators, which should make debugging the issues reported more straight forward.

The only operators I have not (yet) included in the list are the assignment operators as assignments within an output statement should be exceedingly rare anyway.

Note: in all honesty, the sniff should take operator precedence into account properly and, for instance, disregard a `$var` or function call after a boolean operator as that is automatically cast to a boolean. Similarly, the operands before and after an arithmetic operator will always be cast to numeric. Having said that, that's a rabbit hole for a future release as there's enough to fix in the sniff without going down that path.

Includes unit tests, all of which would throw an error on the operator prior to this change.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 17 ++++++-----
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 29 +++++++++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.php   |  1 +
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 564cd5691f..cec1258041 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -81,25 +81,23 @@ class EscapeOutputSniff extends Sniff {
 	/**
 	 * List of tokens which can be considered as safe when directly part of the output.
 	 *
+	 * This list is enhanced with additional tokens in the `register()` method.
+	 *
 	 * @since 0.12.0
 	 *
 	 * @var array
 	 */
 	private $safe_components = array(
-		\T_CONSTANT_ENCAPSED_STRING => \T_CONSTANT_ENCAPSED_STRING,
 		\T_LNUMBER                  => \T_LNUMBER,
-		\T_MINUS                    => \T_MINUS,
-		\T_PLUS                     => \T_PLUS,
-		\T_MULTIPLY                 => \T_MULTIPLY,
-		\T_DIVIDE                   => \T_DIVIDE,
-		\T_MODULUS                  => \T_MODULUS,
+		\T_DNUMBER                  => \T_DNUMBER,
 		\T_TRUE                     => \T_TRUE,
 		\T_FALSE                    => \T_FALSE,
 		\T_NULL                     => \T_NULL,
-		\T_DNUMBER                  => \T_DNUMBER,
+		\T_CONSTANT_ENCAPSED_STRING => \T_CONSTANT_ENCAPSED_STRING,
 		\T_START_NOWDOC             => \T_START_NOWDOC,
 		\T_NOWDOC                   => \T_NOWDOC,
 		\T_END_NOWDOC               => \T_END_NOWDOC,
+		\T_BOOLEAN_NOT              => \T_BOOLEAN_NOT,
 	);
 
 	/**
@@ -108,6 +106,11 @@ class EscapeOutputSniff extends Sniff {
 	 * @return array
 	 */
 	public function register() {
+		// Enrich the list of "safe components" tokens.
+		$this->safe_components += Tokens::$comparisonTokens;
+		$this->safe_components += Tokens::$operators;
+		$this->safe_components += Tokens::$booleanOperators;
+		$this->safe_components += Collections::incrementDecrementOperators();
 
 		return array(
 			\T_ECHO,
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index f9f1261667..eff74c0808 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -317,3 +317,32 @@ echo implode( '<br>', map_deep( callback: 'foo', value: $items, ) ); // Bad, non
 // Note: named params not supported due to the `...$arrays` in array_map()`, but that's not the concern of this sniff.
 echo implode( '<br>', array_map( array: $items, callback: 'esc_html', ) ); // Ok.
 echo implode( '<br>', array_map( array: $items, callback: 'foo', ) ); // Bad.
+
+// Operators should be ignored.
+print 10 ** 20;
+print 10 & 20;
+print 10 | 20;
+print 10 ^ 20;
+print 10 << 20;
+print 10 >> 20;
+print 10 == 20;
+print 10 != 20;
+print 10 === 20;
+print 10 !== 20;
+print 10 < 20;
+print 10 > 20;
+print 10 <= 20;
+print 10 >= 20;
+print 10 <=> 20;
+print ! 'hello';
+print 'hello' && 'world';
+print 'hello' || 'world';
+print 'hello' and 'world';
+print 'hello' or 'world';
+print 'hello' xor 'world';
+print 10++;
+print --10;
+
+// This includes the PHP 7.0+ null coalesce operator.
+echo $var ?? 'default'; // Bad.
+echo esc_html( $var ?? 'default' ); // OK.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 8a65bff2c0..05fa16a4e2 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -105,6 +105,7 @@ public function getErrorList( $testFile = '' ) {
 					314 => 1,
 					315 => 1,
 					319 => 1,
+					347 => 1,
 				);
 
 			default:

From 268d4159943929b247ec7611a638d27149e41d39 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Sun, 23 Jul 2023 23:51:21 +0200
Subject: [PATCH 07/44] Security/EscapeOutput: split `process_token()` method
 [1]

Split off the code which does the actual output escaping check into a separate `check_code_is_escaped()` method.

No functional changes.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 23 +++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index cec1258041..dd93dc495e 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -124,6 +124,8 @@ public function register() {
 	/**
 	 * Processes this test, when one of its tokens is encountered.
 	 *
+	 * @since 3.0.0 This method has been split up.
+	 *
 	 * @param int $stackPtr The position of the current token in the stack.
 	 *
 	 * @return int|void Integer stack pointer to skip forward or void to continue
@@ -217,11 +219,28 @@ public function process_token( $stackPtr ) {
 		// Ignore the function itself.
 		++$stackPtr;
 
+		return $this->check_code_is_escaped( $stackPtr, $end_of_statement, $ternary );
+	}
+
+	/**
+	 * Check whether each relevant part of an arbitrary group of token is output escaped.
+	 *
+	 * @since 3.0.0 Split off from the process_token() method.
+	 *
+	 * @param int       $start   The position to start checking from.
+	 * @param int       $end     The position to stop the check at.
+	 * @param int|false $ternary Stack pointer to the `?` inline then token or
+	 *                           FALSE when no non-parenthesized ternary was found.
+	 *
+	 * @return int Integer stack pointer to skip forward.
+	 */
+	protected function check_code_is_escaped( $start, $end, $ternary = false ) {
+
 		$in_cast = false;
 
 		// Looping through echo'd components.
 		$watch = true;
-		for ( $i = $stackPtr; $i < $end_of_statement; $i++ ) {
+		for ( $i = $start; $i < $end; $i++ ) {
 
 			// Ignore whitespaces and comments.
 			if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) ) {
@@ -412,6 +431,6 @@ public function process_token( $stackPtr ) {
 			);
 		}
 
-		return $end_of_statement;
+		return $end;
 	}
 }

From 42218322ae36461fd52168522b2348ac02bf5a47 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Mon, 24 Jul 2023 02:32:54 +0200
Subject: [PATCH 08/44] Security/EscapeOutput: split `process_token()` method
 [2]

Split off the code which checks for function calls into a separate `process_tstring_token()` method.

No functional changes.

:point_right: This commit will be easier to review while ignoring whitespace changes.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 130 ++++++++++--------
 1 file changed, 74 insertions(+), 56 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index dd93dc495e..320c6a0f43 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -133,51 +133,91 @@ public function register() {
 	 */
 	public function process_token( $stackPtr ) {
 
-		$function = $this->tokens[ $stackPtr ]['content'];
+		// If function, not T_ECHO nor T_PRINT.
+		if ( \T_STRING === $this->tokens[ $stackPtr ]['code'] ) {
+			return $this->process_tstring_token( $stackPtr );
+		}
 
 		// Find the opening parenthesis (if present; T_ECHO might not have it).
 		$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
 
-		// If function, not T_ECHO nor T_PRINT.
-		if ( \T_STRING === $this->tokens[ $stackPtr ]['code'] ) {
-			// Skip if it is a function but is not one of the printing functions.
-			if ( ! $this->is_printing_function( $this->tokens[ $stackPtr ]['content'] ) ) {
-				return;
-			}
+		$ternary = false;
+
+		$end_of_statement = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), $stackPtr );
+		$last_token       = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end_of_statement - 1 ), null, true );
+
+		// Check for the ternary operator. We only need to do this here if this
+		// echo is lacking parenthesis. Otherwise it will be handled below.
+		if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code'] || \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_token ]['code'] ) {
 
-			if ( isset( $this->tokens[ $open_paren ]['parenthesis_closer'] ) ) {
-				$end_of_statement = $this->tokens[ $open_paren ]['parenthesis_closer'];
+			$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $stackPtr, $end_of_statement );
+
+			// If there is a ternary skip over the part before the ?. However, if
+			// the ternary is within parentheses, it will be handled in the loop.
+			if ( false !== $ternary && empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) ) {
+				$stackPtr = $ternary;
 			}
+		}
 
-			// These functions only need to have the first argument escaped.
-			if ( \in_array( $function, array( 'trigger_error', 'user_error' ), true ) ) {
-				$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
-				if ( false === $first_param ) {
-					// First parameter doesn't exist. Nothing to do.
-					return;
-				}
+		// Ignore the function itself.
+		++$stackPtr;
+
+		return $this->check_code_is_escaped( $stackPtr, $end_of_statement, $ternary );
+	}
+
+	/**
+	 * Process a function call token.
+	 *
+	 * @since 3.0.0 Split off from the process_token() method.
+	 *
+	 * @param int $stackPtr The position of the current token in the stack.
+	 *
+	 * @return int|void Integer stack pointer to skip forward or void to continue
+	 *                  normal file processing.
+	 */
+	public function process_tstring_token( $stackPtr ) {
+		// Find the opening parenthesis.
+		$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+
+		$function = $this->tokens[ $stackPtr ]['content'];
+
+		// Skip if it is a function but is not one of the printing functions.
+		if ( ! $this->is_printing_function( $this->tokens[ $stackPtr ]['content'] ) ) {
+			return;
+		}
 
-				$end_of_statement = ( $first_param['end'] + 1 );
-				unset( $first_param );
+		if ( isset( $this->tokens[ $open_paren ]['parenthesis_closer'] ) ) {
+			$end_of_statement = $this->tokens[ $open_paren ]['parenthesis_closer'];
+		}
+
+		// These functions only need to have the first argument escaped.
+		if ( \in_array( $function, array( 'trigger_error', 'user_error' ), true ) ) {
+			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
+			if ( false === $first_param ) {
+				// First parameter doesn't exist. Nothing to do.
+				return;
 			}
 
-			/*
-			 * If the first param to `_deprecated_file()` follows the typical `basename( __FILE__ )`
-			 * pattern, it doesn't need to be escaped.
-			 */
-			if ( '_deprecated_file' === $function ) {
-				$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
-				if ( false === $first_param ) {
-					// First parameter doesn't exist. Nothing to do.
-					return;
-				}
+			$end_of_statement = ( $first_param['end'] + 1 );
+			unset( $first_param );
+		}
 
-				// Quick check. This disregards comments.
-				if ( preg_match( '`^[\\\\]?basename\s*\(\s*__FILE__\s*\)$`', $first_param['raw'] ) === 1 ) {
-					$stackPtr = ( $first_param['end'] + 2 );
-				}
-				unset( $first_param );
+		/*
+		 * If the first param to `_deprecated_file()` follows the typical `basename( __FILE__ )`
+		 * pattern, it doesn't need to be escaped.
+		 */
+		if ( '_deprecated_file' === $function ) {
+			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
+			if ( false === $first_param ) {
+				// First parameter doesn't exist. Nothing to do.
+				return;
 			}
+
+			// Quick check. This disregards comments.
+			if ( preg_match( '`^[\\\\]?basename\s*\(\s*__FILE__\s*\)$`', $first_param['raw'] ) === 1 ) {
+				$stackPtr = ( $first_param['end'] + 2 );
+			}
+			unset( $first_param );
 		}
 
 		if ( isset( $this->unsafePrintingFunctions[ $function ] ) ) {
@@ -194,32 +234,10 @@ public function process_token( $stackPtr ) {
 			}
 		}
 
-		$ternary = false;
-
-		// This is already determined if this is a function and not T_ECHO.
-		if ( ! isset( $end_of_statement ) ) {
-
-			$end_of_statement = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), $stackPtr );
-			$last_token       = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end_of_statement - 1 ), null, true );
-
-			// Check for the ternary operator. We only need to do this here if this
-			// echo is lacking parenthesis. Otherwise it will be handled below.
-			if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code'] || \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_token ]['code'] ) {
-
-				$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $stackPtr, $end_of_statement );
-
-				// If there is a ternary skip over the part before the ?. However, if
-				// the ternary is within parentheses, it will be handled in the loop.
-				if ( false !== $ternary && empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) ) {
-					$stackPtr = $ternary;
-				}
-			}
-		}
-
 		// Ignore the function itself.
 		++$stackPtr;
 
-		return $this->check_code_is_escaped( $stackPtr, $end_of_statement, $ternary );
+		return $this->check_code_is_escaped( $stackPtr, $end_of_statement );
 	}
 
 	/**

From 51dd36493d9ab9935414371e8689191002abda9a Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Mon, 24 Jul 2023 02:45:34 +0200
Subject: [PATCH 09/44] Security/EscapeOutput: bug fix - false positives on
 non-function calls

As things were, the sniff would look for `T_STRING` tokens with certain contents, but never do a proper check if those were actually function calls.

This would lead to false positives, but also potentially to an `Internal.Exception` for the `$end_of_statement` not being defined.

This commit is a first step towards fixing this.

Includes tests for select situations fixed by this particular commit.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php     | 13 +++++++++----
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc |  5 +++++
 WordPress/Tests/Security/EscapeOutputUnitTest.3.inc |  5 +++++
 3 files changed, 19 insertions(+), 4 deletions(-)
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.3.inc

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 320c6a0f43..1bdcbc386b 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -178,6 +178,13 @@ public function process_token( $stackPtr ) {
 	public function process_tstring_token( $stackPtr ) {
 		// Find the opening parenthesis.
 		$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+		if ( false === $open_paren
+			|| \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code']
+			|| isset( $this->tokens[ $open_paren ]['parenthesis_closer'] ) === false
+		) {
+			// Live coding, parse error or not a function _call_.
+			return;
+		}
 
 		$function = $this->tokens[ $stackPtr ]['content'];
 
@@ -186,9 +193,7 @@ public function process_tstring_token( $stackPtr ) {
 			return;
 		}
 
-		if ( isset( $this->tokens[ $open_paren ]['parenthesis_closer'] ) ) {
-			$end_of_statement = $this->tokens[ $open_paren ]['parenthesis_closer'];
-		}
+		$end_of_statement = $this->tokens[ $open_paren ]['parenthesis_closer'];
 
 		// These functions only need to have the first argument escaped.
 		if ( \in_array( $function, array( 'trigger_error', 'user_error' ), true ) ) {
@@ -230,7 +235,7 @@ public function process_tstring_token( $stackPtr ) {
 
 			// If the error was reported, don't bother checking the function's arguments.
 			if ( $error ) {
-				return isset( $end_of_statement ) ? $end_of_statement : null;
+				return $end_of_statement;
 			}
 		}
 
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index eff74c0808..5e8ac7420b 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -346,3 +346,8 @@ print --10;
 // This includes the PHP 7.0+ null coalesce operator.
 echo $var ?? 'default'; // Bad.
 echo esc_html( $var ?? 'default' ); // OK.
+
+// Make sure the sniff does not get confused over constants/properties using the same name as one of the target functions.
+$a = _ex; // OK, constant, not function call.
+$a = $obj->wp_dropdown_pages; // OK, property access, not function call.
+use function wp_dropdown_pages; // OK, import use statement, not function call.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.3.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.3.inc
new file mode 100644
index 0000000000..f4cc5d9fec
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.3.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+_deprecated_function(

From 8d1fffba078c7954c1885b7afb04d3d472035caa Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Sun, 23 Jul 2023 22:27:49 +0200
Subject: [PATCH 10/44] Helpers/PrintingFunctionsTrait: add
 `get_printing_functions()` method

... which is needed for the switch to another base class is the next commit.
---
 WordPress/Helpers/PrintingFunctionsTrait.php | 23 +++++++++++++++-----
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/WordPress/Helpers/PrintingFunctionsTrait.php b/WordPress/Helpers/PrintingFunctionsTrait.php
index ff1df2b190..0b210e7131 100644
--- a/WordPress/Helpers/PrintingFunctionsTrait.php
+++ b/WordPress/Helpers/PrintingFunctionsTrait.php
@@ -87,15 +87,13 @@ trait PrintingFunctionsTrait {
 	private $allPrintingFunctions = array();
 
 	/**
-	 * Check if a particular function is regarded as a printing function.
+	 * Retrieve a list of all known printing functions.
 	 *
 	 * @since 3.0.0
 	 *
-	 * @param string $functionName The name of the function to check.
-	 *
-	 * @return bool
+	 * @return array<string, bool>
 	 */
-	public function is_printing_function( $functionName ) {
+	public function get_printing_functions() {
 		if ( array() === $this->allPrintingFunctions
 			|| $this->customPrintingFunctions !== $this->addedCustomPrintingFunctions
 		) {
@@ -107,6 +105,19 @@ public function is_printing_function( $functionName ) {
 			$this->addedCustomPrintingFunctions = $this->customPrintingFunctions;
 		}
 
-		return isset( $this->allPrintingFunctions[ strtolower( $functionName ) ] );
+		return $this->allPrintingFunctions;
+	}
+
+	/**
+	 * Check if a particular function is regarded as a printing function.
+	 *
+	 * @since 3.0.0
+	 *
+	 * @param string $functionName The name of the function to check.
+	 *
+	 * @return bool
+	 */
+	public function is_printing_function( $functionName ) {
+		return isset( $this->get_printing_functions()[ strtolower( $functionName ) ] );
 	}
 }

From 4f57d4c5a637fc50e11f0ee69bf869a0af439175 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Mon, 24 Jul 2023 02:21:32 +0200
Subject: [PATCH 11/44] Security/EscapeOutput: bug fix - false positives on
 non-global function calls

As things were, the sniff would look for `T_STRING` tokens with certain contents, but never do a proper check if those were actually function calls to the WP/PHP native _global_ function.

This would lead to false positives for calls to methods, function declarations etc.

By changing the parent class to the `AbstractFunctionRestrictionsSniff`, we can re-use the "if this a global function call" logic already in place in that abstract, while still maintaining the handling of the other tokens in the pre-existing manner.

There are three caveats to this:
1. The `AbstractFunctionRestrictionsSniff` contains an `exclude` property, which allows to silence notices about certain function for a complete group.
    As this is a security sniff, we don't want end-users to be able to silence notices about groups of functions in that way.
    To that end, the ability for end-users to set the `exclude` property for this sniff is being disabled.
2. The `AbstractFunctionRestrictionsSniff` class will call the `process_matched_token()` method once for each match in each function group.
    As the "unsafe printing functions" are also included in the "printing functions", this would lead to the `process_matched_token()` method being called twice for those methods, leading to duplicate error notices.
    I'm preventing this by removing the "unsafe printing functions" from the "printing functions" list prior to passing it off to the parent class.
3. Lastly, the `AbstractFunctionRestrictionsSniff` class expects the list of target functions to be available when the sniff is instantiated. However, in test situations, that list can fluctuate, as the "printing functions" supports the ability to add "custom printing functions", which is done on the fly in the test case file using `// phpcs:set ...`.
    To get round that, I'm re-registering the "function groups" every time a `T_STRING` (potential function call) triggers the sniff, but only when the tests are being run.
    While it is possible for end-users to use the `// phpcs:set` syntax, it is little known and is not mentioned anywhere in the documentation as a feature for end-users, so just re-registering the function groups for test runs would seem sufficient.

Includes tests for situations fixed by this particular commit.
Includes a test using the PHP 8.0+ nullsafe object operator.

Note: the `PrintingFunctionsTrait::is_printing_function()` has now become unused and _could_ be removed.
For consistency with other traits/helper classes, I'm electing to leave it be (for now) though.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 69 ++++++++++++++-----
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 11 +++
 2 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 1bdcbc386b..407bfd2ddc 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -13,6 +13,7 @@
 use PHPCSUtils\Tokens\Collections;
 use PHPCSUtils\Utils\PassedParameters;
 use PHPCSUtils\Utils\TextStrings;
+use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff;
 use WordPressCS\WordPress\Helpers\ArrayWalkingFunctionsHelper;
 use WordPressCS\WordPress\Helpers\ContextHelper;
 use WordPressCS\WordPress\Helpers\ConstantsHelper;
@@ -20,7 +21,6 @@
 use WordPressCS\WordPress\Helpers\FormattingFunctionsHelper;
 use WordPressCS\WordPress\Helpers\PrintingFunctionsTrait;
 use WordPressCS\WordPress\Helpers\VariableHelper;
-use WordPressCS\WordPress\Sniff;
 
 /**
  * Verifies that all outputted strings are escaped.
@@ -37,12 +37,15 @@
  *                 echo tags `<?=`.
  * @since   0.13.0 Class name changed: this class is now namespaced.
  * @since   1.0.0  This sniff has been moved from the `XSS` category to the `Security` category.
+ * @since   3.0.0  This class now extends the WordPressCS native
+ *                 `AbstractFunctionRestrictionsSniff` class.
+ *                 The parent `exclude` property is disabled.
  *
  * @uses    \WordPressCS\WordPress\Helpers\EscapingFunctionsTrait::$customEscapingFunctions
  * @uses    \WordPressCS\WordPress\Helpers\EscapingFunctionsTrait::$customAutoEscapedFunctions
  * @uses    \WordPressCS\WordPress\Helpers\PrintingFunctionsTrait::$customPrintingFunctions
  */
-class EscapeOutputSniff extends Sniff {
+class EscapeOutputSniff extends AbstractFunctionRestrictionsSniff {
 
 	use EscapingFunctionsTrait;
 	use PrintingFunctionsTrait;
@@ -112,12 +115,37 @@ public function register() {
 		$this->safe_components += Tokens::$booleanOperators;
 		$this->safe_components += Collections::incrementDecrementOperators();
 
+		// Set up the tokens the sniff should listen too.
+		$targets   = parent::register();
+		$targets[] = \T_ECHO;
+		$targets[] = \T_PRINT;
+		$targets[] = \T_EXIT;
+		$targets[] = \T_OPEN_TAG_WITH_ECHO;
+
+		return $targets;
+	}
+
+	/**
+	 * Groups of functions this sniff is looking for.
+	 *
+	 * @since 3.0.0
+	 *
+	 * @return array
+	 */
+	public function getGroups() {
+		// Make sure all array keys are lowercase (could contain user provided function names).
+		$printing_functions = array_change_key_case( $this->get_printing_functions(), \CASE_LOWER );
+
+		// Remove the unsafe printing functions to prevent duplicate notices.
+		$printing_functions = array_diff_key( $printing_functions, $this->unsafePrintingFunctions );
+
 		return array(
-			\T_ECHO,
-			\T_PRINT,
-			\T_EXIT,
-			\T_STRING,
-			\T_OPEN_TAG_WITH_ECHO,
+			'unsafe_printing_functions' => array(
+				'functions' => array_keys( $this->unsafePrintingFunctions ),
+			),
+			'printing_functions' => array(
+				'functions' => array_keys( $printing_functions ),
+			),
 		);
 	}
 
@@ -132,10 +160,17 @@ public function register() {
 	 *                  normal file processing.
 	 */
 	public function process_token( $stackPtr ) {
+		// Prevent exclusion of any of the function groups.
+		$this->exclude = array();
 
-		// If function, not T_ECHO nor T_PRINT.
+		// Let the abstract parent class handle the initial function call check.
 		if ( \T_STRING === $this->tokens[ $stackPtr ]['code'] ) {
-			return $this->process_tstring_token( $stackPtr );
+			// In the tests, custom printing functions may be added/removed on the fly.
+			if ( defined( 'PHP_CODESNIFFER_IN_TESTS' ) ) {
+				$this->setup_groups( 'functions' );
+			}
+
+			return parent::process_token( $stackPtr );
 		}
 
 		// Find the opening parenthesis (if present; T_ECHO might not have it).
@@ -166,17 +201,20 @@ public function process_token( $stackPtr ) {
 	}
 
 	/**
-	 * Process a function call token.
+	 * Process a matched function call token.
 	 *
 	 * @since 3.0.0 Split off from the process_token() method.
 	 *
-	 * @param int $stackPtr The position of the current token in the stack.
+	 * @param int    $stackPtr        The position of the current token in the stack.
+	 * @param string $group_name      The name of the group which was matched.
+	 * @param string $matched_content The token content (function name) which was matched
+	 *                                in lowercase.
 	 *
 	 * @return int|void Integer stack pointer to skip forward or void to continue
 	 *                  normal file processing.
 	 */
-	public function process_tstring_token( $stackPtr ) {
-		// Find the opening parenthesis.
+	public function process_matched_token( $stackPtr, $group_name, $matched_content ) {
+		// Make sure we only deal with actual function calls, not function import use statements.
 		$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
 		if ( false === $open_paren
 			|| \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code']
@@ -188,11 +226,6 @@ public function process_tstring_token( $stackPtr ) {
 
 		$function = $this->tokens[ $stackPtr ]['content'];
 
-		// Skip if it is a function but is not one of the printing functions.
-		if ( ! $this->is_printing_function( $this->tokens[ $stackPtr ]['content'] ) ) {
-			return;
-		}
-
 		$end_of_statement = $this->tokens[ $open_paren ]['parenthesis_closer'];
 
 		// These functions only need to have the first argument escaped.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 5e8ac7420b..77567a9f65 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -351,3 +351,14 @@ echo esc_html( $var ?? 'default' ); // OK.
 $a = _ex; // OK, constant, not function call.
 $a = $obj->wp_dropdown_pages; // OK, property access, not function call.
 use function wp_dropdown_pages; // OK, import use statement, not function call.
+
+// Make sure the sniff does not get confused over methods/namespaced functions etc vs global function calls.
+$obj->_deprecated_file( $file, '1.3.0' ); // OK.
+$obj?->_deprecated_file( $file, '1.3.0' ); // OK.
+MyClass::_deprecated_file( $file, '1.3.0' ); // OK.
+My\NS\_ex( $some_nasty_var, 'context' ); // OK.
+class IgnoreFunctionDeclarations {
+	function wp_die( $foo ) {} // OK.
+	function &trigger_error( $foo ) {} // OK.
+}
+$obj = new User_Error( $foo ); // OK.

From ecbc52f1d2284600c564feaec4abd258bf223bbc Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Mon, 24 Jul 2023 20:51:15 +0200
Subject: [PATCH 12/44] Security/EscapeOutput: bug fix - function names are
 case-insensitive

While the initial function name check was previously already made case-insensitive in 2273 (commit 53309ac1f26d945268534c8f7f8ccb6080147a13) and remained case-insensitive with the switch to the `AbstractFunctionRestrictionsSniff` base class, the _special casing_ function name checks were **not** done in a case-insensitive manner.

This could lead to false positives as well as false negatives.

Fixed now.

Includes tests proving the bug.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php     | 10 ++++------
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc |  5 +++++
 WordPress/Tests/Security/EscapeOutputUnitTest.php   |  1 +
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 407bfd2ddc..8ce5250077 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -224,12 +224,10 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 			return;
 		}
 
-		$function = $this->tokens[ $stackPtr ]['content'];
-
 		$end_of_statement = $this->tokens[ $open_paren ]['parenthesis_closer'];
 
 		// These functions only need to have the first argument escaped.
-		if ( \in_array( $function, array( 'trigger_error', 'user_error' ), true ) ) {
+		if ( \in_array( $matched_content, array( 'trigger_error', 'user_error' ), true ) ) {
 			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
 			if ( false === $first_param ) {
 				// First parameter doesn't exist. Nothing to do.
@@ -244,7 +242,7 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 		 * If the first param to `_deprecated_file()` follows the typical `basename( __FILE__ )`
 		 * pattern, it doesn't need to be escaped.
 		 */
-		if ( '_deprecated_file' === $function ) {
+		if ( '_deprecated_file' === $matched_content ) {
 			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
 			if ( false === $first_param ) {
 				// First parameter doesn't exist. Nothing to do.
@@ -258,12 +256,12 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 			unset( $first_param );
 		}
 
-		if ( isset( $this->unsafePrintingFunctions[ $function ] ) ) {
+		if ( 'unsafe_printing_functions' === $group_name ) {
 			$error = $this->phpcsFile->addError(
 				"All output should be run through an escaping function (like %s), found '%s'.",
 				$stackPtr,
 				'UnsafePrintingFunction',
-				array( $this->unsafePrintingFunctions[ $function ], $function )
+				array( $this->unsafePrintingFunctions[ $matched_content ], $matched_content )
 			);
 
 			// If the error was reported, don't bother checking the function's arguments.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 77567a9f65..6952031b03 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -362,3 +362,8 @@ class IgnoreFunctionDeclarations {
 	function &trigger_error( $foo ) {} // OK.
 }
 $obj = new User_Error( $foo ); // OK.
+
+// Make sure special casing of select functions is handled case-insensitively.
+Trigger_ERROR( 'This is fine', $second_param_should_be_ignored ); // OK.
+_Deprecated_File( basename( __FILE__ ), '1.3.0' ); // OK.
+_EX( 'all_params_should_be_ignored_if_function_is_reported_as_unsafe', 'another_param' ); // Bad x 1 for unsafe function.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 05fa16a4e2..b17deee01a 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -106,6 +106,7 @@ public function getErrorList( $testFile = '' ) {
 					315 => 1,
 					319 => 1,
 					347 => 1,
+					369 => 1,
 				);
 
 			default:

From 530de77079a59c04257c3cf9cff4c848cea7c15e Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 02:15:09 +0200
Subject: [PATCH 13/44] Security/EscapeOutput: prevent false positive due to
 comments in param

The `'raw'` key in the parameter arrays returned from the `PassedParameters` class contains - as per the name - the _raw_ contents of the parameter.

Since PHPCSUtils 1.0.0-alpha4, the return array also contain a `'clean'` index, which contains the contents of the parameter cleaned of comments.

By switching to using that key, a potential false positive gets fixed.

Includes unit test demonstrating the issue and safeguarding the fix.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php     | 4 ++--
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc | 7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 8ce5250077..fe126aff98 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -249,8 +249,8 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 				return;
 			}
 
-			// Quick check. This disregards comments.
-			if ( preg_match( '`^[\\\\]?basename\s*\(\s*__FILE__\s*\)$`', $first_param['raw'] ) === 1 ) {
+			// Check for a particular code pattern which can safely be ignored.
+			if ( preg_match( '`^[\\\\]?basename\s*\(\s*__FILE__\s*\)$`', $first_param['clean'] ) === 1 ) {
 				$stackPtr = ( $first_param['end'] + 2 );
 			}
 			unset( $first_param );
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 6952031b03..ad67ebb223 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -367,3 +367,10 @@ $obj = new User_Error( $foo ); // OK.
 Trigger_ERROR( 'This is fine', $second_param_should_be_ignored ); // OK.
 _Deprecated_File( basename( __FILE__ ), '1.3.0' ); // OK.
 _EX( 'all_params_should_be_ignored_if_function_is_reported_as_unsafe', 'another_param' ); // Bad x 1 for unsafe function.
+
+// Allow for comments in the $file parameter.
+_deprecated_file(
+	/* comment */
+	basename( __FILE__ ),
+	'1.3.0'
+); // Ok.

From 616c2c9bcd9b0ef1a52d2a449175dbf6e3e5d8de Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 25 Jul 2023 01:45:38 +0200
Subject: [PATCH 14/44] Security/EscapeOutput: code readability improvements
 [1]

Minor tweaks to the code in the `process_token()` method for improved comprehensibility of the code.

Rename some variables, add some documentation, split up long conditions.

Includes preventing the code from changing the `$stackPtr` as that is just really confusing.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index fe126aff98..98c6c82346 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -173,31 +173,31 @@ public function process_token( $stackPtr ) {
 			return parent::process_token( $stackPtr );
 		}
 
-		// Find the opening parenthesis (if present; T_ECHO might not have it).
-		$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
-
+		// Handle the other tokens we're sniffing for.
+		$start   = ( $stackPtr + 1 );
+		$end     = $start;
 		$ternary = false;
 
-		$end_of_statement = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), $stackPtr );
-		$last_token       = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end_of_statement - 1 ), null, true );
+		// Find a potential opening parenthesis (if present).
+		$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+		$end        = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), $stackPtr );
+		$last_token = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end - 1 ), null, true );
 
 		// Check for the ternary operator. We only need to do this here if this
 		// echo is lacking parenthesis. Otherwise it will be handled below.
-		if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code'] || \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_token ]['code'] ) {
-
-			$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $stackPtr, $end_of_statement );
+		if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code']
+			|| \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_token ]['code']
+		) {
+			$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $stackPtr, $end );
 
 			// If there is a ternary skip over the part before the ?. However, if
 			// the ternary is within parentheses, it will be handled in the loop.
 			if ( false !== $ternary && empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) ) {
-				$stackPtr = $ternary;
+				$start = ( $ternary + 1 );
 			}
 		}
 
-		// Ignore the function itself.
-		++$stackPtr;
-
-		return $this->check_code_is_escaped( $stackPtr, $end_of_statement, $ternary );
+		return $this->check_code_is_escaped( $start, $end, $ternary );
 	}
 
 	/**

From 511787d6fe9f03319ac76f50300f107c71cf28af Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 25 Jul 2023 01:52:10 +0200
Subject: [PATCH 15/44] Security/EscapeOutput: bug fix - only examine exit/die
 with parentheses

`exit`/`die` are language constructs and can be used as part of an expression.

Additionally, an `exit`/`die` can be stand-alone or can pass a status code/message. If a status code/message is passed, we want it to be escaped for output, but a status code/message can only be passed within parentheses.

As things were, the sniff would not ignore `exit`/`die` statements without status and when used within an expression, would walk the tokens after the `exit`/`die`, leading to false positives.

Fixed now by using custom logic for `T_EXIT` tokens.
Includes parse error/live coding protection, where the sniff should stay silent.

As the method will now use a `switch` control structure, move the special casing for `T_STRING` also into the `switch`.

Includes tests for situations fixed by this particular commit.

:point_right: This commit will be easier to review while ignoring whitespace changes.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 76 ++++++++++++-------
 .../Tests/Security/EscapeOutputUnitTest.1.inc |  5 ++
 .../Tests/Security/EscapeOutputUnitTest.4.inc |  5 ++
 .../Tests/Security/EscapeOutputUnitTest.5.inc |  5 ++
 .../Tests/Security/EscapeOutputUnitTest.php   |  1 +
 5 files changed, 63 insertions(+), 29 deletions(-)
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.4.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.5.inc

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 98c6c82346..a10a9edbd8 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -160,41 +160,59 @@ public function getGroups() {
 	 *                  normal file processing.
 	 */
 	public function process_token( $stackPtr ) {
-		// Prevent exclusion of any of the function groups.
-		$this->exclude = array();
-
-		// Let the abstract parent class handle the initial function call check.
-		if ( \T_STRING === $this->tokens[ $stackPtr ]['code'] ) {
-			// In the tests, custom printing functions may be added/removed on the fly.
-			if ( defined( 'PHP_CODESNIFFER_IN_TESTS' ) ) {
-				$this->setup_groups( 'functions' );
-			}
-
-			return parent::process_token( $stackPtr );
-		}
-
-		// Handle the other tokens we're sniffing for.
 		$start   = ( $stackPtr + 1 );
 		$end     = $start;
 		$ternary = false;
 
-		// Find a potential opening parenthesis (if present).
-		$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
-		$end        = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), $stackPtr );
-		$last_token = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end - 1 ), null, true );
+		switch ( $this->tokens[ $stackPtr ]['code'] ) {
+			case \T_STRING:
+				// Prevent exclusion of any of the function groups.
+				$this->exclude = array();
 
-		// Check for the ternary operator. We only need to do this here if this
-		// echo is lacking parenthesis. Otherwise it will be handled below.
-		if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code']
-			|| \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_token ]['code']
-		) {
-			$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $stackPtr, $end );
+				// In the tests, custom printing functions may be added/removed on the fly.
+				if ( defined( 'PHP_CODESNIFFER_IN_TESTS' ) ) {
+					$this->setup_groups( 'functions' );
+				}
 
-			// If there is a ternary skip over the part before the ?. However, if
-			// the ternary is within parentheses, it will be handled in the loop.
-			if ( false !== $ternary && empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) ) {
-				$start = ( $ternary + 1 );
-			}
+				// Let the abstract parent class handle the initial function call check.
+				return parent::process_token( $stackPtr );
+
+			case \T_EXIT:
+				$next_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+				if ( false === $next_non_empty
+					|| \T_OPEN_PARENTHESIS !== $this->tokens[ $next_non_empty ]['code']
+					|| isset( $this->tokens[ $next_non_empty ]['parenthesis_closer'] ) === false
+				) {
+					// Live coding/parse error or an exit/die which doesn't pass a status code. Ignore.
+					return;
+				}
+
+				// $end is not examined, so make sure the parentheses are balanced.
+				$start = $next_non_empty;
+				$end   = ( $this->tokens[ $next_non_empty ]['parenthesis_closer'] + 1 );
+				break;
+
+			default:
+				// Find a potential opening parenthesis (if present).
+				$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+				$end        = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), $stackPtr );
+				$last_token = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end - 1 ), null, true );
+
+				// Check for the ternary operator. We only need to do this here if this
+				// echo is lacking parenthesis. Otherwise it will be handled below.
+				if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code']
+					|| \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_token ]['code']
+				) {
+					$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $stackPtr, $end );
+
+					// If there is a ternary skip over the part before the ?. However, if
+					// the ternary is within parentheses, it will be handled in the loop.
+					if ( false !== $ternary && empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) ) {
+						$start = ( $ternary + 1 );
+					}
+				}
+
+				break;
 		}
 
 		return $this->check_code_is_escaped( $start, $end, $ternary );
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index ad67ebb223..8575c1ef57 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -374,3 +374,8 @@ _deprecated_file(
 	basename( __FILE__ ),
 	'1.3.0'
 ); // Ok.
+
+// Exit/die should only be examined when there are parentheses.
+$var = (true || exit ) && empty( $bar ) ? $drink_alone : $drink_together; // OK, exit is not passing status.
+$var = (true or die ) and empty( $bar ) ? $drink_alone : $drink_together; // OK, die is not passing status.
+$var = exit( $var ? $ok : $error ); // Bad x 2.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.4.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.4.inc
new file mode 100644
index 0000000000..99df74f22b
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.4.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+exit
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.5.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.5.inc
new file mode 100644
index 0000000000..bf3d234bca
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.5.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+die(
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index b17deee01a..4c1e54ed54 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -107,6 +107,7 @@ public function getErrorList( $testFile = '' ) {
 					319 => 1,
 					347 => 1,
 					369 => 1,
+					381 => 2,
 				);
 
 			default:

From 793fc7ec33c9b882379ea78fea6478b21a2d736d Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 25 Jul 2023 02:17:13 +0200
Subject: [PATCH 16/44] Security/EscapeOutput: bug fix - improve start/end
 determination for print statements

`print` is a language construct and can be used as part of an expression.

The sniff did not take this into account correctly. This would lead to the sniff walking too far and examining code which is not part of the `print` statement. This would lead to both false positives as well as false negatives.

This commit improves that situation, but doesn't completely fix it.

First off, the sniff now uses the `BCFile::findEndOfStatement()` method to more accurately retrieve the end token for the expression. This should improve the handling of the most common cases.
This part includes live coding/parse error protection for unfinished `print` statements at the end of a file.

However, the `[BC]File::findEndOfStatement()` method does not take ternaries into account when determining the end of the expression.

So, secondly, code has been added to special case `print` statements in the "then" part of a ternary and determine the end of such an expression correctly.

Includes tests for situations fixed by this particular commit.

Additional notes:
* I can still come up with more (exotic) test cases with `print` in ternaries which would still not be handled correctly.
    I'm leaving those as "future scope".
    Example:
    ```php
    ( $fop === 1 ) ? true && print $foo : print $bar; // The first print will now examine everything to the end of the statement, not to the end of the expression, which means that `$bar` will not be flagged as the sniff bow out before it reached $bar.
    ```

Fixes 2209
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 67 +++++++++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 15 +++++
 .../Security/EscapeOutputUnitTest.10.inc      |  4 ++
 .../Tests/Security/EscapeOutputUnitTest.6.inc |  4 ++
 .../Tests/Security/EscapeOutputUnitTest.7.inc |  4 ++
 .../Tests/Security/EscapeOutputUnitTest.8.inc |  5 ++
 .../Tests/Security/EscapeOutputUnitTest.9.inc |  5 ++
 .../Tests/Security/EscapeOutputUnitTest.php   |  9 +++
 8 files changed, 113 insertions(+)
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.10.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.6.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.7.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.8.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.9.inc

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index a10a9edbd8..39216dacdf 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -10,6 +10,7 @@
 namespace WordPressCS\WordPress\Sniffs\Security;
 
 use PHP_CodeSniffer\Util\Tokens;
+use PHPCSUtils\BackCompat\BCFile;
 use PHPCSUtils\Tokens\Collections;
 use PHPCSUtils\Utils\PassedParameters;
 use PHPCSUtils\Utils\TextStrings;
@@ -192,6 +193,72 @@ public function process_token( $stackPtr ) {
 				$end   = ( $this->tokens[ $next_non_empty ]['parenthesis_closer'] + 1 );
 				break;
 
+			case \T_PRINT:
+				$end = BCFile::findEndOfStatement( $this->phpcsFile, $stackPtr );
+				if ( \T_COMMA !== $this->tokens[ $end ]['code']
+					&& \T_SEMICOLON !== $this->tokens[ $end ]['code']
+					&& \T_COLON !== $this->tokens[ $end ]['code']
+					&& \T_DOUBLE_ARROW !== $this->tokens[ $end ]['code']
+					&& isset( $this->tokens[ ( $end + 1 ) ] )
+				) {
+					/*
+					 * FindEndOfStatement includes a comma/(semi-)colon/double arrow if that's the end of
+					 * the statement, but for everything else, it returns the last non-empty token _before_
+					 * the end, which would mean the last non-empty token in the statement would not
+					 * be examined. Let's fix that.
+					 */
+					++$end;
+				}
+
+				// Note: no need to check for close tag as close tag will have the token before the tag as the $end.
+				if ( $end >= ( $this->phpcsFile->numTokens - 1 ) ) {
+					$last_non_empty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $end, null, true );
+					if ( \T_SEMICOLON !== $this->tokens[ $last_non_empty ]['code'] ) {
+						// Live coding/parse error at end of file. Ignore.
+						return;
+					}
+				}
+
+				// Special case for a print statement *within* a ternary, where we need to find the "inline else" as the end token.
+				$prev_non_empty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );
+				if ( \T_INLINE_THEN === $this->tokens[ $prev_non_empty ]['code'] ) {
+					$target_nesting_level = 0;
+					if ( empty( $this->tokens[ $stackPtr ]['nested_parenthesis'] ) === false ) {
+						$target_nesting_level = \count( $this->tokens[ $stackPtr ]['nested_parenthesis'] );
+					}
+
+					$inline_else = false;
+					for ( $i = ( $stackPtr + 1 ); $i < $end; $i++ ) {
+						if ( \T_INLINE_ELSE !== $this->tokens[ $i ]['code'] ) {
+							continue;
+						}
+
+						if ( empty( $this->tokens[ $i ]['nested_parenthesis'] )
+							&& 0 === $target_nesting_level
+						) {
+							$inline_else = $i;
+							break;
+						}
+
+						if ( empty( $this->tokens[ $i ]['nested_parenthesis'] ) === false
+							&& \count( $this->tokens[ $i ]['nested_parenthesis'] ) === $target_nesting_level
+						) {
+							$inline_else = $i;
+							break;
+						}
+					}
+
+					if ( false === $inline_else ) {
+						// Live coding/parse error. Bow out.
+						return;
+					}
+
+					$end = $inline_else;
+				}
+
+				break;
+
+			// Echo, open tag with echo.
 			default:
 				// Find a potential opening parenthesis (if present).
 				$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 8575c1ef57..f9ee2d8528 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -379,3 +379,18 @@ _deprecated_file(
 $var = (true || exit ) && empty( $bar ) ? $drink_alone : $drink_together; // OK, exit is not passing status.
 $var = (true or die ) and empty( $bar ) ? $drink_alone : $drink_together; // OK, die is not passing status.
 $var = exit( $var ? $ok : $error ); // Bad x 2.
+
+// Print can be used in expressions, so end of statement can be all sorts.
+if ( print "hello" ) // OK.
+    $var = 'foo'; // OK, not part of the print.
+
+if ( print("hello") && $var ) // Bad x1, `$var`.
+    $var = 'foo'; // OK, not part of the print.
+
+// Bug #2209.
+( 'auto' === $key ) ? print ' disabled ' : print ' enabled '; // OK.
+( $fop === 1 ) ? print $foo : print $bar; // Bad x 2, each print statement should be examined separately.
+( 1 === $foo ) ? print ( $foo ? 'ten' : $twenty ) : print $bar; // Bad x 2, $twenty & $bar.
+
+// Ensure nesting levels are handled correctly.
+( ( $fop === 1 ) ? print ( $var ? 10 : 20 ) : print 100 ); // OK.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.10.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.10.inc
new file mode 100644
index 0000000000..149338a445
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.10.inc
@@ -0,0 +1,4 @@
+<?php
+
+// Live coding/parse error (missing second "else"). The sniff should ignore this.
+if ( ( $fop === 1 ) ? print ( $var ? 10 : 20 ) ) {}
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.6.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.6.inc
new file mode 100644
index 0000000000..20e2da2fcd
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.6.inc
@@ -0,0 +1,4 @@
+<?php
+
+// Valid print statement at end of file. This should be examined.
+print $foo; // Bad.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.7.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.7.inc
new file mode 100644
index 0000000000..e3e87f96c0
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.7.inc
@@ -0,0 +1,4 @@
+<?php
+
+// Valid print statement at end of file. This should be examined (and flagged).
+print $foo ?>
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.8.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.8.inc
new file mode 100644
index 0000000000..05a328ec13
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.8.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+print $foo
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.9.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.9.inc
new file mode 100644
index 0000000000..5d20a32e22
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.9.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+( 1 === 1 ) ? print $foo
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 4c1e54ed54..5397098aca 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -108,6 +108,15 @@ public function getErrorList( $testFile = '' ) {
 					347 => 1,
 					369 => 1,
 					381 => 2,
+					387 => 1,
+					392 => 2,
+					393 => 2,
+				);
+
+			case 'EscapeOutputUnitTest.6.inc':
+			case 'EscapeOutputUnitTest.7.inc':
+				return array(
+					4 => 1,
 				);
 
 			default:

From f882b53cdcdae880ceb6deefa55f645d48a6fffe Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 25 Jul 2023 15:53:39 +0200
Subject: [PATCH 17/44] Security/EscapeOutput: efficiency tweak for parse
 errors in echo statements

Bow out earlier in case of a parse error for echo statements and short open echo tag statements.

While the sniff would not throw false positives/negatives as the `for()` loop, which does the token walking in the `check_code_is_escaped()` method, would not get started, as `$i < false` as stop condition would always return `true`, we can (and should) bow out earlier for parse errors/live coding.

Fixed now.

Includes switching the order of the token searches for maximum efficiency.

Includes tests safeguarding the tweak.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php      | 7 ++++++-
 WordPress/Tests/Security/EscapeOutputUnitTest.11.inc | 5 +++++
 WordPress/Tests/Security/EscapeOutputUnitTest.12.inc | 5 +++++
 3 files changed, 16 insertions(+), 1 deletion(-)
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.11.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.12.inc

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 39216dacdf..5bca3cc98a 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -260,9 +260,14 @@ public function process_token( $stackPtr ) {
 
 			// Echo, open tag with echo.
 			default:
+				$end = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), $stackPtr );
+				if ( false === $end ) {
+					// Live coding/parse error. Bow out.
+					return;
+				}
+
 				// Find a potential opening parenthesis (if present).
 				$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
-				$end        = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), $stackPtr );
 				$last_token = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end - 1 ), null, true );
 
 				// Check for the ternary operator. We only need to do this here if this
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.11.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.11.inc
new file mode 100644
index 0000000000..23279dcf93
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.11.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+echo $var,
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.12.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.12.inc
new file mode 100644
index 0000000000..48eaa1d0c8
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.12.inc
@@ -0,0 +1,5 @@
+<?php
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+?>
+<?= $var

From 2d76c1d7f23c2ff69f8157115b9321bd5a86c667 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 25 Jul 2023 02:17:13 +0200
Subject: [PATCH 18/44] Security/EscapeOutput: fix up after refactor
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The refactor in the previous few commits "broke" the preliminary check for a non-parenthesized ternary ìn a `print` statement.

There were no tests covering this.

I'm fixing this now by moving that particular code block out of the `switch` and executing it for both `print`, `echo` and `<?=` (though not for `exit`/`die` statements as those will always have parentheses, otherwise there will be nothing to check).

I'm also extracting the actual logic for finding the ternary to a separate method, as this logic will be expanded in follow-up commits.

Includes adding tests safeguarding the logic.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 56 +++++++++++++------
 .../Tests/Security/EscapeOutputUnitTest.1.inc |  5 ++
 .../Tests/Security/EscapeOutputUnitTest.php   |  2 +
 3 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 5bca3cc98a..cd90d09245 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -266,25 +266,27 @@ public function process_token( $stackPtr ) {
 					return;
 				}
 
-				// Find a potential opening parenthesis (if present).
-				$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
-				$last_token = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end - 1 ), null, true );
-
-				// Check for the ternary operator. We only need to do this here if this
-				// echo is lacking parenthesis. Otherwise it will be handled below.
-				if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code']
-					|| \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_token ]['code']
-				) {
-					$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $stackPtr, $end );
+				break;
+		}
 
-					// If there is a ternary skip over the part before the ?. However, if
-					// the ternary is within parentheses, it will be handled in the loop.
-					if ( false !== $ternary && empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) ) {
-						$start = ( $ternary + 1 );
-					}
+		if ( \T_EXIT !== $this->tokens[ $stackPtr ]['code'] ) {
+			/*
+			 * Check for a ternary operator.
+			 * We only need to do this here if this echo/print statement is lacking parenthesis.
+			 * Otherwise it will be handled in the `check_code_is_escaped()` method.
+			 */
+			$next_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+			$last_non_empty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end - 1 ), null, true );
+
+			if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_non_empty ]['code']
+				|| \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_non_empty ]['code']
+			) {
+				// If there is a ternary skip over the part before the ?.
+				$ternary = $this->find_ternary( $start, $end );
+				if ( false !== $ternary ) {
+					$start = ( $ternary + 1 );
 				}
-
-				break;
+			}
 		}
 
 		return $this->check_code_is_escaped( $start, $end, $ternary );
@@ -577,4 +579,24 @@ protected function check_code_is_escaped( $start, $end, $ternary = false ) {
 
 		return $end;
 	}
+
+	/**
+	 * Check whether there is a ternary token in an arbitrary set of tokens.
+	 *
+	 * @since 3.0.0 Split off from the process_token() method.
+	 *
+	 * @param int $start The position to start checking from.
+	 * @param int $end   The position to stop the check at.
+	 *
+	 * @return int|false Stack pointer to the ternary or FALSE if no ternary was found.
+	 */
+	private function find_ternary( $start, $end ) {
+		$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $start, $end );
+
+		if ( false !== $ternary && empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) ) {
+			return $ternary;
+		}
+
+		return false;
+	}
 }
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index f9ee2d8528..c8e884728b 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -394,3 +394,8 @@ if ( print("hello") && $var ) // Bad x1, `$var`.
 
 // Ensure nesting levels are handled correctly.
 ( ( $fop === 1 ) ? print ( $var ? 10 : 20 ) : print 100 ); // OK.
+
+// Ensure print statements with ternaries and without wrapping parentheses are handled correctly.
+( 1 === 1 ) ? print ( $foo ? 'ten' : 'twenty' ) . $baz : print $bar; // Bad x 2, $baz and $bar.
+$var = true && print $foo ? $bar : $baz; // Bad x 2, `$bar` and `$baz` should be flagged, not $foo.
+print $foo ? 'ten' : 'twenty'; // OK.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 5397098aca..33ece492b3 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -111,6 +111,8 @@ public function getErrorList( $testFile = '' ) {
 					387 => 1,
 					392 => 2,
 					393 => 2,
+					399 => 2,
+					400 => 2,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From 0167eb71f08e835d41a4b791a694de2849732115 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 25 Jul 2023 23:01:31 +0200
Subject: [PATCH 19/44] Security/EscapeOutput: bug fix - allow for nested
 statements with ternaries

As things were, the check for statements with ternaries without wrapping parentheses, did not take into account that the whole statement could be nested within parentheses if the statement is a `print`.

This commit updates the code block handling the skipping of the condition for a ternary without wrapping parentheses to allow for the statement being _nested in_ parentheses.

Includes adding tests safeguarding the fix.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 20 +++++++++++++++++--
 .../Tests/Security/EscapeOutputUnitTest.1.inc |  5 +++++
 .../Tests/Security/EscapeOutputUnitTest.php   |  2 ++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index cd90d09245..079b8b1988 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -581,7 +581,7 @@ protected function check_code_is_escaped( $start, $end, $ternary = false ) {
 	}
 
 	/**
-	 * Check whether there is a ternary token in an arbitrary set of tokens.
+	 * Check whether there is a ternary token at the right nesting level in an arbitrary set of tokens.
 	 *
 	 * @since 3.0.0 Split off from the process_token() method.
 	 *
@@ -592,8 +592,24 @@ protected function check_code_is_escaped( $start, $end, $ternary = false ) {
 	 */
 	private function find_ternary( $start, $end ) {
 		$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $start, $end );
+		if ( false === $ternary ) {
+			return false;
+		}
+
+		$target_nesting_level = 0;
+		if ( empty( $this->tokens[ $start ]['nested_parenthesis'] ) === false ) {
+			$target_nesting_level = \count( $this->tokens[ $start ]['nested_parenthesis'] );
+		}
+
+		if ( empty( $this->tokens[ $ternary ]['nested_parenthesis'] )
+			&& 0 === $target_nesting_level
+		) {
+			return $ternary;
+		}
 
-		if ( false !== $ternary && empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) ) {
+		if ( empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) === false
+			&& \count( $this->tokens[ $ternary ]['nested_parenthesis'] ) === $target_nesting_level
+		) {
 			return $ternary;
 		}
 
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index c8e884728b..cbe0dac934 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -399,3 +399,8 @@ if ( print("hello") && $var ) // Bad x1, `$var`.
 ( 1 === 1 ) ? print ( $foo ? 'ten' : 'twenty' ) . $baz : print $bar; // Bad x 2, $baz and $bar.
 $var = true && print $foo ? $bar : $baz; // Bad x 2, `$bar` and `$baz` should be flagged, not $foo.
 print $foo ? 'ten' : 'twenty'; // OK.
+
+// Ensure print statements with ternaries, without wrapping parentheses, but nested within parentheses are also handled correctly.
+if ( print $foo ? 'ten' : 'twenty' ) {} // OK.
+if ( print $foo ? $bar : $baz ) {} // Bad x 2, `$bar` and `$baz` should be flagged, not $foo.
+$var = ( ( $fop === 1 ) ? ( print $foo ? 'ten' : $twenty ) : print $bar ); // Bad x 2, $twenty & $bar.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 33ece492b3..0e4c6de97a 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -113,6 +113,8 @@ public function getErrorList( $testFile = '' ) {
 					393 => 2,
 					399 => 2,
 					400 => 2,
+					405 => 2,
+					406 => 2,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From 6bcb6b0f1dc73b97ef5a5f48b9b7bee54b0d1555 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 03:25:12 +0200
Subject: [PATCH 20/44] Security/EscapeOutput: bug fix - [improved] allow for
 nested statements with ternaries

While the previous commit already prevented a ternary being regarded as the "applicable" ternary when it wasn't at the right nesting level, that still disregarded the fact that ternaries can be chained and that an "applicable" ternary token could be found _after_ a ternary which should be disregarded.

This led to false positives.

This commit changes the logic in the `find_ternary()` method to properly search the token group for an applicable ternary token, which fixes that issue.

Includes tests proving the bug and safeguarding the fix.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 45 ++++++++++++-------
 .../Tests/Security/EscapeOutputUnitTest.1.inc |  6 +++
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 079b8b1988..e0457f5691 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -591,26 +591,37 @@ protected function check_code_is_escaped( $start, $end, $ternary = false ) {
 	 * @return int|false Stack pointer to the ternary or FALSE if no ternary was found.
 	 */
 	private function find_ternary( $start, $end ) {
-		$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $start, $end );
-		if ( false === $ternary ) {
-			return false;
-		}
+		for ( $i = $start; $i < $end; $i++ ) {
+			// Ignore anything within square brackets.
+			if ( isset( $this->tokens[ $i ]['bracket_opener'], $this->tokens[ $i ]['bracket_closer'] )
+				&& $i === $this->tokens[ $i ]['bracket_opener']
+			) {
+				$i = $this->tokens[ $i ]['bracket_closer'];
+				continue;
+			}
 
-		$target_nesting_level = 0;
-		if ( empty( $this->tokens[ $start ]['nested_parenthesis'] ) === false ) {
-			$target_nesting_level = \count( $this->tokens[ $start ]['nested_parenthesis'] );
-		}
+			// Skip past nested arrays, function calls and arbitrary groupings.
+			if ( \T_OPEN_PARENTHESIS === $this->tokens[ $i ]['code']
+				&& isset( $this->tokens[ $i ]['parenthesis_closer'] )
+			) {
+				$i = $this->tokens[ $i ]['parenthesis_closer'];
+				continue;
+			}
 
-		if ( empty( $this->tokens[ $ternary ]['nested_parenthesis'] )
-			&& 0 === $target_nesting_level
-		) {
-			return $ternary;
-		}
+			// Skip past closures, anonymous classes and anything else scope related.
+			if ( isset( $this->tokens[ $i ]['scope_condition'], $this->tokens[ $i ]['scope_closer'] )
+				&& $this->tokens[ $i ]['scope_condition'] === $i
+			) {
+				$i = $this->tokens[ $i ]['scope_closer'];
+				continue;
+			}
 
-		if ( empty( $this->tokens[ $ternary ]['nested_parenthesis'] ) === false
-			&& \count( $this->tokens[ $ternary ]['nested_parenthesis'] ) === $target_nesting_level
-		) {
-			return $ternary;
+			if ( \T_INLINE_THEN !== $this->tokens[ $i ]['code'] ) {
+				continue;
+			}
+
+			// Okay, we found a ternary and it should be at the correct nesting level.
+			return $i;
 		}
 
 		return false;
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index cbe0dac934..30979a783f 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -404,3 +404,9 @@ print $foo ? 'ten' : 'twenty'; // OK.
 if ( print $foo ? 'ten' : 'twenty' ) {} // OK.
 if ( print $foo ? $bar : $baz ) {} // Bad x 2, `$bar` and `$baz` should be flagged, not $foo.
 $var = ( ( $fop === 1 ) ? ( print $foo ? 'ten' : $twenty ) : print $bar ); // Bad x 2, $twenty & $bar.
+
+// Ternary statements can be chained and nested.
+echo ( ! empty( $var ) && ( $var > 10 ? $foo : $bar ) ) ? 'go' : 'stop'; // OK.
+echo isset( $var[ $keyA ? $keyA : $keyB ] ) ? 'go' : 'stop'; // OK.
+echo '' !== implode( '', [ $valueA ? $valueA : $valueB, $valueC ? $valueD : $valueE ] ) ? 'go' : 'stop'; // OK.
+echo '' !== ${true ? $foo : $bar} ? 'go' : 'stop'; // OK.

From a59a60f1d4c8b580d151cc36a883c1455559779b Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 15:46:57 +0200
Subject: [PATCH 21/44] Security/EscapeOutput: bug fix - allow non-matching
 parentheses

As things were, the prelim check for statements with ternaries without wrapping parentheses, did not take into account that there could be a parentheses at the start and at the end of the statement, which didn't belong together.
In that case, the preliminary ternary check should still be executed.

Fixed now.

Includes adding tests safeguarding the fix.

Fixes 677

This also fixes the third case reported in 1507
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php     | 5 +++++
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc | 7 +++++++
 WordPress/Tests/Security/EscapeOutputUnitTest.php   | 1 +
 3 files changed, 13 insertions(+)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index e0457f5691..e4cc38e407 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -280,6 +280,11 @@ public function process_token( $stackPtr ) {
 
 			if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_non_empty ]['code']
 				|| \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_non_empty ]['code']
+				|| ( \T_OPEN_PARENTHESIS === $this->tokens[ $next_non_empty ]['code']
+					&& \T_CLOSE_PARENTHESIS === $this->tokens[ $last_non_empty ]['code']
+					&& isset( $this->tokens[ $next_non_empty ]['parenthesis_closer'] )
+					&& $this->tokens[ $next_non_empty ]['parenthesis_closer'] !== $last_non_empty
+				)
 			) {
 				// If there is a ternary skip over the part before the ?.
 				$ternary = $this->find_ternary( $start, $end );
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 30979a783f..2e2882716f 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -410,3 +410,10 @@ echo ( ! empty( $var ) && ( $var > 10 ? $foo : $bar ) ) ? 'go' : 'stop'; // OK.
 echo isset( $var[ $keyA ? $keyA : $keyB ] ) ? 'go' : 'stop'; // OK.
 echo '' !== implode( '', [ $valueA ? $valueA : $valueB, $valueC ? $valueD : $valueE ] ) ? 'go' : 'stop'; // OK.
 echo '' !== ${true ? $foo : $bar} ? 'go' : 'stop'; // OK.
+
+// Bug #677 (and #1507C).
+echo ( ! empty( $my_bc_title ) ) ? wp_kses( $my_bc_title, allowed_tags() ) : (10 + 20); // OK.
+echo ( ! empty( $my_bc_title ) ) ? wp_kses( $my_bc_title, allowed_tags() ) : get_the_title(); // Bad, should flag get_the_title(), not `!`.
+// Without parentheses wrapping the empty(), this was already okay.
+echo ! empty( $my_bc_title ) ? wp_kses( $my_bc_title, allowed_tags() ) : (10 + 20); // OK.
+echo ( $is_mobile ) ? wp_json_encode( 'true' ) : wp_json_encode( 'false' ); // OK.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 0e4c6de97a..5a5639b62a 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -115,6 +115,7 @@ public function getErrorList( $testFile = '' ) {
 					400 => 2,
 					405 => 2,
 					406 => 2,
+					416 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From fd3101ae60dfbd1db1cdaef47148eeacf7229d9d Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 20:06:40 +0200
Subject: [PATCH 22/44] Security/EscapeOutput: bug fix - fix ternary finding
 within the loop

As things were, when an open parenthesis was found in the loop, the sniff would attempt to find a relevant ternary operator and if found, skip over the condition.

However, this check did not take nested parentheses into account correctly, which meant that if the complete ternary was wrapped in parentheses, but the condition was _not_ wrapped in parentheses and the "then" or the "else" part _would_ contain parentheses, the ternary was incorrectly disregarded.

Fixed now by switching the logic used within the loop over to use the new `find_ternary()` method, which should handle the determination of whether the ternary is applicable correctly.

Note: it is important for the `$start` token passed to the `find_ternary()` method to be the token _after_ the open parenthesis, otherwise, the method will not look _within_ the parentheses.

Includes adding tests safeguarding the fix.

Fixes 1219
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php     | 12 ++----------
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc | 10 ++++++++++
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index e4cc38e407..9accf18d99 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -417,18 +417,10 @@ protected function check_code_is_escaped( $start, $end, $ternary = false ) {
 					$in_cast = false;
 
 				} else {
-
 					// Skip over the condition part of a ternary (i.e., to after the ?).
-					$ternary = $this->phpcsFile->findNext( \T_INLINE_THEN, $i, $this->tokens[ $i ]['parenthesis_closer'] );
-
+					$ternary = $this->find_ternary( ( $i + 1 ), $this->tokens[ $i ]['parenthesis_closer'] );
 					if ( false !== $ternary ) {
-
-						$next_paren = $this->phpcsFile->findNext( \T_OPEN_PARENTHESIS, ( $i + 1 ), $this->tokens[ $i ]['parenthesis_closer'] );
-
-						// We only do it if the ternary isn't within a subset of parentheses.
-						if ( false === $next_paren || ( isset( $this->tokens[ $next_paren ]['parenthesis_closer'] ) && $ternary > $this->tokens[ $next_paren ]['parenthesis_closer'] ) ) {
-							$i = $ternary;
-						}
+						$i = $ternary;
 					}
 				}
 
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 2e2882716f..360388cc3a 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -417,3 +417,13 @@ echo ( ! empty( $my_bc_title ) ) ? wp_kses( $my_bc_title, allowed_tags() ) : get
 // Without parentheses wrapping the empty(), this was already okay.
 echo ! empty( $my_bc_title ) ? wp_kses( $my_bc_title, allowed_tags() ) : (10 + 20); // OK.
 echo ( $is_mobile ) ? wp_json_encode( 'true' ) : wp_json_encode( 'false' ); // OK.
+
+// Bug #1219.
+echo ( $webinar->is_too_late_to_register ? '</a>' : '' ); // OK.
+echo $webinar->is_too_late_to_register ? '</a>' : ''; // OK.
+array_walk(
+	$upcoming_webinars,
+	function ( $webinar ) {
+		echo ( $webinar->is_too_late_to_register ? '<a href="' . esc_url( $webinar->url ) . '">' : '' );
+	}
+); // OK.

From e15a50e6d5e1e6e12f01560d9dd102243d09660c Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 03:59:47 +0200
Subject: [PATCH 23/44] Security/EscapeOutput: bug fix - false negatives for
 short ternary

While the use of short ternaries is/will be discouraged in WP, this doesn't mean it won't be used in non-WP Core code.

When a short ternary is used, the sniff should **not** ignore the code before the ternary operator, but should check it for being correctly escaped as the value will be used if it is "truthy".

Fixed by changing the `find_ternary()` method to a `find_long_ternary()` method, which disregards short ternaries.

Includes tests proving the bug and safeguarding the fix.

Fixes 1617
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 23 +++++++++++++------
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 10 ++++++++
 .../Tests/Security/EscapeOutputUnitTest.php   |  3 +++
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 9accf18d99..cfa89e24e5 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\BackCompat\BCFile;
 use PHPCSUtils\Tokens\Collections;
+use PHPCSUtils\Utils\Operators;
 use PHPCSUtils\Utils\PassedParameters;
 use PHPCSUtils\Utils\TextStrings;
 use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff;
@@ -286,8 +287,8 @@ public function process_token( $stackPtr ) {
 					&& $this->tokens[ $next_non_empty ]['parenthesis_closer'] !== $last_non_empty
 				)
 			) {
-				// If there is a ternary skip over the part before the ?.
-				$ternary = $this->find_ternary( $start, $end );
+				// If there is a (long) ternary skip over the part before the ?.
+				$ternary = $this->find_long_ternary( $start, $end );
 				if ( false !== $ternary ) {
 					$start = ( $ternary + 1 );
 				}
@@ -417,8 +418,8 @@ protected function check_code_is_escaped( $start, $end, $ternary = false ) {
 					$in_cast = false;
 
 				} else {
-					// Skip over the condition part of a ternary (i.e., to after the ?).
-					$ternary = $this->find_ternary( ( $i + 1 ), $this->tokens[ $i ]['parenthesis_closer'] );
+					// Skip over the condition part of a (long) ternary (i.e., to after the ?).
+					$ternary = $this->find_long_ternary( ( $i + 1 ), $this->tokens[ $i ]['parenthesis_closer'] );
 					if ( false !== $ternary ) {
 						$i = $ternary;
 					}
@@ -585,9 +586,10 @@ protected function check_code_is_escaped( $start, $end, $ternary = false ) {
 	 * @param int $start The position to start checking from.
 	 * @param int $end   The position to stop the check at.
 	 *
-	 * @return int|false Stack pointer to the ternary or FALSE if no ternary was found.
+	 * @return int|false Stack pointer to the ternary or FALSE if no ternary was found or
+	 *                   if this is a short ternary.
 	 */
-	private function find_ternary( $start, $end ) {
+	private function find_long_ternary( $start, $end ) {
 		for ( $i = $start; $i < $end; $i++ ) {
 			// Ignore anything within square brackets.
 			if ( isset( $this->tokens[ $i ]['bracket_opener'], $this->tokens[ $i ]['bracket_closer'] )
@@ -617,7 +619,14 @@ private function find_ternary( $start, $end ) {
 				continue;
 			}
 
-			// Okay, we found a ternary and it should be at the correct nesting level.
+			/*
+			 * Okay, we found a ternary and it should be at the correct nesting level.
+			 * If this is a short ternary, it shouldn't be ignored though.
+			 */
+			if ( Operators::isShortTernary( $this->phpcsFile, $i ) === true ) {
+				return false;
+			}
+
 			return $i;
 		}
 
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 360388cc3a..e3dc0ed91e 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -427,3 +427,13 @@ array_walk(
 		echo ( $webinar->is_too_late_to_register ? '<a href="' . esc_url( $webinar->url ) . '">' : '' );
 	}
 ); // OK.
+
+// Bug #1617: code before a ternary should not be ignored if a short ternary is used.
+echo $var ?: ''; // Bad, $var needs escaping.
+echo $var ? /*comment*/ : ''; // Bad, $var needs escaping.
+array_walk(
+	$upcoming_webinars,
+	function ( $webinar ) {
+		echo ( $webinar->is_too_late_to_register ? : '' );
+	}
+); // Bad.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 5a5639b62a..a2a68bc1a1 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -116,6 +116,9 @@ public function getErrorList( $testFile = '' ) {
 					405 => 2,
 					406 => 2,
 					416 => 1,
+					432 => 1,
+					433 => 1,
+					437 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From 05df272c6287f0d25094c79582054e167356d466 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 00:13:58 +0200
Subject: [PATCH 24/44] Security/EscapeOutput: code readability improvements
 [2]

Minor tweaks to the code in the `process_matched_token()` method for improved comprehensibility of the code.

Rename some variables, simplify/split up long conditions.

Includes preventing the code from changing the `$stackPtr` as that is just really confusing.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 27 +++++++++----------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index cfa89e24e5..3d7d9658dc 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -313,26 +313,28 @@ public function process_token( $stackPtr ) {
 	 */
 	public function process_matched_token( $stackPtr, $group_name, $matched_content ) {
 		// Make sure we only deal with actual function calls, not function import use statements.
-		$open_paren = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
-		if ( false === $open_paren
-			|| \T_OPEN_PARENTHESIS !== $this->tokens[ $open_paren ]['code']
-			|| isset( $this->tokens[ $open_paren ]['parenthesis_closer'] ) === false
+		$next_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+		if ( false === $next_non_empty
+			|| \T_OPEN_PARENTHESIS !== $this->tokens[ $next_non_empty ]['code']
+			|| isset( $this->tokens[ $next_non_empty ]['parenthesis_closer'] ) === false
 		) {
 			// Live coding, parse error or not a function _call_.
 			return;
 		}
 
-		$end_of_statement = $this->tokens[ $open_paren ]['parenthesis_closer'];
+		$start = ( $next_non_empty + 1 );
+		$end   = $this->tokens[ $next_non_empty ]['parenthesis_closer'];
 
-		// These functions only need to have the first argument escaped.
-		if ( \in_array( $matched_content, array( 'trigger_error', 'user_error' ), true ) ) {
+		// These functions only need to have their first argument escaped.
+		if ( 'trigger_error' === $matched_content || 'user_error' === $matched_content ) {
 			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
 			if ( false === $first_param ) {
 				// First parameter doesn't exist. Nothing to do.
 				return;
 			}
 
-			$end_of_statement = ( $first_param['end'] + 1 );
+			$start = $first_param['start'];
+			$end   = ( $first_param['end'] + 1 );
 			unset( $first_param );
 		}
 
@@ -349,7 +351,7 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 
 			// Check for a particular code pattern which can safely be ignored.
 			if ( preg_match( '`^[\\\\]?basename\s*\(\s*__FILE__\s*\)$`', $first_param['clean'] ) === 1 ) {
-				$stackPtr = ( $first_param['end'] + 2 );
+				$start = ( $first_param['end'] + 2 );
 			}
 			unset( $first_param );
 		}
@@ -364,14 +366,11 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 
 			// If the error was reported, don't bother checking the function's arguments.
 			if ( $error ) {
-				return $end_of_statement;
+				return $end;
 			}
 		}
 
-		// Ignore the function itself.
-		++$stackPtr;
-
-		return $this->check_code_is_escaped( $stackPtr, $end_of_statement );
+		return $this->check_code_is_escaped( $start, $end );
 	}
 
 	/**

From d49715f8603ec1c0c8c7f86b6f39c08a67f08c47 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 00:29:04 +0200
Subject: [PATCH 25/44] Security/EscapeOutput: efficiency tweak [1]

Bow out earlier.

There is one function call related check which may bow out without checking parameters, so move that check up for efficiency.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 28 +++++++++----------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 3d7d9658dc..29a9b1320e 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -325,6 +325,20 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 		$start = ( $next_non_empty + 1 );
 		$end   = $this->tokens[ $next_non_empty ]['parenthesis_closer'];
 
+		if ( 'unsafe_printing_functions' === $group_name ) {
+			$error = $this->phpcsFile->addError(
+				"All output should be run through an escaping function (like %s), found '%s'.",
+				$stackPtr,
+				'UnsafePrintingFunction',
+				array( $this->unsafePrintingFunctions[ $matched_content ], $matched_content )
+			);
+
+			// If the error was reported, don't bother checking the function's arguments.
+			if ( $error ) {
+				return $end;
+			}
+		}
+
 		// These functions only need to have their first argument escaped.
 		if ( 'trigger_error' === $matched_content || 'user_error' === $matched_content ) {
 			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
@@ -356,20 +370,6 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 			unset( $first_param );
 		}
 
-		if ( 'unsafe_printing_functions' === $group_name ) {
-			$error = $this->phpcsFile->addError(
-				"All output should be run through an escaping function (like %s), found '%s'.",
-				$stackPtr,
-				'UnsafePrintingFunction',
-				array( $this->unsafePrintingFunctions[ $matched_content ], $matched_content )
-			);
-
-			// If the error was reported, don't bother checking the function's arguments.
-			if ( $error ) {
-				return $end;
-			}
-		}
-
 		return $this->check_code_is_escaped( $start, $end );
 	}
 

From f3bd49b752f804162c113ae86123fa43c4527fb7 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 00:32:31 +0200
Subject: [PATCH 26/44] Security/EscapeOutput: efficiency tweak [2]

If the parameter which needs to be examined doesn't exist, we can skip the complete function call.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 29a9b1320e..52d5e24a63 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -344,7 +344,7 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
 			if ( false === $first_param ) {
 				// First parameter doesn't exist. Nothing to do.
-				return;
+				return $end;
 			}
 
 			$start = $first_param['start'];

From b6ec4a2ea7ae78836ec49ac0743ce9c6dce7e49e Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 01:13:07 +0200
Subject: [PATCH 27/44] Security/EscapeOutput: bug fix - sniff would skip too
 much

If a non-function call `T_STRING` token would be named the same as any of the escaping/formatting/array walking functions the sniff looks for, the sniff would walk to the next parenthesis opener (if found) and skip everything in between.

Fixed now.

Includes a test proving the bug and safeguarding the fix.

Note: the test shows the sniff now throwing two errors, one for the `$var` and one for the `unrelatedFunction()` function call.
The sniff does not throw an error for the (unescaped) constant being used. This may need to be looked into separately.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php     | 7 ++++---
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc | 2 ++
 WordPress/Tests/Security/EscapeOutputUnitTest.php   | 1 +
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 52d5e24a63..852bc746e7 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -496,11 +496,12 @@ protected function check_code_is_escaped( $start, $end, $ternary = false ) {
 
 				$ptr                    = $i;
 				$functionName           = $this->tokens[ $i ]['content'];
-				$function_opener        = $this->phpcsFile->findNext( \T_OPEN_PARENTHESIS, ( $i + 1 ), null, false, null, true );
+				$function_opener        = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $i + 1 ), null, true );
 				$is_formatting_function = FormattingFunctionsHelper::is_formatting_function( $functionName );
 
-				if ( false !== $function_opener ) {
-
+				if ( false !== $function_opener
+					&& \T_OPEN_PARENTHESIS === $this->tokens[ $function_opener ]['code']
+				) {
 					if ( ArrayWalkingFunctionsHelper::is_array_walking_function( $functionName ) ) {
 
 						// Get the callback parameter.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index e3dc0ed91e..bdef80e262 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -437,3 +437,5 @@ array_walk(
 		echo ( $webinar->is_too_late_to_register ? : '' );
 	}
 ); // Bad.
+
+echo ESC_HTML . $var . unrelatedFunction( $var ); // Bad x 3.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index a2a68bc1a1..3d2b79b6f8 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -119,6 +119,7 @@ public function getErrorList( $testFile = '' ) {
 					432 => 1,
 					433 => 1,
 					437 => 1,
+					441 => 2,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From 2dcd74271fbd4935338cfb5d8d7db5b893fd0f2b Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 03:00:06 +0200
Subject: [PATCH 28/44] Security/EscapeOutput: bug fix - sniff did not handle
 function call parameters correctly

As things were, the sniff would examine all parameters from a function call as one long code block.

This would easily lead to false positives (and possibly also false negatives) largely related to the sniff not correctly starting/stopping the watching for each new parameter and getting confused over ternaries (again).

This commit make two closely related changes to fix this.
1. It changes the behaviour of the sniff to examine each function call parameter individually.
2. It  moves the "should a ternary condition at the start of the code snippet be skipped ?" check, which was previously only used for `echo`/`print`/`<?=` statements/expressions, to the `check_code_is_escaped()` method, which means it will now be applied to each individual parameter.

This gives much higher accuracy when examining parameters.

As a side-effect of this change, function calls to printing functions using named parameters will not yield false positives on the parameter label.

Includes tests proving the bug(s) and safeguarding the fix.

Fixes the issue reported in WordPress/WordPress-Coding-Standards 677#issuecomment-470407780
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 81 +++++++++----------
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 13 +++
 2 files changed, 53 insertions(+), 41 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 852bc746e7..abd38c8a4c 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -162,9 +162,8 @@ public function getGroups() {
 	 *                  normal file processing.
 	 */
 	public function process_token( $stackPtr ) {
-		$start   = ( $stackPtr + 1 );
-		$end     = $start;
-		$ternary = false;
+		$start = ( $stackPtr + 1 );
+		$end   = $start;
 
 		switch ( $this->tokens[ $stackPtr ]['code'] ) {
 			case \T_STRING:
@@ -270,32 +269,7 @@ public function process_token( $stackPtr ) {
 				break;
 		}
 
-		if ( \T_EXIT !== $this->tokens[ $stackPtr ]['code'] ) {
-			/*
-			 * Check for a ternary operator.
-			 * We only need to do this here if this echo/print statement is lacking parenthesis.
-			 * Otherwise it will be handled in the `check_code_is_escaped()` method.
-			 */
-			$next_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
-			$last_non_empty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end - 1 ), null, true );
-
-			if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_non_empty ]['code']
-				|| \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_non_empty ]['code']
-				|| ( \T_OPEN_PARENTHESIS === $this->tokens[ $next_non_empty ]['code']
-					&& \T_CLOSE_PARENTHESIS === $this->tokens[ $last_non_empty ]['code']
-					&& isset( $this->tokens[ $next_non_empty ]['parenthesis_closer'] )
-					&& $this->tokens[ $next_non_empty ]['parenthesis_closer'] !== $last_non_empty
-				)
-			) {
-				// If there is a (long) ternary skip over the part before the ?.
-				$ternary = $this->find_long_ternary( $start, $end );
-				if ( false !== $ternary ) {
-					$start = ( $ternary + 1 );
-				}
-			}
-		}
-
-		return $this->check_code_is_escaped( $start, $end, $ternary );
+		return $this->check_code_is_escaped( $start, $end );
 	}
 
 	/**
@@ -322,8 +296,7 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 			return;
 		}
 
-		$start = ( $next_non_empty + 1 );
-		$end   = $this->tokens[ $next_non_empty ]['parenthesis_closer'];
+		$end = $this->tokens[ $next_non_empty ]['parenthesis_closer'];
 
 		if ( 'unsafe_printing_functions' === $group_name ) {
 			$error = $this->phpcsFile->addError(
@@ -339,6 +312,8 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 			}
 		}
 
+		$params = PassedParameters::getParameters( $this->phpcsFile, $stackPtr );
+
 		// These functions only need to have their first argument escaped.
 		if ( 'trigger_error' === $matched_content || 'user_error' === $matched_content ) {
 			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
@@ -347,9 +322,7 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 				return $end;
 			}
 
-			$start = $first_param['start'];
-			$end   = ( $first_param['end'] + 1 );
-			unset( $first_param );
+			return $this->check_code_is_escaped( $first_param['start'], ( $first_param['end'] + 1 ) );
 		}
 
 		/*
@@ -365,12 +338,17 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 
 			// Check for a particular code pattern which can safely be ignored.
 			if ( preg_match( '`^[\\\\]?basename\s*\(\s*__FILE__\s*\)$`', $first_param['clean'] ) === 1 ) {
-				$start = ( $first_param['end'] + 2 );
+				unset( $params[1] );
 			}
 			unset( $first_param );
 		}
 
-		return $this->check_code_is_escaped( $start, $end );
+		// Examine each parameter individually.
+		foreach ( $params as $param ) {
+			$this->check_code_is_escaped( $param['start'], ( $param['end'] + 1 ) );
+		}
+
+		return $end;
 	}
 
 	/**
@@ -378,14 +356,35 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 	 *
 	 * @since 3.0.0 Split off from the process_token() method.
 	 *
-	 * @param int       $start   The position to start checking from.
-	 * @param int       $end     The position to stop the check at.
-	 * @param int|false $ternary Stack pointer to the `?` inline then token or
-	 *                           FALSE when no non-parenthesized ternary was found.
+	 * @param int $start The position to start checking from.
+	 * @param int $end   The position to stop the check at.
 	 *
 	 * @return int Integer stack pointer to skip forward.
 	 */
-	protected function check_code_is_escaped( $start, $end, $ternary = false ) {
+	protected function check_code_is_escaped( $start, $end ) {
+		/*
+		 * Check for a ternary operator.
+		 * We only need to do this here if this statement is lacking parenthesis.
+		 * Otherwise it will be handled in the below loop.
+		 */
+		$ternary        = false;
+		$next_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $start + 1 ), null, true );
+		$last_non_empty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $end - 1 ), null, true );
+
+		if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_non_empty ]['code']
+			|| \T_CLOSE_PARENTHESIS !== $this->tokens[ $last_non_empty ]['code']
+			|| ( \T_OPEN_PARENTHESIS === $this->tokens[ $next_non_empty ]['code']
+				&& \T_CLOSE_PARENTHESIS === $this->tokens[ $last_non_empty ]['code']
+				&& isset( $this->tokens[ $next_non_empty ]['parenthesis_closer'] )
+				&& $this->tokens[ $next_non_empty ]['parenthesis_closer'] !== $last_non_empty
+			)
+		) {
+			// If there is a (long) ternary skip over the part before the ?.
+			$ternary = $this->find_long_ternary( $start, $end );
+			if ( false !== $ternary ) {
+				$start = ( $ternary + 1 );
+			}
+		}
 
 		$in_cast = false;
 
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index bdef80e262..96d61a1224 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -439,3 +439,16 @@ array_walk(
 ); // Bad.
 
 echo ESC_HTML . $var . unrelatedFunction( $var ); // Bad x 3.
+
+// Bug #677#issuecomment-470407780: Parameters should be examined individually.
+printf(
+	'<li><a href="#%1$s" class="%2$s"%3$s%4$s>%5$s</a></li>',
+	( '' !== $this->link_title ) ? ' title="' . esc_attr( $this->link_title ) . '"' : '',
+	( '' !== $this->link_aria_label ) ? ' aria-label="' . esc_attr( $this->link_aria_label ) . '"' : '',
+); // OK.
+
+// The fix for #677 will also prevent false positives on parameter labels.
+wp_die(
+	title: 'label',
+	message: 'error message',
+);

From 05de946e306ce51602cd6e33f3ab3bf3bf1f115a Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 01:27:52 +0200
Subject: [PATCH 29/44] Security/EscapeOutput: add tests for PHP 7.4 numeric
 literals + PHP 8.1 octal literals

No changes needed. Just making sure.
---
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 96d61a1224..81fc3d16fd 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -452,3 +452,7 @@ wp_die(
 	title: 'label',
 	message: 'error message',
 );
+
+// Safeguard support for PHP 7.4+ numeric literals with underscores and PHP 8.1 octal literals
+// (and throw in some other non-decimal numbers as well).
+echo 1_000_000 + 2_0_0 + 0o12 + 0o20_00, 0XAB953C, 6.674_083e+11, 0b1010;

From f108735a62ea9590dfdbac432e1acce9fa422d28 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 01:44:01 +0200
Subject: [PATCH 30/44] Security/EscapeOutput: add support for PHP 8.0+ named
 parameters [1]

1. Adjusted the way the correct parameters are retrieved from a call to `trigger_error()`/`user_error()` to use the new PHPCSUtils 1.0.0-alpha4 `PassedParameters::getParameterFromStack()` method.
2. The parameter names used are in line with the names as per the PHP 8.0 release.
    PHP itself renamed a lot of parameters in PHP 8.0. As named parameters did not exist before PHP 8.0, the parameter name as per PHP 8.0 (or above) are the only relevant names.

Includes additional unit tests.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php |  8 ++++----
 .../Tests/Security/EscapeOutputUnitTest.1.inc   | 17 +++++++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.php     |  1 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index abd38c8a4c..5a57d0f683 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -316,13 +316,13 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 
 		// These functions only need to have their first argument escaped.
 		if ( 'trigger_error' === $matched_content || 'user_error' === $matched_content ) {
-			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
-			if ( false === $first_param ) {
-				// First parameter doesn't exist. Nothing to do.
+			$message_param = PassedParameters::getParameterFromStack( $params, 1, 'message' );
+			if ( false === $message_param ) {
+				// Message parameter doesn't exist. Nothing to do.
 				return $end;
 			}
 
-			return $this->check_code_is_escaped( $first_param['start'], ( $first_param['end'] + 1 ) );
+			return $this->check_code_is_escaped( $message_param['start'], ( $message_param['end'] + 1 ) );
 		}
 
 		/*
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 81fc3d16fd..961dd2bc71 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -456,3 +456,20 @@ wp_die(
 // Safeguard support for PHP 7.4+ numeric literals with underscores and PHP 8.1 octal literals
 // (and throw in some other non-decimal numbers as well).
 echo 1_000_000 + 2_0_0 + 0o12 + 0o20_00, 0XAB953C, 6.674_083e+11, 0b1010;
+
+/*
+ * Safeguard handling of PHP 8.0+ function calls with named parameters for [trigger|user]_error().
+ */
+user_error( error_level: E_USER_NOTICE ); // OK, well not really, required $message parameter missing, but that's not our concern.
+trigger_error(
+	messege: "There was an error: {$message}",
+	error_level: E_USER_NOTICE,
+); // OK, well not really, typo in $message param name, but that's not our concern.
+user_error(
+	error_level: E_USER_WARNING,
+	message: 'There was an error: ' . esc_html( $message ),
+); // OK.
+trigger_error(
+	error_level: E_USER_WARNING,
+	message: "There was an error: {$message}",
+); // Bad.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 3d2b79b6f8..ebf0285f8b 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -120,6 +120,7 @@ public function getErrorList( $testFile = '' ) {
 					433 => 1,
 					437 => 1,
 					441 => 2,
+					474 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From 4a4858ceef9f58b0d7123fd37bb8417d923a7595 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 01:54:00 +0200
Subject: [PATCH 31/44] Security/EscapeOutput: add support for PHP 8.0+ named
 parameters [2]

1. Adjusted the way the correct parameters are retrieved from a call to `_deprecated_file()` to use the new PHPCSUtils 1.0.0-alpha4 `PassedParameters::getParameterFromStack()` method.
2. Verified the parameter names used are in line with the name as per the WP 6.3 release.
    WP has been renaming parameters and is probably not done yet, but it doesn't look like those changes (so far) made it into changelog entries....
    For the purposes of this exercise, I've taken the _current_ parameter name as the "truth" as support for named parameters hasn't officially been announced yet, so any renames _after_ this moment are the only ones relevant.

Includes additional unit tests.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php  | 16 +++++++---------
 .../Tests/Security/EscapeOutputUnitTest.1.inc    | 12 ++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.php      |  5 +++++
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 5a57d0f683..312026fb07 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -330,17 +330,15 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 		 * pattern, it doesn't need to be escaped.
 		 */
 		if ( '_deprecated_file' === $matched_content ) {
-			$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
-			if ( false === $first_param ) {
-				// First parameter doesn't exist. Nothing to do.
-				return;
-			}
+			$file_param = PassedParameters::getParameterFromStack( $params, 1, 'file' );
 
-			// Check for a particular code pattern which can safely be ignored.
-			if ( preg_match( '`^[\\\\]?basename\s*\(\s*__FILE__\s*\)$`', $first_param['clean'] ) === 1 ) {
-				unset( $params[1] );
+			if ( false !== $file_param ) {
+				// Check for a particular code pattern which can safely be ignored.
+				if ( preg_match( '`^[\\\\]?basename\s*\(\s*__FILE__\s*\)$`', $file_param['clean'] ) === 1 ) {
+					unset( $params[1], $params['file'] ); // Remove the param, whether passed positionally or named.
+				}
 			}
-			unset( $first_param );
+			unset( $file_param );
 		}
 
 		// Examine each parameter individually.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 961dd2bc71..bb12bf544e 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -473,3 +473,15 @@ trigger_error(
 	error_level: E_USER_WARNING,
 	message: "There was an error: {$message}",
 ); // Bad.
+
+/*
+ * Safeguard handling of PHP 8.0+ function calls with named parameters for _deprecated_file().
+ */
+_deprecated_file( version: '1.3.0', file: basename( __FILE__ ) ); // Ok.
+_deprecated_file( version: '1.3.0', files: basename( __FILE__ ) ); // Error, well not really, typo in $file param name, but that's not our concern.
+_deprecated_file( version: '1.3.0', replacement: $name ); // Error. $file param missing, but that's not our concern.
+_deprecated_file(
+	replacement: $name,
+	version: $version,
+	file: $file,
+); // Error x 3.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index ebf0285f8b..3b2ceda435 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -121,6 +121,11 @@ public function getErrorList( $testFile = '' ) {
 					437 => 1,
 					441 => 2,
 					474 => 1,
+					481 => 1,
+					482 => 1,
+					484 => 1,
+					485 => 1,
+					486 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From eab1f2b0af4c90b02c7ef3ff91250bf025e088a6 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 21:37:35 +0200
Subject: [PATCH 32/44] Security/EscapeOutput: add support for PHP 8.0+ match
 expressions [1]

PHP 8.0 introduced match expressions, which can be used inline, which means they can be used in an output generating statement.

If the match expression is wrapped inside an escaping function, it is already handled correctly.

This commit is the first step in adding additional support for `match` expressions to the sniff and adds logic to disregard a complete match expression if it is preceded by a "safe cast".

Includes tests.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php    | 14 ++++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.1.inc      | 13 +++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.13.inc     |  5 +++++
 .../Tests/Security/EscapeOutputUnitTest.14.inc     |  6 ++++++
 .../Tests/Security/EscapeOutputUnitTest.15.inc     |  7 +++++++
 5 files changed, 45 insertions(+)
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.13.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.14.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.15.inc

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 312026fb07..32715cc92b 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -484,6 +484,20 @@ protected function check_code_is_escaped( $start, $end ) {
 
 			// Allow int/double/bool casted variables.
 			if ( isset( ContextHelper::get_safe_cast_tokens()[ $this->tokens[ $i ]['code'] ] ) ) {
+				/*
+				 * If the next thing is a match expression, skip over it as whatever is
+				 * being returned will be safe casted.
+				 * Do not set `$in_cast` to `true`.
+				 */
+				$next_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $i + 1 ), $end, true );
+				if ( false !== $next_non_empty
+					&& \T_MATCH === $this->tokens[ $next_non_empty ]['code']
+					&& isset( $this->tokens[ $next_non_empty ]['scope_closer'] )
+				) {
+					$i = $this->tokens[ $next_non_empty ]['scope_closer'];
+					continue;
+				}
+
 				$in_cast = true;
 				continue;
 			}
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index bb12bf544e..416b84b8f6 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -485,3 +485,16 @@ _deprecated_file(
 	version: $version,
 	file: $file,
 ); // Error x 3.
+
+/*
+ * Safeguard handling of PHP 8.0+ match expression.
+ */
+echo esc_html( match($var) {
+	$a, $b, $c => $fine,
+	default => $value + $other,
+}); // OK.
+
+echo (int) match($var) {
+	$a, $b, $c => $fine,
+	default => $value + $other,
+}; // OK.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.13.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.13.inc
new file mode 100644
index 0000000000..49b820be8c
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.13.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+echo match($var)
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.14.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.14.inc
new file mode 100644
index 0000000000..8823b6c511
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.14.inc
@@ -0,0 +1,6 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+echo match($var) {
+	$a, $b, $c => $fine,
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.15.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.15.inc
new file mode 100644
index 0000000000..e7e7706efc
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.15.inc
@@ -0,0 +1,7 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+echo match($var) {
+	$a,
+};

From 3987f6d40053666d7c34880d3e24418b221a259d Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Wed, 26 Jul 2023 21:37:35 +0200
Subject: [PATCH 33/44] Security/EscapeOutput: add support for PHP 8.0+ match
 expressions [2]

PHP 8.0 introduced match expressions, which can be used inline, which means they can be used in an output generating statement.

This second commit adds logic to correctly check _inside_ a match expression.

In a `match`, there can be multiple matching conditions before the double arrow and one statement to be returned after the double arrow.
So basically, we should ignore everything _before_ the double arrow as it is effectively a comparison.
And we should verify that everything after the double arrow - up to the first comma at the same "level" as the arrow - is escaped correctly.

Now as the conditions, when there is more than one, are also comma-separated, the parsing of a `match` expression is non-trivial.

With this in mind, I'm introducing a helper method to do the parsing. This helper method will pass each individually identified "return value" part to the `check_code_is_escaped()` method for stand-alone analysis.

Once the method is finished analyzing the `match` expression, it will hand back off to the original call to `check_code_is_escaped()` and continue analyzing the code after the `match` expression.

Note: I'm considering adding a utility method to PHPCSUtils for the parsing of `match` expressions. If and when that method is available, we should be able to simplify the code I've now added to the sniff.

Includes tests.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 94 +++++++++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 34 +++++++
 .../Tests/Security/EscapeOutputUnitTest.php   |  7 ++
 3 files changed, 135 insertions(+)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 32715cc92b..370f678a6e 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -424,6 +424,18 @@ protected function check_code_is_escaped( $start, $end ) {
 				continue;
 			}
 
+			// Handle PHP 8.0+ match expressions.
+			if ( \T_MATCH === $this->tokens[ $i ]['code'] ) {
+				$match_valid = $this->walk_match_expression( $i );
+				if ( false === $match_valid ) {
+					// Live coding or parse error. Shouldn't be possible as PHP[CS] will tokenize the keyword as `T_STRING` in that case.
+					break; // @codeCoverageIgnore
+				}
+
+				$i = $match_valid;
+				continue;
+			}
+
 			// Handle arrays for those functions that accept them.
 			if ( \T_ARRAY === $this->tokens[ $i ]['code'] ) {
 				++$i; // Skip the opening parenthesis.
@@ -643,4 +655,86 @@ private function find_long_ternary( $start, $end ) {
 
 		return false;
 	}
+
+	/**
+	 * Examine a match expression and only check for escaping in the "returned" parts of the match expression.
+	 *
+	 * {@internal PHPCSUtils will likely contain a utility for parsing match expressions in the future.
+	 *            Ref: https://github.com/PHPCSStandards/PHPCSUtils/issues/497}
+	 *
+	 * @since 3.0.0
+	 *
+	 * @param int $stackPtr Pointer to a T_MATCH token.
+	 *
+	 * @return int|false Stack pointer to skip to or FALSE if the match expression contained a parse error.
+	 */
+	private function walk_match_expression( $stackPtr ) {
+		if ( ! isset( $this->tokens[ $stackPtr ]['scope_opener'], $this->tokens[ $stackPtr ]['scope_closer'] ) ) {
+			// Parse error/live coding. Shouldn't be possible as PHP[CS] will tokenize the keyword as `T_STRING` in that case.
+			return false; // @codeCoverageIgnore
+		}
+
+		$current = $this->tokens[ $stackPtr ]['scope_opener'];
+		$end     = $this->tokens[ $stackPtr ]['scope_closer'];
+		do {
+			$current = $this->phpcsFile->findNext( \T_MATCH_ARROW, ( $current + 1 ), $end );
+			if ( false === $current ) {
+				// We must have reached the last match item (or there is a parse error).
+				break;
+			}
+
+			$item_start = ( $current + 1 );
+			$item_end   = false;
+
+			// Find the first comma at the same level.
+			for ( $i = $item_start; $i <= $end; $i++ ) {
+				// Ignore anything within square brackets.
+				if ( isset( $this->tokens[ $i ]['bracket_opener'], $this->tokens[ $i ]['bracket_closer'] )
+					&& $i === $this->tokens[ $i ]['bracket_opener']
+				) {
+					$i = $this->tokens[ $i ]['bracket_closer'];
+					continue;
+				}
+
+				// Skip past nested arrays, function calls and arbitrary groupings.
+				if ( \T_OPEN_PARENTHESIS === $this->tokens[ $i ]['code']
+					&& isset( $this->tokens[ $i ]['parenthesis_closer'] )
+				) {
+					$i = $this->tokens[ $i ]['parenthesis_closer'];
+					continue;
+				}
+
+				// Skip past closures, anonymous classes and anything else scope related.
+				if ( isset( $this->tokens[ $i ]['scope_condition'], $this->tokens[ $i ]['scope_closer'] )
+					&& $this->tokens[ $i ]['scope_condition'] === $i
+				) {
+					$i = $this->tokens[ $i ]['scope_closer'];
+					continue;
+				}
+
+				if ( \T_COMMA !== $this->tokens[ $i ]['code']
+					&& $i !== $end
+				) {
+					continue;
+				}
+
+				$item_end = $i;
+				break;
+			}
+
+			if ( false === $item_end ) {
+				// Parse error/live coding. Shouldn't be possible.
+				return false; // @codeCoverageIgnore
+			}
+
+			// Now check that the value returned by this match "leaf" is correctly escaped.
+			$this->check_code_is_escaped( $item_start, $item_end );
+
+			// Independently of whether or not the check was succesfull or ran into (parse error) problems,
+			// always skip to the identified end of the item.
+			$current = $item_end;
+		} while ( $current < $end );
+
+		return $end;
+	}
 }
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 416b84b8f6..ebdc86ae2c 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -498,3 +498,37 @@ echo (int) match($var) {
 	$a, $b, $c => $fine,
 	default => $value + $other,
 }; // OK.
+
+echo match($var) { // OK.
+	$nr => 10 + 20, // OK.
+	$a, $b, $c => 'this line should NOT be flagged, vars are conditions, not output', // OK.
+	'my array' => [ 1, 2, 'foo' ], // OK.
+	'some value', $key, 'more' => esc_html($escaped), // OK.
+	'callback' => match($foo) {
+		10 => 10,
+		default => 20,
+	}, // OK.
+	false => ( $cond ? esc_html($valueA) : \esc_attr($valueB) ), // OK.
+	101 => sprintf(
+		'some %s format %d',
+		esc_html( $text ), // OK.
+		(int) $nr, // OK.
+	),
+	default => esc_attr($value + $other), // OK.
+};
+
+echo match($var) { // OK.
+	'some value', 'more' => $this_line_SHOULD_be_flagged['key'], // Bad x 1.
+	'other value' => $cond ? $valueA : $valueB['key'], // Bad x 2.
+	false => ( $cond ? $valueA : $valueB ), // Bad x 2.
+	101 => sprintf(
+		'some %s format %d',
+		$text, // Bad.
+		$nr, // Bad.
+	),
+	default => $value . $other, // Bad x 2.
+};
+
+echo match($var) { // OK.
+	'some value', 'more' => $this_line_SHOULD_be_flagged // Bad x 1. Note: no trailing comma!
+};
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 3b2ceda435..f0dad52068 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -126,6 +126,13 @@ public function getErrorList( $testFile = '' ) {
 					484 => 1,
 					485 => 1,
 					486 => 1,
+					521 => 1,
+					522 => 2,
+					523 => 2,
+					526 => 1,
+					527 => 1,
+					529 => 2,
+					533 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From 4ee9469ae56081359589f9b461c27d075f86e09c Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 01:18:50 +0200
Subject: [PATCH 34/44] Security/EscapeOutput: add support for *::class and PHP
 8.0+ $obj::class

The result of `Name::class` or the PHP 8.0+ `$obj::class` will always be a plain string, along the same lines as the use of `__CLASS__`, so this should be allowed and not need escaping.

This commit adds support for that code pattern to the sniff, which fixes some false positives.

Includes tests.

Fixes 1989
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 20 +++++++++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.1.inc |  7 +++++++
 2 files changed, 27 insertions(+)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 370f678a6e..2682426a15 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -492,6 +492,26 @@ protected function check_code_is_escaped( $start, $end ) {
 				continue;
 			}
 
+			// Check for use of *::class.
+			if ( \T_STRING === $this->tokens[ $i ]['code']
+				|| \T_VARIABLE === $this->tokens[ $i ]['code']
+				|| isset( Collections::ooHierarchyKeywords()[ $this->tokens[ $i ]['code'] ] )
+			) {
+				$double_colon = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $i + 1 ), $end, true );
+				if ( false !== $double_colon
+					&& \T_DOUBLE_COLON === $this->tokens[ $double_colon ]['code']
+				) {
+					$class_keyword = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $double_colon + 1 ), $end, true );
+					if ( false !== $class_keyword
+						&& \T_STRING === $this->tokens[ $class_keyword ]['code']
+						&& 'class' === strtolower( $this->tokens[ $class_keyword ]['content'] )
+					) {
+						$i = $class_keyword;
+						continue;
+					}
+				}
+			}
+
 			$watch = false;
 
 			// Allow int/double/bool casted variables.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index ebdc86ae2c..5ea50d95d6 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -532,3 +532,10 @@ echo match($var) { // OK.
 echo match($var) { // OK.
 	'some value', 'more' => $this_line_SHOULD_be_flagged // Bad x 1. Note: no trailing comma!
 };
+
+// Bug #1989: allow for Name::class and PHP 8.0+ $obj::class.
+_deprecated_function( __METHOD__, 'x.x.x', ClassName::class ); // OK.
+die( self::CLASS . ' has been abandoned' ); // OK.
+_deprecated_function( __METHOD__, 'x.x.x', parent::Class ); // OK.
+_deprecated_function( __METHOD__, 'x.x.x', static::class ); // OK.
+echo 'Do not use ' . $object::class ); // OK.

From 90f291ccefac61a2233d258e4239b3211382c017 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 03:58:06 +0200
Subject: [PATCH 35/44] Security/EscapeOutput: add support for examining throw
 statements and PHP 8.0+ throw expressions

Any exception which isn't caught runs the risk of being displayed and should therefore be output escaped, along the same lines as is already required for parameters passed to `trigger_error()` function calls.

This commit add the `T_THROW` token to the tokens the sniff listens for. Parameters passed to the exception creation function call/class instantiation will be examined for being correctly escaped.

Notes:
* As custom exceptions may expect different parameters from the PHP native parameters, *all* parameters will be examined, not just the  `$message` parameter.
* When a `throw` statement is wrapped within a `try - catch` control structure, it is presumed that the exception will be caught and the `throw` statement will be ignored.
* The current logic supports a _lot_ of different ways of exception creation, but does not (yet) support variable variables for the exception name and other exotics like that.

Includes tests.

Fixes 884
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 55 +++++++++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 45 +++++++++++++++
 .../Security/EscapeOutputUnitTest.16.inc      |  5 ++
 .../Security/EscapeOutputUnitTest.17.inc      |  5 ++
 .../Security/EscapeOutputUnitTest.18.inc      |  5 ++
 .../Security/EscapeOutputUnitTest.19.inc      |  5 ++
 .../Tests/Security/EscapeOutputUnitTest.php   | 11 ++++
 7 files changed, 131 insertions(+)
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.16.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.17.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.18.inc
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.19.inc

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 2682426a15..487bcd1fd6 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\BackCompat\BCFile;
 use PHPCSUtils\Tokens\Collections;
+use PHPCSUtils\Utils\Conditions;
 use PHPCSUtils\Utils\Operators;
 use PHPCSUtils\Utils\PassedParameters;
 use PHPCSUtils\Utils\TextStrings;
@@ -121,6 +122,7 @@ public function register() {
 		$targets   = parent::register();
 		$targets[] = \T_ECHO;
 		$targets[] = \T_PRINT;
+		$targets[] = \T_THROW;
 		$targets[] = \T_EXIT;
 		$targets[] = \T_OPEN_TAG_WITH_ECHO;
 
@@ -193,6 +195,59 @@ public function process_token( $stackPtr ) {
 				$end   = ( $this->tokens[ $next_non_empty ]['parenthesis_closer'] + 1 );
 				break;
 
+			case \T_THROW:
+				// Find the open parentheses, while stepping over the exception creation tokens.
+				$ignore  = Tokens::$emptyTokens;
+				$ignore += Collections::namespacedNameTokens();
+				$ignore += Collections::functionCallTokens();
+				$ignore += Collections::objectOperators();
+
+				$next_relevant = $this->phpcsFile->findNext( $ignore, ( $stackPtr + 1 ), null, true );
+				if ( false === $next_relevant ) {
+					return;
+				}
+
+				if ( \T_NEW === $this->tokens[ $next_relevant ]['code'] ) {
+					$next_relevant = $this->phpcsFile->findNext( $ignore, ( $next_relevant + 1 ), null, true );
+					if ( false === $next_relevant ) {
+						return;
+					}
+				}
+
+				if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_relevant ]['code']
+					|| isset( $this->tokens[ $next_relevant ]['parenthesis_closer'] ) === false
+				) {
+					// Live codind/parse error or a pre-created exception. Nothing to do for us.
+					return;
+				}
+
+				$end = $this->tokens[ $next_relevant ]['parenthesis_closer'];
+
+				// Check if the throw is within a `try-catch`.
+				// Doing this here (instead of earlier) to allow skipping to the end of the statement.
+				$search_for           = Collections::closedScopes();
+				$search_for[ \T_TRY ] = \T_TRY;
+
+				$last_condition = Conditions::getLastCondition( $this->phpcsFile, $stackPtr, $search_for );
+				if ( false !== $last_condition && \T_TRY === $this->tokens[ $last_condition ]['code'] ) {
+					// This exception will (probably) be caught, so ignore it.
+					return $end;
+				}
+
+				$call_token = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $next_relevant - 1 ), null, true );
+				$params     = PassedParameters::getParameters( $this->phpcsFile, $call_token );
+				if ( empty( $params ) ) {
+					// No parameters passed, nothing to do.
+					return $end;
+				}
+
+				// Examine each parameter individually.
+				foreach ( $params as $param ) {
+					$this->check_code_is_escaped( $param['start'], ( $param['end'] + 1 ) );
+				}
+
+				return $end;
+
 			case \T_PRINT:
 				$end = BCFile::findEndOfStatement( $this->phpcsFile, $stackPtr );
 				if ( \T_COMMA !== $this->tokens[ $end ]['code']
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 5ea50d95d6..e0281c620e 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -539,3 +539,48 @@ die( self::CLASS . ' has been abandoned' ); // OK.
 _deprecated_function( __METHOD__, 'x.x.x', parent::Class ); // OK.
 _deprecated_function( __METHOD__, 'x.x.x', static::class ); // OK.
 echo 'Do not use ' . $object::class ); // OK.
+
+/*
+ * Examine the parameters passed for exception creation via throw.
+ */
+throw new MyException(); // OK.
+throw new Exception( esc_html( $message ), (int) $code ); // OK.
+throw new /*comment*/ parent( esc_html( $message ), (int) $code ); // OK.
+throw MyException::get( esc_html( $message ), (int) $code ); // OK.
+throw $obj->getException( esc_html( $message ), (int) $code ); // OK.
+
+throw new Exception( $message, $code ); // Bad x 2.
+throw new self( $message, $code ); // Bad x 2.
+throw
+	 MyException  ::   get   /*comment*/ ( $message, $code ); // Bad x 2.
+throw $obj?->getException( $message, $code ); // Bad x 2.
+throw new $exceptionName( $message, $code ); // Bad x 2.
+
+throw static::get( esc_html( $message ), $code ); // Bad x 1.
+throw \Vendor\Name\MyException::get( $message, (int) $code ); // Bad x 1.
+throw Name\MyException::get( esc_html( $message ), $code ); // Bad x 1.
+throw namespace\MyException::get( $message, (int) $code ); // Bad x 1.
+throw new class('text', 0) extends Exception {}; // OK.
+
+// Since PHP 8.0, throw can be used as an expression.
+$var = ( ( $fop === 1 ) ? throw new Exception( esc_html( $message ), (int) $code ) : $not_part_of_the_throw ); // OK.
+$var = ( ( $fop === 1 ) ? throw /*comment*/ new Exception( $message ) : $not_part_of_the_throw; // Bad x 1.
+
+// The following should be ignored as this is not exception creation, so we don't have access to the parameters.
+throw $stored_exceptions['key'];
+throw $obj->stored_exception;
+
+// We should also ignore any exception which is being caught straight away.
+try {
+	throw new Exception( $message, $code ); // OK, as exception is being caught.
+} catch ( Exception $e ) {
+} finally {
+}
+
+// .. but only if it is not within a closed scope nested within the try.
+try {
+	$callback = function() {
+		throw new Exception( $message, 10 ); // Bad. Unclear if the exception will be caught or not.
+	};
+} catch ( Exception $e ) {
+}
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.16.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.16.inc
new file mode 100644
index 0000000000..740dcba37b
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.16.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+throw
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.17.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.17.inc
new file mode 100644
index 0000000000..530d535cbd
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.17.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+throw new 
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.18.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.18.inc
new file mode 100644
index 0000000000..76f3d50838
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.18.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+throw new MyException(
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.19.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.19.inc
new file mode 100644
index 0000000000..8b9e59bd6a
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.19.inc
@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error. The sniff should ignore this.
+// This must be the last test in the file.
+throw MyException::get(
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index f0dad52068..cdf4fdc8c0 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -133,6 +133,17 @@ public function getErrorList( $testFile = '' ) {
 					527 => 1,
 					529 => 2,
 					533 => 1,
+					552 => 2,
+					553 => 2,
+					555 => 2,
+					556 => 2,
+					557 => 2,
+					559 => 1,
+					560 => 1,
+					561 => 1,
+					562 => 1,
+					567 => 1,
+					583 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From b501cd99e5658f6860c92fbd1dd11040cddfe8ba Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 18:53:01 +0200
Subject: [PATCH 36/44] Security/EscapeOutput: improve handling of inline
 expressions

When the sniff would encountered one of the keywords the sniff listens to as an inline expression in the statement which was being examined, the sniff would flag the keyword as needing escaping.

This is wrong as that expression should be examined separately by the sniff (but wouldn't be as the sniff skips to the end of the examined code).

This commit fixes that by adjusting the logic in the loop in the `check_code_is_escaped()` method to check for keywords the sniff listens too and then recursing into the sniff again to examine that expression separately, after which the loop for the original statement will continue from the end of the examined expression.

Includes tests.

Fixes 1861
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 31 ++++++++++++++++---
 .../Tests/Security/EscapeOutputUnitTest.1.inc |  9 ++++++
 .../Tests/Security/EscapeOutputUnitTest.php   |  3 ++
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 487bcd1fd6..21a5c4815e 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -106,6 +106,19 @@ class EscapeOutputSniff extends AbstractFunctionRestrictionsSniff {
 		\T_BOOLEAN_NOT              => \T_BOOLEAN_NOT,
 	);
 
+	/**
+	 * List of keyword tokens this sniff listens for, which can also be used as an inline expression.
+	 *
+	 * @since 3.0.0
+	 *
+	 * @var array<string|int, string|int>
+	 */
+	private $target_keywords = array(
+		\T_EXIT  => \T_EXIT,
+		\T_PRINT => \T_PRINT,
+		\T_THROW => \T_THROW,
+	);
+
 	/**
 	 * Returns an array of tokens this test wants to listen for.
 	 *
@@ -119,11 +132,8 @@ public function register() {
 		$this->safe_components += Collections::incrementDecrementOperators();
 
 		// Set up the tokens the sniff should listen too.
-		$targets   = parent::register();
+		$targets   = array_merge( parent::register(), $this->target_keywords );
 		$targets[] = \T_ECHO;
-		$targets[] = \T_PRINT;
-		$targets[] = \T_THROW;
-		$targets[] = \T_EXIT;
 		$targets[] = \T_OPEN_TAG_WITH_ECHO;
 
 		return $targets;
@@ -479,6 +489,19 @@ protected function check_code_is_escaped( $start, $end ) {
 				continue;
 			}
 
+			/*
+			 * If a keyword is encountered in an inline expression and the keyword is one
+			 * this sniff listens to, recurse into the sniff, handle the expression
+			 * based on the keyword and skip over the code examined.
+			 */
+			if ( isset( $this->target_keywords[ $this->tokens[ $i ]['code'] ] ) ) {
+				$return_value = $this->process_token( $i );
+				if ( isset( $return_value ) ) {
+					$i = $return_value;
+				}
+				continue;
+			}
+
 			// Handle PHP 8.0+ match expressions.
 			if ( \T_MATCH === $this->tokens[ $i ]['code'] ) {
 				$match_valid = $this->walk_match_expression( $i );
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index e0281c620e..8502e70e05 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -584,3 +584,12 @@ try {
 	};
 } catch ( Exception $e ) {
 }
+
+// Bug #1861 - don't throw an error for expression examined separately.
+echo $var == 'foo' ? 'bar' : die( 'world' ); // OK.
+echo $var == 'foo' ? throw new Exception( 'message' ) : 'bar'; // OK.
+echo $var == 'foo' ? print 'message' : 'bar'; // OK.
+
+echo $var == 'foo' ? 'bar' : die( $var ); // Bad x 1, $var, not die().
+echo $var == 'foo' ? throw new Exception( $var ) : 'bar'; // Bad x 1, $var, not throw.
+echo $var == 'foo' ? print $var : 'bar'; // Bad x 1, $var, not print.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index cdf4fdc8c0..71d1975da0 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -144,6 +144,9 @@ public function getErrorList( $testFile = '' ) {
 					562 => 1,
 					567 => 1,
 					583 => 1,
+					593 => 1,
+					594 => 1,
+					595 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From ad2b2a16b9e714a91db58c3954f85a450f78a5c5 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 21:15:43 +0200
Subject: [PATCH 37/44] Security/EscapeOutput: improve handling of params in
 unsafe printing functions

As things were, the sniff will flag "unsafe printing functions" when encountered, but if the `UnsafePrintingFunction` error code is silenced, the sniff would examine all parameters passed to the function for output escaping, while only the first parameter will actually be used in the output.

This updates the sniff to only examine the first (`$text`) parameter for function calls to any of the unsafe printing functions, which should prevent some (rare) false positives.

Includes named parameter support for these functions.

Note: while the target parameter position and name in this case is the same for both functions, I have set it up to be flexible, in case additional functions with different target parameters would be added to the list.

Includes tests.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 33 ++++++++++++++++---
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 12 +++++++
 .../Tests/Security/EscapeOutputUnitTest.php   |  2 ++
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 21a5c4815e..79c31d1886 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -58,12 +58,23 @@ class EscapeOutputSniff extends AbstractFunctionRestrictionsSniff {
 	 *
 	 * @since 0.4.0
 	 * @since 0.11.0 Changed from public static to protected non-static.
+	 * @since 3.0.0  The format of the array values has changed from plain string to array.
 	 *
 	 * @var array
 	 */
 	protected $unsafePrintingFunctions = array(
-		'_e'  => 'esc_html_e() or esc_attr_e()',
-		'_ex' => 'echo esc_html_x() or echo esc_attr_x()',
+		'_e'  => array(
+			'alternative' => 'esc_html_e() or esc_attr_e()',
+			'params'      => array(
+				1 => 'text',
+			),
+		),
+		'_ex' => array(
+			'alternative' => 'echo esc_html_x() or echo esc_attr_x()',
+			'params'      => array(
+				1 => 'text',
+			),
+		),
 	);
 
 	/**
@@ -368,13 +379,27 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 				"All output should be run through an escaping function (like %s), found '%s'.",
 				$stackPtr,
 				'UnsafePrintingFunction',
-				array( $this->unsafePrintingFunctions[ $matched_content ], $matched_content )
+				array( $this->unsafePrintingFunctions[ $matched_content ]['alternative'], $matched_content )
 			);
 
 			// If the error was reported, don't bother checking the function's arguments.
-			if ( $error ) {
+			if ( $error || empty( $this->unsafePrintingFunctions[ $matched_content ]['params'] ) ) {
 				return $end;
 			}
+
+			// If the function was not reported for being unsafe, examine the relevant parameters.
+			$params = PassedParameters::getParameters( $this->phpcsFile, $stackPtr );
+			foreach ( $this->unsafePrintingFunctions[ $matched_content ]['params'] as $position => $name ) {
+				$param = PassedParameters::getParameterFromStack( $params, $position, $name );
+				if ( false === $param ) {
+					// Parameter doesn't exist. Nothing to do.
+					continue;
+				}
+
+				$this->check_code_is_escaped( $param['start'], ( $param['end'] + 1 ) );
+			}
+
+			return $end;
 		}
 
 		$params = PassedParameters::getParameters( $this->phpcsFile, $stackPtr );
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 8502e70e05..d499ceadc5 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -593,3 +593,15 @@ echo $var == 'foo' ? print 'message' : 'bar'; // OK.
 echo $var == 'foo' ? 'bar' : die( $var ); // Bad x 1, $var, not die().
 echo $var == 'foo' ? throw new Exception( $var ) : 'bar'; // Bad x 1, $var, not throw.
 echo $var == 'foo' ? print $var : 'bar'; // Bad x 1, $var, not print.
+
+/*
+ * When the `UnsafePrintingFunction` error code is ignored, the parameters
+ * for the unsafe functions should still be examined, but only the $text
+ * parameter needs escaping.
+ */
+// phpcs:disable WordPress.Security.EscapeOutput.UnsafePrintingFunction
+_e( $text, $domain ); // Bad x 1, only text param.
+_e( domain: $domain ); // OK, well not really, required $text parameter missing, but that's not our concern.
+_ex( domain: $domain, text: 'plain text', context: $context ); // OK.
+_e( domain: $domain, text: $text, ); // Bad x 1, only text param.
+// phpcs:enable
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 71d1975da0..72a376a5ab 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -147,6 +147,8 @@ public function getErrorList( $testFile = '' ) {
 					593 => 1,
 					594 => 1,
 					595 => 1,
+					603 => 1,
+					606 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From 8e7b9a4f481d7614a9dbc207f370e4344c83b6a7 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 21:47:09 +0200
Subject: [PATCH 38/44] Security/EscapeOutput: very minor code readability
 improvements [3]

---
 WordPress/Sniffs/Security/EscapeOutputSniff.php | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 79c31d1886..17384a0dd0 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -475,11 +475,10 @@ protected function check_code_is_escaped( $start, $end ) {
 		}
 
 		$in_cast = false;
+		$watch   = true;
 
 		// Looping through echo'd components.
-		$watch = true;
 		for ( $i = $start; $i < $end; $i++ ) {
-
 			// Ignore whitespaces and comments.
 			if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) ) {
 				continue;
@@ -491,14 +490,12 @@ protected function check_code_is_escaped( $start, $end ) {
 			}
 
 			if ( \T_OPEN_PARENTHESIS === $this->tokens[ $i ]['code'] ) {
-
 				if ( ! isset( $this->tokens[ $i ]['parenthesis_closer'] ) ) {
 					// Live coding or parse error.
 					break;
 				}
 
 				if ( $in_cast ) {
-
 					// Skip to the end of a function call if it has been casted to a safe value.
 					$i       = $this->tokens[ $i ]['parenthesis_closer'];
 					$in_cast = false;
@@ -639,7 +636,6 @@ protected function check_code_is_escaped( $start, $end ) {
 
 			// Now check that next token is a function call.
 			if ( \T_STRING === $this->tokens[ $i ]['code'] ) {
-
 				$ptr                    = $i;
 				$functionName           = $this->tokens[ $i ]['content'];
 				$function_opener        = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $i + 1 ), null, true );
@@ -649,7 +645,6 @@ protected function check_code_is_escaped( $start, $end ) {
 					&& \T_OPEN_PARENTHESIS === $this->tokens[ $function_opener ]['code']
 				) {
 					if ( ArrayWalkingFunctionsHelper::is_array_walking_function( $functionName ) ) {
-
 						// Get the callback parameter.
 						$callback = ArrayWalkingFunctionsHelper::get_callback_parameter( $this->phpcsFile, $ptr );
 
@@ -689,8 +684,7 @@ protected function check_code_is_escaped( $start, $end ) {
 				}
 
 				// If this is a safe function, we don't flag it.
-				if (
-					$is_formatting_function
+				if ( $is_formatting_function
 					|| $this->is_escaping_function( $functionName )
 					|| $this->is_auto_escaped_function( $functionName )
 				) {

From 2f40190ab62e6eb5197ebf38318a678566e42297 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 22:12:39 +0200
Subject: [PATCH 39/44] Security/EscapeOutput: bug fix - sniff did not handle
 arrays correctly

In particular when examining the parameters passed to a function call, the sniff may encounter parameters which are arrays.

As things were, the sniff would skip over the array opener and then continue like before.

This is buggy, in the same way as examining all function parameters as one long code block was buggy and would lead to errors throw on the incorrect token and/or false positives when the array item contains a ternary.

To fix this, I'm special casing arrays, parsing the array items and then recursively calling the `check_code_is_escaped()` method to examine each array item individually. After this, the original call to `check_code_is_escaped()` will continue from the array closer onwards.

This gives much higher accuracy when examining array items and fixes the bug.

Includes tests proving the bug(s) and safeguarding the fix.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 22 ++++++++++++++-----
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 15 +++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.php   |  4 ++++
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 17384a0dd0..19aa07b49e 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\BackCompat\BCFile;
 use PHPCSUtils\Tokens\Collections;
+use PHPCSUtils\Utils\Arrays;
 use PHPCSUtils\Utils\Conditions;
 use PHPCSUtils\Utils\Operators;
 use PHPCSUtils\Utils\PassedParameters;
@@ -536,13 +537,22 @@ protected function check_code_is_escaped( $start, $end ) {
 				continue;
 			}
 
-			// Handle arrays for those functions that accept them.
-			if ( \T_ARRAY === $this->tokens[ $i ]['code'] ) {
-				++$i; // Skip the opening parenthesis.
-				continue;
-			}
+			// Examine the items in an array individually for array parameters.
+			if ( isset( Collections::arrayOpenTokensBC()[ $this->tokens[ $i ]['code'] ] ) ) {
+				$array_open_close = Arrays::getOpenClose( $this->phpcsFile, $i );
+				if ( false === $array_open_close ) {
+					// Short list or misidentified short array token.
+					continue;
+				}
+
+				$array_items = PassedParameters::getParameters( $this->phpcsFile, $i, 0, true );
+				if ( ! empty( $array_items ) ) {
+					foreach ( $array_items as $array_item ) {
+						$this->check_code_is_escaped( $array_item['start'], ( $array_item['end'] + 1 ) );
+					}
+				}
 
-			if ( isset( Collections::shortArrayTokens()[ $this->tokens[ $i ]['code'] ] ) ) {
+				$i = $array_open_close['closer'];
 				continue;
 			}
 
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index d499ceadc5..0ad21737c6 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -605,3 +605,18 @@ _e( domain: $domain ); // OK, well not really, required $text parameter missing,
 _ex( domain: $domain, text: 'plain text', context: $context ); // OK.
 _e( domain: $domain, text: $text, ); // Bad x 1, only text param.
 // phpcs:enable
+
+// Array keys and values should be examined individually.
+wp_die( esc_html( $message ), '', array() ); // OK.
+wp_die(
+	esc_html( $message ), // OK.
+	'',
+	array(
+		'back_link' => true, // OK.
+		'response'  => $response ?? 200, // Bad.
+		'link_url'  => ( '' !== $this->link_url ) ? $this->link_url : '', // Bad.
+		'link_text' => ( '' !== $link_title ) ? esc_attr( $link_title ) : '', // OK.
+		'charset'   => [$set, $rtl] = $array, // Bad x 2 (silly code, but the list shouldn't be treated as an array).
+		'exit'      => $do_exit, // Bad x 1.
+	)
+);
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 72a376a5ab..d64c3d4319 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -149,6 +149,10 @@ public function getErrorList( $testFile = '' ) {
 					595 => 1,
 					603 => 1,
 					606 => 1,
+					616 => 1,
+					617 => 1,
+					619 => 2,
+					620 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From 3ccc769dd17da7ff668ebea4dcda60be841d044e Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 22:23:13 +0200
Subject: [PATCH 40/44] Security/EscapeOutput: group common token check
 together

At various points within the `check_code_is_escaped()` loop, the sniff would check whether the current token is a token which can be ignored.

This commit just groups those simple checks, which are not dependent on other logic in the loop, together and places them at the top of the loop for the highest efficiency.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 19aa07b49e..b4a10727df 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -485,8 +485,12 @@ protected function check_code_is_escaped( $start, $end ) {
 				continue;
 			}
 
-			// Ignore namespace separators.
-			if ( \T_NS_SEPARATOR === $this->tokens[ $i ]['code'] ) {
+			// Skip over irrelevant tokens.
+			if ( isset( Tokens::$magicConstants[ $this->tokens[ $i ]['code'] ] ) // Magic constants for debug functions.
+				|| \T_NS_SEPARATOR === $this->tokens[ $i ]['code']
+				|| \T_DOUBLE_ARROW === $this->tokens[ $i ]['code']
+				|| \T_CLOSE_PARENTHESIS === $this->tokens[ $i ]['code']
+			) {
 				continue;
 			}
 
@@ -556,15 +560,6 @@ protected function check_code_is_escaped( $start, $end ) {
 				continue;
 			}
 
-			if ( \in_array( $this->tokens[ $i ]['code'], array( \T_DOUBLE_ARROW, \T_CLOSE_PARENTHESIS ), true ) ) {
-				continue;
-			}
-
-			// Handle magic constants for debug functions.
-			if ( isset( Tokens::$magicConstants[ $this->tokens[ $i ]['code'] ] ) ) {
-				continue;
-			}
-
 			// Handle safe PHP native constants.
 			if ( \T_STRING === $this->tokens[ $i ]['code']
 				&& isset( $this->safe_php_constants[ $this->tokens[ $i ]['content'] ] )

From 1689ea50b66ffac9c43ca2ca17d7bad2ac72679a Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Fri, 28 Jul 2023 00:46:20 +0200
Subject: [PATCH 41/44] Security/EscapeOutput: improve handling of heredocs,
 include PHP 7.3+ flexible heredocs

As things were, when the sniff would encounter a heredoc, it would throw an error on the heredoc opener (`<<<EOD`) token and be done with it.

However, heredocs _can_, but don't necessarily always contain interpolated variables and expressions and when the heredoc doesn't contain any interpolation, it should be treated the same as any other plain text string.

This commit fixes the sniff to handle heredocs properly and throw errors only on heredoc lines containing interpolation.

Includes tests.
---
 .../Sniffs/Security/EscapeOutputSniff.php     | 19 +++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.1.inc | 21 +++++++++++++++
 .../Security/EscapeOutputUnitTest.20.inc      | 27 +++++++++++++++++++
 .../Tests/Security/EscapeOutputUnitTest.php   | 17 ++++++++++++
 4 files changed, 84 insertions(+)
 create mode 100644 WordPress/Tests/Security/EscapeOutputUnitTest.20.inc

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index b4a10727df..1b0af45c6e 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -639,6 +639,25 @@ protected function check_code_is_escaped( $start, $end ) {
 				continue;
 			}
 
+			// Handle heredocs separately as they only need escaping when interpolation is used.
+			if ( \T_START_HEREDOC === $this->tokens[ $i ]['code'] ) {
+				$current = ( $i + 1 );
+				while ( isset( $this->tokens[ $current ] ) && \T_HEREDOC === $this->tokens[ $current ]['code'] ) {
+					$embeds = TextStrings::getEmbeds( $this->tokens[ $current ]['content'] );
+					if ( ! empty( $embeds ) ) {
+						$this->phpcsFile->addError(
+							'All output should be run through an escaping function (see the Security sections in the WordPress Developer Handbooks), found interpolation in unescaped heredoc.',
+							$current,
+							'HeredocOutputNotEscaped'
+						);
+					}
+					++$current;
+				}
+
+				$i = $current;
+				continue;
+			}
+
 			// Now check that next token is a function call.
 			if ( \T_STRING === $this->tokens[ $i ]['code'] ) {
 				$ptr                    = $i;
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 0ad21737c6..e7d0164b90 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -620,3 +620,24 @@ wp_die(
 		'exit'      => $do_exit, // Bad x 1.
 	)
 );
+
+// Heredocs should only be flagged when they contain interpolated variables or expressions.
+echo <<<EOD
+Some text without interpolation.
+EOD;
+
+echo <<<"EOD"
+Some text without interpolation.
+EOD;
+
+echo <<<EOD
+Some text $with interpolation.
+Some text $with interpolation.
+Some text $with interpolation.
+EOD;
+
+echo <<<"EOD"
+Some text without interpolation.
+Some text {$with->interpolation}.
+Some text without interpolation.
+EOD;
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.20.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.20.inc
new file mode 100644
index 0000000000..baf0e66316
--- /dev/null
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.20.inc
@@ -0,0 +1,27 @@
+<?php
+
+/*
+ * Tests with PHP 7.3+ flexible heredoc/nowdoc.
+ * This must be a separate test case file as it is a parse error in PHP < 7.3.
+ */
+
+// Heredocs should only be flagged when they contain interpolated variables or expressions.
+echo <<<EOD
+	Some text without interpolation.
+	EOD;
+
+echo <<<"EOD"
+    Some text without interpolation.
+    EOD;
+
+echo <<<EOD
+		Some text $with interpolation.
+		Some text $with interpolation.
+		Some text $with interpolation.
+	EOD; // Bad x 3.
+
+echo <<<"EOD"
+	Some text without interpolation.
+	Some text {$with->interpolation}.
+	Some text without interpolation.
+  EOD; // Bad.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index d64c3d4319..30a779c23e 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -153,6 +153,10 @@ public function getErrorList( $testFile = '' ) {
 					617 => 1,
 					619 => 2,
 					620 => 1,
+					634 => 1,
+					635 => 1,
+					636 => 1,
+					641 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':
@@ -161,6 +165,19 @@ public function getErrorList( $testFile = '' ) {
 					4 => 1,
 				);
 
+			case 'EscapeOutputUnitTest.20.inc':
+				// These tests will only yield reliable results when PHPCS is run on PHP 7.3 or higher.
+				if ( \PHP_VERSION_ID < 70300 ) {
+					return array();
+				}
+
+				return array(
+					18 => 1,
+					19 => 1,
+					20 => 1,
+					25 => 1,
+				);
+
 			default:
 				return array();
 		}

From 2db499fc763e32000e371c46e201259592965bfa Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Fri, 28 Jul 2023 01:10:03 +0200
Subject: [PATCH 42/44] Security/EscapeOutput: improve handling of params in
 formatting functions

Similar to multiple other fixes made before, the parameters passed to a formatting function should be examined individually and not as one big code block.

Fixed now.

Includes tests.
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php | 17 ++++++++++++-----
 .../Tests/Security/EscapeOutputUnitTest.1.inc   |  7 +++++++
 .../Tests/Security/EscapeOutputUnitTest.php     |  1 +
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 1b0af45c6e..3fbf587eca 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -693,13 +693,20 @@ protected function check_code_is_escaped( $start, $end ) {
 						}
 					}
 
-					// Skip pointer to after the function.
-					// If this is a formatting function we just skip over the opening
-					// parenthesis. Otherwise we skip all the way to the closing.
+					// If this is a formatting function, we examine the parameters individually.
 					if ( $is_formatting_function ) {
-						$i     = ( $function_opener + 1 );
+						$formatting_params = PassedParameters::getParameters( $this->phpcsFile, $i );
+						if ( ! empty( $formatting_params ) ) {
+							foreach ( $formatting_params as $format_param ) {
+								$this->check_code_is_escaped( $format_param['start'], ( $format_param['end'] + 1 ) );
+							}
+						}
+
 						$watch = true;
-					} elseif ( isset( $this->tokens[ $function_opener ]['parenthesis_closer'] ) ) {
+					}
+
+					// Skip pointer to after the function.
+					if ( isset( $this->tokens[ $function_opener ]['parenthesis_closer'] ) ) {
 						$i = $this->tokens[ $function_opener ]['parenthesis_closer'];
 					} else {
 						// Live coding or parse error.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index e7d0164b90..4c45a48740 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -641,3 +641,10 @@ Some text without interpolation.
 Some text {$with->interpolation}.
 Some text without interpolation.
 EOD;
+
+// Parameters in formatting functions should be examined individually.
+echo sprintf(
+	'<li><a href="#%1$s" class="%2$s"%3$s%4$s>%5$s</a></li>',
+	( '' !== $this->link_title ) ? ' title="' . esc_attr( $this->link_title ) . '"' : '',
+	( '' !== $this->link_aria_label ) ? ' aria-label="' . $this->link_aria_label . '"' : '',
+); // Bad x 1.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 30a779c23e..0f3e971964 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -157,6 +157,7 @@ public function getErrorList( $testFile = '' ) {
 					635 => 1,
 					636 => 1,
 					641 => 1,
+					649 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From f165714a8438101b15d28efe856a0551b26b206d Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Fri, 28 Jul 2023 02:23:54 +0200
Subject: [PATCH 43/44] Security/EscapeOutput: special case get_search_query(
 false )

As per the reported issue, `get_search_query()` is unsafe when the `$escaped` parameter is set, but not set to `true`.

This commit special cases the function and checks the parameter. If the parameter is passed, but not set to `true`, a custom error message will be thrown.

Includes tests.

Fixes 1354
---
 WordPress/Sniffs/Security/EscapeOutputSniff.php     | 12 ++++++++++++
 WordPress/Tests/Security/EscapeOutputUnitTest.1.inc |  7 +++++++
 WordPress/Tests/Security/EscapeOutputUnitTest.php   |  3 +++
 3 files changed, 22 insertions(+)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 3fbf587eca..3c4f3e0504 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -719,6 +719,18 @@ protected function check_code_is_escaped( $start, $end ) {
 					|| $this->is_escaping_function( $functionName )
 					|| $this->is_auto_escaped_function( $functionName )
 				) {
+					// Special case get_search_query() which is unsafe if $escaped = false.
+					if ( 'get_search_query' === strtolower( $functionName ) ) {
+						$escaped_param = PassedParameters::getParameter( $this->phpcsFile, $ptr, 1, 'escaped' );
+						if ( false !== $escaped_param && 'true' !== $escaped_param['clean'] ) {
+							$this->phpcsFile->addError(
+								'Output from get_search_query() is unsafe due to $escaped parameter being set to "false".',
+								$ptr,
+								'UnsafeSearchQuery'
+							);
+						}
+					}
+
 					continue;
 				}
 
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
index 4c45a48740..cfe20caeee 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
@@ -648,3 +648,10 @@ echo sprintf(
 	( '' !== $this->link_title ) ? ' title="' . esc_attr( $this->link_title ) . '"' : '',
 	( '' !== $this->link_aria_label ) ? ' aria-label="' . $this->link_aria_label . '"' : '',
 ); // Bad x 1.
+
+echo '<input type="search" value="' . get_search_query() . '">'; // OK.
+echo '<input type="search" value="' . get_search_query( /*comment*/ true ) . '">'; // OK.
+echo '<input type="search" value="' . get_search_query( false ) . '">'; // Bad.
+echo '<input type="search" value="' . get_search_query( 0 ) . '">'; // Bad.
+echo '<input type="search" value="' . get_search_query( escape: false ) . '">'; // OK, well not really, typo in param name, but that's not our concern.
+echo '<input type="search" value="' . get_search_query( escaped: false ) . '">'; // Bad.
diff --git a/WordPress/Tests/Security/EscapeOutputUnitTest.php b/WordPress/Tests/Security/EscapeOutputUnitTest.php
index 0f3e971964..464affd5f2 100644
--- a/WordPress/Tests/Security/EscapeOutputUnitTest.php
+++ b/WordPress/Tests/Security/EscapeOutputUnitTest.php
@@ -158,6 +158,9 @@ public function getErrorList( $testFile = '' ) {
 					636 => 1,
 					641 => 1,
 					649 => 1,
+					654 => 1,
+					655 => 1,
+					657 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':

From bb0b7d6f72c9c05f978f403c64f963cb9dbcf552 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 27 Jul 2023 18:36:51 +0200
Subject: [PATCH 44/44] Security/EscapeOutput: various minor docs fixes

---
 WordPress/Sniffs/Security/EscapeOutputSniff.php | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/WordPress/Sniffs/Security/EscapeOutputSniff.php b/WordPress/Sniffs/Security/EscapeOutputSniff.php
index 3c4f3e0504..8d3cdcef3c 100644
--- a/WordPress/Sniffs/Security/EscapeOutputSniff.php
+++ b/WordPress/Sniffs/Security/EscapeOutputSniff.php
@@ -61,7 +61,7 @@ class EscapeOutputSniff extends AbstractFunctionRestrictionsSniff {
 	 * @since 0.11.0 Changed from public static to protected non-static.
 	 * @since 3.0.0  The format of the array values has changed from plain string to array.
 	 *
-	 * @var array
+	 * @var array<string, array>
 	 */
 	protected $unsafePrintingFunctions = array(
 		'_e'  => array(
@@ -83,7 +83,7 @@ class EscapeOutputSniff extends AbstractFunctionRestrictionsSniff {
 	 *
 	 * @since 1.0.0
 	 *
-	 * @var array
+	 * @var array<string, bool>
 	 */
 	private $safe_php_constants = array(
 		'PHP_EOL'             => true, // String.
@@ -103,7 +103,7 @@ class EscapeOutputSniff extends AbstractFunctionRestrictionsSniff {
 	 *
 	 * @since 0.12.0
 	 *
-	 * @var array
+	 * @var array<string|int, string|int>
 	 */
 	private $safe_components = array(
 		\T_LNUMBER                  => \T_LNUMBER,
@@ -134,7 +134,7 @@ class EscapeOutputSniff extends AbstractFunctionRestrictionsSniff {
 	/**
 	 * Returns an array of tokens this test wants to listen for.
 	 *
-	 * @return array
+	 * @return string|int[]
 	 */
 	public function register() {
 		// Enrich the list of "safe components" tokens.
@@ -560,7 +560,7 @@ protected function check_code_is_escaped( $start, $end ) {
 				continue;
 			}
 
-			// Handle safe PHP native constants.
+			// Ignore safe PHP native constants.
 			if ( \T_STRING === $this->tokens[ $i ]['code']
 				&& isset( $this->safe_php_constants[ $this->tokens[ $i ]['content'] ] )
 				&& ConstantsHelper::is_use_of_global_constant( $this->phpcsFile, $i )