Skip to content

Commit 26bddea

Browse files
committed
Security/StaticStrreplace: bug fix - false negatives for PHP 5.4+ short array parameters
Fixed now. Includes tests. Note: some of the "should not be flagged" tests already use short arrays since the tests were expanded.
1 parent a14e531 commit 26bddea

File tree

3 files changed

+19
-5
lines changed

3 files changed

+19
-5
lines changed

WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
namespace WordPressVIPMinimum\Sniffs\Security;
1111

1212
use PHP_CodeSniffer\Util\Tokens;
13+
use PHPCSUtils\Tokens\Collections;
14+
use PHPCSUtils\Utils\Arrays;
1315
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
1416

1517
/**
@@ -55,15 +57,20 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
5557
$next_start_ptr = $openBracket + 1;
5658
for ( $i = 0; $i < 3; $i++ ) {
5759
$param_ptr = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_COMMA ] ), $next_start_ptr, null, true );
60+
if ( $param_ptr === false ) {
61+
// Live coding or parse error. Ignore.
62+
return;
63+
}
5864

59-
if ( $this->tokens[ $param_ptr ]['code'] === T_ARRAY ) {
60-
$openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param_ptr + 1, null, true );
61-
if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) {
65+
if ( isset( Collections::arrayOpenTokensBC()[ $this->tokens[ $param_ptr ]['code'] ] ) ) {
66+
$arrayOpenClose = Arrays::getOpenClose( $this->phpcsFile, $param_ptr );
67+
if ( $arrayOpenClose === false ) {
68+
// Short list, parse error or live coding, bow out.
6269
return;
6370
}
6471

65-
// Find the closing bracket.
66-
$closeBracket = $this->tokens[ $openBracket ]['parenthesis_closer'];
72+
$openBracket = $arrayOpenClose['opener'];
73+
$closeBracket = $arrayOpenClose['closer'];
6774

6875
$array_item_ptr = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_COMMA ] ), $openBracket + 1, $closeBracket, true );
6976
while ( $array_item_ptr !== false ) {

WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.inc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,9 @@ Str_replace(
5555
); // Bad.
5656
str_replace( array( 'foo', 'bar' ), array( 'bar', 'foo' ), array( 'foobar', 'foobar' ) ); // Bad.
5757
str_replace( 'foo', 'bar', 'foobar', $count ); // Bad.
58+
59+
// Handle PHP 5.4+ short array syntax correctly.
60+
\str_replace( [ 'foo', 'bar' ], [ 'bar', 'foo' ], [ 'foobar', 'foobar' ], ); // Bad.
61+
62+
// Don't confuse PHP 7.1+ short list with short array.
63+
str_replace( [ $a, $b ] = $search, [ 'bar', 'foo' ], 'foobar', ); // OK.

WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public function getErrorList() {
2727
51 => 1,
2828
56 => 1,
2929
57 => 1,
30+
60 => 1,
3031
];
3132
}
3233

0 commit comments

Comments
 (0)