Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WP/AlternativeFunctions: implement PHPCSUtils and support modern PHP #2169

Merged
merged 13 commits into from
Dec 21, 2022
Merged

WP/AlternativeFunctions: implement PHPCSUtils and support modern PHP #2169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6ac41da
WP/AlternativeFunctions: order lists alphabetically
jrfnl Dec 19, 2022
56a8cc9
WP/AlternativeFunctions: add missing `@since` tags
jrfnl Dec 14, 2022
70154e5
WP/AlternativeFunctions: change property visibility
jrfnl Dec 14, 2022
b503057
WP/AlternativeFunctions: remove stray blank lines at top of test file
jrfnl Dec 15, 2022
744e83b
WP/AlternativeFunctions: move parse_url related tests down
jrfnl Dec 15, 2022
1dffa3a
WP/AlternativeFunctions: bug fix - special casing for parse_url was n…
jrfnl Dec 15, 2022
4f57da1
WP/AlternativeFunctions: add support for PHP 8.0+ named parameters [1…
jrfnl Dec 14, 2022
28e64e7
WP/AlternativeFunctions: add support for PHP 8.0+ named parameters [2…
jrfnl Dec 19, 2022
f3b0c1b
WP/AlternativeFunctions: add support for PHP 8.0+ named parameters [3…
jrfnl Dec 15, 2022
9bed804
WP/AlternativeFunctions: minor code readability improvements
jrfnl Dec 15, 2022
b58f15c
WP/AlternativeFunctions: add support for PHP 8.0+ named parameters [4…
jrfnl Dec 15, 2022
de723fb
WP/AlternativeFunctions: prevent some false positives / file_get_cont…
jrfnl Dec 15, 2022
56a2ef7
WP/AlternativeFunctions: minor code tweak / readfile/fopen/file_put_c…
jrfnl Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 60 additions & 36 deletions WordPress/Sniffs/WP/AlternativeFunctionsSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,12 @@ class AlternativeFunctionsSniff extends AbstractFunctionRestrictionsSniff {
*
* @link https://www.php.net/wrappers.php
*
* @since 2.1.0
* @since 3.0.0 The visibility was changed from `protected` to `private`.
*
* @var array
*/
protected $allowed_local_streams = array(
private $allowed_local_streams = array(
'php://input' => true,
'php://output' => true,
'php://stdin' => true,
Expand All @@ -51,9 +54,12 @@ class AlternativeFunctionsSniff extends AbstractFunctionRestrictionsSniff {
*
* @link https://www.php.net/wrappers.php
*
* @since 2.1.0
* @since 3.0.0 The visibility was changed from `protected` to `private`.
*
* @var array
*/
protected $allowed_local_stream_partials = array(
private $allowed_local_stream_partials = array(
'php://temp/',
'php://fd/',
);
Expand All @@ -63,9 +69,12 @@ class AlternativeFunctionsSniff extends AbstractFunctionRestrictionsSniff {
*
* @link https://www.php.net/wrappers.php
*
* @since 2.1.0
* @since 3.0.0 The visibility was changed from `protected` to `private`.
*
* @var array
*/
protected $allowed_local_stream_constants = array(
private $allowed_local_stream_constants = array(
'STDIN' => true,
'STDOUT' => true,
'STDERR' => true,
Expand Down Expand Up @@ -152,20 +161,20 @@ public function getGroups() {
'chgrp',
'chmod',
'chown',
'is_writable',
'is_writeable',
'mkdir',
'rmdir',
'touch',
'readfile',
'fclose',
'file_put_contents',
'fopen',
'fputs',
'fread',
'fwrite',
'file_put_contents',
'fsockopen',
'fwrite',
'is_writable',
'is_writeable',
'mkdir',
'pfsockopen',
'fputs',
'readfile',
'rmdir',
'touch',
),
),

Expand All @@ -183,8 +192,8 @@ public function getGroups() {
'message' => '%s() is discouraged. Rand seeding is not necessary when using the wp_rand() function (as you should).',
'since' => '2.6.2',
'functions' => array(
'srand',
'mt_srand',
'srand',
),
),

Expand All @@ -193,8 +202,8 @@ public function getGroups() {
'message' => '%s() is discouraged. Use the far less predictable wp_rand() instead.',
'since' => '2.6.2',
'functions' => array(
'rand',
'mt_rand',
'rand',
),
),
);
Expand All @@ -221,27 +230,31 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
case 'strip_tags':
/*
* The function `wp_strip_all_tags()` is only a valid alternative when
* only the first parameter is passed to `strip_tags()`.
* only the first parameter, `$string`, is passed to `strip_tags()`.
*/
if ( PassedParameters::getParameterCount( $this->phpcsFile, $stackPtr ) !== 1 ) {
$has_allowed_tags = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 2, 'allowed_tags' );
if ( false !== $has_allowed_tags ) {
return;
}

unset( $has_allowed_tags );
break;

case 'wp_parse_url':
case 'parse_url':
/*
* Before WP 4.7.0, the function `wp_parse_url()` was only a valid alternative
* if no second param was passed to `parse_url()`.
* if the second param - `$component` - was not passed to `parse_url()`.
*
* @see https://developer.wordpress.org/reference/functions/wp_parse_url/#changelog
*/
if ( PassedParameters::getParameterCount( $this->phpcsFile, $stackPtr ) !== 1
$has_component = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 2, 'component' );
if ( false !== $has_component
&& $this->wp_version_compare( $this->minimum_wp_version, '4.7.0', '<' )
) {
return;
}

unset( $has_component );
break;

case 'file_get_contents':
Expand All @@ -251,62 +264,73 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
*/
$params = PassedParameters::getParameters( $this->phpcsFile, $stackPtr );

if ( isset( $params[2] ) && 'true' === $params[2]['raw'] ) {
$use_include_path_param = PassedParameters::getParameterFromStack( $params, 2, 'use_include_path' );
if ( false !== $use_include_path_param && 'true' === $use_include_path_param['clean'] ) {
// Setting `$use_include_path` to `true` is only relevant for local files.
return;
}

if ( isset( $params[1] ) === false ) {
$filename_param = PassedParameters::getParameterFromStack( $params, 1, 'filename' );
if ( false === $filename_param ) {
// If the file to get is not set, this is a non-issue anyway.
return;
}

if ( strpos( $params[1]['raw'], 'http:' ) !== false
|| strpos( $params[1]['raw'], 'https:' ) !== false
if ( strpos( $filename_param['clean'], 'http:' ) !== false
|| strpos( $filename_param['clean'], 'https:' ) !== false
) {
// Definitely a URL, throw notice.
break;
}

if ( preg_match( '`\b(?:ABSPATH|WP_(?:CONTENT|PLUGIN)_DIR|WPMU_PLUGIN_DIR|TEMPLATEPATH|STYLESHEETPATH|(?:MU)?PLUGINDIR)\b`', $params[1]['raw'] ) === 1 ) {
$contains_wp_path_constant = preg_match(
'`\b(?:ABSPATH|WP_(?:CONTENT|PLUGIN)_DIR|WPMU_PLUGIN_DIR|TEMPLATEPATH|STYLESHEETPATH|(?:MU)?PLUGINDIR)\b`',
$filename_param['clean']
);
if ( 1 === $contains_wp_path_constant ) {
// Using any of the constants matched in this regex is an indicator of a local file.
return;
}

if ( preg_match( '`(?:get_home_path|plugin_dir_path|get_(?:stylesheet|template)_directory|wp_upload_dir)\s*\(`i', $params[1]['raw'] ) === 1 ) {
$contains_wp_path_function_call = preg_match(
'`(?:get_home_path|plugin_dir_path|get_(?:stylesheet|template)_directory|wp_upload_dir)\s*\(`i',
$filename_param['clean']
);
if ( 1 === $contains_wp_path_function_call ) {
// Using any of the functions matched in the regex is an indicator of a local file.
return;
}

if ( $this->is_local_data_stream( $params[1]['raw'] ) === true ) {
if ( $this->is_local_data_stream( $filename_param['clean'] ) === true ) {
// Local data stream.
return;
}

unset( $params );

unset( $params, $use_include_path_param, $filename_param, $contains_wp_path_constant, $contains_wp_path_function_call );
break;

case 'readfile':
case 'fopen':
case 'file_put_contents':
case 'fopen':
case 'readfile':
/*
* Allow for handling raw data streams from the request body.
*
* Note: at this time (December 2022) these three functions use the same parameter name for their
* first parameter. If this would change at any point in the future, this code will need to
* be made more modular and will need to pass the parameter name based on the function call detected.
*/
$first_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );

if ( false === $first_param ) {
$filename_param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1, 'filename' );
if ( false === $filename_param ) {
// If the file to work with is not set, local data streams don't come into play.
break;
}

if ( $this->is_local_data_stream( $first_param['raw'] ) === true ) {
if ( $this->is_local_data_stream( $filename_param['clean'] ) === true ) {
// Local data stream.
return;
}

unset( $first_param );

unset( $filename_param );
break;
}

Expand Down
81 changes: 69 additions & 12 deletions WordPress/Tests/WP/AlternativeFunctionsUnitTest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
<?php



curl_init(); // Warning.
curl_close( $ch ); // Warning.
CURL_getinfo(); // Warning.
Expand All @@ -28,18 +26,8 @@ srand(); // Warning.
mt_srand(); // Warning.
wp_rand(); // OK.

// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.0
parse_url( 'http://example.com/' ); // OK.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.6

strip_tags( $something, '<iframe>' ); // OK.

// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.0
parse_url($url, PHP_URL_QUERY); // OK.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.7
parse_url($url, PHP_URL_SCHEME); // Warning.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.6

file_get_contents( $local_file, true ); // OK.
file_get_contents( $url, false ); // Warning.
file_get_contents(); // OK - no params, so nothing to do.
Expand Down Expand Up @@ -90,3 +78,72 @@ mkdir(); // Warning.
rmdir(); // Warning.
touch(); // Warning.
fputs(); // Warning.

// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.0
parse_url( 'http://example.com/' ); // OK, alternative was not yet available.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.4
parse_url( 'http://example.com/' ); // Warning, not using $component param, so can switch over.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 5.8

// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.0
parse_url($url, PHP_URL_QUERY); // OK, alternative was not yet available.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.5
parse_url($url, PHP_URL_QUERY); // OK, $component param not yet available.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.7
parse_url($url, PHP_URL_SCHEME); // Warning, using $component param, but also using WP 4.7+, so can switch over.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 5.8

/*
* Tests for support for PHP 8.0+ named parameters.
*/
// Safeguard support for PHP 8.0+ named parameters for the custom logic related to strip_tags().
strip_tags( allowed_tags: '<iframe>' ); // OK. Well not really, missing required param, but that's not the concern of this sniff.
strip_tags( allowed_tags: '<iframe>', string: $something ); // OK.
strip_tags( string: $something ); // Warning.

// Safeguard support for PHP 8.0+ named parameters for the custom logic related to parse_url().
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.4
parse_url(component : PHP_URL_QUERY); // OK. Well, not really, missing required parameter, but that's not the concern of this sniff.
parse_url(component : PHP_URL_QUERY, url : $url); // OK.
parse_url(url : $url); // Warning.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 4.7
parse_url(component: PHP_URL_SCHEME, url: $url, ); // Warning.
// phpcs:set WordPress.WP.AlternativeFunctions minimum_wp_version 5.8

// Safeguard support for PHP 8.0+ named parameters for the custom logic related to file_get_contents().
file_get_contents( use_include_path: true, filename: $local_file, ); // OK.
file_get_contents( use_include_path: false, filename: $url, ); // Warning.

file_get_contents( use_include_path: false ); // OK, well not really, missing required param, but that's not the concern of this sniff.

file_get_contents( use_include_path: false, filename: 'http://remoteurl.com/file/?w=1' ); // Warning.
file_get_contents( use_include_path: false, filename: 'https://wordpress.org' ); // Warning.
file_get_contents( use_include_path: false, filename: ABSPATH . 'wp-admin/css/some-file.css'); // OK.
file_get_contents( use_include_path: false, filename: 'php://input' ); // OK.
file_get_contents(use_include_path: false, filename: MYABSPATH . 'plugin-file.json'); // Warning.

// Safeguard support for PHP 8.0+ named parameters for the custom logic related to fopen(), readfile() and file_put_contents().
fopen (mode: 'w'); // Warning. Missing required param, but that's not the concern of this sniff.
$output_stream = fopen( mode: 'w', filename: 'php://output', ); // OK.
$output_stream = fopen( mode: 'w', context: $stream, filename: STDOUT ); // OK.
$bytes = readfile( use_include_path: false, context: $stream, filename: 'php://fd/3' ); // OK.
file_put_contents( data: $data, flags : LOCK_EX, filename: STDERR ); // OK.
$output_stream = fopen( mode: 'r', filename: $url, ); // Warning.

// Safeguard that comments in the parameters are ignored for the custom logic related to file_get_contents().
file_get_contents(
$local_file,
true // Using local include path.
); // OK.
file_get_contents(
// Not using an https: URL for reasons.
ABSPATH . 'wp-admin/css/some-file.css'
); // OK.
file_get_contents(
// Not using ABSPATH for reasons.
$url
); // Warning.
file_get_contents(
// Not using plugin_dir_path() for reasons.
$url
); // Warning.
Loading