WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#12723 closed defect (bug) (fixed)

apply_filters_ref_array bug

Reported by: jfarthing84 Owned by: westi
Milestone: 3.0 Priority: highest omg bbq
Severity: blocker Version: 3.0
Component: Plugins Keywords: has-patch tested commit
Focuses: Cc:

Description

I believe (unless I am not using it correctly) that there is a bug with the new apply_filters_ref_array() function. Particularly, I am applying two different functions certain 'posts_' filters in the query, and only the one with the highest priority is processed. Small example:

function posts_where_filter_one($where) {
  global $wpdb;
  $where .= " AND $wpdb->posts.post_type = 'post'";
}
add_filter('posts_where', 'posts_where_filter_one');

function posts_where_filter_two($where) {
  global $wpdb;
  $where .= " AND $wpdb->posts.post_status = 'publish'";
}
add_filter('posts_where', 'posts_where_filter_two');

You would expect $where to end up as:

AND wp_posts.post_type = 'post' AND wp_posts.post_status = 'publish'

But instead it end up as:

AND wp_posts.post_status = 'publish'

Attachments (1)

12723.diff (982 bytes) - added by scribu 5 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 @scribu5 years ago

  • Milestone Unassigned deleted
  • Resolution set to invalid
  • Status changed from new to closed

Erm, you forgot to do return $where in both cases.

comment:2 @scribu5 years ago

  • Milestone set to 3.0
  • Resolution invalid deleted
  • Status changed from closed to reopened

On a closer look, bug confirmed.

comment:3 @scribu5 years ago

  • Keywords dev-feedback removed
  • Owner changed from westi to scribu
  • Status changed from reopened to accepted

@scribu5 years ago

comment:4 @scribu5 years ago

  • Keywords has-patch needs-testing added

comment:5 @jfarthing845 years ago

Huh, oops. I sure did. I didn'tforget in my code though, just on typing the example here.

comment:6 @scribu5 years ago

jfarthing84, can you confirm that the patch fixes the problem?

comment:7 @jfarthing845 years ago

Yes, it does.

comment:8 @scribu5 years ago

  • Keywords tested commit added; needs-testing removed

Good enough. :)

comment:9 @scribu5 years ago

  • Owner changed from scribu to westi
  • Status changed from accepted to assigned

comment:10 @westi5 years ago

  • Milestone 3.0 deleted
  • Resolution set to invalid
  • Status changed from assigned to closed

Your filters are not written correctly.

A filter hooked into a WordPress filter must return something to be passed back out.

By default the first argument passed to apply_filters / apply_filters_ref_array is returned unless a function is hooked in.

If something is hooked in then the result of that function is returned instead.

If there are a number of filters then the result of one gets passed to the next as an argument.

Your examples should be:

function posts_where_filter_one($where) {
  global $wpdb;
  $where .= " AND $wpdb->posts.post_type = 'post'";
  return $where;
}
add_filter('posts_where', 'posts_where_filter_one');

function posts_where_filter_two($where) {
  global $wpdb;
  $where .= " AND $wpdb->posts.post_status = 'publish'";
  return $where;
}
add_filter('posts_where', 'posts_where_filter_two');

Closing as invalid

comment:11 @jfarthing845 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Wow, if you look up I said that I just forgot to type the return in my mockup examples here, not in my code. It is a bug. Try it yourself.

comment:12 @jfarthing845 years ago

  • Milestone set to 3.0

comment:13 @westi5 years ago

Ok I missed the subtlety of the bug and the need for multiple filters.

Sorry.

I've added a test case for this now to wordpress-tests so it won't reappear in the future.

comment:14 @westi5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [13906]) Ensure that apply_filters_ref_array passes the result of the filter to the next filter. Fixes #12723 props scribue.

Note: See TracTickets for help on using tickets.