From 0302431888d457d254f152a502946e6ffe7935e4 Mon Sep 17 00:00:00 2001 From: Floatingghost Date: Fri, 31 May 2024 09:04:00 -0400 Subject: [PATCH] Use proper workers for fetching pins instead of an ad-hoc task BUG: https://git.pleroma.social/pleroma/pleroma/-/issues/3276 --- changelog.d/pinned-collection-fetch.security | 1 + lib/pleroma/web/activity_pub/activity_pub.ex | 25 ++++++++-------- .../web/activity_pub/activity_pub_test.exs | 30 +++++++++++++++---- 3 files changed, 38 insertions(+), 18 deletions(-) create mode 100644 changelog.d/pinned-collection-fetch.security diff --git a/changelog.d/pinned-collection-fetch.security b/changelog.d/pinned-collection-fetch.security new file mode 100644 index 000000000..4e8746924 --- /dev/null +++ b/changelog.d/pinned-collection-fetch.security @@ -0,0 +1 @@ +Use proper workers for fetching pins instead of an ad-hoc task, fixing a potential fetch loop diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 5bb0fba6e..1247ae7ce 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1794,24 +1794,25 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do end end - def pinned_fetch_task(nil), do: nil - - def pinned_fetch_task(%{pinned_objects: pins}) do - if Enum.all?(pins, fn {ap_id, _} -> - Object.get_cached_by_ap_id(ap_id) || - match?({:ok, _object}, Fetcher.fetch_object_from_id(ap_id)) - end) do - :ok - else - :error - end + def enqueue_pin_fetches(%{pinned_objects: pins}) do + # enqueue a task to fetch all pinned objects + Enum.each(pins, fn {ap_id, _} -> + if is_nil(Object.get_cached_by_ap_id(ap_id)) do + Pleroma.Workers.RemoteFetcherWorker.enqueue("fetch_remote", %{ + "id" => ap_id, + "depth" => 1 + }) + end + end) end + def enqueue_pin_fetches(_), do: nil + def make_user_from_ap_id(ap_id, additional \\ []) do user = User.get_cached_by_ap_id(ap_id) with {:ok, data} <- fetch_and_prepare_user_from_ap_id(ap_id, additional) do - {:ok, _pid} = Task.start(fn -> pinned_fetch_task(data) end) + enqueue_pin_fetches(data) if user do user diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index 524294385..d278125ee 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -291,9 +291,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do body: featured_data, headers: [{"content-type", "application/activity+json"}] } - end) - Tesla.Mock.mock_global(fn %{ method: :get, url: ^object_url @@ -306,7 +304,18 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do end) {:ok, user} = ActivityPub.make_user_from_ap_id(ap_id) - Process.sleep(50) + + assert_enqueued( + worker: Pleroma.Workers.RemoteFetcherWorker, + args: %{ + "op" => "fetch_remote", + "id" => object_url, + "depth" => 1 + } + ) + + # wait for oban + Pleroma.Tests.ObanHelpers.perform_all() assert user.featured_address == featured_url assert Map.has_key?(user.pinned_objects, object_url) @@ -368,9 +377,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do body: featured_data, headers: [{"content-type", "application/activity+json"}] } - end) - Tesla.Mock.mock_global(fn %{ method: :get, url: ^object_url @@ -383,7 +390,18 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do end) {:ok, user} = ActivityPub.make_user_from_ap_id(ap_id) - Process.sleep(50) + + assert_enqueued( + worker: Pleroma.Workers.RemoteFetcherWorker, + args: %{ + "op" => "fetch_remote", + "id" => object_url, + "depth" => 1 + } + ) + + # wait for oban + Pleroma.Tests.ObanHelpers.perform_all() assert user.featured_address == featured_url assert Map.has_key?(user.pinned_objects, object_url)