[wp-trac] [WordPress Trac] #40685: /wp-includes/class-wp-hook.php function resort_active_iterations not working correct

WordPress Trac noreply at wordpress.org
Thu May 11 20:37:42 UTC 2017


#40685: /wp-includes/class-wp-hook.php function resort_active_iterations not
working correct
--------------------------+------------------------------
 Reporter:  tim2017       |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  General       |     Version:  4.7.4
 Severity:  normal        |  Resolution:
 Keywords:                |     Focuses:
--------------------------+------------------------------

Comment (by gitlost):

 Nice bug!

 At first I thought it might be possible to fix this by manipulating the
 internal array pointer of `iterations` to point to the previous element on
 `remove_filter()` so that the `next()` in `apply_filters()` would work
 correctly - but a major flaw with this is that if the filter being removed
 is the first in `iterations` then there's no previous element to go back
 to! So the `next()` in `apply_filters()` will always skip an element.

 So just not calling `resort_active_iterations()` on `remove_filter()`
 seems like a simple way to fix the issue. But this leaves the `iterations`
 array invalid, so its return needs to be checked before using it in
 `apply_filters()`, which affects the performance of the loop. (It might
 still be a good, more BC solution though.)

 The approach taken in the attached demo patch is to not call
 `resort_active_iterations()` either, but also to not unset the callback
 priority entries at all but leave them empty, so `iterations` is left
 pointing at a "zombie" entry, avoiding the need for a check.

 This has some behavioural issues - for instance to get the full unit tests
 working required changing some of them to not test for `isset(
 $hook->callbacks[ $priority ] )` but `! empty()` instead, and also not do
 `assertArrayNotHasKey( 'blah', $wp_filter )` but `has_filter( 'blah' )` -
 so it's very much a demo.

 The patch includes unit tests that fail appropriately.

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


More information about the wp-trac mailing list