diff --git a/lib/pleroma/collections/fetcher.ex b/lib/pleroma/collections/fetcher.ex index 0c81f0b56..ab69f4b84 100644 --- a/lib/pleroma/collections/fetcher.ex +++ b/lib/pleroma/collections/fetcher.ex @@ -11,10 +11,7 @@ defmodule Akkoma.Collections.Fetcher do alias Pleroma.Config require Logger - def fetch_collection_by_ap_id(ap_id) when is_binary(ap_id) do - fetch_collection(ap_id) - end - + @spec fetch_collection(String.t() | map()) :: {:ok, [Pleroma.Object.t()]} | {:error, any()} def fetch_collection(ap_id) when is_binary(ap_id) do with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id) do {:ok, objects_from_collection(page)} @@ -26,7 +23,7 @@ defmodule Akkoma.Collections.Fetcher do end def fetch_collection(%{"type" => type} = page) - when type in ["Collection", "OrderedCollection"] do + when type in ["Collection", "OrderedCollection", "CollectionPage", "OrderedCollectionPage"] do {:ok, objects_from_collection(page)} end @@ -38,12 +35,13 @@ defmodule Akkoma.Collections.Fetcher do when is_list(items) and type in ["Collection", "CollectionPage"], do: items - defp objects_from_collection(%{"type" => "OrderedCollection", "orderedItems" => items}) - when is_list(items), - do: items + defp objects_from_collection(%{"type" => type, "orderedItems" => items} = page) + when is_list(items) and type in ["OrderedCollection", "OrderedCollectionPage"], + do: maybe_next_page(page, items) - defp objects_from_collection(%{"type" => "Collection", "items" => items}) when is_list(items), - do: items + defp objects_from_collection(%{"type" => type, "items" => items} = page) + when is_list(items) and type in ["Collection", "CollectionPage"], + do: maybe_next_page(page, items) defp objects_from_collection(%{"type" => type, "first" => first}) when is_binary(first) and type in ["Collection", "OrderedCollection"] do @@ -55,17 +53,27 @@ defmodule Akkoma.Collections.Fetcher do fetch_page_items(id) end + defp objects_from_collection(_page), do: [] + defp fetch_page_items(id, items \\ []) do if Enum.count(items) >= Config.get([:activitypub, :max_collection_objects]) do items else - {:ok, page} = Fetcher.fetch_and_contain_remote_object_from_id(id) - objects = items_in_page(page) + with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(id) do + objects = items_in_page(page) - if Enum.count(objects) > 0 do - maybe_next_page(page, items ++ objects) + if Enum.count(objects) > 0 do + maybe_next_page(page, items ++ objects) + else + items + end else - items + {:error, "Object has been deleted"} -> + items + + {:error, error} -> + Logger.error("Could not fetch page #{id} - #{inspect(error)}") + {:error, error} end end end diff --git a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex index 5cadcd0b0..91fe8f928 100644 --- a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex @@ -6,7 +6,6 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do use Ecto.Schema alias Pleroma.User alias Pleroma.EctoType.ActivityPub.ObjectValidators - alias Pleroma.Object.Fetcher alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes alias Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations @@ -58,22 +57,10 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do defp fix_tag(%{"tag" => tag} = data) when is_map(tag), do: Map.put(data, "tag", [tag]) defp fix_tag(data), do: Map.drop(data, ["tag"]) - defp fix_replies(%{"replies" => %{"first" => %{"items" => replies}}} = data) - when is_list(replies), - do: Map.put(data, "replies", replies) - - defp fix_replies(%{"replies" => %{"items" => replies}} = data) when is_list(replies), - do: Map.put(data, "replies", replies) - - # TODO: Pleroma does not have any support for Collections at the moment. - # If the `replies` field is not something the ObjectID validator can handle, - # the activity/object would be rejected, which is bad behavior. - defp fix_replies(%{"replies" => replies} = data) when not is_list(replies), - do: Map.drop(data, ["replies"]) + defp fix_replies(%{"replies" => replies} = data) when is_list(replies), do: data defp fix_replies(%{"replies" => %{"first" => first}} = data) do - with {:ok, %{"orderedItems" => replies}} <- - Fetcher.fetch_and_contain_remote_object_from_id(first) do + with {:ok, replies} <- Akkoma.Collections.Fetcher.fetch_collection(first) do Map.put(data, "replies", replies) else {:error, _} -> @@ -82,7 +69,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do end end - defp fix_replies(data), do: data + defp fix_replies(data), do: Map.delete(data, "replies") defp remote_mention_resolver( %{"id" => ap_id, "tag" => tags}, diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex index a62178deb..afecd4832 100644 --- a/lib/pleroma/web/plugs/http_signature_plug.ex +++ b/lib/pleroma/web/plugs/http_signature_plug.ex @@ -64,7 +64,9 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do if has_signature_header?(conn) do # set (request-target) header to the appropriate value # we also replace the digest header with the one we computed - possible_paths = route_aliases(conn) ++ [conn.request_path, conn.request_path <> "?#{conn.query_string}"] + possible_paths = + route_aliases(conn) ++ [conn.request_path, conn.request_path <> "?#{conn.query_string}"] + assign_valid_signature_on_route_aliases(conn, possible_paths) else Logger.debug("No signature header!") diff --git a/test/pleroma/collections/collections_fetcher_test.exs b/test/pleroma/collections/collections_fetcher_test.exs index b9f84f5c4..7a582a3d7 100644 --- a/test/pleroma/collections/collections_fetcher_test.exs +++ b/test/pleroma/collections/collections_fetcher_test.exs @@ -30,7 +30,7 @@ defmodule Akkoma.Collections.FetcherTest do } end) - {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id) + {:ok, objects} = Fetcher.fetch_collection(ap_id) assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects end @@ -53,7 +53,7 @@ defmodule Akkoma.Collections.FetcherTest do } end) - {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id) + {:ok, objects} = Fetcher.fetch_collection(ap_id) assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects end @@ -106,7 +106,7 @@ defmodule Akkoma.Collections.FetcherTest do } end) - {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id) + {:ok, objects} = Fetcher.fetch_collection(ap_id) assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects end @@ -161,7 +161,58 @@ defmodule Akkoma.Collections.FetcherTest do } end) - {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id) + {:ok, objects} = Fetcher.fetch_collection(ap_id) + assert [%{"type" => "Create"}] = objects + end + + test "it should stop fetching when we hit a 404" do + clear_config([:activitypub, :max_collection_objects], 1) + + unordered_collection = + "test/fixtures/collections/unordered_page_reference.json" + |> File.read!() + + first_page = + "test/fixtures/collections/unordered_page_first.json" + |> File.read!() + + ap_id = "https://example.com/collection/unordered_page_reference" + first_page_id = "https://example.com/collection/unordered_page_reference?page=1" + second_page_id = "https://example.com/collection/unordered_page_reference?page=2" + + Tesla.Mock.mock(fn + %{ + method: :get, + url: ^ap_id + } -> + %Tesla.Env{ + status: 200, + body: unordered_collection, + headers: [{"content-type", "application/activity+json"}] + } + + %{ + method: :get, + url: ^first_page_id + } -> + %Tesla.Env{ + status: 200, + body: first_page, + headers: [{"content-type", "application/activity+json"}] + } + + %{ + method: :get, + url: ^second_page_id + } -> + %Tesla.Env{ + status: 404, + body: nil, + headers: [{"content-type", "application/activity+json"}] + } + end) + + {:ok, objects} = Fetcher.fetch_collection(ap_id) assert [%{"type" => "Create"}] = objects end end diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index da5c87bd8..e209bb46b 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -782,6 +782,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do |> String.replace("{{status_id}}", status_id) status_url = "https://example.com/users/lain/statuses/#{status_id}" + replies_url = status_url <> "/replies?only_other_accounts=true&page=true" user = File.read!("test/fixtures/users_mock/user.json") @@ -820,6 +821,16 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do |> String.replace("{{nickname}}", "lain"), headers: [{"content-type", "application/activity+json"}] } + + %{ + method: :get, + url: ^replies_url + } -> + %Tesla.Env{ + status: 404, + body: "", + headers: [{"content-type", "application/activity+json"}] + } end) data = %{ diff --git a/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs index 24df5ea61..002042802 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs @@ -380,7 +380,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.NoteHandlingTest do clear_config([:instance, :federation_incoming_replies_max_depth], 10) {:ok, activity} = Transmogrifier.handle_incoming(data) - object = Object.normalize(activity.data["object"]) assert object.data["replies"] == items diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index 476e0ce04..ab44c489b 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -407,6 +407,15 @@ defmodule HttpRequestMock do }} end + def get( + "http://mastodon.example.org/users/admin/statuses/99512778738411822/replies?min_id=99512778738411824&page=true", + _, + _, + _ + ) do + {:ok, %Tesla.Env{status: 404, body: ""}} + end + def get("http://mastodon.example.org/users/relay", _, _, [ {"accept", "application/activity+json"} ]) do