Skip to content

Commit 2e09cb3

Browse files
rebeccahumrebeccahum
committed
Add new OptionsRaceConditionSniff
1 parent 164d67f commit 2e09cb3

File tree

3 files changed

+387
-0
lines changed

3 files changed

+387
-0
lines changed

‎WordPressVIPMinimum/Sniffs/Functions/OptionsRaceConditionSniff.php

+222
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
<?php
2+
/**
3+
* WordPressVIPMinimum Coding Standard.
4+
*
5+
* @package VIPCS\WordPressVIPMinimum
6+
*/
7+
8+
namespace WordPressVIPMinimum\Sniffs\Functions;
9+
10+
use WordPressVIPMinimum\Sniffs\Sniff;
11+
use PHP_CodeSniffer\Util\Tokens;
12+
use PHP_CodeSniffer\Files\File;
13+
use PHP_CodeSniffer\Sniffs\AbstractScopeSniff;
14+
15+
/**
16+
* This sniff checks to see if the deletion of an option is immediately followed by an addition of the same option.
17+
*/
18+
class OptionsRaceConditionSniff extends AbstractScopeSniff {
19+
20+
/**
21+
* Function name to delete option.
22+
*
23+
* @var string
24+
*/
25+
private $delete_option = 'delete_option';
26+
27+
/**
28+
* Function name to add option.
29+
*
30+
* @var string
31+
*/
32+
private $add_option = 'add_option';
33+
34+
/**
35+
* Constructs the test with the tokens it wishes to listen for.
36+
*/
37+
public function __construct() {
38+
parent::__construct(
39+
[ T_FUNCTION ],
40+
[ T_STRING ]
41+
);
42+
}
43+
44+
/**
45+
* Processes the function tokens within the class.
46+
*
47+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file where this token was found.
48+
* @param int $stackPtr The position where the token was found.
49+
* @param int $currScope The current scope opener token.
50+
*
51+
* @return void
52+
*/
53+
protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currScope ) {
54+
$tokens = $phpcsFile->getTokens();
55+
56+
if ( $tokens[ $stackPtr ]['content'] !== $this->delete_option ) {
57+
$stackPtr = $phpcsFile->findNext(
58+
[ T_STRING ],
59+
$stackPtr + 1,
60+
null,
61+
true,
62+
$this->delete_option
63+
);
64+
65+
if ( ! $stackPtr ) {
66+
return; // No delete_option found, bail.
67+
}
68+
}
69+
70+
$delete_option_scope_start = $phpcsFile->findNext(
71+
Tokens::$emptyTokens,
72+
$stackPtr + 1,
73+
null,
74+
true
75+
);
76+
77+
if ( ! $delete_option_scope_start || $tokens[ $delete_option_scope_start ]['code'] !== T_OPEN_PARENTHESIS ) {
78+
return; // Not a function call, bail.
79+
}
80+
81+
$delete_option_semicolon = $phpcsFile->findNext(
82+
[ T_SEMICOLON ],
83+
$tokens[ $delete_option_scope_start ]['parenthesis_closer'] + 1,
84+
null,
85+
false
86+
);
87+
88+
$delete_option_key = $phpcsFile->findNext(
89+
Tokens::$emptyTokens,
90+
$delete_option_scope_start + 1,
91+
null,
92+
true
93+
);
94+
95+
$add_option = $phpcsFile->findNext(
96+
Tokens::$emptyTokens,
97+
$delete_option_semicolon + 1,
98+
null,
99+
true
100+
);
101+
102+
$message = 'Concurrent calls to `delete_option()` and `add_option()` for %s may lead to race conditions in persistent object caching. Please consider using `update_option()` in place of both function calls, as it will also add the option if it does not exist.';
103+
104+
if ( $tokens[ $add_option ]['code'] === T_IF ) {
105+
// Check inside scope of first IF statement after for `add_option()` being called.
106+
$add_option_inside_if = $phpcsFile->findNext(
107+
[ T_STRING ],
108+
$tokens[ $add_option ]['scope_opener'] + 1,
109+
null,
110+
false,
111+
$this->add_option
112+
);
113+
114+
if ( ! $add_option_inside_if || $add_option_inside_if > $tokens[ $add_option ]['scope_closer'] ) {
115+
return; // No add_option() call inside first IF statement or add_option found not in IF scope.
116+
}
117+
118+
$add_option_inside_if_scope_start = $phpcsFile->findNext(
119+
Tokens::$emptyTokens,
120+
$add_option_inside_if + 1,
121+
null,
122+
true
123+
);
124+
125+
if ( ! $add_option_inside_if_scope_start || $tokens[ $add_option_inside_if_scope_start ]['code'] !== T_OPEN_PARENTHESIS ) {
126+
return; // Not a function call, bail.
127+
}
128+
129+
$add_option_inside_if_option_key = $phpcsFile->findNext(
130+
Tokens::$emptyTokens,
131+
$add_option_inside_if_scope_start + 1,
132+
null,
133+
true
134+
);
135+
136+
if ( $add_option_inside_if_option_key && $this->is_same_option_key( $tokens, $add_option_inside_if_option_key, $delete_option_key ) ) {
137+
$phpcsFile->addWarning( $message, $add_option_inside_if_option_key, 'OptionsRaceCondition' );
138+
}
139+
140+
// Walk ahead out of IF control structure.
141+
$add_option = $phpcsFile->findNext(
142+
Tokens::$emptyTokens,
143+
$tokens[ $add_option ]['scope_closer'] + 1,
144+
null,
145+
true
146+
);
147+
}
148+
149+
if ( $tokens[ $add_option ]['code'] === T_VARIABLE ) {
150+
// Account for variable assignments.
151+
$equals = $phpcsFile->findNext(
152+
Tokens::$emptyTokens,
153+
$add_option + 1,
154+
null,
155+
true
156+
);
157+
158+
if ( $tokens[ $equals ]['code'] === T_EQUAL ) {
159+
$add_option = $phpcsFile->findNext(
160+
Tokens::$emptyTokens,
161+
$equals + 1,
162+
null,
163+
true
164+
);
165+
}
166+
}
167+
168+
if ( $tokens[ $add_option ]['code'] !== T_STRING || $tokens[ $add_option ]['content'] !== $this->add_option ) {
169+
return; // add_option() isn't immediately following delete_option(), bail.
170+
}
171+
172+
$add_option_scope_start = $phpcsFile->findNext(
173+
Tokens::$emptyTokens,
174+
$add_option + 1,
175+
null,
176+
true
177+
);
178+
179+
if ( ! $add_option_scope_start || $tokens[ $add_option_scope_start ]['code'] !== T_OPEN_PARENTHESIS ) {
180+
return; // Not a function call, bail.
181+
}
182+
183+
// Check if it's the same option being deleted earlier.
184+
$add_option_key = $phpcsFile->findNext(
185+
Tokens::$emptyTokens,
186+
$add_option_scope_start + 1,
187+
null,
188+
true
189+
);
190+
191+
if ( $this->is_same_option_key( $tokens, $add_option_key, $delete_option_key ) ) {
192+
$phpcsFile->addWarning( $message, $add_option_key, 'OptionsRaceCondition' );
193+
return;
194+
}
195+
}
196+
197+
/**
198+
* Processes a token that is found within the scope that this test is
199+
* listening to.
200+
*
201+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file where this token was found.
202+
* @param int $stackPtr The position in the stack where this
203+
* token was found.
204+
* @return void
205+
*/
206+
protected function processTokenOutsideScope( File $phpcsFile, $stackPtr ) {
207+
}
208+
209+
/**
210+
* Check if option is the same.
211+
*
212+
* @param array $tokens List of PHPCS tokens.
213+
* @param int $first_option Stack position of first option.
214+
* @param int $second_option Stack position of second option to match against.
215+
*
216+
* @return false
217+
*/
218+
private function is_same_option_key( $tokens, $first_option, $second_option ) {
219+
return $tokens[ $first_option ]['code'] === $tokens[ $second_option ]['code'] &&
220+
$tokens[ $first_option ]['content'] === $tokens[ $second_option ]['content'];
221+
}
222+
}

‎WordPressVIPMinimum/Tests/Functions/OptionsRaceConditionUnitTest.inc

+112
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
<?php
2+
3+
function test_with_function_between() {
4+
$defaults = get_default_settings();
5+
delete_option( 'my_settings' );
6+
7+
do_stuff();
8+
9+
add_option( 'my_settings', $defaults ); // OK - since there is stuff being done in between.
10+
}
11+
12+
function test_different_option_name() {
13+
$defaults = get_default_settings();
14+
delete_option( 'my_settings' );
15+
add_option( 'settings', [] ); // OK - Not same key as delete_option().
16+
}
17+
18+
function test_same_option_key_by_var_assignment() {
19+
$key = 'option_key';
20+
delete_option( $key );
21+
$add = add_option( $key, $key ); // Warning.
22+
}
23+
24+
function test_same_option_key() {
25+
$key = 'option_key';
26+
delete_option( $key );
27+
add_option( $key, 'data' ); // Warning.
28+
}
29+
30+
function test_same_option_key_var_assignment2() {
31+
$delete = delete_option( 'option_key' );
32+
add_option( 'option_key', [ 'stuff', '123' ] ); // Warning.
33+
}
34+
35+
function test_same_option_key_string() {
36+
$delete = delete_option( 'option_key' );
37+
add_option( 'option_key', [] ); // Warning.
38+
}
39+
40+
function test_line_breaks() {
41+
$delete = delete_option( 'option_key' );
42+
43+
44+
45+
add_option( 'option_key', [] ); // Warning.
46+
}
47+
48+
function test_bad_spacing() {
49+
$delete = delete_option( 'option_key' );
50+
add_option('option_key', []); // Warning.
51+
}
52+
53+
function test_empty_tokens_between() {
54+
delete_option( 'key' );
55+
56+
// Random comment here.
57+
/* Random stuff. */
58+
59+
add_option ( 'key' ); // Warning.
60+
}
61+
62+
function test_first_if() {
63+
$delete = delete_option ( 'option_key' );
64+
if ( $delete ) {
65+
add_option( 'option_key', [] ); // Warning.
66+
}
67+
}
68+
69+
function test_inside_first_if_but_different_key() {
70+
$delete = delete_option ( 'option_key' );
71+
if ( $delete ) {
72+
add_option( 'different_option_key', [] ); // OK.
73+
}
74+
}
75+
76+
function test_second_if() {
77+
$delete = delete_option ( 'option_key' );
78+
if ( do_something() ) {
79+
// Stuff.
80+
}
81+
if ( $delete ) {
82+
add_option( 'option_key', [] ); // OK - Not on first IF.
83+
}
84+
}
85+
86+
function concurrent_option_writes() {
87+
delete_option( 'test_option' );
88+
add_option( 'test_option', [ 1, 2, 3 ] ); // Warning.
89+
90+
do_stuff();
91+
92+
$delete = delete_option( 'another_test_option' );
93+
if ( $delete ) {
94+
add_option( 'another_test_option', $data ); // Warning.
95+
}
96+
}
97+
98+
function concurrent_option_writes_with_one_if_different_key() {
99+
$test = delete_option( 'test_option' );
100+
if ( $test ) {
101+
add_option( 'option', [ 1, 2, 3 ] );
102+
}
103+
$test = add_option( 'test_option', [ 1, 2, 3 ] ); // Warning.
104+
}
105+
106+
function concurrent_option_writes_with_one_if_same_key() {
107+
$test = delete_option( 'test_option' );
108+
if ( $test ) {
109+
add_option( 'test_option', [] ); // Warning.
110+
}
111+
$test = add_option( 'test_option', [ 1, 2, 3 ] ); // Warning.
112+
}

‎WordPressVIPMinimum/Tests/Functions/OptionsRaceConditionUnitTest.php

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
/**
3+
* Unit test class for WordPressVIPMinimum Coding Standard.
4+
*
5+
* @package VIPCS\WordPressVIPMinimum
6+
*/
7+
8+
namespace WordPressVIPMinimum\Tests\Functions;
9+
10+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
11+
12+
/**
13+
* Unit test class for the OptionsRaceCondition sniff.
14+
*
15+
* @package VIPCS\WordPressVIPMinimum
16+
*
17+
* @covers \WordPressVIPMinimum\Sniffs\Functions\OptionsRaceConditionSniff
18+
*/
19+
class OptionsRaceConditionUnitTest extends AbstractSniffUnitTest {
20+
21+
/**
22+
* Returns the lines where errors should occur.
23+
*
24+
* @return array <int line number> => <int number of errors>
25+
*/
26+
public function getErrorList() {
27+
return [];
28+
}
29+
30+
/**
31+
* Returns the lines where warnings should occur.
32+
*
33+
* @return array <int line number> => <int number of warnings>
34+
*/
35+
public function getWarningList() {
36+
return [
37+
21 => 1,
38+
27 => 1,
39+
32 => 1,
40+
37 => 1,
41+
45 => 1,
42+
50 => 1,
43+
59 => 1,
44+
65 => 1,
45+
88 => 1,
46+
94 => 1,
47+
103 => 1,
48+
109 => 1,
49+
111 => 1,
50+
];
51+
}
52+
53+
}

0 commit comments

Comments
 (0)