Skip to content

Commit ef84c20

Browse files
committed
Hooks/RestrictedHooks: bug fix - false positives for dynamic name with concatenation
As things were, the code in the `normalize_hook_name_from_parameter()` method would check for concatenation and if found, would gather the contents of all `T_CONSTANT_ENCAPSED_STRING` tokens found in the parameter. However, this presumes all the concatenated parts are text strings and disregards the possibility that variables would be concatenated in, leading to false positives. Fixed now. Includes tests.
1 parent 52ee722 commit ef84c20

2 files changed

Lines changed: 25 additions & 20 deletions

File tree

WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace WordPressVIPMinimum\Sniffs\Hooks;
1111

12+
use PHP_CodeSniffer\Util\Tokens;
1213
use PHPCSUtils\Utils\MessageHelper;
1314
use PHPCSUtils\Utils\PassedParameters;
1415
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
@@ -91,6 +92,10 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
9192
}
9293

9394
$normalized_hook_name = $this->normalize_hook_name_from_parameter( $hook_name_param );
95+
if ( $normalized_hook_name === '' ) {
96+
// Dynamic hook name. Cannot reliably determine if it's one of the targets. Bow out.
97+
return;
98+
}
9499

95100
foreach ( $this->restricted_hook_groups as $group => $group_args ) {
96101
foreach ( $group_args['hooks'] as $hook ) {
@@ -107,31 +112,27 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
107112
*
108113
* @param array $parameter Array with information about a parameter.
109114
*
110-
* @return string Normalized hook name.
115+
* @return string Normalized hook name or an empty string if the hook name could not be determined.
111116
*/
112117
private function normalize_hook_name_from_parameter( $parameter ) {
113-
// If concatenation is found, build hook name.
114-
$concat_ptr = $this->phpcsFile->findNext(
115-
T_STRING_CONCAT,
116-
$parameter['start'],
117-
$parameter['end'],
118-
false,
119-
null,
120-
true
121-
);
118+
$allowed_tokens = Tokens::$emptyTokens;
119+
$allowed_tokens += [
120+
T_STRING_CONCAT => T_STRING_CONCAT,
121+
T_CONSTANT_ENCAPSED_STRING => T_CONSTANT_ENCAPSED_STRING,
122+
];
122123

123-
if ( $concat_ptr ) {
124-
$hook_name = '';
125-
for ( $i = $parameter['start']; $i <= $parameter['end']; $i++ ) {
126-
if ( $this->tokens[ $i ]['code'] === T_CONSTANT_ENCAPSED_STRING ) {
127-
$hook_name .= str_replace( [ "'", '"' ], '', $this->tokens[ $i ]['content'] );
128-
}
124+
$has_disallowed_token = $this->phpcsFile->findNext( $allowed_tokens, $parameter['start'], ( $parameter['end'] + 1 ), true );
125+
if ( $has_disallowed_token !== false ) {
126+
return '';
127+
}
128+
129+
$hook_name = '';
130+
for ( $i = $parameter['start']; $i <= $parameter['end']; $i++ ) {
131+
if ( $this->tokens[ $i ]['code'] === T_CONSTANT_ENCAPSED_STRING ) {
132+
$hook_name .= str_replace( [ "'", '"' ], '', $this->tokens[ $i ]['content'] );
129133
}
130-
} else {
131-
$hook_name = $parameter['clean'];
132134
}
133135

134-
// Remove quotes (double and single).
135-
return str_replace( [ "'", '"' ], '', $hook_name );
136+
return $hook_name;
136137
}
137138
}

WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.inc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,7 @@ add_filter( 'upLoad_mimeS' , $callback); // OK, not our target.
7474

7575
// Bug fix - spacing vs concatenation.
7676
add_filter('do_' . 'robots' . 'txt', 'bad_example_function'); // Warning.
77+
78+
// Ignore partially dynamic hook names.
79+
add_filter( 'robots_' . $something . 'txt' , $callback); // OK, ignored as undetermined.
80+
add_filter( 'http_request_timeout' . $something, $callback); // OK, ignored as undetermined.

0 commit comments

Comments
 (0)