Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#16137 closed defect (bug) (fixed)

wp_list_filter() is inexact when dealing with nested arrays

Reported by: scribu Owned by: ryan
Milestone: 3.3 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description (last modified by scribu)

To reproduce:

add_action('init', 'test_admin_notices', 11);
function test_admin_notices() {
    global $wp_taxonomies;
    print_r( wp_filter_object_list( array_values( $wp_taxonomies ), array( 'object_type' => array( 'post' ) ), 'and', 'object_type' ) );

This is because array_intersect_assoc() does a strict comparison: (string) $elem1 === (string) $elem2

Using a manual loop fixes the problem.

Attachments (6)

16137.diff (639 bytes) - added by scribu 5 years ago.
16137.patch (668 bytes) - added by SergeyBiryukov 4 years ago.
16137.2.patch (679 bytes) - added by SergeyBiryukov 4 years ago.
16137_tests.diff (3.1 KB) - added by ampt 4 years ago.
add unit tests
wplf-test.php (2.6 KB) - added by scribu 4 years ago.
Speed test
16137_tests.2.diff (3.1 KB) - added by ampt 4 years ago.

Download all attachments as: .zip

Change History (23)

5 years ago

#1 @scribu
5 years ago

Context: #14084

#2 @scribu
5 years ago

  • Keywords needs-refresh added

#3 @mark8barnes
5 years ago

  • Cc mark@… added

I just came across this problem. The bug breaks get_taxonomies( $args, $output, $operator ) when $args = array('object_type' => array($custom_post_type)), but the patch fixes it. Can we get this in 3.2?

#4 @SergeyBiryukov
4 years ago

  • Keywords needs-refresh 3.2-early removed
  • Milestone changed from Future Release to 3.3

#5 follow-up: @scribu
4 years ago

While we're at it, we should use get_object_vars() instead of casting to array, in case the object has non-public properties, which we don't care about.

Last edited 4 years ago by scribu (previous) (diff)

#6 @scribu
4 years ago

  • Description modified (diff)

#7 in reply to: ↑ 5 @SergeyBiryukov
4 years ago

Replying to scribu:

While we're at it, we should use get_object_vars() instead of casting to array


#8 @scribu
4 years ago

  • Keywords commit added

#10 @scribu
4 years ago

  • Keywords needs-unit-tests added; commit removed

#11 @nacin
4 years ago

Performance benchmark?

4 years ago

add unit tests

#12 @ampt
4 years ago

add unit tests, i'm not exactly certain of the correct behaviour and cases for test_filter_object_list_nested_array_or and also test_filter_object_list_or_nested_array_or_field, any guidance would be appreciated. any other cases required?

#13 @scribu
4 years ago

It seems using a foreach is actually faster than array_intersect_assoc().

Using wplf-test.php I get:

wp_list_filter_old: 1.6865890026093
wp_list_filter_new: 1.3801169395447
wp_list_filter_new2: 1.871533870697

wp_list_filter_new is the one in 16137.patch.

wp_list_filter_new2 uses an if + get_object_vars(). Although I made this suggestion, I think it's better to just use casting to arrays.

4 years ago

Speed test

4 years ago

#14 @ampt
4 years ago

Fix typo in function names

#15 @ryan
4 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [18606]:

Properly handle nested arrays in wp_list_filter(). Props scribu, SergeyBiryukov. fixes #16137

#17 @SergeyBiryukov
4 years ago

  • Keywords needs-unit-tests removed
Note: See TracTickets for help on using tickets.