Skip to content

Commit d462b85

Browse files
GaryJonesjrfnl
andauthored
Merge pull request #1648 from NielsdeBlaauw/feature/ini-set-sniff
Fixes #1447, adds new sniff for blacklisted ini_set directives. Co-authored-by: Juliette <[email protected]>
2 parents 1f4d912 + 9f6c94b commit d462b85

File tree

7 files changed

+296
-9
lines changed

7 files changed

+296
-9
lines changed

WordPress-Extra/ruleset.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@
113113
https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/242 -->
114114
<rule ref="WordPress.PHP.StrictComparisons"/>
115115

116+
<!-- Detect incorrect or risky use of the `ini_set()` function.
117+
https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1447 -->
118+
<rule ref="WordPress.PHP.IniSet"/>
119+
116120
<!-- Check that in_array() and array_search() use strict comparisons.
117121
https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/399
118122
https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/503 -->

WordPress/Sniffs/PHP/DiscouragedPHPFunctionsSniff.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,10 @@ public function getGroups() {
5656

5757
'runtime_configuration' => array(
5858
'type' => 'warning',
59-
'message' => '%s() found. Changing configuration at runtime is rarely necessary.',
59+
'message' => '%s() found. Changing configuration values at runtime is strongly discouraged.',
6060
'functions' => array(
6161
'error_reporting',
62-
'ini_alter',
6362
'ini_restore',
64-
'ini_set',
6563
'apache_setenv',
6664
'putenv',
6765
'set_include_path',
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
<?php
2+
/**
3+
* WordPress Coding Standard.
4+
*
5+
* @package WPCS\WordPressCodingStandards
6+
* @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards
7+
* @license https://opensource.org/licenses/MIT MIT
8+
*/
9+
10+
namespace WordPressCS\WordPress\Sniffs\PHP;
11+
12+
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
13+
14+
/**
15+
* Detect use of the `ini_set()` function.
16+
*
17+
* - Won't throw notices for "safe" ini directives as listed in the whitelist.
18+
* - Throws errors for ini directives listed in the blacklist.
19+
* - A warning will be thrown in all other cases.
20+
*
21+
* @package WPCS\WordPressCodingStandards
22+
*
23+
* @since 2.1.0
24+
*/
25+
class IniSetSniff extends AbstractFunctionParameterSniff {
26+
27+
/**
28+
* Array of functions that must be checked.
29+
*
30+
* @since 2.1.0
31+
*
32+
* @var array Multidimensional array with parameter details.
33+
* $target_functions = array(
34+
* (string) Function name.
35+
* );
36+
*/
37+
protected $target_functions = array(
38+
'ini_set' => true,
39+
'ini_alter' => true,
40+
);
41+
42+
/**
43+
* Array of PHP configuration options that are allowed to be manipulated.
44+
*
45+
* @since 2.1.0
46+
*
47+
* @var array Multidimensional array with parameter details.
48+
* $whitelisted_options = array(
49+
* (string) option name. = array(
50+
* (string[]) 'valid_values' = array()
51+
* )
52+
* );
53+
*/
54+
protected $whitelisted_options = array(
55+
'auto_detect_line_endings' => array(),
56+
'highlight.bg' => array(),
57+
'highlight.comment' => array(),
58+
'highlight.default' => array(),
59+
'highlight.html' => array(),
60+
'highlight.keyword' => array(),
61+
'highlight.string' => array(),
62+
'short_open_tag' => array(
63+
'valid_values' => array( 'true', '1', 'on' ),
64+
),
65+
);
66+
67+
/**
68+
* Array of PHP configuration options that are not allowed to be manipulated.
69+
*
70+
* @since 2.1.0
71+
*
72+
* @var array Multidimensional array with parameter details.
73+
* $blacklisted_options = array(
74+
* (string) option name. = array(
75+
* (string[]) 'invalid_values' = array()
76+
* (string) 'message'
77+
* )
78+
* );
79+
*/
80+
protected $blacklisted_options = array(
81+
'bcmath.scale' => array(
82+
'message' => 'Use `bcscale()` instead.',
83+
),
84+
'display_errors' => array(
85+
'message' => 'Use `WP_DEBUG_DISPLAY` instead.',
86+
),
87+
'error_reporting' => array(
88+
'message' => 'Use `WP_DEBUG` instead.',
89+
),
90+
'filter.default' => array(
91+
'message' => 'Changing the option value can break other plugins. Use the filter flag constants when calling the Filter functions instead.',
92+
),
93+
'filter.default_flags' => array(
94+
'message' => 'Changing the option value can break other plugins. Use the filter flag constants when calling the Filter functions instead.',
95+
),
96+
'iconv.input_encoding' => array(
97+
'message' => 'PHP < 5.6 only - use `iconv_set_encoding()` instead.',
98+
),
99+
'iconv.internal_encoding' => array(
100+
'message' => 'PHP < 5.6 only - use `iconv_set_encoding()` instead.',
101+
),
102+
'iconv.output_encoding' => array(
103+
'message' => 'PHP < 5.6 only - use `iconv_set_encoding()` instead.',
104+
),
105+
'ignore_user_abort' => array(
106+
'message' => 'Use `ignore_user_abort()` instead.',
107+
),
108+
'log_errors' => array(
109+
'message' => 'Use `WP_DEBUG_LOG` instead.',
110+
),
111+
'max_execution_time' => array(
112+
'message' => 'Use `set_time_limit()` instead.',
113+
),
114+
'memory_limit' => array(
115+
'message' => 'Use `wp_raise_memory_limit()` or hook into the filters in that function.',
116+
),
117+
'short_open_tag' => array(
118+
'invalid_values' => array( 'false', '0', 'off' ),
119+
'message' => 'Turning off short_open_tag is prohibited as it can break other plugins.',
120+
),
121+
);
122+
123+
/**
124+
* Process the parameter of a matched function.
125+
*
126+
* Errors if an option is found in the blacklist. Warns as
127+
* 'risky' when the option is not found in the whitelist.
128+
*
129+
* @since 2.1.0
130+
*
131+
* @param int $stackPtr The position of the current token in the stack.
132+
* @param array $group_name The name of the group which was matched.
133+
* @param string $matched_content The token content (function name) which was matched.
134+
* @param array $parameters Array with information about the parameters.
135+
*
136+
* @return void
137+
*/
138+
public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
139+
$option_name = $this->strip_quotes( $parameters[1]['raw'] );
140+
$option_value = $this->strip_quotes( $parameters[2]['raw'] );
141+
if ( isset( $this->whitelisted_options[ $option_name ] ) ) {
142+
$whitelisted_option = $this->whitelisted_options[ $option_name ];
143+
if ( ! isset( $whitelisted_option['valid_values'] ) || in_array( strtolower( $option_value ), $whitelisted_option['valid_values'], true ) ) {
144+
return;
145+
}
146+
}
147+
148+
if ( isset( $this->blacklisted_options[ $option_name ] ) ) {
149+
$blacklisted_option = $this->blacklisted_options[ $option_name ];
150+
if ( ! isset( $blacklisted_option['invalid_values'] ) || in_array( strtolower( $option_value ), $blacklisted_option['invalid_values'], true ) ) {
151+
$this->phpcsFile->addError(
152+
'%s(%s, %s) found. %s',
153+
$stackPtr,
154+
$this->string_to_errorcode( $option_name . '_Blacklisted' ),
155+
array(
156+
$matched_content,
157+
$parameters[1]['raw'],
158+
$parameters[2]['raw'],
159+
$blacklisted_option['message'],
160+
)
161+
);
162+
return;
163+
}
164+
}
165+
166+
$this->phpcsFile->addWarning(
167+
'%s(%s, %s) found. Changing configuration values at runtime is strongly discouraged.',
168+
$stackPtr,
169+
'Risky',
170+
array(
171+
$matched_content,
172+
$parameters[1]['raw'],
173+
$parameters[2]['raw'],
174+
)
175+
);
176+
}
177+
}

WordPress/Tests/PHP/DiscouragedPHPFunctionsUnitTest.inc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ rawurlencode(); // Ok.
1414

1515
dl(); // Warning.
1616
error_reporting(); // Warning.
17-
ini_alter(); // Warning.
17+
1818
ini_restore(); // Warning.
19-
ini_set(); // Warning.
2019
magic_quotes_runtime(); // Warning.
2120
set_magic_quotes_runtime(); // Warning.
2221
apache_setenv(); // Warning.

WordPress/Tests/PHP/DiscouragedPHPFunctionsUnitTest.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,24 @@ public function getWarningList() {
4242
12 => 1,
4343
15 => 1,
4444
16 => 1,
45-
17 => 1,
4645
18 => 1,
4746
19 => 1,
4847
20 => 1,
4948
21 => 1,
5049
22 => 1,
5150
23 => 1,
5251
24 => 1,
53-
25 => 1,
52+
27 => 1,
5453
28 => 1,
5554
29 => 1,
5655
30 => 1,
5756
31 => 1,
58-
32 => 1,
57+
34 => 1,
5958
35 => 1,
6059
36 => 1,
6160
37 => 1,
6261
38 => 1,
6362
39 => 1,
64-
40 => 1,
6563
);
6664
}
6765

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
ini_set('auto_detect_line_endings', true); // Ok.
4+
ini_set( 'auto_detect_line_endings',true) ; // Ok.
5+
ini_set('highlight.bg', '#FFFFFF'); // Ok.
6+
ini_set('highlight.comment', '#FFFFFF'); // Ok.
7+
ini_set('highlight.default', '#FFFFFF'); // Ok.
8+
ini_set('highlight.html', '#FFFFFF'); // Ok.
9+
ini_set('highlight.keyword', '#FFFFFF'); // Ok.
10+
ini_set('highlight.string', '#FFFFFF'); // Ok.
11+
ini_set('short_open_tag', true); // Ok.
12+
ini_set('short_open_tag', 1); // Ok.
13+
ini_set('short_open_tag', 'On'); // Ok.
14+
ini_set('short_open_tag', 'on'); // Ok.
15+
16+
ini_set('bcmath.scale', 0); // Error.
17+
ini_set( 'bcmath.scale' ,0 ); // Error.
18+
ini_set('display_errors', 0); // Error.
19+
ini_set('error_reporting', 0); // Error.
20+
ini_set('filter.default', 'full_special_chars'); // Error.
21+
ini_set('filter.default_flags', 0); // Error.
22+
ini_set('iconv.input_encoding', ''); // Error.
23+
ini_set('iconv.internal_encoding', ''); // Error.
24+
ini_set('iconv.output_encoding', ''); // Error.
25+
ini_set('iconv.output_encoding', ''); // Error.
26+
ini_set('ignore_user_abort', true); // Error.
27+
ini_set('log_errors', true); // Error.
28+
ini_set('max_execution_time', 60); // Error.
29+
ini_set('memory_limit', -1); // Error.
30+
ini_set('short_open_tag', false); // Error.
31+
ini_set('short_open_tag', FALSE); // Error.
32+
ini_set('short_open_tag', FaLsE); // Error.
33+
ini_set('short_open_tag', 0); // Error.
34+
ini_set('short_open_tag', 'Off'); // Error.
35+
36+
ini_set('report_memleaks', true); // Warning.
37+
ini_set('report_memleaks',true); // Warning.
38+
ini_set('short_open_tag', 1230); // Warning.
39+
ini_set($test, 1230); // Warning.
40+
41+
ini_alter('auto_detect_line_endings', true); // Ok.
42+
ini_alter('display_errors', false); // Error.
43+
ini_alter('report_memleaks', 1230); // Warning.
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
/**
3+
* Unit test class for WordPress Coding Standard.
4+
*
5+
* @package WPCS\WordPressCodingStandards
6+
* @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards
7+
* @license https://opensource.org/licenses/MIT MIT
8+
*/
9+
10+
namespace WordPressCS\WordPress\Tests\PHP;
11+
12+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
13+
14+
/**
15+
* Unit test class for the IniSet sniff.
16+
*
17+
* @package WPCS\WordPressCodingStandards
18+
*
19+
* @since 2.1.0
20+
*/
21+
class IniSetUnitTest extends AbstractSniffUnitTest {
22+
23+
/**
24+
* Returns the lines where errors should occur.
25+
*
26+
* @return array <int line number> => <int number of errors>
27+
*/
28+
public function getErrorList() {
29+
return array(
30+
16 => 1,
31+
17 => 1,
32+
18 => 1,
33+
19 => 1,
34+
20 => 1,
35+
21 => 1,
36+
22 => 1,
37+
23 => 1,
38+
24 => 1,
39+
25 => 1,
40+
26 => 1,
41+
27 => 1,
42+
28 => 1,
43+
29 => 1,
44+
30 => 1,
45+
31 => 1,
46+
32 => 1,
47+
33 => 1,
48+
34 => 1,
49+
42 => 1,
50+
);
51+
}
52+
53+
/**
54+
* Returns the lines where warnings should occur.
55+
*
56+
* @return array <int line number> => <int number of warnings>
57+
*/
58+
public function getWarningList() {
59+
return array(
60+
36 => 1,
61+
37 => 1,
62+
38 => 1,
63+
39 => 1,
64+
43 => 1,
65+
);
66+
}
67+
68+
}

0 commit comments

Comments
 (0)