From 818f517f56c995a00d5597c1d242ce9baca8303c Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Fri, 24 Oct 2025 20:04:36 -0500 Subject: [PATCH 1/3] Fix infinite recursion when storing remote actors with mentions When storing remote actors with @mentions in their bios, the mention processing filter would fetch the mentioned actor, which would then process mentions in their bio, creating an infinite loop that fills error logs and crashes with stack overflow. The fix temporarily removes mention/hashtag/link filters around the to_json() call when storing remote actors, then re-adds them afterward. Added comprehensive tests that reproduce the recursion scenario: - Self-mention: actor mentioning themselves in their bio - Cross-mention: two actors mentioning each other The code includes detailed comments documenting this approach's shortcomings and exploring better long-term solutions that distinguish between incoming (storage) and outgoing (federation) contexts. --- includes/collection/class-remote-actors.php | 35 ++- .../collection/class-test-remote-actors.php | 201 ++++++++++++++++++ 2 files changed, 235 insertions(+), 1 deletion(-) diff --git a/includes/collection/class-remote-actors.php b/includes/collection/class-remote-actors.php index ad169dfee..e334c89e9 100644 --- a/includes/collection/class-remote-actors.php +++ b/includes/collection/class-remote-actors.php @@ -482,12 +482,45 @@ private static function prepare_custom_post_type( $actor ) { ); } + /* + * Temporarily remove mention/hashtag/link filters to prevent infinite recursion when + * storing remote actors with mentions/hashtags in their bios. + * + * PROBLEM: These filters are globally registered on 'init' for all to_json() calls, + * but they're designed for OUTGOING content (federation). When processing mentions in + * an actor's bio during storage, the Mention filter fetches the mentioned actor, which + * then processes mentions in THEIR bio, creating infinite recursion. + * + * SHORTCOMINGS: + * - Fragile: Easy to forget when adding new storage locations (e.g., Inbox storage). + * - Scattered: Same pattern would need to be repeated anywhere we store remote content. + * - Race conditions: If filters are re-added/removed elsewhere, this could break. + * - Not semantic: We're working around a design issue rather than fixing it. + * + * BETTER LONG-TERM SOLUTION: + * Distinguish between "incoming" (storage) and "outgoing" (federation) contexts: + * - INCOMING: Store received ActivityPub data as-is, don't process mentions/hashtags. + * (Remote_Actors::prepare_custom_post_type, Inbox storage) + * - OUTGOING: Process mentions/hashtags when serving our content to other servers. + * (Dispatcher, REST API controllers, Transformers) + */ + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Mention', 'filter_activity_object' ), 99 ); + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Hashtag', 'filter_activity_object' ), 99 ); + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Link', 'filter_activity_object' ), 99 ); + + $actor_json = $actor->to_json(); + + // Re-add the filters. + \add_filter( 'activitypub_activity_object_array', array( 'Activitypub\Mention', 'filter_activity_object' ), 99 ); + \add_filter( 'activitypub_activity_object_array', array( 'Activitypub\Hashtag', 'filter_activity_object' ), 99 ); + \add_filter( 'activitypub_activity_object_array', array( 'Activitypub\Link', 'filter_activity_object' ), 99 ); + return array( 'guid' => \esc_url_raw( $actor->get_id() ), 'post_title' => \wp_strip_all_tags( \wp_slash( $actor->get_name() ?? $actor->get_preferred_username() ) ), 'post_author' => 0, 'post_type' => self::POST_TYPE, - 'post_content' => \wp_slash( $actor->to_json() ), + 'post_content' => \wp_slash( $actor_json ), 'post_excerpt' => \wp_kses( \wp_slash( (string) $actor->get_summary() ), 'user_description' ), 'post_status' => 'publish', 'meta_input' => array( diff --git a/tests/phpunit/tests/includes/collection/class-test-remote-actors.php b/tests/phpunit/tests/includes/collection/class-test-remote-actors.php index 0ac84c23b..e77924f64 100644 --- a/tests/phpunit/tests/includes/collection/class-test-remote-actors.php +++ b/tests/phpunit/tests/includes/collection/class-test-remote-actors.php @@ -873,6 +873,207 @@ function ( $preempt, $parsed_args, $url ) { \wp_delete_post( $post_id4, true ); } + /** + * Test that saving a remote actor with a self-mention doesn't cause infinite recursion. + * + * @covers ::create + * @covers ::prepare_custom_post_type + */ + public function test_create_actor_with_self_mention_no_recursion() { + // Ensure the Mention filter is active to test for recursion. + \Activitypub\Mention::init(); + + // Create an actor with a self-mention in their summary. + $actor = array( + 'id' => 'https://remote.example.com/actor/self-mention', + 'type' => 'Person', + 'url' => 'https://remote.example.com/actor/self-mention', + 'inbox' => 'https://remote.example.com/actor/self-mention/inbox', + 'name' => 'Self Mention User', + 'preferredUsername' => 'selfmention', + 'summary' => 'Hello, I am @selfmention@remote.example.com and I like to mention myself!', + 'endpoints' => array( + 'sharedInbox' => 'https://remote.example.com/inbox', + ), + ); + + // Mock webfinger to resolve the mention. + \add_filter( + 'pre_http_request', + function ( $preempt, $parsed_args, $url ) use ( $actor ) { + if ( strpos( $url, '.well-known/webfinger' ) !== false ) { + return array( + 'response' => array( 'code' => 200 ), + 'body' => wp_json_encode( + array( + 'subject' => 'acct:selfmention@remote.example.com', + 'links' => array( + array( + 'rel' => 'self', + 'type' => 'application/activity+json', + 'href' => 'https://remote.example.com/actor/self-mention', + ), + ), + ) + ), + ); + } + return $preempt; + }, + 10, + 3 + ); + + // Mock remote actor fetch to return the same actor (creating potential recursion). + \add_filter( + 'activitypub_pre_http_get_remote_object', + function ( $pre, $url_or_object ) use ( $actor ) { + if ( $url_or_object === $actor['id'] ) { + return $actor; + } + return $pre; + }, + 10, + 2 + ); + + // This should not cause infinite recursion. + $post_id = Remote_Actors::create( $actor ); + + $this->assertIsInt( $post_id ); + $this->assertGreaterThan( 0, $post_id ); + + $post = \get_post( $post_id ); + $this->assertInstanceOf( '\WP_Post', $post ); + $this->assertEquals( 'https://remote.example.com/actor/self-mention', $post->guid ); + + // Verify the summary was stored correctly (without being processed for mentions). + $this->assertStringContainsString( '@selfmention@remote.example.com', $post->post_excerpt ); + + // Clean up. + \remove_all_filters( 'pre_http_request' ); + \remove_all_filters( 'activitypub_pre_http_get_remote_object' ); + \remove_all_filters( 'activitypub_activity_object_array' ); + \wp_delete_post( $post_id, true ); + } + + /** + * Test that saving a remote actor with mentions of other actors doesn't cause recursion. + * + * @covers ::create + * @covers ::prepare_custom_post_type + */ + public function test_create_actor_with_cross_mentions_no_recursion() { + // Ensure the Mention filter is active to test for recursion. + \Activitypub\Mention::init(); + + // Create two actors that mention each other in their bios. + $actor_a = array( + 'id' => 'https://remote.example.com/actor/alice-cross', + 'type' => 'Person', + 'url' => 'https://remote.example.com/actor/alice-cross', + 'inbox' => 'https://remote.example.com/actor/alice-cross/inbox', + 'name' => 'Alice', + 'preferredUsername' => 'alice', + 'summary' => 'Best friends with @bob@remote.example.com', + 'endpoints' => array( + 'sharedInbox' => 'https://remote.example.com/inbox', + ), + ); + + $actor_b = array( + 'id' => 'https://remote.example.com/actor/bob-cross', + 'type' => 'Person', + 'url' => 'https://remote.example.com/actor/bob-cross', + 'inbox' => 'https://remote.example.com/actor/bob-cross/inbox', + 'name' => 'Bob', + 'preferredUsername' => 'bob', + 'summary' => 'Best friends with @alice@remote.example.com', + 'endpoints' => array( + 'sharedInbox' => 'https://remote.example.com/inbox', + ), + ); + + // Mock webfinger to resolve the mentions. + \add_filter( + 'pre_http_request', + function ( $preempt, $parsed_args, $url ) { + if ( strpos( $url, '.well-known/webfinger' ) !== false ) { + if ( strpos( $url, 'bob@remote.example.com' ) !== false ) { + return array( + 'response' => array( 'code' => 200 ), + 'body' => wp_json_encode( + array( + 'subject' => 'acct:bob@remote.example.com', + 'links' => array( + array( + 'rel' => 'self', + 'type' => 'application/activity+json', + 'href' => 'https://remote.example.com/actor/bob-cross', + ), + ), + ) + ), + ); + } elseif ( strpos( $url, 'alice@remote.example.com' ) !== false ) { + return array( + 'response' => array( 'code' => 200 ), + 'body' => wp_json_encode( + array( + 'subject' => 'acct:alice@remote.example.com', + 'links' => array( + array( + 'rel' => 'self', + 'type' => 'application/activity+json', + 'href' => 'https://remote.example.com/actor/alice-cross', + ), + ), + ) + ), + ); + } + } + return $preempt; + }, + 10, + 3 + ); + + // Mock the remote fetch to return the cross-mentioned actors. + \add_filter( + 'activitypub_pre_http_get_remote_object', + function ( $pre, $url_or_object ) use ( $actor_a, $actor_b ) { + if ( $url_or_object === $actor_a['id'] ) { + return $actor_a; + } + if ( $url_or_object === $actor_b['id'] ) { + return $actor_b; + } + return $pre; + }, + 10, + 2 + ); + + // This should not cause infinite recursion when creating both actors. + $post_id_a = Remote_Actors::create( $actor_a ); + $this->assertIsInt( $post_id_a ); + + $post_id_b = Remote_Actors::create( $actor_b ); + $this->assertIsInt( $post_id_b ); + + // Verify both were created successfully. + $this->assertGreaterThan( 0, $post_id_a ); + $this->assertGreaterThan( 0, $post_id_b ); + + // Clean up. + \remove_all_filters( 'pre_http_request' ); + \remove_all_filters( 'activitypub_pre_http_get_remote_object' ); + \remove_all_filters( 'activitypub_activity_object_array' ); + \wp_delete_post( $post_id_a, true ); + \wp_delete_post( $post_id_b, true ); + } + /** * Pre get remote metadata by actor. * From cb4f4d798b0f09ca89431603c0c970782d8bc14c Mon Sep 17 00:00:00 2001 From: Automattic Bot Date: Sat, 25 Oct 2025 03:05:49 +0200 Subject: [PATCH 2/3] Add changelog --- .github/changelog/2369-from-description | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .github/changelog/2369-from-description diff --git a/.github/changelog/2369-from-description b/.github/changelog/2369-from-description new file mode 100644 index 000000000..72e3cacce --- /dev/null +++ b/.github/changelog/2369-from-description @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Fix infinite recursion when storing remote actors with mentions in their bios From 785fe1ae8dfc10d142fbae74643dbcc35ade6e40 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Fri, 24 Oct 2025 20:16:12 -0500 Subject: [PATCH 3/3] Use explicit filter removal in recursion tests Instead of using remove_all_filters() which could interfere with other tests, extract filter callbacks into variables and remove them individually using remove_filter(). This ensures only the filters added by these specific tests are removed during cleanup, preventing interference with filters from other test sources. --- .../collection/class-test-remote-actors.php | 191 +++++++++--------- 1 file changed, 92 insertions(+), 99 deletions(-) diff --git a/tests/phpunit/tests/includes/collection/class-test-remote-actors.php b/tests/phpunit/tests/includes/collection/class-test-remote-actors.php index e77924f64..1328fc77d 100644 --- a/tests/phpunit/tests/includes/collection/class-test-remote-actors.php +++ b/tests/phpunit/tests/includes/collection/class-test-remote-actors.php @@ -8,6 +8,7 @@ namespace Activitypub\Tests\Collection; use Activitypub\Collection\Remote_Actors; +use Activitypub\Mention; /** * Class Test_Remote_Actors @@ -881,7 +882,7 @@ function ( $preempt, $parsed_args, $url ) { */ public function test_create_actor_with_self_mention_no_recursion() { // Ensure the Mention filter is active to test for recursion. - \Activitypub\Mention::init(); + Mention::init(); // Create an actor with a self-mention in their summary. $actor = array( @@ -898,44 +899,38 @@ public function test_create_actor_with_self_mention_no_recursion() { ); // Mock webfinger to resolve the mention. - \add_filter( - 'pre_http_request', - function ( $preempt, $parsed_args, $url ) use ( $actor ) { - if ( strpos( $url, '.well-known/webfinger' ) !== false ) { - return array( - 'response' => array( 'code' => 200 ), - 'body' => wp_json_encode( - array( - 'subject' => 'acct:selfmention@remote.example.com', - 'links' => array( - array( - 'rel' => 'self', - 'type' => 'application/activity+json', - 'href' => 'https://remote.example.com/actor/self-mention', - ), + $webfinger_callback = function ( $preempt, $parsed_args, $url ) { + if ( strpos( $url, '.well-known/webfinger' ) !== false ) { + return array( + 'response' => array( 'code' => 200 ), + 'body' => wp_json_encode( + array( + 'subject' => 'acct:selfmention@remote.example.com', + 'links' => array( + array( + 'rel' => 'self', + 'type' => 'application/activity+json', + 'href' => 'https://remote.example.com/actor/self-mention', ), - ) - ), - ); - } - return $preempt; - }, - 10, - 3 - ); + ), + ) + ), + ); + } + + return $preempt; + }; + \add_filter( 'pre_http_request', $webfinger_callback, 10, 3 ); // Mock remote actor fetch to return the same actor (creating potential recursion). - \add_filter( - 'activitypub_pre_http_get_remote_object', - function ( $pre, $url_or_object ) use ( $actor ) { - if ( $url_or_object === $actor['id'] ) { - return $actor; - } - return $pre; - }, - 10, - 2 - ); + $actor_fetch_callback = function ( $pre, $url_or_object ) use ( $actor ) { + if ( $url_or_object === $actor['id'] ) { + return $actor; + } + + return $pre; + }; + \add_filter( 'activitypub_pre_http_get_remote_object', $actor_fetch_callback, 10, 2 ); // This should not cause infinite recursion. $post_id = Remote_Actors::create( $actor ); @@ -950,10 +945,12 @@ function ( $pre, $url_or_object ) use ( $actor ) { // Verify the summary was stored correctly (without being processed for mentions). $this->assertStringContainsString( '@selfmention@remote.example.com', $post->post_excerpt ); - // Clean up. - \remove_all_filters( 'pre_http_request' ); - \remove_all_filters( 'activitypub_pre_http_get_remote_object' ); - \remove_all_filters( 'activitypub_activity_object_array' ); + // Clean up - remove only the specific filters we added. + \remove_filter( 'pre_http_request', $webfinger_callback ); + \remove_filter( 'activitypub_pre_http_get_remote_object', $actor_fetch_callback ); + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Mention', 'filter_activity_object' ), 99 ); + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Hashtag', 'filter_activity_object' ), 99 ); + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Link', 'filter_activity_object' ), 99 ); \wp_delete_post( $post_id, true ); } @@ -965,7 +962,7 @@ function ( $pre, $url_or_object ) use ( $actor ) { */ public function test_create_actor_with_cross_mentions_no_recursion() { // Ensure the Mention filter is active to test for recursion. - \Activitypub\Mention::init(); + Mention::init(); // Create two actors that mention each other in their bios. $actor_a = array( @@ -995,65 +992,59 @@ public function test_create_actor_with_cross_mentions_no_recursion() { ); // Mock webfinger to resolve the mentions. - \add_filter( - 'pre_http_request', - function ( $preempt, $parsed_args, $url ) { - if ( strpos( $url, '.well-known/webfinger' ) !== false ) { - if ( strpos( $url, 'bob@remote.example.com' ) !== false ) { - return array( - 'response' => array( 'code' => 200 ), - 'body' => wp_json_encode( - array( - 'subject' => 'acct:bob@remote.example.com', - 'links' => array( - array( - 'rel' => 'self', - 'type' => 'application/activity+json', - 'href' => 'https://remote.example.com/actor/bob-cross', - ), + $webfinger_callback = function ( $preempt, $parsed_args, $url ) { + if ( strpos( $url, '.well-known/webfinger' ) !== false ) { + if ( strpos( $url, 'bob@remote.example.com' ) !== false ) { + return array( + 'response' => array( 'code' => 200 ), + 'body' => wp_json_encode( + array( + 'subject' => 'acct:bob@remote.example.com', + 'links' => array( + array( + 'rel' => 'self', + 'type' => 'application/activity+json', + 'href' => 'https://remote.example.com/actor/bob-cross', ), - ) - ), - ); - } elseif ( strpos( $url, 'alice@remote.example.com' ) !== false ) { - return array( - 'response' => array( 'code' => 200 ), - 'body' => wp_json_encode( - array( - 'subject' => 'acct:alice@remote.example.com', - 'links' => array( - array( - 'rel' => 'self', - 'type' => 'application/activity+json', - 'href' => 'https://remote.example.com/actor/alice-cross', - ), + ), + ) + ), + ); + } elseif ( strpos( $url, 'alice@remote.example.com' ) !== false ) { + return array( + 'response' => array( 'code' => 200 ), + 'body' => wp_json_encode( + array( + 'subject' => 'acct:alice@remote.example.com', + 'links' => array( + array( + 'rel' => 'self', + 'type' => 'application/activity+json', + 'href' => 'https://remote.example.com/actor/alice-cross', ), - ) - ), - ); - } + ), + ) + ), + ); } - return $preempt; - }, - 10, - 3 - ); + } + + return $preempt; + }; + \add_filter( 'pre_http_request', $webfinger_callback, 10, 3 ); // Mock the remote fetch to return the cross-mentioned actors. - \add_filter( - 'activitypub_pre_http_get_remote_object', - function ( $pre, $url_or_object ) use ( $actor_a, $actor_b ) { - if ( $url_or_object === $actor_a['id'] ) { - return $actor_a; - } - if ( $url_or_object === $actor_b['id'] ) { - return $actor_b; - } - return $pre; - }, - 10, - 2 - ); + $actor_fetch_callback = function ( $pre, $url_or_object ) use ( $actor_a, $actor_b ) { + if ( $url_or_object === $actor_a['id'] ) { + return $actor_a; + } + if ( $url_or_object === $actor_b['id'] ) { + return $actor_b; + } + + return $pre; + }; + \add_filter( 'activitypub_pre_http_get_remote_object', $actor_fetch_callback, 10, 2 ); // This should not cause infinite recursion when creating both actors. $post_id_a = Remote_Actors::create( $actor_a ); @@ -1066,10 +1057,12 @@ function ( $pre, $url_or_object ) use ( $actor_a, $actor_b ) { $this->assertGreaterThan( 0, $post_id_a ); $this->assertGreaterThan( 0, $post_id_b ); - // Clean up. - \remove_all_filters( 'pre_http_request' ); - \remove_all_filters( 'activitypub_pre_http_get_remote_object' ); - \remove_all_filters( 'activitypub_activity_object_array' ); + // Clean up - remove only the specific filters we added. + \remove_filter( 'pre_http_request', $webfinger_callback ); + \remove_filter( 'activitypub_pre_http_get_remote_object', $actor_fetch_callback ); + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Mention', 'filter_activity_object' ), 99 ); + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Hashtag', 'filter_activity_object' ), 99 ); + \remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Link', 'filter_activity_object' ), 99 ); \wp_delete_post( $post_id_a, true ); \wp_delete_post( $post_id_b, true ); }