Merge pull request 'Handle failed fetches a bit better' (#743) from failed-fetch-processing into develop

Reviewed-on: AkkomaGang/akkoma#743

CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Verified support for elixir 1.16
 
 ## Changed
+- Inbound pipeline error handing was modified somewhat, which should lead to less incomprehensible log spam. Hopefully.
 
 ## Fixed
 - Issue preventing fetching anything from IPv6-only instances

lib/pleroma/collections/fetcher.ex

@@ -68,7 +68,10 @@ defmodule Akkoma.Collections.Fetcher do
           items
         end
       else
-        {:error, {"Object has been deleted", _, _}} ->
+        {:error, :not_found} ->
+          items
+
+        {:error, :forbidden} ->
           items
 
         {:error, error} ->

lib/pleroma/object.ex

@@ -178,7 +178,10 @@ defmodule Pleroma.Object do
         ap_id
 
       Keyword.get(options, :fetch) ->
-        Fetcher.fetch_object_from_id!(ap_id, options)
+        case Fetcher.fetch_object_from_id(ap_id, options) do
+          {:ok, object} -> object
+          _ -> nil
+        end
 
       true ->
         get_cached_by_ap_id(ap_id)

lib/pleroma/object/fetcher.ex

@@ -122,7 +122,7 @@ defmodule Pleroma.Object.Fetcher do
       {:ok, object}
     else
       {:local, true} -> {:ok, object}
-      {:id, false} -> {:error, "Object id changed on refetch"}
+      {:id, false} -> {:error, :id_mismatch}
       e -> {:error, e}
     end
   end
@@ -136,10 +136,13 @@ defmodule Pleroma.Object.Fetcher do
   def fetch_object_from_id(id, options \\ []) do
     with %URI{} = uri <- URI.parse(id),
          # let's check the URI is even vaguely valid first
-         {:scheme, true} <- {:scheme, uri.scheme == "http" or uri.scheme == "https"},
+         {:valid_uri_scheme, true} <-
+           {:valid_uri_scheme, uri.scheme == "http" or uri.scheme == "https"},
          # If we have instance restrictions, apply them here to prevent fetching from unwanted instances
-         {:ok, nil} <- Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_reject(uri),
+         {:mrf_reject_check, {:ok, nil}} <-
-         {:ok, _} <- Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_accept(uri),
+           {:mrf_reject_check, Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_reject(uri)},
+         {:mrf_accept_check, {:ok, _}} <-
+           {:mrf_accept_check, Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_accept(uri)},
          {_, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)},
          {_, true} <- {:allowed_depth, Federator.allowed_thread_distance?(options[:depth])},
          {_, {:ok, data}} <- {:fetch, fetch_and_contain_remote_object_from_id(id)},
@@ -151,20 +154,37 @@ defmodule Pleroma.Object.Fetcher do
            {:object, data, Object.normalize(activity, fetch: false)} do
       {:ok, object}
     else
-      {:allowed_depth, false} ->
+      {:allowed_depth, false} = e ->
-        {:error, "Max thread distance exceeded."}
+        log_fetch_error(id, e)
+        {:error, :allowed_depth}
 
-      {:scheme, false} ->
+      {:valid_uri_scheme, _} = e ->
-        {:error, "URI Scheme Invalid"}
+        log_fetch_error(id, e)
+        {:error, :invalid_uri_scheme}
 
-      {:transmogrifier, {:error, {:reject, e}}} ->
+      {:mrf_reject_check, _} = e ->
-        {:reject, e}
+        log_fetch_error(id, e)
+        {:reject, :mrf}
 
-      {:transmogrifier, {:reject, e}} ->
+      {:mrf_accept_check, _} = e ->
-        {:reject, e}
+        log_fetch_error(id, e)
+        {:reject, :mrf}
 
-      {:transmogrifier, _} = e ->
+      {:containment, reason} = e ->
-        {:error, e}
+        log_fetch_error(id, e)
+        {:error, reason}
+
+      {:transmogrifier, {:error, {:reject, reason}}} = e ->
+        log_fetch_error(id, e)
+        {:reject, reason}
+
+      {:transmogrifier, {:reject, reason}} = e ->
+        log_fetch_error(id, e)
+        {:reject, reason}
+
+      {:transmogrifier, reason} = e ->
+        log_fetch_error(id, e)
+        {:error, reason}
 
       {:object, data, nil} ->
         reinject_object(%Object{}, data)
@@ -175,17 +195,21 @@ defmodule Pleroma.Object.Fetcher do
       {:fetch_object, %Object{} = object} ->
         {:ok, object}
 
-      {:fetch, {:error, error}} ->
+      {:fetch, {:error, reason}} = e ->
-        {:error, error}
+        log_fetch_error(id, e)
-
+        {:error, reason}
-      {:reject, reason} ->
-        {:reject, reason}
 
       e ->
-        e
+        log_fetch_error(id, e)
+        {:error, e}
     end
   end
 
+  defp log_fetch_error(id, error) do
+    Logger.metadata(object: id)
+    Logger.error("Object rejected while fetching #{id} #{inspect(error)}")
+  end
+
   defp prepare_activity_params(data) do
     %{
       "type" => "Create",
@@ -199,27 +223,6 @@ defmodule Pleroma.Object.Fetcher do
     |> Maps.put_if_present("bcc", data["bcc"])
   end
 
-  @doc "Identical to `fetch_object_from_id/2` but just directly returns the object or on error `nil`"
-  def fetch_object_from_id!(id, options \\ []) do
-    with {:ok, object} <- fetch_object_from_id(id, options) do
-      object
-    else
-      {:error, %Tesla.Mock.Error{}} ->
-        nil
-
-      {:error, {"Object has been deleted", _id, _code}} ->
-        nil
-
-      {:reject, reason} ->
-        Logger.debug("Rejected #{id} while fetching: #{inspect(reason)}")
-        nil
-
-      e ->
-        Logger.error("Error while fetching #{id}: #{inspect(e)}")
-        nil
-    end
-  end
-
   defp make_signature(id, date) do
     uri = URI.parse(id)
 
@@ -259,8 +262,13 @@ defmodule Pleroma.Object.Fetcher do
   def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do
     Logger.debug("Fetching object #{id} via AP")
 
-    with {:scheme, true} <- {:scheme, String.starts_with?(id, "http")},
+    with {:valid_uri_scheme, true} <- {:valid_uri_scheme, String.starts_with?(id, "http")},
-         {_, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)},
+         %URI{} = uri <- URI.parse(id),
+         {:mrf_reject_check, {:ok, nil}} <-
+           {:mrf_reject_check, Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_reject(uri)},
+         {:mrf_accept_check, {:ok, _}} <-
+           {:mrf_accept_check, Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_accept(uri)},
+         {:local_fetch, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)},
          {:ok, final_id, body} <- get_object(id),
          {:ok, data} <- safe_json_decode(body),
          {_, :ok} <- {:strict_id, Containment.contain_id_to_fetch(final_id, data)},
@@ -271,17 +279,29 @@ defmodule Pleroma.Object.Fetcher do
 
       {:ok, data}
     else
-      {:strict_id, _} ->
+      {:strict_id, _} = e ->
-        {:error, "Object's ActivityPub id/url does not match final fetch URL"}
+        log_fetch_error(id, e)
+        {:error, :id_mismatch}
 
-      {:scheme, _} ->
+      {:mrf_reject_check, _} = e ->
-        {:error, "Unsupported URI scheme"}
+        log_fetch_error(id, e)
+        {:reject, :mrf}
 
-      {:local_fetch, _} ->
+      {:mrf_accept_check, _} = e ->
-        {:error, "Trying to fetch local resource"}
+        log_fetch_error(id, e)
+        {:reject, :mrf}
 
-      {:containment, _} ->
+      {:valid_uri_scheme, _} = e ->
-        {:error, "Object containment failed."}
+        log_fetch_error(id, e)
+        {:error, :invalid_uri_scheme}
+
+      {:local_fetch, _} = e ->
+        log_fetch_error(id, e)
+        {:error, :local_resource}
+
+      {:containment, reason} ->
+        log_fetch_error(id, reason)
+        {:error, reason}
 
       {:error, e} ->
         {:error, e}
@@ -292,7 +312,7 @@ defmodule Pleroma.Object.Fetcher do
   end
 
   def fetch_and_contain_remote_object_from_id(_id),
-    do: {:error, "id must be a string"}
+    do: {:error, :invalid_id}
 
   defp check_crossdomain_redirect(final_host, original_url)
 
@@ -356,8 +376,11 @@ defmodule Pleroma.Object.Fetcher do
           {:error, {:content_type, content_type}}
       end
     else
+      {:ok, %{status: code}} when code in [401, 403] ->
+        {:error, :forbidden}
+
       {:ok, %{status: code}} when code in [404, 410] ->
-        {:error, {"Object has been deleted", id, code}}
+        {:error, :not_found}
 
       {:error, e} ->
         {:error, e}

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -1705,9 +1705,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
            Fetcher.fetch_and_contain_remote_object_from_id(first) do
       {:ok, false}
     else
-      {:error, {:ok, %{status: code}}} when code in [401, 403] -> {:ok, true}
+      {:error, _} -> {:ok, true}
-      {:error, _} = e -> e
-      e -> {:error, e}
     end
   end
 
@@ -1732,7 +1730,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
         Logger.debug("Could not decode user at fetch #{ap_id}, #{inspect(e)}")
         {:error, e}
 
-      {:error, {:reject, reason} = e} ->
+      {:reject, reason} = e ->
         Logger.debug("Rejected user #{ap_id}: #{inspect(reason)}")
         {:error, e}
 

lib/pleroma/web/activity_pub/transmogrifier.ex

@@ -25,8 +25,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
 
   import Ecto.Query
 
-  require Logger
   require Pleroma.Constants
+  require Logger
 
   @doc """
   Modifies an incoming AP object (mastodon format) to our internal format.
@@ -135,8 +135,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
         |> Map.put("context", replied_object.data["context"] || object["conversation"])
         |> Map.drop(["conversation", "inReplyToAtomUri"])
       else
-        e ->
+        _ ->
-          Logger.warning("Couldn't fetch reply@#{inspect(in_reply_to_id)}, error: #{inspect(e)}")
           object
       end
     else
@@ -833,8 +832,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
            relative_object do
       Map.put(data, "object", external_url)
     else
-      {:fetch, e} ->
+      {:fetch, _} ->
-        Logger.error("Couldn't fetch fixed_object@#{object} #{inspect(e)}")
         data
 
       _ ->

lib/pleroma/web/xml.ex

@@ -26,7 +26,7 @@ defmodule Pleroma.Web.XML do
 
   def parse_document(text) do
     try do
-      doc = SweetXml.parse(text, dtd: :none)
+      doc = SweetXml.parse(text, dtd: :none, quiet: true)
 
       {:ok, doc}
     rescue

lib/pleroma/workers/remote_fetcher_worker.ex

@@ -5,10 +5,42 @@
 defmodule Pleroma.Workers.RemoteFetcherWorker do
   alias Pleroma.Object.Fetcher
 
-  use Pleroma.Workers.WorkerHelper, queue: "remote_fetcher"
+  use Pleroma.Workers.WorkerHelper,
+    queue: "remote_fetcher",
+    unique: [period: 300, states: Oban.Job.states(), keys: [:op, :id]]
 
   @impl Oban.Worker
   def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do
-    {:ok, _object} = Fetcher.fetch_object_from_id(id, depth: args["depth"])
+    case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do
+      {:ok, _object} ->
+        :ok
+
+      {:error, :forbidden} ->
+        {:discard, :forbidden}
+
+      {:error, :not_found} ->
+        {:discard, :not_found}
+
+      {:error, :allowed_depth} ->
+        {:discard, :allowed_depth}
+
+      {:error, :invalid_uri_scheme} ->
+        {:discard, :invalid_uri_scheme}
+
+      {:error, :local_resource} ->
+        {:discard, :local_resource}
+
+      {:reject, _} ->
+        {:discard, :reject}
+
+      {:error, :id_mismatch} ->
+        {:discard, :id_mismatch}
+
+      {:error, _} = e ->
+        e
+
+      e ->
+        {:error, e}
+    end
   end
 end

lib/pleroma/workers/worker_helper.ex

@@ -25,12 +25,16 @@ defmodule Pleroma.Workers.WorkerHelper do
   defmacro __using__(opts) do
     caller_module = __CALLER__.module
     queue = Keyword.fetch!(opts, :queue)
+    # by default just stop unintended duplicates - this can and should be overridden
+    # if you want to have a more complex uniqueness constraint
+    uniqueness = Keyword.get(opts, :unique, period: 1)
 
     quote do
       # Note: `max_attempts` is intended to be overridden in `new/2` call
       use Oban.Worker,
         queue: unquote(queue),
-        max_attempts: 1
+        max_attempts: 1,
+        unique: unquote(uniqueness)
 
       alias Oban.Job
 

mix.exs

@@ -125,7 +125,7 @@ defmodule Pleroma.Mixfile do
       {:ecto_enum, "~> 1.4"},
       {:ecto_sql, "~> 3.10.0"},
       {:postgrex, "~> 0.17.2"},
-      {:oban, "~> 2.15.2"},
+      {:oban, "~> 2.17.8"},
       {:gettext, "~> 0.22.3"},
       {:bcrypt_elixir, "~> 3.0.1"},
       {:fast_sanitize, "~> 0.2.3"},

mix.lock

@@ -83,7 +83,7 @@
   "nimble_options": {:hex, :nimble_options, "1.1.0", "3b31a57ede9cb1502071fade751ab0c7b8dbe75a9a4c2b5bbb0943a690b63172", [:mix], [], "hexpm", "8bbbb3941af3ca9acc7835f5655ea062111c9c27bcac53e004460dfd19008a99"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
   "nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"},
-  "oban": {:hex, :oban, "2.15.4", "d49ab4ffb7153010e32f80fe9e56f592706238149ec579eb50f8a4e41d218856", [:mix], [{:ecto_sql, "~> 3.6", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, "~> 0.9", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.16", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "5fce611fdfffb13e9148df883116e5201adf1e731eb302cc88cde0588510079c"},
+  "oban": {:hex, :oban, "2.17.8", "7fd7c8e82c7819afc1b5b5ed8d6d92bf0ecdd7ba170328fb043301eb06d32521", [:mix], [{:ecto_sql, "~> 3.10", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, "~> 0.9", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.16", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "a2165bf93843b7bcb68182c82725ddd4cb43c0c3719f114e7aa3b6c99c4b6129"},
   "open_api_spex": {:hex, :open_api_spex, "3.18.3", "fefb84fe323cacfc92afdd0ecb9e89bc0261ae00b7e3167ffc2028ce3944de42", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 3.0 or ~> 4.0 or ~> 5.0", [hex: :poison, repo: "hexpm", optional: true]}, {:ymlr, "~> 2.0 or ~> 3.0 or ~> 4.0 or ~> 5.0", [hex: :ymlr, repo: "hexpm", optional: true]}], "hexpm", "c0cfc31570199ce7e7520b494a591027da609af45f6bf9adce51e2469b1609fb"},
   "parse_trans": {:hex, :parse_trans, "3.4.1", "6e6aa8167cb44cc8f39441d05193be6e6f4e7c2946cb2759f015f8c56b76e5ff", [:rebar3], [], "hexpm", "620a406ce75dada827b82e453c19cf06776be266f5a67cff34e1ef2cbb60e49a"},
   "phoenix": {:hex, :phoenix, "1.7.12", "1cc589e0eab99f593a8aa38ec45f15d25297dd6187ee801c8de8947090b5a9d3", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.1", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.7", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:websock_adapter, "~> 0.5.3", [hex: :websock_adapter, repo: "hexpm", optional: false]}], "hexpm", "d646192fbade9f485b01bc9920c139bfdd19d0f8df3d73fd8eaf2dfbe0d2837c"},

test/fixtures/tesla_mock/aimu@misskey.io.json

@@ -0,0 +1,64 @@
+{
+  "@context": [
+    "https://www.w3.org/ns/activitystreams",
+    "https://w3id.org/security/v1",
+    {
+      "manuallyApprovesFollowers": "as:manuallyApprovesFollowers",
+      "sensitive": "as:sensitive",
+      "Hashtag": "as:Hashtag",
+      "quoteUrl": "as:quoteUrl",
+      "toot": "http://joinmastodon.org/ns#",
+      "Emoji": "toot:Emoji",
+      "featured": "toot:featured",
+      "discoverable": "toot:discoverable",
+      "schema": "http://schema.org#",
+      "PropertyValue": "schema:PropertyValue",
+      "value": "schema:value",
+      "misskey": "https://misskey.io/ns#",
+      "_misskey_content": "misskey:_misskey_content",
+      "_misskey_quote": "misskey:_misskey_quote",
+      "_misskey_reaction": "misskey:_misskey_reaction",
+      "_misskey_votes": "misskey:_misskey_votes",
+      "_misskey_talk": "misskey:_misskey_talk",
+      "isCat": "misskey:isCat",
+      "vcard": "http://www.w3.org/2006/vcard/ns#"
+    }
+  ],
+  "type": "Person",
+  "id": "https://misskey.io/users/83ssedkv53",
+  "inbox": "https://misskey.io/users/83ssedkv53/inbox",
+  "outbox": "https://misskey.io/users/83ssedkv53/outbox",
+  "followers": "https://misskey.io/users/83ssedkv53/followers",
+  "following": "https://misskey.io/users/83ssedkv53/following",
+  "sharedInbox": "https://misskey.io/inbox",
+  "endpoints": {
+    "sharedInbox": "https://misskey.io/inbox"
+  },
+  "url": "https://misskey.io/@aimu",
+  "preferredUsername": "aimu",
+  "name": "あいむ",
+  "summary": "<p><span>わずかな作曲要素 巣穴で独り言<br>Twitter </span><a href=\"https://twitter.com/aimu_53\">https://twitter.com/aimu_53</a><span><br>Soundcloud </span><a href=\"https://soundcloud.com/aimu-53\">https://soundcloud.com/aimu-53</a></p>",
+  "icon": {
+    "type": "Image",
+    "url": "https://s3.arkjp.net/misskey/webpublic-3f7e93c0-34f5-443c-acc0-f415cb2342b4.jpg",
+    "sensitive": false,
+    "name": null
+  },
+  "image": {
+    "type": "Image",
+    "url": "https://s3.arkjp.net/misskey/webpublic-2db63d1d-490b-488b-ab62-c93c285f26b6.png",
+    "sensitive": false,
+    "name": null
+  },
+  "tag": [],
+  "manuallyApprovesFollowers": false,
+  "discoverable": true,
+  "publicKey": {
+    "id": "https://misskey.io/users/83ssedkv53#main-key",
+    "type": "Key",
+    "owner": "https://misskey.io/users/83ssedkv53",
+    "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA1ylhePJ6qGHmwHSBP17b\nIosxGaiFKvgDBgZdm8vzvKeRSqJV9uLHfZL3pO/Zt02EwaZd2GohZAtBZEF8DbMA\n3s93WAesvyGF9mjGrYYKlhp/glwyrrrbf+RdD0DLtyDwRRlrxp3pS2lLmv5Tp1Zl\npH+UKpOnNrpQqjHI5P+lEc9bnflzbRrX+UiyLNsVAP80v4wt7SZfT/telrU6mDru\n998UdfhUo7bDKeDsHG1PfLpyhhtfdoZub4kBpkyacHiwAd+CdCjR54Eu7FDwVK3p\nY3JcrT2q5stgMqN1m4QgSL4XAADIotWwDYttTJejM1n9dr+6VWv5bs0F2Q/6gxOp\nu5DQZLk4Q+64U4LWNox6jCMOq3fYe0g7QalJIHnanYQQo+XjoH6S1Aw64gQ3Ip2Y\nZBmZREAOR7GMFVDPFnVnsbCHnIAv16TdgtLgQBAihkWEUuPqITLi8PMu6kMr3uyq\nYkObEfH0TNTcqaiVpoXv791GZLEUV5ROl0FSUANLNkHZZv29xZ5JDOBOR1rNBLyH\ngVtW8rpszYqOXwzX23hh4WsVXfB7YgNvIijwjiaWbzsecleaENGEnLNMiVKVumTj\nmtyTeFJpH0+OaSrUYpemRRJizmqIjklKsNwUEwUb2WcUUg92o56T2obrBkooabZe\nwgSXSKTOcjsR/ju7+AuIyvkCAwEAAQ==\n-----END PUBLIC KEY-----\n"
+  },
+  "isCat": true,
+  "vcard:bday": "5353-05-03"
+}

test/fixtures/tesla_mock/misskey.io_8vs6wxufd0.json

@@ -0,0 +1,44 @@
+{
+  "@context": [
+    "https://www.w3.org/ns/activitystreams",
+    "https://w3id.org/security/v1",
+    {
+      "manuallyApprovesFollowers": "as:manuallyApprovesFollowers",
+      "sensitive": "as:sensitive",
+      "Hashtag": "as:Hashtag",
+      "quoteUrl": "as:quoteUrl",
+      "toot": "http://joinmastodon.org/ns#",
+      "Emoji": "toot:Emoji",
+      "featured": "toot:featured",
+      "discoverable": "toot:discoverable",
+      "schema": "http://schema.org#",
+      "PropertyValue": "schema:PropertyValue",
+      "value": "schema:value",
+      "misskey": "https://misskey.io/ns#",
+      "_misskey_content": "misskey:_misskey_content",
+      "_misskey_quote": "misskey:_misskey_quote",
+      "_misskey_reaction": "misskey:_misskey_reaction",
+      "_misskey_votes": "misskey:_misskey_votes",
+      "_misskey_talk": "misskey:_misskey_talk",
+      "isCat": "misskey:isCat",
+      "vcard": "http://www.w3.org/2006/vcard/ns#"
+    }
+  ],
+  "id": "https://misskey.io/notes/8vs6wxufd0",
+  "type": "Note",
+  "attributedTo": "https://misskey.io/users/83ssedkv53",
+  "summary": null,
+  "content": "<p><span>Fantiaこれできないように過去のやつは従量課金だった気がする</span></p>",
+  "_misskey_content": "Fantiaこれできないように過去のやつは従量課金だった気がする",
+  "published": "2022-01-21T16:37:12.663Z",
+  "to": [
+    "https://www.w3.org/ns/activitystreams#Public"
+  ],
+  "cc": [
+    "https://misskey.io/users/83ssedkv53/followers"
+  ],
+  "inReplyTo": null,
+  "attachment": [],
+  "sensitive": false,
+  "tag": []
+}

test/pleroma/object/fetcher_test.exs

@@ -57,6 +57,9 @@ defmodule Pleroma.Object.FetcherTest do
           body: spoofed_object_with_ids("https://patch.cx/objects/spoof_content_type")
         }
 
+      %{method: :get, url: "https://octodon.social/users/cwebber/statuses/111647596861000656"} ->
+        %Tesla.Env{status: 403}
+
       # Spoof: mismatching ids
       # Variant 1: Non-exisitng fake id
       %{
@@ -203,8 +206,7 @@ defmodule Pleroma.Object.FetcherTest do
     test "it returns thread depth exceeded error if thread depth is exceeded" do
       clear_config([:instance, :federation_incoming_replies_max_depth], 0)
 
-      assert {:error, "Max thread distance exceeded."} =
+      assert {:error, :allowed_depth} = Fetcher.fetch_object_from_id(@ap_id, depth: 1)
-               Fetcher.fetch_object_from_id(@ap_id, depth: 1)
     end
 
     test "it fetches object if max thread depth is restricted to 0 and depth is not specified" do
@@ -250,12 +252,12 @@ defmodule Pleroma.Object.FetcherTest do
     end
 
     test "it does not fetch a spoofed object with id different from URL" do
-      assert {:error, "Object's ActivityPub id/url does not match final fetch URL"} =
+      assert {:error, :id_mismatch} =
                Fetcher.fetch_and_contain_remote_object_from_id(
                  "https://patch.cx/media/03ca3c8b4ac3ddd08bf0f84be7885f2f88de0f709112131a22d83650819e36c2.json"
                )
 
-      assert {:error, "Object's ActivityPub id/url does not match final fetch URL"} =
+      assert {:error, :id_mismatch} =
                Fetcher.fetch_and_contain_remote_object_from_id(
                  "https://patch.cx/media/spoof_stage1.json"
                )
@@ -285,14 +287,14 @@ defmodule Pleroma.Object.FetcherTest do
     end
 
     test "it does not fetch a spoofed object with a foreign actor" do
-      assert {:error, "Object containment failed."} =
+      assert {:error, _} =
                Fetcher.fetch_and_contain_remote_object_from_id(
                  "https://patch.cx/objects/spoof_foreign_actor"
                )
     end
 
     test "it does not fetch from localhost" do
-      assert {:error, "Trying to fetch local resource"} =
+      assert {:error, :local_resource} =
                Fetcher.fetch_and_contain_remote_object_from_id(
                  Pleroma.Web.Endpoint.url() <> "/spoof_local"
                )
@@ -402,16 +404,14 @@ defmodule Pleroma.Object.FetcherTest do
     end
 
     test "handle HTTP 410 Gone response" do
-      assert {:error,
+      assert {:error, :not_found} ==
-              {"Object has been deleted", "https://mastodon.example.org/users/userisgone", 410}} ==
                Fetcher.fetch_and_contain_remote_object_from_id(
                  "https://mastodon.example.org/users/userisgone"
                )
     end
 
     test "handle HTTP 404 response" do
-      assert {:error,
+      assert {:error, :not_found} ==
-              {"Object has been deleted", "https://mastodon.example.org/users/userisgone404", 404}} ==
                Fetcher.fetch_and_contain_remote_object_from_id(
                  "https://mastodon.example.org/users/userisgone404"
                )

test/pleroma/web/activity_pub/transmogrifier_test.exs

@@ -124,6 +124,28 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
 
       assert activity.data["context"] == object.data["context"]
     end
+
+    test "it accepts quote posts" do
+      insert(:user, ap_id: "https://misskey.io/users/7rkrarq81i")
+
+      object = File.read!("test/fixtures/quote_post/misskey_quote_post.json") |> Jason.decode!()
+
+      message = %{
+        "@context" => "https://www.w3.org/ns/activitystreams",
+        "type" => "Create",
+        "actor" => "https://misskey.io/users/7rkrarq81i",
+        "object" => object
+      }
+
+      assert {:ok, activity} = Transmogrifier.handle_incoming(message)
+
+      # Object was created in the database
+      object = Object.normalize(activity)
+      assert object.data["quoteUri"] == "https://misskey.io/notes/8vs6wxufd0"
+
+      # It fetched the quoted post
+      assert Object.normalize("https://misskey.io/notes/8vs6wxufd0")
+    end
   end
 
   describe "prepare outgoing" do
@@ -413,7 +435,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
 
       assert capture_log(fn ->
                {:error, _} = Transmogrifier.handle_incoming(data)
-             end) =~ "Object containment failed"
+             end) =~ "Object rejected while fetching"
     end
 
     test "it rejects activities which reference objects that have an incorrect attribution (variant 1)" do
@@ -428,7 +450,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
 
       assert capture_log(fn ->
                {:error, _} = Transmogrifier.handle_incoming(data)
-             end) =~ "Object containment failed"
+             end) =~ "Object rejected while fetching"
     end
 
     test "it rejects activities which reference objects that have an incorrect attribution (variant 2)" do
@@ -443,7 +465,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
 
       assert capture_log(fn ->
                {:error, _} = Transmogrifier.handle_incoming(data)
-             end) =~ "Object containment failed"
+             end) =~ "Object rejected while fetching"
     end
   end
 
@@ -536,7 +558,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
     test "returns nil when cannot normalize object" do
       assert capture_log(fn ->
                refute Transmogrifier.get_obj_helper("test-obj-id")
-             end) =~ "URI Scheme Invalid"
+             end) =~ ":valid_uri_scheme"
     end
 
     @tag capture_log: true

test/pleroma/web/twitter_api/remote_follow_controller_test.exs

@@ -132,7 +132,7 @@ defmodule Pleroma.Web.TwitterAPI.RemoteFollowControllerTest do
                  |> html_response(200)
 
                assert response =~ "Error fetching user"
-             end) =~ "Object has been deleted"
+             end) =~ ":not_found"
     end
   end
 

test/pleroma/workers/remote_fetcher_worker_test.exs

@@ -0,0 +1,69 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2023 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Workers.RemoteFetcherWorkerTest do
+  use Pleroma.DataCase
+  use Oban.Testing, repo: Pleroma.Repo
+
+  alias Pleroma.Workers.RemoteFetcherWorker
+
+  @deleted_object_one "https://deleted-404.example.com/"
+  @deleted_object_two "https://deleted-410.example.com/"
+  @unauthorized_object "https://unauthorized.example.com/"
+  @depth_object "https://depth.example.com/"
+
+  describe "RemoteFetcherWorker" do
+    setup do
+      Tesla.Mock.mock(fn
+        %{method: :get, url: @deleted_object_one} ->
+          %Tesla.Env{
+            status: 404
+          }
+
+        %{method: :get, url: @deleted_object_two} ->
+          %Tesla.Env{
+            status: 410
+          }
+
+        %{method: :get, url: @unauthorized_object} ->
+          %Tesla.Env{
+            status: 403
+          }
+
+        %{method: :get, url: @depth_object} ->
+          %Tesla.Env{
+            status: 200
+          }
+      end)
+    end
+
+    test "does not requeue a deleted object" do
+      assert {:discard, _} =
+               RemoteFetcherWorker.perform(%Oban.Job{
+                 args: %{"op" => "fetch_remote", "id" => @deleted_object_one}
+               })
+
+      assert {:discard, _} =
+               RemoteFetcherWorker.perform(%Oban.Job{
+                 args: %{"op" => "fetch_remote", "id" => @deleted_object_two}
+               })
+    end
+
+    test "does not requeue an unauthorized object" do
+      assert {:discard, _} =
+               RemoteFetcherWorker.perform(%Oban.Job{
+                 args: %{"op" => "fetch_remote", "id" => @unauthorized_object}
+               })
+    end
+
+    test "does not requeue an object that exceeded depth" do
+      clear_config([:instance, :federation_incoming_replies_max_depth], 0)
+
+      assert {:discard, _} =
+               RemoteFetcherWorker.perform(%Oban.Job{
+                 args: %{"op" => "fetch_remote", "id" => @depth_object, "depth" => 1}
+               })
+    end
+  end
+end

test/support/http_request_mock.ex

@@ -1321,6 +1321,25 @@ defmodule HttpRequestMock do
      }}
   end
 
+  # A misskey quote
+  def get("https://misskey.io/notes/8vs6wxufd0", _, _, _) do
+    {:ok,
+     %Tesla.Env{
+       status: 200,
+       body: File.read!("test/fixtures/tesla_mock/misskey.io_8vs6wxufd0.json"),
+       headers: activitypub_object_headers()
+     }}
+  end
+
+  def get("https://misskey.io/users/83ssedkv53", _, _, _) do
+    {:ok,
+     %Tesla.Env{
+       status: 200,
+       body: File.read!("test/fixtures/tesla_mock/aimu@misskey.io.json"),
+       headers: activitypub_object_headers()
+     }}
+  end
+
   def get("https://example.org/emoji/firedfox.png", _, _, _) do
     {:ok, %Tesla.Env{status: 200, body: File.read!("test/fixtures/image.jpg")}}
   end