Make WordPress Core

Opened 8 years ago

Last modified 13 months ago

#33341 reopened defect (bug)

WP_Meta_Query IN operator with empty array does not return expected result

Reported by: flixos90's profile flixos90 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: critical Version: 3.2
Component: Query Keywords: dev-feedback
Focuses: Cc:

Description

When using the IN compare mode in a WP_Meta_Query where the value is an empty array, this query is evaluated with an unexpected behavior.

For example, I have some posts with a meta field called 'some-textfield':

$posts = get_posts( array(
	'posts_per_page' => -1,
	'post_type' => 'post',
	'post_status' => 'publish',
	'meta_query' => array(
		array(
			'key'	 => 'some-textfield',
			'value' => array(),
			'compare' => 'IN',
		),
	),
) );

This code returns all posts although I would rather expect it to return no posts at all since the value of 'some-textfield' (whatever it may be) is not in those provided in the value field.

I discovered it when I needed to perform a meta query where the value was an array that was returned by a previous operation. It is obviously a minor issue since we can simply check if the array is empty (and in that case do not make the query at all) - but I thought it's not really the expected result of such a query.

The solution would be to:

  • ignore this condition if the relation is set to OR and there are other conditions available
  • evaluate this condition to false otherwise (and thus return no results)

Change History (6)

#1 @helen
8 years ago

  • Keywords reporter-feedback added
  • Version changed from trunk to 3.2

Hi flixos90 - welcome to Trac, and thanks for the detailed report. My expectation in this case would be that the condition is discarded, which in your example would indeed lead to all posts being returned. Does it currently work per the first part of your solution, ignoring/discarding that relation but honoring the others?

#2 @flixos90
8 years ago

  • Keywords dev-feedback added; reporter-feedback removed

Hi Helen, yes that works indeed, so it is not really a functional problem. It probably depends on the point of view, but I personally would not expect the condition to be discarded. I would think that it is processed like the other conditions, causing the query (in my case) to return no posts at all.

As far as I checked in core, the query is sent to MySQL, but it is basically being discarded by MySQL, so it does not happen directly in core. But the behavior is, like I said, just not the one I was expecting - why would any condition in a meta query be discarded? I would propose to manually work around that in the code by ignoring the condition only if it happens within an 'or' relation with at least one more condition provided (since it is not relevant in this case), but otherwise by returning an empty array immediately (since this condition does not apply to any post).

#3 @boonebgorges
8 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I agree with flixos90 that an empty array should be interpreted so that no records are matched.

The problem is that the WordPress never handles IN params like this. See post__in, post_parent__in, various include params (like get_terms()), and so forth. All of these are processed like this:

if ( ! empty( $include ) ) {
    // ...
    $sql[] = "ID NOT IN {$include_sql}";
}

Your suggestion would be something like:

if ( is_array( $include ) || ! empty( $include ) ) { // ...

But if we change the expected behavior in one place, we have to change it everywhere. And we can't do this, as it'll surely break lots of plugins.

As a workaround, I suggest that your code converts clauses like the one you suggest to something like this when it's determined that the value array is empty:

array(
    'key' => 'some-textfield',
    'compare' => 'NOT EXISTS',
)

More generally, use array( 0 ) for post__in, include, etc. Not ideal, but the best we can do given what we've got.

(I know there have been other tickets where this issue has been discussed, but I can't find them right now.)

Thanks for the ticket, flixos90 :)

#4 @flixos90
8 years ago

Yeah that's true, in terms of backwards compatibility this change is definitely not justifiable. Thanks for the feedback anyway!

#5 @patkemper
2 years ago

  • Resolution wontfix deleted
  • Severity changed from normal to critical
  • Status changed from closed to reopened

Well, it's have been several years now and this is still an issue.

I don't know, whether it's just me, but this bug is critical, as it causes a severe security issue. As you can see, WordPress isn't handling empty arrays properly, which causes the query to fetch the full dataset.

This faulty behavior might get you in big trouble, if you are not aware of this.
Example situation: Imagine saving sensitive data using a custom post type. Associate some of them to specific users. Users who have just no assiciation might see all the data.

For anyone, who needs a quick fix. This will set the value of the meta-query to [-1], if the value is empty:

<?php
function custom_query($query) {
  $the_meta_query = $query->get( 'meta_query' );
  if( is_array( $the_meta_query ) ) {
      foreach( $the_meta_query as $id => $meta_query ) {
          if ( isset( $meta_query[ 'compare' ] ) && isset( $meta_query [ 'value' ] ) ) {
              if ( $meta_query[ 'compare' ] == 'IN' ) {
                  if ( empty( $meta_query[ 'value' ] ) ) {
                      $the_meta_query[ $id ][ 'value' ] = [ -1 ];
                      $query->set( 'meta_query', $the_meta_query );
                  }
              }
          }
      }
  }   
}

add_action( 'pre_get_posts', 'custom_query' );

#6 @SergeyBiryukov
13 months ago

  • Milestone set to Awaiting Review
Note: See TracTickets for help on using tickets.