Modularizing the discouraged functions #633
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,45 @@ | |
| <!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#commented-out-code-debug-code-or-output --> | ||
| <rule ref="Squiz.PHP.CommentedOutCode" /> | ||
|
|
||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#eval-and-create_function --> | ||
| <rule ref="WordPress.PHP.RestrictedFunctions" /> | ||
|
|
||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#eval-and-create_function --> | ||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#serializing-data --> | ||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#encoding-values-used-when-creating-a-url-or-passed-to-add_query_arg --> | ||
| <!-- https://github.com/Automattic/vip-scanner/blob/master/vip-scanner/checks/ForbiddenPHPFunctionsCheck.php --> | ||
| <rule ref="WordPress.PHP.DiscouragedPHPFunctions"> | ||
| <!-- https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/pull/633#issuecomment-266634811 --> | ||
| <exclude name="WordPress.PHP.DiscouragedPHPFunctions.obfuscation" /> | ||
| </rule> | ||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#settings-alteration --> | ||
| <rule ref="WordPress.PHP.DiscouragedPHPFunctions.runtime_configuration"> | ||
| <type>error</type> | ||
| </rule> | ||
|
|
||
| <!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#commented-out-code-debug-code-or-output --> | ||
|
There was a problem hiding this comment. This link should probably also be added to cover the |
||
| <rule ref="WordPress.PHP.DevelopmentFunctions" /> | ||
| <rule ref="WordPress.PHP.DevelopmentFunctions.error_log"> | ||
| <type>error</type> | ||
| </rule> | ||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#settings-alteration --> | ||
| <rule ref="WordPress.PHP.DevelopmentFunctions.prevent_path_disclosure"> | ||
| <type>error</type> | ||
| </rule> | ||
|
|
||
| <!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#using-in_array-without-strict-parameter --> | ||
| <rule ref="WordPress.PHP.StrictInArray" /> | ||
|
|
||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#use-wp_parse_url-instead-of-parse_url --> | ||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#use-wp_json_encode-over-json_encode --> | ||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#filesystem-writes --> | ||
| <!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#remote-calls --> | ||
| <rule ref="WordPress.WP.AlternativeFunctions" /> | ||
| <!-- VIP recommends other functions --> | ||
| <rule ref="WordPress.WP.AlternativeFunctions.curl"> | ||
| <message>Using cURL functions is highly discouraged within VIP context. Check (Fetching Remote Data) on VIP Documentation.</message> | ||
| </rule> | ||
| <rule ref="WordPress.WP.AlternativeFunctions.file_get_contents"> | ||
| <message>%s() is highly discouraged, please use vip_safe_wp_remote_get() instead.</message> | ||
| </rule> | ||
| </ruleset> | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| <?php | ||
| /** | ||
| * WordPress Coding Standard. | ||
| * | ||
| * @package WPCS\WordPressCodingStandards | ||
| * @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards | ||
| * @license https://opensource.org/licenses/MIT MIT | ||
| */ | ||
|
|
||
| /** | ||
| * Restrict the use of various development functions. | ||
| * | ||
| * @package WPCS\WordPressCodingStandards | ||
| * | ||
| * @since 0.11.0 | ||
| */ | ||
| class WordPress_Sniffs_PHP_DevelopmentFunctionsSniff extends WordPress_AbstractFunctionRestrictionsSniff { | ||
|
|
||
| /** | ||
| * Groups of functions to restrict. | ||
| * | ||
| * Example: groups => array( | ||
| * 'lambda' => array( | ||
| * 'type' => 'error' | 'warning', | ||
| * 'message' => 'Use anonymous functions instead please!', | ||
| * 'functions' => array( 'eval', 'create_function' ), | ||
| * ) | ||
| * ) | ||
| * | ||
| * @return array | ||
| */ | ||
| public function getGroups() { | ||
| return array( | ||
| 'error_log' => array( | ||
| 'type' => 'warning', | ||
| 'message' => '%s() found. Debug code should not normally be used in production.', | ||
| 'functions' => array( | ||
| 'error_log', | ||
| 'var_dump', | ||
| 'var_export', | ||
| 'print_r', | ||
| 'trigger_error', | ||
| 'set_error_handler', | ||
| 'debug_backtrace', | ||
| 'debug_print_backtrace', | ||
| 'wp_debug_backtrace_summary', | ||
| ), | ||
| ), | ||
|
|
||
| 'prevent_path_disclosure' => array( | ||
|
There was a problem hiding this comment. This group is new to the The error message itself is also worded too strongly for usage in There was a problem hiding this comment. I am fine with changing it to By having the development functions together it can be easily enabled or disabled. There was a problem hiding this comment. Both |
||
| 'type' => 'warning', | ||
| 'message' => '%s() can lead to full path disclosure.', | ||
| 'functions' => array( | ||
| 'error_reporting', | ||
| 'phpinfo', | ||
| ), | ||
| ), | ||
|
|
||
| ); | ||
| } // end getGroups() | ||
|
|
||
| } // end class | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| <?php | ||
| /** | ||
| * WordPress Coding Standard. | ||
| * | ||
| * @package WPCS\WordPressCodingStandards | ||
| * @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards | ||
| * @license https://opensource.org/licenses/MIT MIT | ||
| */ | ||
|
|
||
| /** | ||
| * Discourages the use of various native PHP functions and suggests alternatives. | ||
| * | ||
| * @package WPCS\WordPressCodingStandards | ||
| * | ||
| * @since 0.11.0 | ||
| */ | ||
| class WordPress_Sniffs_PHP_DiscouragedPHPFunctionsSniff extends WordPress_AbstractFunctionRestrictionsSniff { | ||
|
|
||
| /** | ||
| * Groups of functions to discourage. | ||
| * | ||
| * Example: groups => array( | ||
| * 'lambda' => array( | ||
| * 'type' => 'error' | 'warning', | ||
| * 'message' => 'Use anonymous functions instead please!', | ||
| * 'functions' => array( 'eval', 'create_function' ), | ||
| * ) | ||
| * ) | ||
| * | ||
| * @return array | ||
| */ | ||
| public function getGroups() { | ||
| return array( | ||
| 'create_function' => array( | ||
| 'type' => 'warning', | ||
| 'message' => '%s() is discouraged, please use anonymous functions instead.', | ||
| 'functions' => array( | ||
| 'create_function', | ||
| ), | ||
| ), | ||
|
|
||
| 'serialize' => array( | ||
| 'type' => 'warning', | ||
| 'message' => '%s() found. Serialized data has known vulnerability problems with Object Injection. JSON is generally a better approach for serializing data. See https://www.owasp.org/index.php/PHP_Object_Injection', | ||
| 'functions' => array( | ||
| 'serialize', | ||
| 'unserialize', | ||
| ), | ||
| ), | ||
|
|
||
| 'urlencode' => array( | ||
| 'type' => 'warning', | ||
| 'message' => '%s() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See http://php.net/manual/en/function.rawurlencode.php and http://www.faqs.org/rfcs/rfc3986.html', | ||
| 'functions' => array( | ||
| 'urlencode', | ||
| ), | ||
| ), | ||
|
|
||
| 'runtime_configuration' => array( | ||
| 'type' => 'warning', | ||
| 'message' => '%s() found. Changing configuration at runtime is rarely necessary.', | ||
| 'functions' => array( | ||
| 'error_reporting', | ||
| 'ini_alter', | ||
| 'ini_restore', | ||
| 'ini_set', | ||
| 'apache_setenv', | ||
| 'putenv', | ||
| 'set_include_path', | ||
| 'restore_include_path', | ||
| // This alias was DEPRECATED in PHP 5.3.0, and REMOVED as of PHP 7.0.0. | ||
|
There was a problem hiding this comment. Are these last three functions included here because they are deprecated/removed of because they touch the runtime_configuration ? There was a problem hiding this comment. They are included because they touch the runtime. For PHP compatibility we can use the PHPcompatibility sniffs. |
||
| 'magic_quotes_runtime', | ||
| // Warning This function was DEPRECATED in PHP 5.3.0, and REMOVED as of PHP 7.0.0. | ||
| 'set_magic_quotes_runtime', | ||
| // Warning This function was removed from most SAPIs in PHP 5.3.0, and was removed from PHP-FPM in PHP 7.0.0. | ||
| 'dl', | ||
| ), | ||
| ), | ||
|
|
||
| 'system_calls' => array( | ||
| 'type' => 'warning', | ||
| 'message' => '%s() found. PHP system calls are often disabled by server admins.', | ||
| 'functions' => array( | ||
| 'exec', | ||
| 'passthru', | ||
| 'proc_open', | ||
| 'shell_exec', | ||
| 'system', | ||
| 'popen', | ||
| ), | ||
| ), | ||
|
|
||
| 'obfuscation' => array( | ||
| 'type' => 'warning', | ||
| 'message' => '%s() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.', | ||
| 'functions' => array( | ||
| 'base64_decode', | ||
| 'base64_encode', | ||
| 'convert_uudecode', | ||
| 'convert_uuencode', | ||
| 'str_rot13', | ||
| ), | ||
| ), | ||
|
|
||
| ); | ||
| } // end getGroups() | ||
|
|
||
| } // End class. | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The links justify adding this sniff for the
evalandruntime_configurationsections.Additional links to the VIP documentation should be added for the newly introduced
system_callsandobfuscationsections. If these are not documented, they should be excluded from the ruleset or added to the VIP documentation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of us can update the VIP documentation. There is a check in the VIP scanner that does the same thing that could backup including the two new sections. https://github.com/Automattic/vip-scanner/blob/master/vip-scanner/checks/ForbiddenPHPFunctionsCheck.php