diff --git a/src/wp-includes/class-wp-comment.php b/src/wp-includes/class-wp-comment.php index f6b4d8d75f5e3..f14fac6614901 100644 --- a/src/wp-includes/class-wp-comment.php +++ b/src/wp-includes/class-wp-comment.php @@ -246,6 +246,7 @@ final class WP_Comment { * Retrieves a WP_Comment instance. * * @since 4.4.0 + * @since 7.2.0 Cache values that are not usable as a comment object are now treated as a cache miss and replaced. * * @global wpdb $wpdb WordPress database abstraction object. * @@ -263,7 +264,8 @@ public static function get_instance( $id ) { $_comment = wp_cache_get( $comment_id, 'comment' ); - if ( ! is_object( $_comment ) ) { + // A cached value that is not usable as a comment is treated as a cache miss. + if ( ! is_object( $_comment ) || ! isset( $_comment->comment_ID ) ) { /** @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 */ $_comment = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->comments WHERE comment_ID = %d LIMIT 1", $comment_id ) ); @@ -271,7 +273,8 @@ public static function get_instance( $id ) { return false; } - wp_cache_add( $_comment->comment_ID, $_comment, 'comment' ); + // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced. + wp_cache_set( $_comment->comment_ID, $_comment, 'comment' ); } return new WP_Comment( $_comment ); diff --git a/src/wp-includes/class-wp-network.php b/src/wp-includes/class-wp-network.php index 23056cd5a80f5..b9f6b01f2938a 100644 --- a/src/wp-includes/class-wp-network.php +++ b/src/wp-includes/class-wp-network.php @@ -90,6 +90,7 @@ class WP_Network { * Retrieves a network from the database by its ID. * * @since 4.4.0 + * @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. * * @global wpdb $wpdb WordPress database abstraction object. * @@ -106,14 +107,20 @@ public static function get_instance( $network_id ) { $_network = wp_cache_get( $network_id, 'networks' ); - if ( false === $_network ) { + // 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. + if ( + ( ! is_object( $_network ) || ! isset( $_network->id ) ) + && + ! is_numeric( $_network ) + ) { $_network = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$wpdb->site} WHERE id = %d LIMIT 1", $network_id ) ); if ( empty( $_network ) || is_wp_error( $_network ) ) { $_network = -1; } - wp_cache_add( $network_id, $_network, 'networks' ); + // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced. + wp_cache_set( $network_id, $_network, 'networks' ); } if ( is_numeric( $_network ) ) { diff --git a/src/wp-includes/class-wp-post.php b/src/wp-includes/class-wp-post.php index 5531c1301ce3c..4008c6bc12880 100644 --- a/src/wp-includes/class-wp-post.php +++ b/src/wp-includes/class-wp-post.php @@ -267,6 +267,7 @@ final class WP_Post { * Retrieve WP_Post instance. * * @since 3.5.0 + * @since 7.2.0 Cache values that are not usable as a post object are now treated as a cache miss and replaced. * * @global wpdb $wpdb WordPress database abstraction object. * @@ -285,7 +286,8 @@ public static function get_instance( $post_id ) { $_post = wp_cache_get( $post_id, 'posts' ); - if ( ! ( $_post instanceof stdClass ) && ! ( $_post instanceof WP_Post ) ) { + // A cached value that is not usable as a post is treated as a cache miss. + if ( ! ( $_post instanceof stdClass || $_post instanceof WP_Post ) || ! isset( $_post->ID ) ) { $_post = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE ID = %d LIMIT 1", $post_id ) ); if ( ! $_post ) { @@ -293,7 +295,9 @@ public static function get_instance( $post_id ) { } $_post = sanitize_post( $_post, 'raw' ); - wp_cache_add( (int) $_post->ID, $_post, 'posts' ); + + // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced. + wp_cache_set( (int) $_post->ID, $_post, 'posts' ); } elseif ( empty( $_post->filter ) || 'raw' !== $_post->filter ) { $_post = sanitize_post( $_post, 'raw' ); } diff --git a/src/wp-includes/class-wp-site.php b/src/wp-includes/class-wp-site.php index ce7bc411518e1..bb6c2d50a3428 100644 --- a/src/wp-includes/class-wp-site.php +++ b/src/wp-includes/class-wp-site.php @@ -159,6 +159,7 @@ final class WP_Site { * Retrieves a site from the database by its ID. * * @since 4.5.0 + * @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. * * @global wpdb $wpdb WordPress database abstraction object. * @@ -175,14 +176,20 @@ public static function get_instance( $site_id ) { $_site = wp_cache_get( $site_id, 'sites' ); - if ( false === $_site ) { + // 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. + if ( + ( ! is_object( $_site ) || ! isset( $_site->blog_id ) ) + && + ! is_numeric( $_site ) + ) { $_site = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$wpdb->blogs} WHERE blog_id = %d LIMIT 1", $site_id ) ); if ( empty( $_site ) || is_wp_error( $_site ) ) { $_site = -1; } - wp_cache_add( $site_id, $_site, 'sites' ); + // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced. + wp_cache_set( $site_id, $_site, 'sites' ); } if ( is_numeric( $_site ) ) { diff --git a/src/wp-includes/class-wp-term.php b/src/wp-includes/class-wp-term.php index 0cefa3097b393..33547e4cbef98 100644 --- a/src/wp-includes/class-wp-term.php +++ b/src/wp-includes/class-wp-term.php @@ -103,6 +103,7 @@ final class WP_Term { * Retrieve WP_Term instance. * * @since 4.4.0 + * @since 7.2.0 Cache values that are not usable as a term object are now treated as a cache miss and replaced. * * @global wpdb $wpdb WordPress database abstraction object. * @@ -123,8 +124,15 @@ public static function get_instance( $term_id, $taxonomy = null ) { $_term = wp_cache_get( $term_id, 'terms' ); - // If there isn't a cached version, hit the database. - if ( ! $_term || ( $taxonomy && $taxonomy !== $_term->taxonomy ) ) { + /* + * If there isn't a usable cached version, hit the database. A cached value that is + * not a term object, or that belongs to another taxonomy, is treated as a cache miss. + */ + if ( + ! is_object( $_term ) + || ! isset( $_term->term_id, $_term->taxonomy ) + || ( $taxonomy && $taxonomy !== $_term->taxonomy ) + ) { // Any term found in the cache is not a match, so don't use it. $_term = false; @@ -177,7 +185,8 @@ public static function get_instance( $term_id, $taxonomy = null ) { // Don't cache terms that are shared between taxonomies. if ( 1 === count( $terms ) ) { - wp_cache_add( $term_id, $_term, 'terms' ); + // Not wp_cache_add(), since an unusable cached value may still be present and must be replaced. + wp_cache_set( $term_id, $_term, 'terms' ); } } diff --git a/tests/phpunit/tests/comment/wpComment.php b/tests/phpunit/tests/comment/wpComment.php index 1f8688d3aeaf9..7361b18346419 100644 --- a/tests/phpunit/tests/comment/wpComment.php +++ b/tests/phpunit/tests/comment/wpComment.php @@ -6,7 +6,7 @@ * @covers WP_Comment::get_instance */ class Tests_Comment_WpComment extends WP_UnitTestCase { - protected static $comment_id; + protected static int $comment_id; public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { global $wpdb; @@ -63,6 +63,78 @@ public function test_get_instance_should_succeed_for_float_that_is_equal_to_post $this->assertSame( '1', $found->comment_ID ); } + /** + * Tests that a cached value which cannot be used as a comment is treated as a cache miss. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void { + wp_cache_set( self::$comment_id, $cache_value, 'comment' ); + + $num_queries = get_num_queries(); + + $comment = WP_Comment::get_instance( self::$comment_id ); + + $this->assertInstanceOf( WP_Comment::class, $comment, 'A comment object was not returned.' ); + $this->assertSame( (string) self::$comment_id, $comment->comment_ID, 'The wrong comment was returned.' ); + $this->assertSame( $num_queries + 1, get_num_queries(), 'The comment was not fetched from the database.' ); + } + + /** + * Tests that the refetched comment replaces the poisoned cache value. + * + * Otherwise the poisoned value survives and every subsequent lookup queries the database again. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void { + wp_cache_set( self::$comment_id, $cache_value, 'comment' ); + + // Prime the object cache, replacing the poisoned value. + WP_Comment::get_instance( self::$comment_id ); + + $num_queries = get_num_queries(); + + $comment = WP_Comment::get_instance( self::$comment_id ); + + $this->assertInstanceOf( WP_Comment::class, $comment, 'A comment object was not returned.' ); + $this->assertSame( (string) self::$comment_id, $comment->comment_ID, 'The wrong comment was returned.' ); + $this->assertSame( $num_queries, get_num_queries(), 'The database was queried again.' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array { + return array( + 'true' => array( true ), + 'a non-numeric string' => array( 'not-a-comment' ), + 'an empty array' => array( array() ), + 'an array of comment data' => array( + array( + 'comment_ID' => '1', + 'comment_content' => 'Hello world.', + ), + ), + 'an object without comment_ID' => array( + (object) array( + 'comment_content' => 'Hello world.', + ), + ), + 'a WP_Comment without comment_ID' => array( new WP_Comment( new stdClass() ) ), + ); + } + /** * @ticket 64898 * diff --git a/tests/phpunit/tests/multisite/network.php b/tests/phpunit/tests/multisite/network.php index a5741e91f93f7..17fcbfaa49fd3 100644 --- a/tests/phpunit/tests/multisite/network.php +++ b/tests/phpunit/tests/multisite/network.php @@ -11,7 +11,7 @@ class Tests_Multisite_Network extends WP_UnitTestCase { protected $plugin_hook_count = 0; - protected static $different_network_id; + protected static int $different_network_id; protected static $different_site_ids = array(); public function tear_down() { @@ -683,6 +683,73 @@ public function test_get_network_not_found_cache_clear() { $this->assertSame( $new_network_id, $fetched_network->id ); } + /** + * Tests that a cached value which is neither a network object nor the miss sentinel is treated as a cache miss. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void { + wp_cache_set( self::$different_network_id, $cache_value, 'networks' ); + + $network = WP_Network::get_instance( self::$different_network_id ); + + $this->assertInstanceOf( WP_Network::class, $network, 'A network object was not returned.' ); + $this->assertSame( self::$different_network_id, $network->id, 'The wrong network was returned.' ); + } + + /** + * Tests that the refetched network replaces the poisoned cache value. + * + * Otherwise the poisoned value survives and every subsequent lookup queries the database again. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void { + wp_cache_set( self::$different_network_id, $cache_value, 'networks' ); + + // Prime the object cache, replacing the poisoned value. + WP_Network::get_instance( self::$different_network_id ); + + $cached = wp_cache_get( self::$different_network_id, 'networks' ); + + $this->assertInstanceOf( stdClass::class, $cached, 'The poisoned value was not replaced in the object cache.' ); + $this->assertSame( self::$different_network_id, (int) $cached->id, 'The wrong network was added to the object cache.' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array { + return array( + 'true' => array( true ), + 'a non-numeric string' => array( 'not-a-network' ), + 'an empty array' => array( array() ), + 'an array of network data' => array( + array( + 'id' => '1', + 'domain' => 'wordpress.org', + 'path' => '/', + ), + ), + 'an object without an id' => array( + (object) array( + 'domain' => 'wordpress.org', + 'path' => '/', + ), + ), + ); + } + /** * Gets the ID of the site with the highest ID. * @return int diff --git a/tests/phpunit/tests/multisite/wpSite.php b/tests/phpunit/tests/multisite/wpSite.php new file mode 100644 index 0000000000000..7c3439eae10ff --- /dev/null +++ b/tests/phpunit/tests/multisite/wpSite.php @@ -0,0 +1,310 @@ +blog->create( + array( + 'domain' => 'wordpress.org', + 'path' => '/', + ) + ); + + self::$nonexistent_site_id = self::$site_id + 1000; + } + + public static function wpTearDownAfterClass(): void { + /* + * The object cache is flushed before each test, but not between the last test + * and here. A poisoned value left in the 'sites' group would make the get_site() + * call inside wp_delete_site() consider the site to no longer exist. + */ + wp_cache_delete( self::$site_id, 'sites' ); + + wp_delete_site( self::$site_id ); + + wp_update_network_site_counts(); + } + + /** + * Tests that a site ID which cannot reference a site returns false without querying the database. + * + * @global wpdb $wpdb WordPress database abstraction object. + * + * @dataProvider data_get_instance_returns_false_for_an_empty_site_id + * + * @param mixed $site_id Site ID to look up. + */ + public function test_get_instance_returns_false_for_an_empty_site_id( $site_id ): void { + global $wpdb; + + $num_queries = $wpdb->num_queries; + + $this->assertFalse( WP_Site::get_instance( $site_id ), 'A site object was returned.' ); // @phpstan-ignore argument.type (Intentionally passing a value which is not an integer.) + $this->assertSame( $num_queries, $wpdb->num_queries, 'The database was queried.' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_get_instance_returns_false_for_an_empty_site_id(): array { + return array( + 'zero as an integer' => array( 0 ), + 'zero as a string' => array( '0' ), + 'a non-numeric string' => array( 'not-a-site-id' ), + 'false' => array( false ), + 'null' => array( null ), + ); + } + + /** + * Tests that an uncached site is fetched from the database and then added to the object cache. + * + * @global wpdb $wpdb WordPress database abstraction object. + */ + public function test_get_instance_queries_the_database_when_the_site_is_not_cached(): void { + global $wpdb; + + wp_cache_delete( self::$site_id, 'sites' ); + + $num_queries = $wpdb->num_queries; + + $site = WP_Site::get_instance( self::$site_id ); + + $this->assertInstanceOf( WP_Site::class, $site, 'A site object was not returned.' ); + $this->assertSame( (string) self::$site_id, $site->blog_id, 'The wrong site was returned.' ); + $this->assertSame( $num_queries + 1, $wpdb->num_queries, 'The site was not fetched from the database.' ); + + $cached = wp_cache_get( self::$site_id, 'sites' ); + + $this->assertInstanceOf( stdClass::class, $cached, 'The database row was not added to the object cache.' ); + $this->assertSame( (string) self::$site_id, $cached->blog_id, 'The wrong site was added to the object cache.' ); + } + + /** + * Tests that a cached site is returned without querying the database. + * + * @global wpdb $wpdb WordPress database abstraction object. + */ + public function test_get_instance_does_not_query_the_database_when_the_site_is_cached(): void { + global $wpdb; + + wp_cache_delete( self::$site_id, 'sites' ); + + // Prime the object cache. + WP_Site::get_instance( self::$site_id ); + + $num_queries = $wpdb->num_queries; + + $site = WP_Site::get_instance( self::$site_id ); + + $this->assertInstanceOf( WP_Site::class, $site, 'A site object was not returned.' ); + $this->assertSame( (string) self::$site_id, $site->blog_id, 'The wrong site was returned.' ); + $this->assertSame( $num_queries, $wpdb->num_queries, 'The database was queried.' ); + } + + /** + * Tests that a cached WP_Site object is treated as a cache hit. + * + * The object cache is normally primed with the raw database row, but a WP_Site + * instance may be cached by other means. + * + * @global wpdb $wpdb WordPress database abstraction object. + */ + public function test_get_instance_treats_a_cached_wp_site_object_as_a_cache_hit(): void { + global $wpdb; + + wp_cache_set( self::$site_id, WP_Site::get_instance( self::$site_id ), 'sites' ); + + $num_queries = $wpdb->num_queries; + + $site = WP_Site::get_instance( self::$site_id ); + + $this->assertInstanceOf( WP_Site::class, $site, 'A site object was not returned.' ); + $this->assertSame( (string) self::$site_id, $site->blog_id, 'The wrong site was returned.' ); + $this->assertSame( $num_queries, $wpdb->num_queries, 'The database was queried.' ); + } + + /** + * Tests that a site which is not in the database returns false and that the miss is cached. + * + * @global wpdb $wpdb WordPress database abstraction object. + */ + public function test_get_instance_returns_false_for_a_nonexistent_site_and_caches_the_miss(): void { + global $wpdb; + + wp_cache_delete( self::$nonexistent_site_id, 'sites' ); + + $num_queries = $wpdb->num_queries; + + $this->assertFalse( WP_Site::get_instance( self::$nonexistent_site_id ), 'A site object was returned.' ); + $this->assertSame( $num_queries + 1, $wpdb->num_queries, 'The database was not queried.' ); + $this->assertSame( -1, wp_cache_get( self::$nonexistent_site_id, 'sites' ), 'The miss was not cached as -1.' ); + } + + /** + * Tests that a cached miss is not looked up in the database again. + * + * @global wpdb $wpdb WordPress database abstraction object. + */ + public function test_get_instance_does_not_query_the_database_for_a_cached_miss(): void { + global $wpdb; + + wp_cache_delete( self::$nonexistent_site_id, 'sites' ); + + // Prime the object cache with the miss. + WP_Site::get_instance( self::$nonexistent_site_id ); + + $num_queries = $wpdb->num_queries; + + $this->assertFalse( WP_Site::get_instance( self::$nonexistent_site_id ), 'A site object was returned.' ); + $this->assertSame( $num_queries, $wpdb->num_queries, 'The database was queried.' ); + } + + /** + * Tests that a cached value which is neither a site object nor the miss sentinel is treated as a cache miss. + * + * @ticket 65962 + * + * @global wpdb $wpdb WordPress database abstraction object. + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void { + global $wpdb; + + wp_cache_set( self::$site_id, $cache_value, 'sites' ); + + $num_queries = $wpdb->num_queries; + + $site = WP_Site::get_instance( self::$site_id ); + + $this->assertInstanceOf( WP_Site::class, $site, 'A site object was not returned.' ); + $this->assertSame( (string) self::$site_id, $site->blog_id, 'The wrong site was returned.' ); + $this->assertSame( $num_queries + 1, $wpdb->num_queries, 'The site was not fetched from the database.' ); + } + + /** + * Tests that the refetched site replaces the poisoned cache value. + * + * Otherwise the poisoned value survives and every subsequent lookup queries the database again. + * + * @ticket 65962 + * + * @global wpdb $wpdb WordPress database abstraction object. + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void { + global $wpdb; + + wp_cache_set( self::$site_id, $cache_value, 'sites' ); + + // Prime the object cache, replacing the poisoned value. + WP_Site::get_instance( self::$site_id ); + + $cached = wp_cache_get( self::$site_id, 'sites' ); + + $this->assertInstanceOf( stdClass::class, $cached, 'The poisoned value was not replaced in the object cache.' ); + $this->assertSame( (string) self::$site_id, $cached->blog_id, 'The wrong site was added to the object cache.' ); + + $num_queries = $wpdb->num_queries; + + $site = WP_Site::get_instance( self::$site_id ); + + $this->assertInstanceOf( WP_Site::class, $site, 'A site object was not returned.' ); + $this->assertSame( (string) self::$site_id, $site->blog_id, 'The wrong site was returned.' ); + $this->assertSame( $num_queries, $wpdb->num_queries, 'The database was queried again.' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array { + return array( + 'true' => array( true ), + 'a non-numeric string' => array( 'not-a-site' ), + 'an empty array' => array( array() ), + 'an array of site data' => array( + array( + 'blog_id' => '1', + 'domain' => 'wordpress.org', + 'path' => '/', + ), + ), + 'an object without blog_id' => array( + (object) array( + 'domain' => 'wordpress.org', + 'path' => '/', + ), + ), + ); + } + + /** + * Tests that any numeric cached value is treated as a cached miss. + * + * Only -1 is written to the cache to record a miss, but every numeric value is + * currently trusted as one, so an existing site is reported as not found. + * + * @ticket 65962 + * + * @global wpdb $wpdb WordPress database abstraction object. + * + * @dataProvider data_get_instance_treats_a_numeric_cache_value_as_a_cached_miss + * + * @param int|numeric-string $cache_value Value to poison the object cache with. + */ + public function test_get_instance_treats_a_numeric_cache_value_as_a_cached_miss( $cache_value ): void { + global $wpdb; + + wp_cache_set( self::$site_id, $cache_value, 'sites' ); + + $num_queries = $wpdb->num_queries; + + $this->assertFalse( WP_Site::get_instance( self::$site_id ), 'A site object was returned.' ); + $this->assertSame( $num_queries, $wpdb->num_queries, 'The database was queried.' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_get_instance_treats_a_numeric_cache_value_as_a_cached_miss(): array { + return array( + 'the -1 miss sentinel' => array( -1 ), + '-1 as a string' => array( '-1' ), + 'zero' => array( 0 ), + 'a positive integer' => array( 42 ), + 'a float as a string' => array( '3.5' ), + ); + } +} diff --git a/tests/phpunit/tests/post/wpPost.php b/tests/phpunit/tests/post/wpPost.php index b35a9ba5c406a..e738ef25e7036 100644 --- a/tests/phpunit/tests/post/wpPost.php +++ b/tests/phpunit/tests/post/wpPost.php @@ -4,7 +4,7 @@ * @group post */ class Tests_Post_wpPost extends WP_UnitTestCase { - protected static $post_id; + protected static int $post_id; public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { global $wpdb; @@ -68,4 +68,76 @@ public function test_get_instance_should_succeed_for_float_that_is_equal_to_post $this->assertSame( 1, $found->ID ); } + + /** + * Tests that a cached value which cannot be used as a post is treated as a cache miss. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void { + wp_cache_set( self::$post_id, $cache_value, 'posts' ); + + $num_queries = get_num_queries(); + + $post = WP_Post::get_instance( self::$post_id ); + + $this->assertInstanceOf( WP_Post::class, $post, 'A post object was not returned.' ); + $this->assertSame( self::$post_id, $post->ID, 'The wrong post was returned.' ); + $this->assertSame( $num_queries + 1, get_num_queries(), 'The post was not fetched from the database.' ); + } + + /** + * Tests that the refetched post replaces the poisoned cache value. + * + * Otherwise the poisoned value survives and every subsequent lookup queries the database again. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void { + wp_cache_set( self::$post_id, $cache_value, 'posts' ); + + // Prime the object cache, replacing the poisoned value. + WP_Post::get_instance( self::$post_id ); + + $num_queries = get_num_queries(); + + $post = WP_Post::get_instance( self::$post_id ); + + $this->assertInstanceOf( WP_Post::class, $post, 'A post object was not returned.' ); + $this->assertSame( self::$post_id, $post->ID, 'The wrong post was returned.' ); + $this->assertSame( $num_queries, get_num_queries(), 'The database was queried again.' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array { + return array( + 'true' => array( true ), + 'a non-numeric string' => array( 'not-a-post' ), + 'an empty array' => array( array() ), + 'an array of post data' => array( + array( + 'ID' => 1, + 'post_title' => 'Post 1', + ), + ), + 'an object without ID' => array( + (object) array( + 'post_title' => 'Post 1', + ), + ), + 'a WP_Post without ID' => array( new WP_Post( new stdClass() ) ), + ); + } } diff --git a/tests/phpunit/tests/term/wpTerm.php b/tests/phpunit/tests/term/wpTerm.php index e640cf8120732..f7a2934baa473 100644 --- a/tests/phpunit/tests/term/wpTerm.php +++ b/tests/phpunit/tests/term/wpTerm.php @@ -4,7 +4,7 @@ * @group taxonomy */ class Tests_Term_WpTerm extends WP_UnitTestCase { - protected static $term_id; + protected static int $term_id; public function set_up() { parent::set_up(); @@ -89,4 +89,102 @@ public function test_get_instance_should_respect_taxonomy_when_term_id_is_found_ $found = WP_Term::get_instance( self::$term_id, 'wptests_tax2' ); $this->assertFalse( $found ); } + + /** + * Tests that a cached value which cannot be used as a term is treated as a cache miss. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss( $cache_value ): void { + wp_cache_set( self::$term_id, $cache_value, 'terms' ); + + $num_queries = get_num_queries(); + + $term = WP_Term::get_instance( self::$term_id ); + + $this->assertInstanceOf( WP_Term::class, $term, 'A term object was not returned.' ); + $this->assertSame( self::$term_id, $term->term_id, 'The wrong term was returned.' ); + $this->assertSame( 'wptests_tax', $term->taxonomy, 'The term was returned without its taxonomy.' ); + $this->assertSame( $num_queries + 1, get_num_queries(), 'The term was not fetched from the database.' ); + } + + /** + * Tests that a poisoned cache value is treated as a miss when a taxonomy is given. + * + * The taxonomy comparison reads a property off whatever is cached, so the guard has to + * reject an unusable value before that point. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss_with_a_taxonomy( $cache_value ): void { + wp_cache_set( self::$term_id, $cache_value, 'terms' ); + + $term = WP_Term::get_instance( self::$term_id, 'wptests_tax' ); + + $this->assertInstanceOf( WP_Term::class, $term, 'A term object was not returned.' ); + $this->assertSame( self::$term_id, $term->term_id, 'The wrong term was returned.' ); + } + + /** + * Tests that the refetched term replaces the poisoned cache value. + * + * Otherwise the poisoned value survives and every subsequent lookup queries the database again. + * + * @ticket 65962 + * + * @dataProvider data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss + * + * @param mixed $cache_value Value to poison the object cache with. + */ + public function test_get_instance_replaces_a_poisoned_cache_value( $cache_value ): void { + wp_cache_set( self::$term_id, $cache_value, 'terms' ); + + // Prime the object cache, replacing the poisoned value. + WP_Term::get_instance( self::$term_id ); + + $num_queries = get_num_queries(); + + $term = WP_Term::get_instance( self::$term_id ); + + $this->assertInstanceOf( WP_Term::class, $term, 'A term object was not returned.' ); + $this->assertSame( self::$term_id, $term->term_id, 'The wrong term was returned.' ); + $this->assertSame( $num_queries, get_num_queries(), 'The database was queried again.' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_get_instance_treats_a_poisoned_cache_value_as_a_cache_miss(): array { + return array( + 'true' => array( true ), + 'a non-numeric string' => array( 'not-a-term' ), + 'an array of term data' => array( + array( + 'term_id' => 1, + 'taxonomy' => 'wptests_tax', + ), + ), + 'an object without term_id' => array( + (object) array( + 'taxonomy' => 'wptests_tax', + ), + ), + 'an object without taxonomy' => array( + (object) array( + 'term_id' => 1, + ), + ), + 'a WP_Term without term_id' => array( new WP_Term( new stdClass() ) ), + ); + } }