Skip to content

Commit d2b5218

Browse files
authored
Merge branch 'WordPress:trunk' into trunk
2 parents b271aca + cc80c3a commit d2b5218

11 files changed

Lines changed: 664 additions & 15 deletions

File tree

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ final class WP_Comment {
246246
* Retrieves a WP_Comment instance.
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.
251252
*
@@ -263,15 +264,17 @@ public static function get_instance( $id ) {
263264

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 ) );
269271

270272
if ( ! $_comment ) {
271273
return false;
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

277280
return new WP_Comment( $_comment );

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class WP_Network {
9090
* Retrieves a network from the database by its ID.
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.
9596
*
@@ -106,14 +107,20 @@ public static function get_instance( $network_id ) {
106107

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

112118
if ( empty( $_network ) || is_wp_error( $_network ) ) {
113119
$_network = -1;
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

119126
if ( is_numeric( $_network ) ) {

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ final class WP_Post {
267267
* Retrieve WP_Post instance.
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.
272273
*
@@ -285,15 +286,18 @@ public static function get_instance( $post_id ) {
285286

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

291293
if ( ! $_post ) {
292294
return false;
293295
}
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' );
299303
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ final class WP_Site {
159159
* Retrieves a site from the database by its ID.
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.
164165
*
@@ -175,14 +176,20 @@ public static function get_instance( $site_id ) {
175176

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

181187
if ( empty( $_site ) || is_wp_error( $_site ) ) {
182188
$_site = -1;
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

188195
if ( is_numeric( $_site ) ) {

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ final class WP_Term {
103103
* Retrieve WP_Term instance.
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.
108109
*
@@ -123,8 +124,15 @@ public static function get_instance( $term_id, $taxonomy = null ) {
123124

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;
130138

@@ -177,7 +185,8 @@ public static function get_instance( $term_id, $taxonomy = null ) {
177185

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
}
183192

tests/phpstan/baselines/greater.invalid.neon

Whitespace-only changes.

tests/phpunit/tests/comment/wpComment.php

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* @covers WP_Comment::get_instance
77
*/
88
class 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 ) {
1212
global $wpdb;
@@ -63,6 +63,78 @@ public function test_get_instance_should_succeed_for_float_that_is_equal_to_post
6363
$this->assertSame( '1', $found->comment_ID );
6464
}
6565

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+
);
136+
}
137+
66138
/**
67139
* @ticket 64898
68140
*

tests/phpunit/tests/multisite/network.php

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class Tests_Multisite_Network extends WP_UnitTestCase {
1111

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

1717
public function tear_down() {
@@ -683,6 +683,73 @@ public function test_get_network_not_found_cache_clear() {
683683
$this->assertSame( $new_network_id, $fetched_network->id );
684684
}
685685

686+
/**
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+
686753
/**
687754
* Gets the ID of the site with the highest ID.
688755
* @return int

0 commit comments

Comments
 (0)