[wp-trac] [WordPress Trac] #48556: Query for multiple post types not considering user permission to retrieve private posts

[WordPress Trac]

#48556: Query for multiple post types not considering user permission to retrieve
private posts
--------------------------------------------+-----------------------------
 Reporter:  leogermani                      |       Owner:  SergeyBiryukov
     Type:  defect (bug)                    |      Status:  reviewing
 Priority:  normal                          |   Milestone:  5.7
Component:  Query                           |     Version:
 Severity:  normal                          |  Resolution:
 Keywords:  has-patch has-unit-tests early  |     Focuses:
--------------------------------------------+-----------------------------

Comment (by leogermani):

 Replying to [comment:21 boonebgorges]:
 > @leogermani Thanks for your patience as I reviewed your PR.

 No problem!

 > I've attached a new patch [attachment:"48556.2.diff"] which makes a few
 changes:
 > - Fixes a bug where certain combinations of query parameters could cause
 the list of 'where' clauses to be empty, resulting in invalid SQL syntax
 of the form `AND ()`.
 > - Updates code formatting and variable names. I know you were matching
 existing variable names in other clauses, but they're really difficult to
 understand, so I've tried to make them clearer.
 > - Rewritten and relocated the tests, to make them a bit more independent
 of built-in post types and to isolate the capability issue more clearly.

 The patch looks good, thanks for it! Maybe it's a good idea to have a test
 that reproduces this scenario you fixed.

 > Have you had a chance to think about backward compatibility concerns?
 How can we summarize the changes to SQL syntax?

 I was thinking of something like:

 (We can either edit it later and make it smaller for a dev note, or expand
 it with more examples for a full blog post)

 This fix changes the structure of the WHERE clause when querying for posts
 filtering by multiple post types. Any code that relies on the
 `posts_where` filter and expecting a specific pattern should check if it
 will be affected by this change.

 The changes will be noticed whenever the `post_type` argument of the query
 is set.

 Before this change, the clauses for post types and post statuses were
 separated:


 {{{
 ... wp_posts.post_type IN ('post', 'page') AND (wp_posts.post_status =
 'publish' ...
 }}}

 Now, each clause for a post type will hold a clause for the post status as
 well.


 {{{
 ... ((wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish'))
 OR (wp_posts.post_type = 'page' AND (wp_posts.post_status = 'publish')))
 ...
 }}}


 Example:

 Sample query:
 {{{#!php
 $query = new WP_Query(
         array(
                 'post_type' => array(
                         'post',
                         'page',
                 )
         )
 );
 }}}

 SQL Query before this change:

 {{{
 SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND
 wp_posts.post_type IN ('post', 'page') AND (wp_posts.post_status =
 'publish' OR wp_posts.post_author = 1 AND wp_posts.post_status =
 'private') ORDER BY wp_posts.post_date DESC LIMIT 0, 10
 }}}


 SQL Query after this change:

 {{{
 SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND
 ((wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish'
 wp_posts.post_author = 1 AND wp_posts.post_status = 'private')) OR
 (wp_posts.post_type = 'page' AND (wp_posts.post_status = 'publish' OR
 wp_posts.post_author = 1 AND wp_posts.post_status = 'private'))) ORDER BY
 wp_posts.post_date DESC LIMIT 0, 10
 }}}


 > > There's still another case to take care, which is when we also query
 for multiple post statuses. I'm wondering if that will be too much for a
 single diff and maybe it's better to wait for this to move forward and do
 this in a following one. Anyway, I already started looking into it and the
 solution will be similar.
 >
 > Could you start by writing tests that describe how this is currently
 broken?

 Sure, I'll open a new ticket and start by describing the issue and writing
 tests that reproduce it... it would be easier to write a patch only after
 this one is merged though, to avoid conflicts and merge hell. (unless you
 think it's fine to work on this same ticket)

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