From 303bcfe7784246d59a27fa806ccebd293f95b19f Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 5 May 2026 11:20:46 -0500 Subject: [PATCH 1/2] Plugins: Don't skip the next priority's callbacks when a callback empties its bucket mid-iteration. When a callback running inside apply_filters() is the only one at its priority and removes itself (or another callback empties that bucket), WP_Hook::resort_active_iterations() walks the iterator forward to the first remaining priority greater than $current. The trailing next() in apply_filters() then overshoots that priority and silently skips its callbacks. The bug only manifests when at least one earlier-priority callback has already run; otherwise the existing array_unshift fast-path handles it. This adds the symmetric counterpart to the existing add-side handling: if $current's bucket was emptied during iteration, step the iterator back one so that apply_filters()'s trailing next() lands on the right priority instead of skipping past it. Fixes #65167. --- src/wp-includes/class-wp-hook.php | 10 ++++++ tests/phpunit/tests/hooks/removeFilter.php | 38 ++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/wp-includes/class-wp-hook.php b/src/wp-includes/class-wp-hook.php index cd6860c0f81f2..0d62917cba61d 100644 --- a/src/wp-includes/class-wp-hook.php +++ b/src/wp-includes/class-wp-hook.php @@ -150,6 +150,16 @@ private function resort_active_iterations( $new_priority = false, $priority_exis } } + /* + * If $current's bucket was emptied during iteration, the iterator now + * sits on the first remaining priority greater than $current. The + * trailing next() in ::apply_filters() would skip past it, so step + * back one so that next() lands on it instead. + */ + if ( false !== current( $iteration ) && ! isset( $this->callbacks[ $current ] ) ) { + prev( $iteration ); + } + // If we have a new priority that didn't exist, but ::apply_filters() or ::do_action() thinks it's the current priority... if ( $new_priority === $this->current_priority[ $index ] && ! $priority_existed ) { /* diff --git a/tests/phpunit/tests/hooks/removeFilter.php b/tests/phpunit/tests/hooks/removeFilter.php index d3065bb9bd425..2c2eb313da80d 100644 --- a/tests/phpunit/tests/hooks/removeFilter.php +++ b/tests/phpunit/tests/hooks/removeFilter.php @@ -86,6 +86,44 @@ public function test_remove_filter_with_another_at_different_priority() { $this->check_priority_exists( $hook, $priority + 1, 'Should priority of 3' ); } + /** + * Removing the last callback at the currently iterating priority must not + * cause the next remaining priority to be silently skipped. + * + * @ticket 65167 + * + * @covers WP_Hook::remove_filter + * @covers WP_Hook::apply_filters + */ + public function test_remove_filter_during_iteration_does_not_skip_next_priority() { + $hook = new WP_Hook(); + $fired = array(); + + $early = static function ( $value ) use ( &$fired ) { + $fired[] = 'early'; + return $value; + }; + + $self_removing = static function ( $value ) use ( &$hook, &$self_removing, &$fired ) { + $fired[] = 'self_removing'; + $hook->remove_filter( __FUNCTION__, $self_removing, 10 ); + return $value; + }; + + $later = static function ( $value ) use ( &$fired ) { + $fired[] = 'later'; + return $value; + }; + + $hook->add_filter( __FUNCTION__, $early, 5, 1 ); + $hook->add_filter( __FUNCTION__, $self_removing, 10, 1 ); + $hook->add_filter( __FUNCTION__, $later, 20, 1 ); + + $hook->apply_filters( null, array( null ) ); + + $this->assertSame( array( 'early', 'self_removing', 'later' ), $fired ); + } + protected function check_priority_non_existent( $hook, $priority ) { $priorities = $this->get_priorities( $hook ); From ef837acc900ca93659ca702dd00e4950170159a9 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 5 May 2026 11:34:12 -0500 Subject: [PATCH 2/2] Tests: Use a hook name variable instead of __FUNCTION__ inside the self-removing closure. Inside a closure, __FUNCTION__ resolves to the closure's synthetic name (e.g. {closure:Tests_Hooks_RemoveFilter::...}), not the test method name used at the call site. As a result the closure's remove_filter() targeted a hook that was never registered, the priority bucket never emptied, and the regression test passed for the wrong reason. Capture the hook name in a local variable in the test method and use it consistently for add_filter() and remove_filter(). Verified that the test now fails on trunk without the WP_Hook fix and passes with it. --- tests/phpunit/tests/hooks/removeFilter.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/phpunit/tests/hooks/removeFilter.php b/tests/phpunit/tests/hooks/removeFilter.php index 2c2eb313da80d..ef5ee717caa49 100644 --- a/tests/phpunit/tests/hooks/removeFilter.php +++ b/tests/phpunit/tests/hooks/removeFilter.php @@ -96,17 +96,18 @@ public function test_remove_filter_with_another_at_different_priority() { * @covers WP_Hook::apply_filters */ public function test_remove_filter_during_iteration_does_not_skip_next_priority() { - $hook = new WP_Hook(); - $fired = array(); + $hook = new WP_Hook(); + $hook_name = __FUNCTION__; + $fired = array(); $early = static function ( $value ) use ( &$fired ) { $fired[] = 'early'; return $value; }; - $self_removing = static function ( $value ) use ( &$hook, &$self_removing, &$fired ) { + $self_removing = static function ( $value ) use ( &$hook, $hook_name, &$self_removing, &$fired ) { $fired[] = 'self_removing'; - $hook->remove_filter( __FUNCTION__, $self_removing, 10 ); + $hook->remove_filter( $hook_name, $self_removing, 10 ); return $value; }; @@ -115,9 +116,9 @@ public function test_remove_filter_during_iteration_does_not_skip_next_priority( return $value; }; - $hook->add_filter( __FUNCTION__, $early, 5, 1 ); - $hook->add_filter( __FUNCTION__, $self_removing, 10, 1 ); - $hook->add_filter( __FUNCTION__, $later, 20, 1 ); + $hook->add_filter( $hook_name, $early, 5, 1 ); + $hook->add_filter( $hook_name, $self_removing, 10, 1 ); + $hook->add_filter( $hook_name, $later, 20, 1 ); $hook->apply_filters( null, array( null ) );