From d93f76dca87687d72951bc0be18bd56f4b23bb4d Mon Sep 17 00:00:00 2001 From: Sergey Biryukov Date: Mon, 16 Aug 2021 22:16:32 +0000 Subject: [PATCH] Code Modernization: Correct handling of `null` in `wp_parse_str()`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes `parse_str(): Passing null to parameter #1 ($string) of type string is deprecated` notices on PHP 8.1, without change in behaviour. Impact: 311 of the pre-existing tests are affected by this issue. The PHP native `parse_str()` function expects a string, however, based on the failing tests, it is clear there are functions in WordPress which passes a non-string – including `null` – value to the `wp_parse_str()` function, which would subsequently pass it onto the PHP native function without further input validation. Most notable offender is the `wp_parse_args()` function which special cases arrays and objects, but passes everything else off to `wp_parse_str()`. Several ways to fix this issue have been explored, including checking the received value with `is_string()` or `is_scalar()` before passing it off to the PHP native `parse_str()` function. In the end it was decided against these in favor of a string cast as: * `is_string()` would significantly change the behavior for anything non-string. * `is_scalar()` up to a point as well, as it does not take objects with a `__toString()` method into account. Executing a string cast on the received value before passing it on maintains the pre-existing behavior while still preventing the deprecation notice coming from PHP 8.1. Reference: [https://www.php.net/manual/en/function.parse-str.php PHP Manual: parse_str()] Follow-up to [5709]. Props jrf, hellofromTonya, lucatume, SergeyBiryukov. See #53635. git-svn-id: https://develop.svn.wordpress.org/trunk@51624 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/formatting.php | 2 +- tests/phpunit/tests/formatting/wpParseStr.php | 144 ++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 tests/phpunit/tests/formatting/wpParseStr.php diff --git a/src/wp-includes/formatting.php b/src/wp-includes/formatting.php index edfdd1e416..b80c40cdda 100644 --- a/src/wp-includes/formatting.php +++ b/src/wp-includes/formatting.php @@ -4964,7 +4964,7 @@ function map_deep( $value, $callback ) { * @param array $array Variables will be stored in this array. */ function wp_parse_str( $string, &$array ) { - parse_str( $string, $array ); + parse_str( (string) $string, $array ); /** * Filters the array of variables derived from a parsed string. diff --git a/tests/phpunit/tests/formatting/wpParseStr.php b/tests/phpunit/tests/formatting/wpParseStr.php new file mode 100644 index 0000000000..b8c3d063d2 --- /dev/null +++ b/tests/phpunit/tests/formatting/wpParseStr.php @@ -0,0 +1,144 @@ +assertSame( $expected, $output ); + } + + /** + * Data Provider. + * + * @return array + */ + public function data_wp_parse_str() { + return array( + 'null' => array( + 'input' => null, + 'expected' => array(), + ), + 'boolean false' => array( + 'input' => false, + 'expected' => array(), + ), + 'boolean true' => array( + 'input' => true, + 'expected' => array( + 1 => '', + ), + ), + 'integer 0' => array( + 'input' => 0, + 'expected' => array( + 0 => '', + ), + ), + 'integer 456' => array( + 'input' => 456, + 'expected' => array( + 456 => '', + ), + ), + 'float 12.53' => array( + 'input' => 12.53, + 'expected' => array( + '12_53' => '', + ), + ), + 'plain string' => array( + 'input' => 'foobar', + 'expected' => array( + 'foobar' => '', + ), + ), + 'query string' => array( + 'input' => 'x=5&_baba=dudu&', + 'expected' => array( + 'x' => '5', + '_baba' => 'dudu', + ), + ), + 'stringable object' => array( + 'input' => new Fixture_Formatting_wpParseStr(), + 'expected' => array( + 'foobar' => '', + ), + ), + ); + } + + /** + * Tests that the result array only contains the result of the string parsing + * when provided with different types of input for the `$output` parameter. + * + * @dataProvider data_wp_parse_str_result_array_is_always_overwritten + * + * @param array|null $output Value for the `$output` parameter. + * @param array $expected Expected function output. + */ + public function test_wp_parse_str_result_array_is_always_overwritten( $output, $expected ) { + wp_parse_str( 'key=25&thing=text', $output ); + $this->assertSame( $expected, $output ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_wp_parse_str_result_array_is_always_overwritten() { + // Standard value for expected output. + $expected = array( + 'key' => '25', + 'thing' => 'text', + ); + + return array( + 'output null' => array( + 'output' => null, + 'expected' => $expected, + ), + 'output empty array' => array( + 'output' => array(), + 'expected' => $expected, + ), + 'output non empty array, no conflicting keys' => array( + 'output' => array( + 'foo' => 'bar', + ), + 'expected' => $expected, + ), + 'output non empty array, conflicting keys' => array( + 'output' => array( + 'key' => 'value', + ), + 'expected' => $expected, + ), + ); + } +} + +/** + * Fixture for use in the tests. + */ +class Fixture_Formatting_wpParseStr { + public function __toString() { + return 'foobar'; + } +}