diff --git a/changelog.d/get-actor-crash.fix b/changelog.d/get-actor-crash.fix new file mode 100644 index 000000000..591ac3ba2 --- /dev/null +++ b/changelog.d/get-actor-crash.fix @@ -0,0 +1 @@ +Reject fetched objects whose author has no id instead of crashing diff --git a/lib/pleroma/object/containment.ex b/lib/pleroma/object/containment.ex index fe42bfa69..531100cbb 100644 --- a/lib/pleroma/object/containment.ex +++ b/lib/pleroma/object/containment.ex @@ -44,6 +44,9 @@ defmodule Pleroma.Object.Containment do id end + # remote data can carry any shape here; callers treat nil as "no actor" + def get_actor(_), do: nil + def get_object(%{"object" => id}) when is_binary(id) do id end diff --git a/test/pleroma/object/containment_test.exs b/test/pleroma/object/containment_test.exs index 1319064f2..2fdc83605 100644 --- a/test/pleroma/object/containment_test.exs +++ b/test/pleroma/object/containment_test.exs @@ -128,6 +128,24 @@ defmodule Pleroma.Object.ContainmentTest do end end + describe "get_actor/1" do + test "returns nil for actor shapes without an id" do + for actor <- [%{"type" => "Person"}, %{"id" => 1}, 1, true] do + assert nil == Containment.get_actor(%{"actor" => actor}) + assert nil == Containment.get_actor(%{"actor" => nil, "attributedTo" => actor}) + end + + assert nil == Containment.get_actor(%{}) + end + + test "contain_origin/2 rejects actor shapes without an id" do + assert :error == + Containment.contain_origin("https://a.test/o/1", %{ + "attributedTo" => %{"type" => "Person"} + }) + end + end + describe "contain_id_to_fetch/2" do test "matches the exact id" do assert :ok = diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index c9c4890cd..61a6fb517 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -488,6 +488,24 @@ defmodule Pleroma.Object.FetcherTest do assert Object.get_by_ap_id("https://mastodon.social/1").data["content"] == "test 1" end + test "it rejects a refetched object whose author has no id", %{object1: object1} do + {:ok, o} = Object.create(object1) + + mock(fn %{method: :get, url: "https://mastodon.social/1"} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: + object1 + |> Map.delete("actor") + |> Map.put("attributedTo", %{"type" => "Person"}) + |> Jason.encode!() + } + end) + + assert {:error, _} = Fetcher.refetch_object(o) + end + test "it rejects a refetched object whose id changed", %{object1: object1} do {:ok, o} = Object.create(object1) @@ -909,6 +927,25 @@ defmodule Pleroma.Object.FetcherTest do ) end + test "it rejects objects whose author has no id" do + mock(fn %{method: :get, url: "https://host.test/objects/anon" = url} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: + Jason.encode!(%{ + "id" => url, + "type" => "Note", + "content" => "anon", + "attributedTo" => %{"type" => "Person", "name" => "nobody"} + }) + } + end) + + assert {:containment, :error} = + Fetcher.fetch_object_from_id("https://host.test/objects/anon") + end + test "it rejects documents without an id" do assert {:error, _} = Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/objects/no-id")