Fix infinite recursion in Outbox::add() causing OOM#3146
Fix infinite recursion in Outbox::add() causing OOM#3146
Conversation
Outbox::add() calls wp_insert_post() which fires wp_after_insert_post, re-entering Post::triage() and creating an infinite loop on sites where the outbox post type passes through content filters. Temporarily unhook Post::triage before inserting, mirroring the existing kses guard pattern.
There was a problem hiding this comment.
Pull request overview
Fixes an infinite recursion/OOM scenario in the ActivityPub outbox insertion path by temporarily disabling the post triage hook while creating/updating ap_outbox posts.
Changes:
- Temporarily unhooks
Activitypub\Scheduler\Post::triagearoundwp_insert_post()/wp_update_post()insideOutbox::add(). - Adds PHPUnit coverage to assert the triage hook is unhooked during the outbox insert and restored afterward.
- Adds a patch-level changelog entry describing the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
includes/collection/class-outbox.php |
Prevents wp_after_insert_post → Post::triage() re-entry during outbox item creation/update. |
tests/phpunit/tests/includes/collection/class-test-outbox.php |
Adds regression tests ensuring triage is unhooked during insert and restored after. |
.github/changelog/fix-outbox-add-infinite-recursion |
Documents the patch fix in the changelog system. |
| // Prevent infinite recursion: wp_insert_post fires wp_after_insert_post, | ||
| // which would re-enter Post::triage() -> add_to_outbox() -> Outbox::add(). | ||
| $has_triage = false !== \has_action( 'wp_after_insert_post', array( Scheduler\Post::class, 'triage' ) ); | ||
| if ( $has_triage ) { | ||
| \remove_action( 'wp_after_insert_post', array( Scheduler\Post::class, 'triage' ), 33 ); | ||
| } |
There was a problem hiding this comment.
$has_triage is derived from has_action(), but the removal is hard-coded to priority 33. If Post::triage is attached at a different priority (or multiple priorities), this will leave it hooked and the recursion/OOM can still occur; additionally, the re-add always forces priority 33 even if it was different before. Consider capturing the priority returned by has_action() and removing/restoring at that priority (and/or removing all priorities where the callback is registered).
|
|
||
| // Check whether Post::triage is hooked when wp_after_insert_post fires | ||
| // during the outbox insert. | ||
| \add_action( | ||
| 'wp_after_insert_post', | ||
| function ( $post_id ) use ( &$triage_hooked_during_insert ) { | ||
| if ( Outbox::POST_TYPE === \get_post_type( $post_id ) ) { | ||
| $triage_hooked_during_insert = \has_action( | ||
| 'wp_after_insert_post', | ||
| array( \Activitypub\Scheduler\Post::class, 'triage' ) | ||
| ); | ||
| } | ||
| }, | ||
| 0 // Run before priority 33 to inspect hook state. | ||
| ); | ||
|
|
||
| $object = new Base_Object(); | ||
| $object->set_id( 'https://example.com/recursion-test' ); | ||
| $object->set_type( 'Note' ); | ||
| $object->set_content( '<p>Recursion test</p>' ); | ||
|
|
||
| $id = \Activitypub\add_to_outbox( $object, 'Create', self::$user_id ); | ||
|
|
||
| $this->assertIsInt( $id ); | ||
| $this->assertFalse( $triage_hooked_during_insert, 'Post::triage should be unhooked during Outbox::add() to prevent recursion.' ); |
There was a problem hiding this comment.
This test adds an anonymous wp_after_insert_post action and never removes it. Because the closure captures $triage_hooked_during_insert by reference, it will persist beyond this test and can leak state / affect later tests that insert outbox posts. Store the closure in a variable and remove_action() it at the end of the test (or in tear_down()).
| // Check whether Post::triage is hooked when wp_after_insert_post fires | |
| // during the outbox insert. | |
| \add_action( | |
| 'wp_after_insert_post', | |
| function ( $post_id ) use ( &$triage_hooked_during_insert ) { | |
| if ( Outbox::POST_TYPE === \get_post_type( $post_id ) ) { | |
| $triage_hooked_during_insert = \has_action( | |
| 'wp_after_insert_post', | |
| array( \Activitypub\Scheduler\Post::class, 'triage' ) | |
| ); | |
| } | |
| }, | |
| 0 // Run before priority 33 to inspect hook state. | |
| ); | |
| $object = new Base_Object(); | |
| $object->set_id( 'https://example.com/recursion-test' ); | |
| $object->set_type( 'Note' ); | |
| $object->set_content( '<p>Recursion test</p>' ); | |
| $id = \Activitypub\add_to_outbox( $object, 'Create', self::$user_id ); | |
| $this->assertIsInt( $id ); | |
| $this->assertFalse( $triage_hooked_during_insert, 'Post::triage should be unhooked during Outbox::add() to prevent recursion.' ); | |
| $inspect_triage_hook = function ( $post_id ) use ( &$triage_hooked_during_insert ) { | |
| if ( Outbox::POST_TYPE === \get_post_type( $post_id ) ) { | |
| $triage_hooked_during_insert = \has_action( | |
| 'wp_after_insert_post', | |
| array( \Activitypub\Scheduler\Post::class, 'triage' ) | |
| ); | |
| } | |
| }; | |
| // Check whether Post::triage is hooked when wp_after_insert_post fires | |
| // during the outbox insert. | |
| \add_action( | |
| 'wp_after_insert_post', | |
| $inspect_triage_hook, | |
| 0 // Run before priority 33 to inspect hook state. | |
| ); | |
| try { | |
| $object = new Base_Object(); | |
| $object->set_id( 'https://example.com/recursion-test' ); | |
| $object->set_type( 'Note' ); | |
| $object->set_content( '<p>Recursion test</p>' ); | |
| $id = \Activitypub\add_to_outbox( $object, 'Create', self::$user_id ); | |
| $this->assertIsInt( $id ); | |
| $this->assertFalse( $triage_hooked_during_insert, 'Post::triage should be unhooked during Outbox::add() to prevent recursion.' ); | |
| } finally { | |
| \remove_action( 'wp_after_insert_post', $inspect_triage_hook, 0 ); | |
| } |
Fixes #3145
Proposed changes:
Post::triagefromwp_after_insert_postbeforeOutbox::add()callswp_insert_post()andwp_update_post(), preventing an infinite recursion loop that exhausts memory.$has_triage) mirroring the existing$has_ksespattern in the same method, so the hook is only removed/restored if it was registered in the first place.Other information:
Testing instructions:
ap_outboxposts in the database or via the REST API).activitypubsupport to theap_outboxpost type, or overridesactivitypub_is_post_disabledto returnfalsefor outbox posts — this was the scenario that triggered the infinite loop.Changelog entry
Changelog Entry Details
Significance
Type
Message
Fix an infinite loop when saving activities to the outbox on sites where the outbox post type passes through content filters.