diff --git a/changelog.d/publisher-worker-atoms.fix b/changelog.d/publisher-worker-atoms.fix new file mode 100644 index 000000000..2df6608ba --- /dev/null +++ b/changelog.d/publisher-worker-atoms.fix @@ -0,0 +1 @@ +Avoid creating atoms from serialized PublisherWorker parameters. diff --git a/lib/pleroma/workers/publisher_worker.ex b/lib/pleroma/workers/publisher_worker.ex index f799af77a..4ac0e218e 100644 --- a/lib/pleroma/workers/publisher_worker.ex +++ b/lib/pleroma/workers/publisher_worker.ex @@ -9,6 +9,13 @@ defmodule Pleroma.Workers.PublisherWorker do use Oban.Worker, queue: :federator_outgoing, max_attempts: 13 + @publish_one_param_keys %{ + "activity_id" => :activity_id, + "cc" => :cc, + "inbox" => :inbox, + "unreachable_since" => :unreachable_since + } + @impl true def perform(%Job{args: %{"op" => "publish", "activity_id" => activity_id}}) do activity = Activity.get_by_id(activity_id) @@ -16,7 +23,13 @@ defmodule Pleroma.Workers.PublisherWorker do end def perform(%Job{args: %{"op" => "publish_one", "params" => params}} = job) do - params = Map.new(params, fn {k, v} -> {String.to_atom(k), v} end) + params = + Enum.reduce(params, %{}, fn {key, value}, acc -> + case @publish_one_param_keys do + %{^key => atom_key} -> Map.put(acc, atom_key, value) + _ -> acc + end + end) # Cancel / skip the job if this server believed to be unreachable now if not Instances.reachable?(params.inbox) do diff --git a/test/pleroma/workers/publisher_worker_test.exs b/test/pleroma/workers/publisher_worker_test.exs index 4519864a5..0c71415fd 100644 --- a/test/pleroma/workers/publisher_worker_test.exs +++ b/test/pleroma/workers/publisher_worker_test.exs @@ -11,6 +11,7 @@ defmodule Pleroma.Workers.PublisherWorkerTest do alias Pleroma.Instances alias Pleroma.Object + alias Pleroma.Workers.PublisherWorker alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Builder alias Pleroma.Web.CommonAPI @@ -70,12 +71,12 @@ defmodule Pleroma.Workers.PublisherWorkerTest do max_attempts: 5 } - assert {:error, :connection_error} = Pleroma.Workers.PublisherWorker.perform(job) + assert {:error, :connection_error} = PublisherWorker.perform(job) assert Instances.reachable?("https://example.com/inbox") # Final attempt job = %{job | attempt: 5} - assert {:error, :connection_error} = Pleroma.Workers.PublisherWorker.perform(job) + assert {:error, :connection_error} = PublisherWorker.perform(job) refute Instances.reachable?("https://example.com/inbox") end end @@ -95,11 +96,36 @@ defmodule Pleroma.Workers.PublisherWorkerTest do max_attempts: 5 } - assert :ok = Pleroma.Workers.PublisherWorker.perform(job) + assert :ok = PublisherWorker.perform(job) assert Instances.reachable?("https://example.com/inbox") end end + test "does not create atoms from unexpected job params", %{activity: activity} do + unknown_key = "unknown_#{System.unique_integer([:positive])}" + + assert_raise ArgumentError, fn -> String.to_existing_atom(unknown_key) end + + with_mock Pleroma.Web.Federator, + perform: fn :publish_one, _params -> {:ok, %{status: 200}} end do + job = %Oban.Job{ + args: %{ + "op" => "publish_one", + "params" => %{ + "inbox" => "https://example.com/inbox", + "activity_id" => activity.id, + unknown_key => "ignored" + } + }, + attempt: 1, + max_attempts: 5 + } + + assert :ok = PublisherWorker.perform(job) + assert_raise ArgumentError, fn -> String.to_existing_atom(unknown_key) end + end + end + test "cancels job if server is unreachable", %{activity: activity} do # First mark the server as unreachable Instances.set_unreachable("https://example.com/inbox") @@ -117,7 +143,7 @@ defmodule Pleroma.Workers.PublisherWorkerTest do max_attempts: 5 } - assert {:cancel, :unreachable} = Pleroma.Workers.PublisherWorker.perform(job) + assert {:cancel, :unreachable} = PublisherWorker.perform(job) end end end