Changeset 52370

Timestamp:

12/14/2021 02:59:33 PM (3 years ago)

Author:

hellofromTonya

Message:

Formatting: Use is_scalar() in sanitize_key().

This is a follow-up to [52292] which introduced is_string() to check the given key is a string to be sanitized, else the key is set to an empty string.

sanitize_key() is clearly identified (in the documentation) to only work with string keys. However, it had a bug in it that allowed non-strings to pass through it:

• A non-scalar "key" would throw a PHP Warning (which was resolved in [52292].
• A non-string scalar "key" was handled by the PHP native strtolower() which converted it into a string.

While is_string() is valid, non-string scalar types passed as the key to be sanitized were being set to an empty string. Given that strtolower() handles these without error or deprecation as of PHP 8.1, is_scalar() protects the website from issues while retaining the past behavior of converting integer keys (for example) into a string.

Changes include:

• Using is_scalar() instead of is_string()
• Refactor for readability and less code
• More tests

Please note, this does not change the behavior of the function, nor redefine it to now accept non-string scalars.

References:

• ​https://developer.wordpress.org/reference/functions/sanitize_key/
• ​https://www.php.net/manual/en/function.strtolower.php

Follow-up [52292].

Props wppunk, hellofromTonya, costdev, jrf.
Fixes #54160.

Location:

trunk

Files:

• src/wp-includes/formatting.php (modified) (2 diffs)
• tests/phpunit/tests/formatting/sanitizeKey.php (modified) (3 diffs)

trunk/src/wp-includes/formatting.php

@@ -2133 +2133 @@
  * @since 3.0.0
  *
- * @param string $key String key
- * @return string Sanitized key
+ * @param string $key String key.
+ * @return string Sanitized key.
  */
 function sanitize_key( $key ) {
-    $raw_key = $key;
-
-    if ( ! is_string( $key ) ) {
-        $key = '';
-    }
-
-    if ( '' !== $key ) {
-        $key = strtolower( $key );
-        $key = preg_replace( '/[^a-z0-9_\-]/', '', $key );
+    $sanitized_key = '';
+
+    if ( is_scalar( $key ) ) {
+        $sanitized_key = strtolower( $key );
+        $sanitized_key = preg_replace( '/[^a-z0-9_\-]/', '', $sanitized_key );
     }
 
@@ -2153 +2149 @@
      * @since 3.0.0
      *
-     * @param string $key     Sanitized key.
-     * @param string $raw_key The key prior to sanitization.
+     * @param string $sanitized_key Sanitized key.
+     * @param string $key           The key prior to sanitization.
      */
-    return apply_filters( 'sanitize_key', $key, $raw_key );
+    return apply_filters( 'sanitize_key', $sanitized_key, $key );
 }
 

trunk/tests/phpunit/tests/formatting/sanitizeKey.php

@@ -8 +8 @@
 
     /**
-     * @ticket 54160
+     * @ticket       54160
      * @dataProvider data_sanitize_key
      *
-     * @param mixed $key       The key to sanitize.
-     * @param mixed $expected  The expected value.
+     * @param string $key      The key to sanitize.
+     * @param string $expected The expected value.
      */
     public function test_sanitize_key( $key, $expected ) {
@@ -18 +18 @@
     }
 
+    /**
+     * Data provider.
+     *
+     * @return array
+     */
     public function data_sanitize_key() {
         return array(
             'an empty string key'            => array(
                 'key'      => '',
-                'expected' => '',
-            ),
-            'a null key'                     => array(
-                'key'      => null,
-                'expected' => '',
-            ),
-            'an int 0 key'                   => array(
-                'key'      => 0,
-                'expected' => '',
-            ),
-            'a true key'                     => array(
-                'key'      => true,
-                'expected' => '',
-            ),
-            'an array key'                   => array(
-                'key'      => array( 'Howdy, admin!' ),
                 'expected' => '',
             ),
@@ -72 +61 @@
 
     /**
-     * @ticket 54160
+     * @ticket       54160
+     * @dataProvider data_sanitize_key_nonstring_scalar
      *
-     * @covers WP_Hook::sanitize_key
+     * @param mixed  $key      The key to sanitize.
+     * @param string $expected The expected value.
      */
-    public function test_filter_sanitize_key() {
-        $key = 'Howdy, admin!';
+    public function test_sanitize_key_nonstring_scalar( $key, $expected ) {
+        $this->assertSame( $expected, sanitize_key( $key ) );
+    }
 
+    /**
+     * Data provider.
+     *
+     * @return array
+     */
+    public function data_sanitize_key_nonstring_scalar() {
+        return array(
+            'integer type'  => array(
+                'key'      => 0,
+                'expected' => '0',
+            ),
+            'boolean true'  => array(
+                'key'      => true,
+                'expected' => '1',
+            ),
+            'boolean false' => array(
+                'key'      => false,
+                'expected' => '',
+            ),
+            'float type'    => array(
+                'key'      => 0.123,
+                'expected' => '0123',
+            ),
+        );
+    }
+
+    /**
+     * @ticket       54160
+     * @dataProvider data_sanitize_key_with_non_scalars
+     *
+     * @param mixed $nonscalar_key A non-scalar data type given as a key.
+     */
+    public function test_sanitize_key_with_non_scalars( $nonscalar_key ) {
         add_filter(
             'sanitize_key',
-            static function( $key ) {
-                return 'Howdy, admin!';
-            }
+            function ( $sanitized_key, $key ) use ( $nonscalar_key ) {
+                $this->assertEmpty( $sanitized_key, 'Empty string not passed as first filtered argument' );
+                $this->assertSame( $nonscalar_key, $key, 'Given unsanitized key not passed as second filtered argument' );
+                return $sanitized_key;
+            },
+            10,
+            2
         );
+        $this->assertEmpty( sanitize_key( $nonscalar_key ), 'Non-scalar key did not return empty string' );
+    }
 
-        $this->assertSame( $key, sanitize_key( $key ) );
+    /**
+     * Data provider.
+     *
+     * @return array
+     */
+    public function data_sanitize_key_with_non_scalars() {
+        return array(
+            'array type' => array(
+                'key'      => array( 'key' ),
+                'expected' => '',
+            ),
+            'null'       => array(
+                'key'      => null,
+                'expected' => '',
+            ),
+            'object'     => array(
+                'key'      => new stdClass(),
+                'expected' => '',
+            ),
+        );
     }
 }