Skip to content

Commit b54105f

Browse files
committed
RestrictedVariables: don't report on "use" in isset()
Disregard when the existence of a restricted variable is being checked. This uses the upstream WPCS `Sniff::is_in_isset_or_empty()` method. This means that variables will not be reported as "used" when they are wrapped in a call to: * `isset()` * `empty()` * `array_key_exists()` This also means that the `if ( isset( $_SERVER['REMOTE_ADDR'] ) ) {` test on line 13 will no longer report a warning. As `$_SERVER['REMOTE_ADDR']` was then no longer tested for and `$_SERVER['HTTP_USER_AGENT']` was being tested twice (line 14 and line 28), I've changed the occurrence on line 14 to use `$_SERVER['REMOTE_ADDR']` to make sure both are still tested. Includes additional unit test for the case as reported by the op. Fixes 568
1 parent 8ff3fa9 commit b54105f

File tree

4 files changed

+12
-4
lines changed

4 files changed

+12
-4
lines changed

WordPress-VIP-Go/ruleset-test.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ setcookie( 'cookie[three]', 'cookiethree' ); // Error + Message.
5353
$x = sanitize_key( $_COOKIE['bar'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated -- Error + Message.
5454

5555
// WordPressVIPMinimum.Variables.RestrictedVariables.cache_constraints___SERVER__HTTP_USER_AGENT__
56-
if ( ! isset( $_SERVER['HTTP_USER_AGENT'] ) ) { // Error + Message.
56+
if ( isset( $_SERVER['HTTP_USER_AGENT'] ) && $_SERVER['HTTP_USER_AGENT'] === 'some_value' ) { // Error + Message.
5757
}
5858

5959
// WordPress.WP.AlternativeFunctions.file_system_read_fclose

WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,11 @@ public function process_token( $stackPtr ) {
143143
}
144144
}
145145

146+
if ( $this->is_in_isset_or_empty( $stackPtr ) === true ) {
147+
// Checking whether a variable exists is not the same as using it.
148+
return;
149+
}
150+
146151
foreach ( $this->groups_cache as $groupName => $group ) {
147152

148153
if ( isset( $this->excluded_groups[ $groupName ] ) ) {

WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.inc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ $wp_db->update( $wpdb->usermeta, array( 'meta_value' => 'bar!' ), array( 'user_i
1010

1111
$query = "SELECT * FROM $wpdb->posts"; // Ok.
1212

13-
if ( isset( $_SERVER['REMOTE_ADDR'] ) ) { // Warning.
14-
foo( $_SERVER['HTTP_USER_AGENT'] ); // Warning.
13+
if ( isset( $_SERVER['REMOTE_ADDR'] ) ) { // OK.
14+
foo( $_SERVER['REMOTE_ADDR'] ); // Warning.
1515
}
1616

1717
$x = $_COOKIE['bar']; // Warning.
@@ -35,3 +35,7 @@ $query = "SELECT * FROM $wpdb->usermeta"; // Ok, excluded.
3535

3636
foo( $_SESSION ); // Error.
3737
foo( $_SESSION['bar'] ); // Error.
38+
39+
if ( isset( $_SESSION ) ) { // OK.
40+
$cache = false;
41+
}

WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ public function getErrorList() {
4343
*/
4444
public function getWarningList() {
4545
return [
46-
13 => 1,
4746
14 => 1,
4847
17 => 1,
4948
28 => 1,

0 commit comments

Comments
 (0)