From: floatingghost <hannah@coffee-and-dreams.uk>
Date: Sat, 27 Aug 2022 18:05:48 +0000 (+0000)
Subject: GTS: cherry-picks and collection usage (#186)
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=772c209914d5cbfd4f763edc266d0f1541f656f8;p=akkoma

GTS: cherry-picks and collection usage (#186)

https://git.pleroma.social/pleroma/pleroma/-/merge_requests/3725?commit_id=61254111e59f02118cad15de49d1e0704c07030e

what is this, a yoink of a yoink? good times

Co-authored-by: Hélène <pleroma-dev@helene.moe>
Co-authored-by: FloatingGhost <hannah@coffee-and-dreams.uk>
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/186
---

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 28053ea3a..55323bc2e 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,19 +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)
-
-  defp fix_replies(%{"replies" => replies} = data) when is_bitstring(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, _} ->
@@ -79,7 +69,10 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do
     end
   end
 
-  defp fix_replies(data), do: data
+  defp fix_replies(%{"replies" => %{"items" => replies}} = data) when is_list(replies),
+    do: Map.put(data, "replies", replies)
+
+  defp fix_replies(data), do: Map.delete(data, "replies")
 
   defp remote_mention_resolver(
          %{"id" => ap_id, "tag" => tags},
diff --git a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex b/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex
index 779c8b622..6fa2bbb99 100644
--- a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex
+++ b/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex
@@ -7,8 +7,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do
   alias Pleroma.Object
   alias Pleroma.Object.Containment
   alias Pleroma.User
-  alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.ActivityPub.Utils
+  require Pleroma.Constants
 
   def cast_and_filter_recipients(message, field, follower_collection, field_fallback \\ []) do
     {:ok, data} = ObjectValidators.Recipients.cast(message[field] || field_fallback)
@@ -32,7 +32,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do
     |> cast_and_filter_recipients("cc", follower_collection)
     |> cast_and_filter_recipients("bto", follower_collection)
     |> cast_and_filter_recipients("bcc", follower_collection)
-    |> Transmogrifier.fix_implicit_addressing(follower_collection)
+    |> fix_implicit_addressing(follower_collection)
   end
 
   def fix_activity_addressing(activity) do
@@ -43,7 +43,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do
     |> cast_and_filter_recipients("cc", follower_collection)
     |> cast_and_filter_recipients("bto", follower_collection)
     |> cast_and_filter_recipients("bcc", follower_collection)
-    |> Transmogrifier.fix_implicit_addressing(follower_collection)
+    |> fix_implicit_addressing(follower_collection)
   end
 
   def fix_actor(data) do
@@ -73,4 +73,27 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do
 
     Map.put(data, "to", to)
   end
+
+  # if as:Public is addressed, then make sure the followers collection is also addressed
+  # so that the activities will be delivered to local users.
+  def fix_implicit_addressing(%{"to" => to, "cc" => cc} = object, followers_collection) do
+    recipients = to ++ cc
+
+    if followers_collection not in recipients do
+      cond do
+        Pleroma.Constants.as_public() in cc ->
+          to = to ++ [followers_collection]
+          Map.put(object, "to", to)
+
+        Pleroma.Constants.as_public() in to ->
+          cc = cc ++ [followers_collection]
+          Map.put(object, "cc", cc)
+
+        true ->
+          object
+      end
+    else
+      object
+    end
+  end
 end
diff --git a/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex b/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex
index 803b5d5a1..d868c3915 100644
--- a/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex
@@ -13,7 +13,6 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CreateGenericValidator do
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes
   alias Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations
-  alias Pleroma.Web.ActivityPub.Transmogrifier
 
   import Ecto.Changeset
 
@@ -67,7 +66,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CreateGenericValidator do
     |> CommonFixes.cast_and_filter_recipients("cc", follower_collection, object["cc"])
     |> CommonFixes.cast_and_filter_recipients("bto", follower_collection, object["bto"])
     |> CommonFixes.cast_and_filter_recipients("bcc", follower_collection, object["bcc"])
-    |> Transmogrifier.fix_implicit_addressing(follower_collection)
+    |> CommonFixes.fix_implicit_addressing(follower_collection)
   end
 
   def fix(data, meta) do
diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex
index d2077967c..8ec4b0fec 100644
--- a/lib/pleroma/web/activity_pub/transmogrifier.ex
+++ b/lib/pleroma/web/activity_pub/transmogrifier.ex
@@ -19,6 +19,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
   alias Pleroma.Web.ActivityPub.Pipeline
   alias Pleroma.Web.ActivityPub.Utils
   alias Pleroma.Web.ActivityPub.Visibility
+  alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes
   alias Pleroma.Web.Federator
   alias Pleroma.Workers.TransmogrifierWorker
 
@@ -95,29 +96,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     |> Map.put("cc", final_cc)
   end
 
-  # if as:Public is addressed, then make sure the followers collection is also addressed
-  # so that the activities will be delivered to local users.
-  def fix_implicit_addressing(%{"to" => to, "cc" => cc} = object, followers_collection) do
-    recipients = to ++ cc
-
-    if followers_collection not in recipients do
-      cond do
-        Pleroma.Constants.as_public() in cc ->
-          to = to ++ [followers_collection]
-          Map.put(object, "to", to)
-
-        Pleroma.Constants.as_public() in to ->
-          cc = cc ++ [followers_collection]
-          Map.put(object, "cc", cc)
-
-        true ->
-          object
-      end
-    else
-      object
-    end
-  end
-
   def fix_addressing(object) do
     {:ok, %User{follower_address: follower_collection}} =
       object
@@ -130,7 +108,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     |> fix_addressing_list("bto")
     |> fix_addressing_list("bcc")
     |> fix_explicit_addressing(follower_collection)
-    |> fix_implicit_addressing(follower_collection)
+    |> CommonFixes.fix_implicit_addressing(follower_collection)
   end
 
   def fix_actor(%{"attributedTo" => actor} = object) do
diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex
index cfee392c8..c906a4eec 100644
--- a/lib/pleroma/web/plugs/http_signature_plug.ex
+++ b/lib/pleroma/web/plugs/http_signature_plug.ex
@@ -27,11 +27,11 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
     end
   end
 
-  def route_aliases(%{path_info: ["objects", id]}) do
+  def route_aliases(%{path_info: ["objects", id], query_string: query_string}) do
     ap_id = Router.Helpers.o_status_url(Pleroma.Web.Endpoint, :object, id)
 
     with %Activity{} = activity <- Activity.get_by_object_ap_id_with_object(ap_id) do
-      ["/notice/#{activity.id}"]
+      ["/notice/#{activity.id}", "/notice/#{activity.id}?#{query_string}"]
     else
       _ -> []
     end
@@ -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]
+      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/object_validators/article_note_page_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs
index 1d73d6765..7c8e5a4e1 100644
--- a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs
+++ b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs
@@ -146,4 +146,17 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidatorTest
                "<span class=\"h-card\"><a class=\"u-url mention\" data-user=\"#{local_user.id}\" href=\"#{local_user.ap_id}\" rel=\"ugc\">@<span>akkoma_user</span></a></span>"
     end
   end
+
+  test "a Note without replies/first/items validates" do
+    insert(:user, ap_id: "https://mastodon.social/users/emelie")
+
+    note =
+      "test/fixtures/tesla_mock/status.emelie.json"
+      |> File.read!()
+      |> Jason.decode!()
+      |> pop_in(["replies", "first", "items"])
+      |> elem(1)
+
+    %{valid?: true} = ArticleNotePageValidator.cast_and_validate(note)
+  end
 end
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/pleroma/web/plugs/http_signature_plug_test.exs b/test/pleroma/web/plugs/http_signature_plug_test.exs
index 02e8b3092..8ce956510 100644
--- a/test/pleroma/web/plugs/http_signature_plug_test.exs
+++ b/test/pleroma/web/plugs/http_signature_plug_test.exs
@@ -86,10 +86,12 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do
     test "aliases redirected /object endpoints", _ do
       obj = insert(:note)
       act = insert(:note_activity, note: obj)
-      params = %{"actor" => "http://mastodon.example.org/users/admin"}
+      params = %{"actor" => "someparam"}
       path = URI.parse(obj.data["id"]).path
       conn = build_conn(:get, path, params)
-      assert ["/notice/#{act.id}"] == HTTPSignaturePlug.route_aliases(conn)
+
+      assert ["/notice/#{act.id}", "/notice/#{act.id}?actor=someparam"] ==
+               HTTPSignaturePlug.route_aliases(conn)
     end
   end
 end
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