WordPress.org
Get WordPress

Make WordPress Core

Changeset 53818


Ignore:
Timestamp:
08/03/2022 02:34:58 PM (4 years ago)
Author:
SergeyBiryukov
Message:

Cache API: Validate cache key in WP_Object_Cache methods.

Some plugins may call the wp_cache_*() functions with an empty string, false, or null as cache key, usually as a result of not checking the return value of another function that's used as the key.

Previously, this was silently failing, leading to odd behavior at best and often breakage due to key collisions.

A valid cache key must be either an integer number or a non-empty string.

This commit introduces a quick type check on the given cache keys and adds a _doing_it_wrong() message that should help plugin developers to notice these issues quicker.

Includes:

  • A check in update_user_caches() and clean_user_cache() to make sure user email is not empty before being cached or removed from cache, as it is technically possible to create a user with empty email via wp_insert_user().
  • Some minor cleanup in unit tests where the email was passed to wp_insert_user() incorrectly or was unintentionally reset.

Props tillkruess, malthert, dd32, spacedmonkey, flixos90, peterwilsoncc, SergeyBiryukov.
Fixes #56198.

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/wp-includes/class-wp-object-cache.php

    r53767 r53818  
    131131
    132132        /**
     133         * Serves as a utility function to determine whether a key is valid.
     134         *
     135         * @since 6.1.0
     136         *
     137         * @param int|string $key Cache key to check for validity.
     138         * @return bool Whether the key is valid.
     139         */
     140        protected function is_valid_key( $key ) {
     141                if ( is_int( $key ) ) {
     142                        return true;
     143                }
     144
     145                if ( is_string( $key ) && trim( $key ) !== '' ) {
     146                        return true;
     147                }
     148
     149                $type = gettype( $key );
     150
     151                if ( ! function_exists( '__' ) ) {
     152                        wp_load_translations_early();
     153                }
     154
     155                $message = is_string( $key )
     156                        ? __( 'Cache key must not be an empty string.' )
     157                        /* translators: %s: The type of the given cache key. */
     158                        : sprintf( __( 'Cache key must be integer or non-empty string, %s given.' ), $type );
     159
     160                _doing_it_wrong(
     161                        sprintf( '%s::%s', __CLASS__, debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS, 2 )[1]['function'] ),
     162                        $message,
     163                        '6.1.0'
     164                );
     165
     166                return false;
     167        }
     168
     169        /**
    133170         * Serves as a utility function to determine whether a key exists in the cache.
    134171         *
    … …  
    164201                }
    165202
     203                if ( ! $this->is_valid_key( $key ) ) {
     204                        return false;
     205                }
     206
    166207                if ( empty( $group ) ) {
    167208                        $group = 'default';
    … …  
    217258         */
    218259        public function replace( $key, $data, $group = 'default', $expire = 0 ) {
     260                if ( ! $this->is_valid_key( $key ) ) {
     261                        return false;
     262                }
     263
    219264                if ( empty( $group ) ) {
    220265                        $group = 'default';
    … …  
    246291         *
    247292         * @since 2.0.0
     293         * @since 6.1.0 Returns false if cache key is invalid.
    248294         *
    249295         * @param int|string $key    What to call the contents in the cache.
    … …  
    251297         * @param string     $group  Optional. Where to group the cache contents. Default 'default'.
    252298         * @param int        $expire Optional. Not used.
    253          * @return true Always returns true.
     299         * @return bool True if contents were set, false if key is invalid.
    254300         */
    255301        public function set( $key, $data, $group = 'default', $expire = 0 ) {
     302                if ( ! $this->is_valid_key( $key ) ) {
     303                        return false;
     304                }
     305
    256306                if ( empty( $group ) ) {
    257307                        $group = 'default';
    … …  
    311361         */
    312362        public function get( $key, $group = 'default', $force = false, &$found = null ) {
     363                if ( ! $this->is_valid_key( $key ) ) {
     364                        return false;
     365                }
     366
    313367                if ( empty( $group ) ) {
    314368                        $group = 'default';
    … …  
    369423         */
    370424        public function delete( $key, $group = 'default', $deprecated = false ) {
     425                if ( ! $this->is_valid_key( $key ) ) {
     426                        return false;
     427                }
     428
    371429                if ( empty( $group ) ) {
    372430                        $group = 'default';
    … …  
    417475         */
    418476        public function incr( $key, $offset = 1, $group = 'default' ) {
     477                if ( ! $this->is_valid_key( $key ) ) {
     478                        return false;
     479                }
     480
    419481                if ( empty( $group ) ) {
    420482                        $group = 'default';
    … …  
    456518         */
    457519        public function decr( $key, $offset = 1, $group = 'default' ) {
     520                if ( ! $this->is_valid_key( $key ) ) {
     521                        return false;
     522                }
     523
    458524                if ( empty( $group ) ) {
    459525                        $group = 'default';

  • trunk/src/wp-includes/user.php

    r53501 r53818  
    18511851        wp_cache_add( $user->ID, $user, 'users' );
    18521852        wp_cache_add( $user->user_login, $user->ID, 'userlogins' );
    1853         wp_cache_add( $user->user_email, $user->ID, 'useremail' );
    18541853        wp_cache_add( $user->user_nicename, $user->ID, 'userslugs' );
     1854
     1855        if ( ! empty( $user->user_email ) ) {
     1856                wp_cache_add( $user->user_email, $user->ID, 'useremail' );
     1857        }
    18551858}
    18561859
    … …  
    18791882        wp_cache_delete( $user->ID, 'users' );
    18801883        wp_cache_delete( $user->user_login, 'userlogins' );
    1881         wp_cache_delete( $user->user_email, 'useremail' );
    18821884        wp_cache_delete( $user->user_nicename, 'userslugs' );
     1885
     1886        if ( ! empty( $user->user_email ) ) {
     1887                wp_cache_delete( $user->user_email, 'useremail' );
     1888        }
    18831889
    18841890        /**

  • trunk/tests/phpunit/tests/user.php

    r53698 r53818  
    20112011                        'ID'         => $create_user,
    20122012                        'user_login' => 'test_user',
     2013                        'user_email' => 'user@example.com',
    20132014                        'meta_input' => array(
    20142015                                'test_meta_key' => 'test_meta_updated',

  • trunk/tests/phpunit/tests/user/getUserCount.php

    r53011 r53818  
    4646
    4747                // Add another user to fake the network user count to be different.
    48                 wpmu_create_user( 'user', 'pass', 'email' );
     48                wpmu_create_user( 'user', 'pass', 'user@example.com' );
    4949
    5050                wp_update_network_user_counts( $different_network_id );
    … …  
    6969                $start_count = get_user_count();
    7070
    71                 wpmu_create_user( 'user', 'pass', 'email' );
     71                wpmu_create_user( 'user', 'pass', 'user@example.com' );
    7272
    7373                // No change, cache not refreshed.
    … …  
    133133                $current_network_user_count = get_user_count();
    134134
    135                 $u1 = wpmu_create_user( 'user', 'pass', 'email' );
     135                $u1 = wpmu_create_user( 'user', 'pass', 'user@example.com' );
    136136
    137137                $user_count = get_user_count();

  • trunk/tests/phpunit/tests/user/slashes.php

    r53557 r53818  
    149149                                'user_login'   => 'slash_example_user_3',
    150150                                'role'         => 'subscriber',
    151                                 'email'        => 'user3@example.com',
     151                                'user_email'   => 'user3@example.com',
    152152                                'first_name'   => self::SLASH_1,
    153153                                'last_name'    => self::SLASH_3,
    … …  
    170170                                'user_login'   => 'slash_example_user_4',
    171171                                'role'         => 'subscriber',
    172                                 'email'        => 'user3@example.com',
     172                                'user_email'   => 'user4@example.com',
    173173                                'first_name'   => self::SLASH_2,
    174174                                'last_name'    => self::SLASH_4,
Note: See TracChangeset for help on using the changeset viewer.