Skip to content

Commit

Permalink
ContextHelper::is_in_array_comparison(): add support for PHP 8.0 name…
Browse files Browse the repository at this point in the history
…d parameters

With named parameters, the function call may have two parameters, without the required `$filter_value` param being one of them. Would make for an invalid function call, but that's not the concern of this sniff.

Fixed now by looking for a specific named parameter.

Includes tests in the `WordPress.Security.NonceVerification` test file (which include PHP 7.3+ trailing commas in function calls).
  • Loading branch information
jrfnl committed Apr 24, 2023
1 parent 3266b92 commit 75410b4
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 5 deletions.
15 changes: 10 additions & 5 deletions WordPress/Helpers/ContextHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,19 +94,22 @@ final class ContextHelper {
/**
* Array functions to compare a $needle to a predefined set of values.
*
* If the value is set to an integer, the function needs to have at least that
* many parameters for it to be considered as a comparison.
* If the value is set to an array, the parameter specified in the array is
* required for the function call to be considered as a comparison.
*
* @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 <string function name> => <true|int>
* @var array <string function name> => <true|array>
*/
private static $arrayCompareFunctions = array(
'in_array' => true,
'array_search' => true,
'array_keys' => 2,
'array_keys' => array(
'position' => 2,
'name' => 'filter_value',
),
);

/**
Expand Down Expand Up @@ -347,7 +350,9 @@ public static function is_in_array_comparison( File $phpcsFile, $stackPtr ) {
return true;
}

if ( PassedParameters::getParameterCount( $phpcsFile, $function_ptr ) >= self::$arrayCompareFunctions[ $function_name ] ) {
$target_param = self::$arrayCompareFunctions[ $function_name ];
$found_param = PassedParameters::getParameter( $phpcsFile, $function_ptr, $target_param['position'], $target_param['name'] );
if ( false !== $found_param ) {
return true;
}

Expand Down
14 changes: 14 additions & 0 deletions WordPress/Tests/Security/NonceVerificationUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -377,3 +377,17 @@ function disallow_for_non_array_comparison_in_condition() {
foo();
}
}

function allow_for_array_comparison_in_condition_with_named_params() {
if ( array_keys( filter_value: 'my_action', array: $_GET['actions'], strict: true, ) ) { // OK.
check_admin_referer( 'foo' );
foo();
}
}

function disallow_for_non_array_comparison_in_condition_with_named_params() {
if ( array_keys( strict: true, array: $_GET['actions'], ) ) { // Bad, missing $filter_value param. Invalid function call, but not our concern.
check_admin_referer( 'foo' );
foo();
}
}
1 change: 1 addition & 0 deletions WordPress/Tests/Security/NonceVerificationUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public function getErrorList() {
public function getWarningList() {
return array(
375 => 1,
389 => 1,
);
}
}

0 comments on commit 75410b4

Please sign in to comment.