Skip to content

Commit 2565a9a

Browse files
committed
Hooks: Fix pointer over-advance in resort_active_iterations()
When a callback removes itself during execution and is the only entry at that priority, the internal array pointer is re-positioned by resort_active_iterations(). Before this fix, the pointer ended up at the next priority, but the subsequent next() call in the main apply_filters() loop would then advance it once more, skipping the next priority entirely. Calling prev() ensures the pointer is balanced so the next() call lands correctly on the intended priority. Fixes #64653.
1 parent 4d3b0b9 commit 2565a9a

File tree

2 files changed

+74
-0
lines changed

2 files changed

+74
-0
lines changed

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: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,75 @@ 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+
$callback_10 = function () use ( &$log ) {
312+
$log[] = 10;
313+
};
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+
$log[] = 50;
319+
};
320+
321+
$callback_100 = function () use ( &$log ) {
322+
$log[] = 100;
323+
};
324+
325+
$hook->add_filter( $hook_name, $callback_10, 10, 0 );
326+
$hook->add_filter( $hook_name, $self_removing, 50, 0 );
327+
$hook->add_filter( $hook_name, $callback_100, 100, 0 );
328+
329+
$hook->do_action( array() );
330+
331+
$this->assertSame( array( 10, 50, 100 ), $log, 'Priority 100 should not be skipped when priority 50 removes itself during iteration.' );
332+
}
333+
334+
/**
335+
* Verify the fix when the self-removing callback is at the first priority.
336+
*
337+
* @ticket 64653
338+
*/
339+
public function test_self_removing_callback_at_lowest_priority() {
340+
$hook = new WP_Hook();
341+
$hook_name = __FUNCTION__;
342+
$log = array();
343+
344+
$self_removing = function () use ( &$log, &$self_removing, $hook, $hook_name ) {
345+
$hook->remove_filter( $hook_name, $self_removing, 10 );
346+
$log[] = 10;
347+
};
348+
349+
$callback_50 = function () use ( &$log ) {
350+
$log[] = 50;
351+
};
352+
353+
$hook->add_filter( $hook_name, $self_removing, 10, 0 );
354+
$hook->add_filter( $hook_name, $callback_50, 50, 0 );
355+
356+
$hook->do_action( array() );
357+
358+
$this->assertSame( array( 10, 50 ), $log, 'Priority 50 should execute when priority 10 removes itself.' );
359+
}
360+
361+
362+
294363
/**
295364
* Use this rather than MockAction so we can test callbacks with no args
296365
*

0 commit comments

Comments
 (0)