WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 4 weeks ago

#51094 new defect (bug)

WP_Query.query with invalid post_status will return all

Reported by: carsonreinke Owned by:
Milestone: Awaiting Review Priority: normal
Severity: critical Version: 3.9
Component: Query Keywords: needs-patch
Focuses: Cc:

Description

Hello, I noticed an issue with WooCommerce method wc_get_products and dove into the issue.

The problem was wc_get_products(array('status' => 'published')); would return ALL posts, even trashed ones because of the "published" status is not a valid post status.

I believe that behavior is unexpected that it should return NO posts.

The issue appears to be this portion of WP_Query.get_posts that only known post statuses are added to the query:

<?php
                                foreach ( get_post_stati() as $status ) {
                                        if ( in_array( $status, $q_status, true ) ) {
                                                if ( 'private' === $status ) {
                                                        $p_status[] = "{$wpdb->posts}.post_status = '$status'";
                                                } else {
                                                        $r_status[] = "{$wpdb->posts}.post_status = '$status'";
                                                }
                                        }
                                }

I believe it would best if this method checks for any unrequested post status supplied in the query and either applies them to the query or produces an error. I would imagine this is a pretty major change.

Change History (5)

#1 @kishanjasani
3 months ago

Hey @carsonreinke, Welcome to WordPress Core Trac! This issue seems like issue of the WooCommerce.
So you have to report it here: https://github.com/woocommerce/woocommerce/issues

#2 @carsonreinke
3 months ago

@kishanjasani Sorry for the confusion, my reference to WooCommerce was simply how I came to this area in WordPress.

The following below seems wrong to me, since instead of returning NOTHING, it will return ALL posts:

<?php
$q = new WP_Query( array( 'post_status' => 'publish' ) );
$q->found_posts; //All published posts

$q = new WP_Query( array( 'post_status' => 'fake status' ) );
$q->found_posts; //Should be zero, instead ALL posts

#3 @xParham
5 weeks ago

  • Severity changed from minor to critical

I also believe this is a bug in WP_Query and actually one with potentially serious consequences. I came across the bug running a wp post CLI command, in my case passing an unregistered custom post status.

But here is another case, if you simply have a typo in your command, passing pendin instead of pending here:
wp post delete $(wp post list --post_type='post' --post_status=pendin --format=ids) --force

You will end up having ALL of your posts deleted.

Version 0, edited 5 weeks ago by xParham (next)

#4 @metalandcoffee
4 weeks ago

  • Keywords needs-patch added

I've also confirmed this issue. I tested the bug all the way back to WordPress version 3.9 and it was still an issue. Maybe this has always been a thing?

It looks like the logic inside of WP_Query's get_posts method (wp-includes/class-wp-query.php) only accommodates for the following cases:

  1. No post_status argument was provided in the new WP_query object so it generates the SQL statement with the default values ('publish' or 'private'):

Example: new WP_Query( array( 'author' => '1') );

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

  1. A post_status argument is provided in the new WP_query object and matches one of the available post statuses.

Example: new WP_Query( array( 'post_status' => 'publish') );

Generated SQL:
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')) ORDER BY wp_posts.post_date DESC LIMIT 0, 10

It doesn't accommodate for the case where a post_status argument is provided but it doesn't match any of the available post statuses.

So in that case, because none of the conditionals are met in order to append one of the relevant post_status SQL conditonals, the following SQL is generated:
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND wp_posts.post_type = 'post' ORDER BY wp_posts.post_date DESC LIMIT 0, 10

Which obviously results in all posts being returned.

#5 @johnbillion
4 weeks ago

  • Version changed from trunk to 3.9
Note: See TracTickets for help on using tickets.