Skip to content

Commit 05558d3

Browse files
obenlandobenland
authored
Fix infinite recursion when storing remote actors with mentions (#2369)
1 parent b1f1b29 commit 05558d3

File tree

3 files changed

+232
-1
lines changed

3 files changed

+232
-1
lines changed

‎.github/changelog/2369-from-description‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: patch
2+
Type: fixed
3+
4+
Fix infinite recursion when storing remote actors with mentions in their bios

‎includes/collection/class-remote-actors.php‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,12 +482,45 @@ private static function prepare_custom_post_type( $actor ) {
482482
);
483483
}
484484

485+
/*
486+
* Temporarily remove mention/hashtag/link filters to prevent infinite recursion when
487+
* storing remote actors with mentions/hashtags in their bios.
488+
*
489+
* PROBLEM: These filters are globally registered on 'init' for all to_json() calls,
490+
* but they're designed for OUTGOING content (federation). When processing mentions in
491+
* an actor's bio during storage, the Mention filter fetches the mentioned actor, which
492+
* then processes mentions in THEIR bio, creating infinite recursion.
493+
*
494+
* SHORTCOMINGS:
495+
* - Fragile: Easy to forget when adding new storage locations (e.g., Inbox storage).
496+
* - Scattered: Same pattern would need to be repeated anywhere we store remote content.
497+
* - Race conditions: If filters are re-added/removed elsewhere, this could break.
498+
* - Not semantic: We're working around a design issue rather than fixing it.
499+
*
500+
* BETTER LONG-TERM SOLUTION:
501+
* Distinguish between "incoming" (storage) and "outgoing" (federation) contexts:
502+
* - INCOMING: Store received ActivityPub data as-is, don't process mentions/hashtags.
503+
* (Remote_Actors::prepare_custom_post_type, Inbox storage)
504+
* - OUTGOING: Process mentions/hashtags when serving our content to other servers.
505+
* (Dispatcher, REST API controllers, Transformers)
506+
*/
507+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Mention', 'filter_activity_object' ), 99 );
508+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Hashtag', 'filter_activity_object' ), 99 );
509+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Link', 'filter_activity_object' ), 99 );
510+
511+
$actor_json = $actor->to_json();
512+
513+
// Re-add the filters.
514+
\add_filter( 'activitypub_activity_object_array', array( 'Activitypub\Mention', 'filter_activity_object' ), 99 );
515+
\add_filter( 'activitypub_activity_object_array', array( 'Activitypub\Hashtag', 'filter_activity_object' ), 99 );
516+
\add_filter( 'activitypub_activity_object_array', array( 'Activitypub\Link', 'filter_activity_object' ), 99 );
517+
485518
return array(
486519
'guid' => \esc_url_raw( $actor->get_id() ),
487520
'post_title' => \wp_strip_all_tags( \wp_slash( $actor->get_name() ?? $actor->get_preferred_username() ) ),
488521
'post_author' => 0,
489522
'post_type' => self::POST_TYPE,
490-
'post_content' => \wp_slash( $actor->to_json() ),
523+
'post_content' => \wp_slash( $actor_json ),
491524
'post_excerpt' => \wp_kses( \wp_slash( (string) $actor->get_summary() ), 'user_description' ),
492525
'post_status' => 'publish',
493526
'meta_input' => array(

‎tests/phpunit/tests/includes/collection/class-test-remote-actors.php‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace Activitypub\Tests\Collection;
99

1010
use Activitypub\Collection\Remote_Actors;
11+
use Activitypub\Mention;
1112

1213
/**
1314
* Class Test_Remote_Actors
@@ -873,6 +874,199 @@ function ( $preempt, $parsed_args, $url ) {
873874
\wp_delete_post( $post_id4, true );
874875
}
875876

877+
/**
878+
* Test that saving a remote actor with a self-mention doesn't cause infinite recursion.
879+
*
880+
* @covers ::create
881+
* @covers ::prepare_custom_post_type
882+
*/
883+
public function test_create_actor_with_self_mention_no_recursion() {
884+
// Ensure the Mention filter is active to test for recursion.
885+
Mention::init();
886+
887+
// Create an actor with a self-mention in their summary.
888+
$actor = array(
889+
'id' => 'https://remote.example.com/actor/self-mention',
890+
'type' => 'Person',
891+
'url' => 'https://remote.example.com/actor/self-mention',
892+
'inbox' => 'https://remote.example.com/actor/self-mention/inbox',
893+
'name' => 'Self Mention User',
894+
'preferredUsername' => 'selfmention',
895+
'summary' => 'Hello, I am @[email protected] and I like to mention myself!',
896+
'endpoints' => array(
897+
'sharedInbox' => 'https://remote.example.com/inbox',
898+
),
899+
);
900+
901+
// Mock webfinger to resolve the mention.
902+
$webfinger_callback = function ( $preempt, $parsed_args, $url ) {
903+
if ( strpos( $url, '.well-known/webfinger' ) !== false ) {
904+
return array(
905+
'response' => array( 'code' => 200 ),
906+
'body' => wp_json_encode(
907+
array(
908+
'subject' => 'acct:[email protected]',
909+
'links' => array(
910+
array(
911+
'rel' => 'self',
912+
'type' => 'application/activity+json',
913+
'href' => 'https://remote.example.com/actor/self-mention',
914+
),
915+
),
916+
)
917+
),
918+
);
919+
}
920+
921+
return $preempt;
922+
};
923+
\add_filter( 'pre_http_request', $webfinger_callback, 10, 3 );
924+
925+
// Mock remote actor fetch to return the same actor (creating potential recursion).
926+
$actor_fetch_callback = function ( $pre, $url_or_object ) use ( $actor ) {
927+
if ( $url_or_object === $actor['id'] ) {
928+
return $actor;
929+
}
930+
931+
return $pre;
932+
};
933+
\add_filter( 'activitypub_pre_http_get_remote_object', $actor_fetch_callback, 10, 2 );
934+
935+
// This should not cause infinite recursion.
936+
$post_id = Remote_Actors::create( $actor );
937+
938+
$this->assertIsInt( $post_id );
939+
$this->assertGreaterThan( 0, $post_id );
940+
941+
$post = \get_post( $post_id );
942+
$this->assertInstanceOf( '\WP_Post', $post );
943+
$this->assertEquals( 'https://remote.example.com/actor/self-mention', $post->guid );
944+
945+
// Verify the summary was stored correctly (without being processed for mentions).
946+
$this->assertStringContainsString( '@[email protected]', $post->post_excerpt );
947+
948+
// Clean up - remove only the specific filters we added.
949+
\remove_filter( 'pre_http_request', $webfinger_callback );
950+
\remove_filter( 'activitypub_pre_http_get_remote_object', $actor_fetch_callback );
951+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Mention', 'filter_activity_object' ), 99 );
952+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Hashtag', 'filter_activity_object' ), 99 );
953+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Link', 'filter_activity_object' ), 99 );
954+
\wp_delete_post( $post_id, true );
955+
}
956+
957+
/**
958+
* Test that saving a remote actor with mentions of other actors doesn't cause recursion.
959+
*
960+
* @covers ::create
961+
* @covers ::prepare_custom_post_type
962+
*/
963+
public function test_create_actor_with_cross_mentions_no_recursion() {
964+
// Ensure the Mention filter is active to test for recursion.
965+
Mention::init();
966+
967+
// Create two actors that mention each other in their bios.
968+
$actor_a = array(
969+
'id' => 'https://remote.example.com/actor/alice-cross',
970+
'type' => 'Person',
971+
'url' => 'https://remote.example.com/actor/alice-cross',
972+
'inbox' => 'https://remote.example.com/actor/alice-cross/inbox',
973+
'name' => 'Alice',
974+
'preferredUsername' => 'alice',
975+
'summary' => 'Best friends with @[email protected]',
976+
'endpoints' => array(
977+
'sharedInbox' => 'https://remote.example.com/inbox',
978+
),
979+
);
980+
981+
$actor_b = array(
982+
'id' => 'https://remote.example.com/actor/bob-cross',
983+
'type' => 'Person',
984+
'url' => 'https://remote.example.com/actor/bob-cross',
985+
'inbox' => 'https://remote.example.com/actor/bob-cross/inbox',
986+
'name' => 'Bob',
987+
'preferredUsername' => 'bob',
988+
'summary' => 'Best friends with @[email protected]',
989+
'endpoints' => array(
990+
'sharedInbox' => 'https://remote.example.com/inbox',
991+
),
992+
);
993+
994+
// Mock webfinger to resolve the mentions.
995+
$webfinger_callback = function ( $preempt, $parsed_args, $url ) {
996+
if ( strpos( $url, '.well-known/webfinger' ) !== false ) {
997+
if ( strpos( $url, '[email protected]' ) !== false ) {
998+
return array(
999+
'response' => array( 'code' => 200 ),
1000+
'body' => wp_json_encode(
1001+
array(
1002+
'subject' => 'acct:[email protected]',
1003+
'links' => array(
1004+
array(
1005+
'rel' => 'self',
1006+
'type' => 'application/activity+json',
1007+
'href' => 'https://remote.example.com/actor/bob-cross',
1008+
),
1009+
),
1010+
)
1011+
),
1012+
);
1013+
} elseif ( strpos( $url, '[email protected]' ) !== false ) {
1014+
return array(
1015+
'response' => array( 'code' => 200 ),
1016+
'body' => wp_json_encode(
1017+
array(
1018+
'subject' => 'acct:[email protected]',
1019+
'links' => array(
1020+
array(
1021+
'rel' => 'self',
1022+
'type' => 'application/activity+json',
1023+
'href' => 'https://remote.example.com/actor/alice-cross',
1024+
),
1025+
),
1026+
)
1027+
),
1028+
);
1029+
}
1030+
}
1031+
1032+
return $preempt;
1033+
};
1034+
\add_filter( 'pre_http_request', $webfinger_callback, 10, 3 );
1035+
1036+
// Mock the remote fetch to return the cross-mentioned actors.
1037+
$actor_fetch_callback = function ( $pre, $url_or_object ) use ( $actor_a, $actor_b ) {
1038+
if ( $url_or_object === $actor_a['id'] ) {
1039+
return $actor_a;
1040+
}
1041+
if ( $url_or_object === $actor_b['id'] ) {
1042+
return $actor_b;
1043+
}
1044+
1045+
return $pre;
1046+
};
1047+
\add_filter( 'activitypub_pre_http_get_remote_object', $actor_fetch_callback, 10, 2 );
1048+
1049+
// This should not cause infinite recursion when creating both actors.
1050+
$post_id_a = Remote_Actors::create( $actor_a );
1051+
$this->assertIsInt( $post_id_a );
1052+
1053+
$post_id_b = Remote_Actors::create( $actor_b );
1054+
$this->assertIsInt( $post_id_b );
1055+
1056+
// Verify both were created successfully.
1057+
$this->assertGreaterThan( 0, $post_id_a );
1058+
$this->assertGreaterThan( 0, $post_id_b );
1059+
1060+
// Clean up - remove only the specific filters we added.
1061+
\remove_filter( 'pre_http_request', $webfinger_callback );
1062+
\remove_filter( 'activitypub_pre_http_get_remote_object', $actor_fetch_callback );
1063+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Mention', 'filter_activity_object' ), 99 );
1064+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Hashtag', 'filter_activity_object' ), 99 );
1065+
\remove_filter( 'activitypub_activity_object_array', array( 'Activitypub\Link', 'filter_activity_object' ), 99 );
1066+
\wp_delete_post( $post_id_a, true );
1067+
\wp_delete_post( $post_id_b, true );
1068+
}
1069+
8761070
/**
8771071
* Pre get remote metadata by actor.
8781072
*

0 commit comments

Comments
 (0)