From 74c0a2f226b345db1ead1709a93a4eec5665b96c Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Apr 2023 17:01:53 +0200 Subject: [PATCH] Move `is_in_type_test()` utility method to dedicated `ContextHelper` The `is_in_type_test()` utility method is only used by a small set of sniffs, so is better placed in a dedicated class. This commit moves the `is_in_type_test()` method and the associated `$typeTestFunctions` property to the new `WordPressCS\WordPress\Helpers\ContextHelper` class and starts using that class in the relevant sniffs. Related to 1465 This method will be tested via the `WordPress.Security.NonceVerification` sniff (via pre-existing tests). --- WordPress/Helpers/ContextHelper.php | 55 +++++++++++++++++++ WordPress/Sniff.php | 50 ----------------- .../Security/NonceVerificationSniff.php | 2 +- .../Security/ValidatedSanitizedInputSniff.php | 3 +- .../Security/NonceVerificationUnitTest.php | 1 + 5 files changed, 59 insertions(+), 52 deletions(-) diff --git a/WordPress/Helpers/ContextHelper.php b/WordPress/Helpers/ContextHelper.php index 8ddbb29c4e..79e8d4e6f7 100644 --- a/WordPress/Helpers/ContextHelper.php +++ b/WordPress/Helpers/ContextHelper.php @@ -27,6 +27,40 @@ */ final class ContextHelper { + /** + * List of PHP native functions to test the type of a variable. + * + * Using these functions is safe in combination with superglobals without + * unslashing or sanitization. + * + * They should, however, not be regarded as unslashing or sanitization functions. + * + * @since 2.1.0 + * @since 3.0.0 - Moved from the Sniff class to this class. + * - The property visibility was changed from `protected` to `private static`. + * + * @var array + */ + private static $typeTestFunctions = array( + 'is_array' => true, + 'is_bool' => true, + 'is_callable' => true, + 'is_countable' => true, + 'is_double' => true, + 'is_float' => true, + 'is_int' => true, + 'is_integer' => true, + 'is_iterable' => true, + 'is_long' => true, + 'is_null' => true, + 'is_numeric' => true, + 'is_object' => true, + 'is_real' => true, + 'is_resource' => true, + 'is_scalar' => true, + 'is_string' => true, + ); + /** * Check if a particular token acts - statically or non-statically - on an object. * @@ -163,4 +197,25 @@ public static function is_in_function_call( File $phpcsFile, $stackPtr, array $v return false; } + + /** + * Check if a token is inside of an is_...() statement. + * + * @since 2.1.0 + * @since 3.0.0 - Moved from the Sniff class to this class. + * - The method visibility was changed from `protected` to `public static`. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The index of the token in the stack. + * + * @return bool Whether the token is being type tested. + */ + public static function is_in_type_test( File $phpcsFile, $stackPtr ) { + /* + * Casting the potential integer stack pointer return value to boolean here is fine. + * The return can never be `0` as there will always be a PHP open tag before the + * function call. + */ + return (bool) self::is_in_function_call( $phpcsFile, $stackPtr, self::$typeTestFunctions ); + } } diff --git a/WordPress/Sniff.php b/WordPress/Sniff.php index e1a8ad89d3..37542e8019 100644 --- a/WordPress/Sniff.php +++ b/WordPress/Sniff.php @@ -249,38 +249,6 @@ abstract class Sniff implements PHPCS_Sniff { 'wp_unslash' => true, ); - /** - * List of PHP native functions to test the type of a variable. - * - * Using these functions is safe in combination with superglobals without - * unslashing or sanitization. - * - * They should, however, not be regarded as unslashing or sanitization functions. - * - * @since 2.1.0 - * - * @var array - */ - protected $typeTestFunctions = array( - 'is_array' => true, - 'is_bool' => true, - 'is_callable' => true, - 'is_countable' => true, - 'is_double' => true, - 'is_float' => true, - 'is_int' => true, - 'is_integer' => true, - 'is_iterable' => true, - 'is_long' => true, - 'is_null' => true, - 'is_numeric' => true, - 'is_object' => true, - 'is_real' => true, - 'is_resource' => true, - 'is_scalar' => true, - 'is_string' => true, - ); - /** * Token which when they preceed code indicate the value is safely casted. * @@ -508,24 +476,6 @@ protected function is_in_isset_or_empty( $stackPtr ) { return false; } - /** - * Check if a token is inside of an is_...() statement. - * - * @since 2.1.0 - * - * @param int $stackPtr The index of the token in the stack. - * - * @return bool Whether the token is being type tested. - */ - protected function is_in_type_test( $stackPtr ) { - /* - * Casting the potential integer stack pointer return value to boolean here is fine. - * The return can never be `0` as there will always be a PHP open tag before the - * function call. - */ - return (bool) ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $this->typeTestFunctions ); - } - /** * Check if something is only being sanitized. * diff --git a/WordPress/Sniffs/Security/NonceVerificationSniff.php b/WordPress/Sniffs/Security/NonceVerificationSniff.php index e7a6dc4114..bf3a44f36c 100644 --- a/WordPress/Sniffs/Security/NonceVerificationSniff.php +++ b/WordPress/Sniffs/Security/NonceVerificationSniff.php @@ -201,7 +201,7 @@ private function has_nonce_check( $stackPtr ) { $allow_nonce_after = false; if ( $this->is_in_isset_or_empty( $stackPtr ) - || $this->is_in_type_test( $stackPtr ) + || ContextHelper::is_in_type_test( $this->phpcsFile, $stackPtr ) || VariableHelper::is_comparison( $this->phpcsFile, $stackPtr ) || $this->is_in_array_comparison( $stackPtr ) || ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $this->unslashingFunctions ) !== false diff --git a/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php b/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php index 8067714a43..dc3a6fc619 100644 --- a/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php +++ b/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php @@ -11,6 +11,7 @@ use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Utils\TextStrings; +use WordPressCS\WordPress\Helpers\ContextHelper; use WordPressCS\WordPress\Helpers\RulesetPropertyHelper; use WordPressCS\WordPress\Helpers\VariableHelper; use WordPressCS\WordPress\Sniff; @@ -177,7 +178,7 @@ function( $embed ) { } // If this variable is being tested with one of the `is_..()` functions, sanitization isn't needed. - if ( $this->is_in_type_test( $stackPtr ) ) { + if ( ContextHelper::is_in_type_test( $this->phpcsFile, $stackPtr ) ) { return; } diff --git a/WordPress/Tests/Security/NonceVerificationUnitTest.php b/WordPress/Tests/Security/NonceVerificationUnitTest.php index 3cf402baee..ea3ad7ceb5 100644 --- a/WordPress/Tests/Security/NonceVerificationUnitTest.php +++ b/WordPress/Tests/Security/NonceVerificationUnitTest.php @@ -21,6 +21,7 @@ * @since 1.0.0 This sniff has been moved from the `CSRF` category to the `Security` category. * * @covers \WordPressCS\WordPress\Helpers\ContextHelper::is_in_function_call + * @covers \WordPressCS\WordPress\Helpers\ContextHelper::is_in_type_test * @covers \WordPressCS\WordPress\Sniffs\Security\NonceVerificationSniff */ final class NonceVerificationUnitTest extends AbstractSniffUnitTest {