[wp-trac] [WordPress Trac] #58599: WP_Query->get_posts: cache updated also when query is set to be not cacheable

[WordPress Trac]

#58599: WP_Query->get_posts: cache updated also when query is set to be not
cacheable
--------------------------+------------------------------
 Reporter:  saulirajala   |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Query         |     Version:  6.2.2
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:  performance
--------------------------+------------------------------

Comment (by saulirajala):

 Hmm, what would this break? What is the use case where we would want the
 query to be uncacheable in all the other if-statements in the
 `get_posts()`-function, but in the last one we want it to be cacheable?
 Isn't the code comment [https://github.com/WordPress/wordpress-
 develop/blob/6.2/src/wp-includes/class-wp-query.php#L3138-L3149 in rows
 3138-3149] ''"Random queries are expected to have unpredictable results
 and cannot be cached…If $fields has been modified by the posts_fields,
 posts_fields_request, post_clauses or posts_clauses_request filters, then
 caching is disabled to prevent caching collisions."'' true also
 [https://github.com/WordPress/wordpress-develop/blob/6.2/src/wp-includes
 /class-wp-query.php#L3516-L3518, in the last if-statement]? If query is
 set to be uncacheable, then it should be treated as one consistently
 throughout the function. And thus `update_post_cache()` shouldn't be
 executed, when `$id_query_is_cacheable` is false.

 I understand that the situation where I am, can be a rare edge case (using
 persistent object cache and modifying the query with
 `posts_fields_request`-hook). But I would still argue that this patch is a
 fix to a bug and not a breaking change. To my knowledge the breaking
 change was introduced in [https://core.trac.wordpress.org/changeset/55035
 r55035]. `$this->posts` has wrong field values for posts when we have
 modified the query via `posts_fields_request` -hook. For example, if the
 `foo_allowed_fields` -hook function (mentioned above) is enabled,
 `$this->posts` would have posts that [https://github.com/WordPress
 /wordpress-develop/blob/6.2/src/wp-includes/class-wp-post.php#L32-L220 has
 default values of `WP_Post` object] for fields like `post_date`,
 `ping_status`, `comment_status`, `post_modified` and `post_password`. This
 can cause a serious problems on the site using persistent object cache
 since the wrong values are saved persistently for each post. Now that I
 think about this, this can even lead to a somekind of security issue,
 since for post that are password protect can now have the value in the
 persistent object cache, where `post_password` is empty string and thus
 the page will not be password protected.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/58599#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform