Skip to content

Commit

Permalink
Merge pull request #2169 from WordPress/feature/alternativefunctions-…
Browse files Browse the repository at this point in the history 
…phpcsutils-and-modern-php
  • Loading branch information
@GaryJones
GaryJones authored Dec 21, 2022
2 parents f6fda8b + 56a2ef7 commit ad3c73b
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 88 deletions.
96 changes: 60 additions & 36 deletions WordPress/Sniffs/WP/AlternativeFunctionsSniff.php
View file
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
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

0 comments on commit ad3c73b

Please sign in to comment.