[wp-trac] [WordPress Trac] #30911: Overhaul post_status logic in WP_Query

[WordPress Trac]

#30911: Overhaul post_status logic in WP_Query
--------------------------+------------------------------------------
 Reporter:  boonebgorges  |      Owner:
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Future Release
Component:  Query         |    Version:
 Severity:  normal        |   Keywords:  needs-unit-tests needs-patch
  Focuses:                |
--------------------------+------------------------------------------
 `WP_Query` filters based on post_status in two different places:

 1. When building the main SQL query (`SELECT ID FROM $wpdb->posts`...).
 https://core.trac.wordpress.org/browser/tags/4.1/src/wp-
 includes/query.php#L2946 This block is responsible for parsing the
 'post_status' query var, and rectifying these passed post_statuses with
 the 'perm' param, the logged-in user's permissions, and whether
 `is_admin()`. (The ability to query by 'post_status' dates from [5575].)

 2. On `single` queries (`is_page()`, `is_single()`), additional filtering
 happens *after* the query.
 https://core.trac.wordpress.org/browser/tags/4.1/src/wp-
 includes/query.php#L3511 The intended purpose of this block is to ensure
 that Preview works (since posts being previewed generally are not
 published). The current incarnation of the preview logic (ie, living in
 `WP_Query`) was introduced in [2523].

 These two 'post_status' blocks have two different purposes and two
 different histories, but they interact in a number of problematic ways.
 Just a few of the problems:

 a. Querying posts with 'fields=ids' means that the post ids are returned
 before the second filter block gets a chance to run. As a result, certain
 `WP_Query` objects (eg 'p=123&post_status=trash') can return different
 post IDs depending on whether you request 'fields=ids'.
 b. Values passed to the post_status parameter of `WP_Query` are sometimes
 overridden forcibly, based on logged-in user status. In an ideal world,
 `WP_Query` would not make any essential reference to the logged-in user.
 Realistically, we can't change some of this behavior for reasons having to
 do with backward compatibility. But there are places where the current
 user logic could be reworked so that it provides defaults, which can then
 be overridden by the params passed to `WP_Query`. Some relevant tickets:
 #28568, #29167
 c. Filtering out non-previewable posts after the main query has taken
 place means doing more SQL queries than necessary in cases where
 post_status=draft and `is_single()`.
 d. Filtering out non-previewable posts after the main query has taken
 place can result in certain sorts of data leakage. See eg #30910.
 e. Having two separate blocks that filter results based on post_status
 makes unnecessarily complicated to fix post_status bugs. See eg #30530,
 #22001, #23309.

 My initial thought is that the second block should be removed. Preview
 logic should either be merged with the first block of post_status parsing
 logic that runs when building the main post query, or it should be moved
 into a separate query_var, which would then be passed when making a
 Preview request. In general, the challenge here is to ensure that the
 default post_status whitelist is properly sensitive to the permissions of
 the logged-in user - which sometimes means building clauses like
 `(post_status = 'publish' OR ( post_status = 'private' AND post_author =
 123 ))`.

 I've started to write some unit tests that describe the (weird and
 complicated) existing behavior. See #29167 [31047].

--
Ticket URL: <https://core.trac.wordpress.org/ticket/30911>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform