Opened 10 years ago
Last modified 5 years ago
#30911 new defect (bug)
Overhaul post_status logic in WP_Query
Reported by: | boonebgorges | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Query | Keywords: | needs-unit-tests needs-patch |
Focuses: | Cc: |
Description
WP_Query
filters based on post_status in two different places:
- 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 whetheris_admin()
. (The ability to query by 'post_status' dates from [5575].)
- 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 inWP_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:
- 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'. - 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 toWP_Query
. Some relevant tickets: #28568, #29167 - 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()
. - Filtering out non-previewable posts after the main query has taken place can result in certain sorts of data leakage. See eg #30910.
- 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].
In 31302: