[wp-trac] [WordPress Trac] #17817: do_action/apply_filters/etc. recursion on same filter kills underlying call

[WordPress Trac]

#17817: do_action/apply_filters/etc. recursion on same filter kills underlying call
-------------------------------------------------+-------------------------
 Reporter:  kernfel                              |       Owner:  pento
     Type:  defect (bug)                         |      Status:  reviewing
 Priority:  normal                               |   Milestone:  4.7
Component:  Plugins                              |     Version:  2.2
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch needs-testing needs-unit-  |     Focuses:
  tests                                          |
-------------------------------------------------+-------------------------
Changes (by pento):

 * keywords:  has-unit-tests has-patch needs-testing => has-patch needs-
     testing needs-unit-tests


Comment:

 [attachment:17817.12.diff] fixes some unit tests, and updates the `@since`
 docs.

 There was a bug I've been running into, triggered by this code. It's kind
 of ugly, but not insane.

 {{{#!php
 <?php
 /*
 Plugin Name: lol
  */
 add_filter( 'the_content', 'foo', 999999 );

 function foo( $content ) {
         return $content . bar();
 }

 function bar() {
         remove_filter( 'the_content', 'foo', 999999 );
         $bar = apply_filters( 'the_content', 'bar' );
         add_filter( 'the_content', 'foo', 999999 );
         return $bar;
 }
 }}}

 When `remove_filter()` is called, it eventually gets to
 `WP_Hook::resort_active_iterations()`. While the logic in this method was
 sound, the `foreach()` was resetting the array pointer for each item in
 `$this->iterations`.

 In the above example, the result was that, when `foo()` returned to
 `WP_Hook::apply_filters()`, the internal pointer of `$this->iterations[
 $nesting_level ]` had been reset to the first element, instead of staying
 where it was supposed to.

 [attachment:17817.13.diff] fixes this bug by operating on a reference to
 each element in the `WP_Hook::resort_active_iterations()` loop.

 It also bails on the loop early when the `current()` element is `false`,
 there's no need to loop through an array that we're already at the end of.

 [attachment:17817.13.diff] needs unit tests to confirm that this behaviour
 is fixed.

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