Skip to content

Commit 90f0d19

Browse files
committed
Hooks: Fix resort_active_iterations() skipping next priority on self-removal.
When a callback removes itself during `do_action()`/`apply_filters()` and is the sole callback at its priority, `resort_active_iterations()` repositions the internal array pointer one position too far. The main loop's `next()` call then skips the following priority entirely. The fix detects when the current priority was removed (the pointer advanced past it) and calls `prev()` so that `next()` in the calling loop lands on the correct priority. Fixes #64653.
1 parent e1d2179 commit 90f0d19

2 files changed

Lines changed: 70 additions & 0 deletions

File tree

src/wp-includes/class-wp-hook.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,11 @@ private function resort_active_iterations( $new_priority = false, $priority_exis
150150
}
151151
}
152152

153+
// If the current priority was removed, step back so the next() call in the main loop lands correctly.
154+
if ( false !== current( $iteration ) && current( $iteration ) !== $current ) {
155+
prev( $iteration );
156+
}
157+
153158
// If we have a new priority that didn't exist, but ::apply_filters() or ::do_action() thinks it's the current priority...
154159
if ( $new_priority === $this->current_priority[ $index ] && ! $priority_existed ) {
155160
/*

tests/phpunit/tests/hooks/doAction.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,71 @@ public function _filter_do_action_doesnt_change_value3( $value ) {
291291
return 'x3';
292292
}
293293

294+
/**
295+
* Verify that a callback removing itself during execution does not cause
296+
* the next priority to be skipped.
297+
*
298+
* When a callback is the sole entry at its priority and removes itself
299+
* mid-iteration, resort_active_iterations() repositions the internal
300+
* array pointer. Before the fix, the pointer ended up one position too
301+
* far, causing apply_filters()'s next() call to skip the following
302+
* priority entirely.
303+
*
304+
* @ticket 64653
305+
*/
306+
public function test_self_removing_callback_does_not_skip_next_priority() {
307+
$hook = new WP_Hook();
308+
$hook_name = __FUNCTION__;
309+
$log = array();
310+
311+
$hook->add_filter( $hook_name, function () use ( &$log ) {
312+
$log[] = 10;
313+
}, 10, 0 );
314+
315+
// Callback that removes itself -- the only callback at priority 50.
316+
$self_removing = function () use ( &$log, &$self_removing, $hook, $hook_name ) {
317+
$hook->remove_filter( $hook_name, $self_removing, 50 );
318+
319+
$log[] = 50;
320+
};
321+
322+
$hook->add_filter( $hook_name, $self_removing, 50, 0 );
323+
324+
$hook->add_filter( $hook_name, function () use ( &$log ) {
325+
$log[] = 100;
326+
}, 100, 0 );
327+
328+
$hook->do_action( array() );
329+
330+
$this->assertSame( array( 10, 50, 100 ), $log, 'Priority 100 should not be skipped when priority 50 removes itself during iteration.' );
331+
}
332+
333+
/**
334+
* Verify the fix when the self-removing callback is at the first priority.
335+
*
336+
* @ticket 64653
337+
*/
338+
public function test_self_removing_callback_at_lowest_priority() {
339+
$hook = new WP_Hook();
340+
$hook_name = __FUNCTION__;
341+
$log = array();
342+
343+
$self_removing = function () use ( &$log, &$self_removing, $hook, $hook_name ) {
344+
$hook->remove_filter( $hook_name, $self_removing, 10 );
345+
$log[] = 10;
346+
};
347+
348+
$hook->add_filter( $hook_name, $self_removing, 10, 0 );
349+
350+
$hook->add_filter( $hook_name, function () use ( &$log ) {
351+
$log[] = 50;
352+
}, 50, 0 );
353+
354+
$hook->do_action( array() );
355+
356+
$this->assertSame( array( 10, 50 ), $log, 'Priority 50 should execute when priority 10 removes itself.' );
357+
}
358+
294359
/**
295360
* Use this rather than MockAction so we can test callbacks with no args
296361
*

0 commit comments

Comments
 (0)