WordPress.org
Get WordPress

Make WordPress Core

Changeset 63354


Ignore:
Timestamp:
08/26/2026 10:20:49 PM (26 hours ago)
Author:
westonruter
Message:

Cache API: Treat an unusable cached value as a miss.

WP_Site::get_instance() treated only false as a cache miss, but wp_cache_get() can return whatever a persistent object cache holds under that key. A cached string or array therefore skipped the refetch and reached the constructor, where get_object_vars() raised a TypeError. The same lookup is repeated in WP_Network::get_instance(), WP_Post::get_instance(), WP_Comment::get_instance() and WP_Term::get_instance(); networks and terms carried the identical fatal.

All five now refetch unless the cached value is an object carrying the property that identifies it. That also closes a quieter bug in the same code: an object missing that property was accepted as a cache hit, so posts came back with an ID of 0, comments with a null comment_ID, and terms as an entirely different term. Sites and networks keep their -1 sentinel recording a lookup that previously found nothing.

The refetched row is now stored with wp_cache_set() rather than wp_cache_add(). Widening the guard makes the branch reachable while an entry is still present, and add() does not overwrite, so the unusable value survived and every later lookup for that object queried the database again.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/13270.
Follow-up to r33891, r35537, r45910, r62648.

Props josephscott, westonruter.
Fixes #65962.

Location:
trunk
Files:
1 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/wp-includes/class-wp-comment.php

    r62822 r63354  
    247247         *
    248248         * @since 4.4.0
     249         * @since 7.2.0 Cache values that are not usable as a comment object are now treated as a cache miss and replaced.
    249250         *
    250251         * @global wpdb $wpdb WordPress database abstraction object.
    … …  
    264265                $_comment = wp_cache_get( $comment_id, 'comment' );
    265266
    266                 if ( ! is_object( $_comment ) ) {
     267                // A cached value that is not usable as a comment is treated as a cache miss.
     268                if ( ! is_object( $_comment ) || ! isset( $_comment->comment_ID ) ) {
    267269                        /** @var object{ comment_ID: string, comment_post_ID: string, comment_author: string, comment_author_email: string, comment_author_url: string, comment_author_IP: string, comment_date: string, comment_date_gmt: string, comment_content: string, comment_karma: string, comment_approved: string, comment_agent: string, comment_type: string, comment_parent: string, user_id: string }|null $_comment */
    268270                        $_comment = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->comments WHERE comment_ID = %d LIMIT 1", $comment_id ) );
    … …  
    272274                        }
    273275
    274                         wp_cache_add( $_comment->comment_ID, $_comment, 'comment' );
     276                        // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
     277                        wp_cache_set( $_comment->comment_ID, $_comment, 'comment' );
    275278                }
    276279

  • trunk/src/wp-includes/class-wp-network.php

    r62640 r63354  
    9191         *
    9292         * @since 4.4.0
     93         * @since 7.2.0 Cache values that are neither a network object nor the -1 miss sentinel are now treated as a cache miss and replaced.
    9394         *
    9495         * @global wpdb $wpdb WordPress database abstraction object.
    … …  
    107108                $_network = wp_cache_get( $network_id, 'networks' );
    108109
    109                 if ( false === $_network ) {
     110                // A cached -1 records a previous lookup that found nothing. Any other non-numeric value that is not a network object is treated as a cache miss.
     111                if (
     112                        ( ! is_object( $_network ) || ! isset( $_network->id ) )
     113                        &&
     114                        ! is_numeric( $_network )
     115                ) {
    110116                        $_network = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$wpdb->site} WHERE id = %d LIMIT 1", $network_id ) );
    111117
    … …  
    114120                        }
    115121
    116                         wp_cache_add( $network_id, $_network, 'networks' );
     122                        // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
     123                        wp_cache_set( $network_id, $_network, 'networks' );
    117124                }
    118125

  • trunk/src/wp-includes/class-wp-post.php

    r62717 r63354  
    268268         *
    269269         * @since 3.5.0
     270         * @since 7.2.0 Cache values that are not usable as a post object are now treated as a cache miss and replaced.
    270271         *
    271272         * @global wpdb $wpdb WordPress database abstraction object.
    … …  
    286287                $_post = wp_cache_get( $post_id, 'posts' );
    287288
    288                 if ( ! ( $_post instanceof stdClass ) && ! ( $_post instanceof WP_Post ) ) {
     289                // A cached value that is not usable as a post is treated as a cache miss.
     290                if ( ! ( $_post instanceof stdClass || $_post instanceof WP_Post ) || ! isset( $_post->ID ) ) {
    289291                        $_post = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE ID = %d LIMIT 1", $post_id ) );
    290292
    … …  
    294296
    295297                        $_post = sanitize_post( $_post, 'raw' );
    296                         wp_cache_add( (int) $_post->ID, $_post, 'posts' );
     298
     299                        // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
     300                        wp_cache_set( (int) $_post->ID, $_post, 'posts' );
    297301                } elseif ( empty( $_post->filter ) || 'raw' !== $_post->filter ) {
    298302                        $_post = sanitize_post( $_post, 'raw' );

  • trunk/src/wp-includes/class-wp-site.php

    r62640 r63354  
    160160         *
    161161         * @since 4.5.0
     162         * @since 7.2.0 Cache values that are neither a site object nor the -1 miss sentinel are now treated as a cache miss and replaced.
    162163         *
    163164         * @global wpdb $wpdb WordPress database abstraction object.
    … …  
    176177                $_site = wp_cache_get( $site_id, 'sites' );
    177178
    178                 if ( false === $_site ) {
     179                // A cached -1 records a previous lookup that found nothing. Any other non-numeric value that is not a site object is treated as a cache miss.
     180                if (
     181                        ( ! is_object( $_site ) || ! isset( $_site->blog_id ) )
     182                        &&
     183                        ! is_numeric( $_site )
     184                ) {
    179185                        $_site = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$wpdb->blogs} WHERE blog_id = %d LIMIT 1", $site_id ) );
    180186
    … …  
    183189                        }
    184190
    185                         wp_cache_add( $site_id, $_site, 'sites' );
     191                        // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
     192                        wp_cache_set( $site_id, $_site, 'sites' );
    186193                }
    187194

  • trunk/src/wp-includes/class-wp-term.php

    r61789 r63354  
    104104         *
    105105         * @since 4.4.0
     106         * @since 7.2.0 Cache values that are not usable as a term object are now treated as a cache miss and replaced.
    106107         *
    107108         * @global wpdb $wpdb WordPress database abstraction object.
    … …  
    124125                $_term = wp_cache_get( $term_id, 'terms' );
    125126
    126                 // If there isn't a cached version, hit the database.
    127                 if ( ! $_term || ( $taxonomy && $taxonomy !== $_term->taxonomy ) ) {
     127                /*
     128                 * If there isn't a usable cached version, hit the database. A cached value that is
     129                 * not a term object, or that belongs to another taxonomy, is treated as a cache miss.
     130                 */
     131                if (
     132                        ! is_object( $_term )
     133                        || ! isset( $_term->term_id, $_term->taxonomy )
     134                        || ( $taxonomy && $taxonomy !== $_term->taxonomy )
     135                ) {
    128136                        // Any term found in the cache is not a match, so don't use it.
    129137                        $_term = false;
    … …  
    178186                        // Don't cache terms that are shared between taxonomies.
    179187                        if ( 1 === count( $terms ) ) {
    180                                 wp_cache_add( $term_id, $_term, 'terms' );
     188                                // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced.
     189                                wp_cache_set( $term_id, $_term, 'terms' );
    181190                        }
    182191                }

  • trunk/tests/phpunit/tests/comment/wpComment.php

    r62822 r63354  
    77 */
    88class Tests_Comment_WpComment extends WP_UnitTestCase {
    9         protected static $comment_id;
     9        protected static int $comment_id;
    1010
    1111        public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) {
    … …  
    6262
    6363                $this->assertSame( '1', $found->comment_ID );
     64        }
     65
     66        /**
     67         * Tests that a cached value which cannot be used as a comment is treated as a cache miss.
     68         *
     69         * @ticket 65962
     70         *
     71         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     72         *
     73         * @param mixed $cache_value Value to poison the object cache with.
     74         */
     75        public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void {
     76                wp_cache_set( self::$comment_id, $cache_value, 'comment' );
     77
     78                $num_queries = get_num_queries();
     79
     80                $comment = WP_Comment::get_instance( self::$comment_id );
     81
     82                $this->assertInstanceOf( WP_Comment::class, $comment, 'A comment object was not returned.' );
     83                $this->assertSame( (string) self::$comment_id, $comment->comment_ID, 'The wrong comment was returned.' );
     84                $this->assertSame( $num_queries + 1, get_num_queries(), 'The comment was not fetched from the database.' );
     85        }
     86
     87        /**
     88         * Tests that the refetched comment replaces the poisoned cache value.
     89         *
     90         * Otherwise the poisoned value survives and every subsequent lookup queries the database again.
     91         *
     92         * @ticket 65962
     93         *
     94         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     95         *
     96         * @param mixed $cache_value Value to poison the object cache with.
     97         */
     98        public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void {
     99                wp_cache_set( self::$comment_id, $cache_value, 'comment' );
     100
     101                // Prime the object cache, replacing the poisoned value.
     102                WP_Comment::get_instance( self::$comment_id );
     103
     104                $num_queries = get_num_queries();
     105
     106                $comment = WP_Comment::get_instance( self::$comment_id );
     107
     108                $this->assertInstanceOf( WP_Comment::class, $comment, 'A comment object was not returned.' );
     109                $this->assertSame( (string) self::$comment_id, $comment->comment_ID, 'The wrong comment was returned.' );
     110                $this->assertSame( $num_queries, get_num_queries(), 'The database was queried again.' );
     111        }
     112
     113        /**
     114         * Data provider.
     115         *
     116         * @return array<non-falsy-string, array{ mixed }>
     117         */
     118        public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array {
     119                return array(
     120                        'true'                            => array( true ),
     121                        'a non-numeric string'            => array( 'not-a-comment' ),
     122                        'an empty array'                  => array( array() ),
     123                        'an array of comment data'        => array(
     124                                array(
     125                                        'comment_ID'      => '1',
     126                                        'comment_content' => 'Hello world.',
     127                                ),
     128                        ),
     129                        'an object without comment_ID'    => array(
     130                                (object) array(
     131                                        'comment_content' => 'Hello world.',
     132                                ),
     133                        ),
     134                        'a WP_Comment without comment_ID' => array( new WP_Comment( new stdClass() ) ),
     135                );
    64136        }
    65137

  • trunk/tests/phpunit/tests/multisite/network.php

    r61006 r63354  
    1212        protected $plugin_hook_count = 0;
    1313
    14         protected static $different_network_id;
     14        protected static int $different_network_id;
    1515        protected static $different_site_ids = array();
    1616
    … …  
    685685
    686686        /**
     687         * Tests that a cached value which is neither a network object nor the miss sentinel is treated as a cache miss.
     688         *
     689         * @ticket 65962
     690         *
     691         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     692         *
     693         * @param mixed $cache_value Value to poison the object cache with.
     694         */
     695        public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void {
     696                wp_cache_set( self::$different_network_id, $cache_value, 'networks' );
     697
     698                $network = WP_Network::get_instance( self::$different_network_id );
     699
     700                $this->assertInstanceOf( WP_Network::class, $network, 'A network object was not returned.' );
     701                $this->assertSame( self::$different_network_id, $network->id, 'The wrong network was returned.' );
     702        }
     703
     704        /**
     705         * Tests that the refetched network replaces the poisoned cache value.
     706         *
     707         * Otherwise the poisoned value survives and every subsequent lookup queries the database again.
     708         *
     709         * @ticket 65962
     710         *
     711         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     712         *
     713         * @param mixed $cache_value Value to poison the object cache with.
     714         */
     715        public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void {
     716                wp_cache_set( self::$different_network_id, $cache_value, 'networks' );
     717
     718                // Prime the object cache, replacing the poisoned value.
     719                WP_Network::get_instance( self::$different_network_id );
     720
     721                $cached = wp_cache_get( self::$different_network_id, 'networks' );
     722
     723                $this->assertInstanceOf( stdClass::class, $cached, 'The poisoned value was not replaced in the object cache.' );
     724                $this->assertSame( self::$different_network_id, (int) $cached->id, 'The wrong network was added to the object cache.' );
     725        }
     726
     727        /**
     728         * Data provider.
     729         *
     730         * @return array<non-falsy-string, array{ mixed }>
     731         */
     732        public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array {
     733                return array(
     734                        'true'                     => array( true ),
     735                        'a non-numeric string'     => array( 'not-a-network' ),
     736                        'an empty array'           => array( array() ),
     737                        'an array of network data' => array(
     738                                array(
     739                                        'id'     => '1',
     740                                        'domain' => 'wordpress.org',
     741                                        'path'   => '/',
     742                                ),
     743                        ),
     744                        'an object without an id'  => array(
     745                                (object) array(
     746                                        'domain' => 'wordpress.org',
     747                                        'path'   => '/',
     748                                ),
     749                        ),
     750                );
     751        }
     752
     753        /**
    687754         * Gets the ID of the site with the highest ID.
    688755         * @return int

  • trunk/tests/phpunit/tests/post/wpPost.php

    r60654 r63354  
    55 */
    66class Tests_Post_wpPost extends WP_UnitTestCase {
    7         protected static $post_id;
     7        protected static int $post_id;
    88
    99        public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) {
    … …  
    6969                $this->assertSame( 1, $found->ID );
    7070        }
     71
     72        /**
     73         * Tests that a cached value which cannot be used as a post is treated as a cache miss.
     74         *
     75         * @ticket 65962
     76         *
     77         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     78         *
     79         * @param mixed $cache_value Value to poison the object cache with.
     80         */
     81        public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void {
     82                wp_cache_set( self::$post_id, $cache_value, 'posts' );
     83
     84                $num_queries = get_num_queries();
     85
     86                $post = WP_Post::get_instance( self::$post_id );
     87
     88                $this->assertInstanceOf( WP_Post::class, $post, 'A post object was not returned.' );
     89                $this->assertSame( self::$post_id, $post->ID, 'The wrong post was returned.' );
     90                $this->assertSame( $num_queries + 1, get_num_queries(), 'The post was not fetched from the database.' );
     91        }
     92
     93        /**
     94         * Tests that the refetched post replaces the poisoned cache value.
     95         *
     96         * Otherwise the poisoned value survives and every subsequent lookup queries the database again.
     97         *
     98         * @ticket 65962
     99         *
     100         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     101         *
     102         * @param mixed $cache_value Value to poison the object cache with.
     103         */
     104        public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void {
     105                wp_cache_set( self::$post_id, $cache_value, 'posts' );
     106
     107                // Prime the object cache, replacing the poisoned value.
     108                WP_Post::get_instance( self::$post_id );
     109
     110                $num_queries = get_num_queries();
     111
     112                $post = WP_Post::get_instance( self::$post_id );
     113
     114                $this->assertInstanceOf( WP_Post::class, $post, 'A post object was not returned.' );
     115                $this->assertSame( self::$post_id, $post->ID, 'The wrong post was returned.' );
     116                $this->assertSame( $num_queries, get_num_queries(), 'The database was queried again.' );
     117        }
     118
     119        /**
     120         * Data provider.
     121         *
     122         * @return array<non-falsy-string, array{ mixed }>
     123         */
     124        public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array {
     125                return array(
     126                        'true'                  => array( true ),
     127                        'a non-numeric string'  => array( 'not-a-post' ),
     128                        'an empty array'        => array( array() ),
     129                        'an array of post data' => array(
     130                                array(
     131                                        'ID'         => 1,
     132                                        'post_title' => 'Post 1',
     133                                ),
     134                        ),
     135                        'an object without ID'  => array(
     136                                (object) array(
     137                                        'post_title' => 'Post 1',
     138                                ),
     139                        ),
     140                        'a WP_Post without ID'  => array( new WP_Post( new stdClass() ) ),
     141                );
     142        }
    71143}

  • trunk/tests/phpunit/tests/term/wpTerm.php

    r51568 r63354  
    55 */
    66class Tests_Term_WpTerm extends WP_UnitTestCase {
    7         protected static $term_id;
     7        protected static int $term_id;
    88
    99        public function set_up() {
    … …  
    9090                $this->assertFalse( $found );
    9191        }
     92
     93        /**
     94         * Tests that a cached value which cannot be used as a term is treated as a cache miss.
     95         *
     96         * @ticket 65962
     97         *
     98         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     99         *
     100         * @param mixed $cache_value Value to poison the object cache with.
     101         */
     102        public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void {
     103                wp_cache_set( self::$term_id, $cache_value, 'terms' );
     104
     105                $num_queries = get_num_queries();
     106
     107                $term = WP_Term::get_instance( self::$term_id );
     108
     109                $this->assertInstanceOf( WP_Term::class, $term, 'A term object was not returned.' );
     110                $this->assertSame( self::$term_id, $term->term_id, 'The wrong term was returned.' );
     111                $this->assertSame( 'wptests_tax', $term->taxonomy, 'The term was returned without its taxonomy.' );
     112                $this->assertSame( $num_queries + 1, get_num_queries(), 'The term was not fetched from the database.' );
     113        }
     114
     115        /**
     116         * Tests that a poisoned cache value is treated as a miss when a taxonomy is given.
     117         *
     118         * The taxonomy comparison reads a property off whatever is cached, so the guard has to
     119         * reject an unusable value before that point.
     120         *
     121         * @ticket 65962
     122         *
     123         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     124         *
     125         * @param mixed $cache_value Value to poison the object cache with.
     126         */
     127        public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss_with_a_taxonomy( $cache_value ): void {
     128                wp_cache_set( self::$term_id, $cache_value, 'terms' );
     129
     130                $term = WP_Term::get_instance( self::$term_id, 'wptests_tax' );
     131
     132                $this->assertInstanceOf( WP_Term::class, $term, 'A term object was not returned.' );
     133                $this->assertSame( self::$term_id, $term->term_id, 'The wrong term was returned.' );
     134        }
     135
     136        /**
     137         * Tests that the refetched term replaces the poisoned cache value.
     138         *
     139         * Otherwise the poisoned value survives and every subsequent lookup queries the database again.
     140         *
     141         * @ticket 65962
     142         *
     143         * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss
     144         *
     145         * @param mixed $cache_value Value to poison the object cache with.
     146         */
     147        public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void {
     148                wp_cache_set( self::$term_id, $cache_value, 'terms' );
     149
     150                // Prime the object cache, replacing the poisoned value.
     151                WP_Term::get_instance( self::$term_id );
     152
     153                $num_queries = get_num_queries();
     154
     155                $term = WP_Term::get_instance( self::$term_id );
     156
     157                $this->assertInstanceOf( WP_Term::class, $term, 'A term object was not returned.' );
     158                $this->assertSame( self::$term_id, $term->term_id, 'The wrong term was returned.' );
     159                $this->assertSame( $num_queries, get_num_queries(), 'The database was queried again.' );
     160        }
     161
     162        /**
     163         * Data provider.
     164         *
     165         * @return array<non-falsy-string, array{ mixed }>
     166         */
     167        public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array {
     168                return array(
     169                        'true'                       => array( true ),
     170                        'a non-numeric string'       => array( 'not-a-term' ),
     171                        'an array of term data'      => array(
     172                                array(
     173                                        'term_id'  => 1,
     174                                        'taxonomy' => 'wptests_tax',
     175                                ),
     176                        ),
     177                        'an object without term_id'  => array(
     178                                (object) array(
     179                                        'taxonomy' => 'wptests_tax',
     180                                ),
     181                        ),
     182                        'an object without taxonomy' => array(
     183                                (object) array(
     184                                        'term_id' => 1,
     185                                ),
     186                        ),
     187                        'a WP_Term without term_id'  => array( new WP_Term( new stdClass() ) ),
     188                );
     189        }
    92190}
Note: See TracChangeset for help on using the changeset viewer.