diff --git a/CHANGELOG.md b/CHANGELOG.md index e397a75d0..18ed8c8ae 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/pleroma/collections/fetcher.ex b/lib/pleroma/collections/fetcher.ex index a2fcb7794..9ab883cc2 100644 --- a/lib/pleroma/collections/fetcher.ex +++ b/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} -> diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 844251a18..379b361f8 100644 --- a/lib/pleroma/object.ex +++ b/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) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 267a82b27..b9d8dbaaa 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/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), - {:ok, _} <- Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_accept(uri), + {: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)}, {_, 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} -> - {:error, "Max thread distance exceeded."} + {:allowed_depth, false} = e -> + log_fetch_error(id, e) + {:error, :allowed_depth} - {:scheme, false} -> - {:error, "URI Scheme Invalid"} + {:valid_uri_scheme, _} = e -> + log_fetch_error(id, e) + {:error, :invalid_uri_scheme} - {:transmogrifier, {:error, {:reject, e}}} -> - {:reject, e} + {:mrf_reject_check, _} = e -> + log_fetch_error(id, e) + {:reject, :mrf} - {:transmogrifier, {:reject, e}} -> - {:reject, e} + {:mrf_accept_check, _} = e -> + log_fetch_error(id, e) + {:reject, :mrf} - {:transmogrifier, _} = e -> - {:error, e} + {:containment, reason} = 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}} -> - {:error, error} - - {:reject, reason} -> - {:reject, reason} + {:fetch, {:error, reason}} = e -> + log_fetch_error(id, e) + {:error, 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")}, - {_, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)}, + with {:valid_uri_scheme, true} <- {:valid_uri_scheme, String.starts_with?(id, "http")}, + %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, _} -> - {:error, "Object's ActivityPub id/url does not match final fetch URL"} + {:strict_id, _} = e -> + log_fetch_error(id, e) + {:error, :id_mismatch} - {:scheme, _} -> - {:error, "Unsupported URI scheme"} + {:mrf_reject_check, _} = e -> + log_fetch_error(id, e) + {:reject, :mrf} - {:local_fetch, _} -> - {:error, "Trying to fetch local resource"} + {:mrf_accept_check, _} = e -> + log_fetch_error(id, e) + {:reject, :mrf} - {:containment, _} -> - {:error, "Object containment failed."} + {:valid_uri_scheme, _} = e -> + 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} diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 1e06bc809..505fa7462 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/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, _} = e -> e - e -> {:error, e} + {:error, _} -> {:ok, true} 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} diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index a72a431b2..033fc9e78 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/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} -> - Logger.error("Couldn't fetch fixed_object@#{object} #{inspect(e)}") + {:fetch, _} -> data _ -> diff --git a/lib/pleroma/web/xml.ex b/lib/pleroma/web/xml.ex index e68341e20..7fe7730ea 100644 --- a/lib/pleroma/web/xml.ex +++ b/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 diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index ad4d785a1..1bc0e5d0c 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/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 diff --git a/lib/pleroma/workers/worker_helper.ex b/lib/pleroma/workers/worker_helper.ex index 4c0a55774..6d27151de 100644 --- a/lib/pleroma/workers/worker_helper.ex +++ b/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 diff --git a/mix.exs b/mix.exs index b942c6c23..237503f84 100644 --- a/mix.exs +++ b/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"}, diff --git a/mix.lock b/mix.lock index c62df06f8..118f17699 100644 --- a/mix.lock +++ b/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"}, diff --git a/test/fixtures/tesla_mock/aimu@misskey.io.json b/test/fixtures/tesla_mock/aimu@misskey.io.json new file mode 100644 index 000000000..9ff4cb6d0 --- /dev/null +++ b/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": "

わずかな作曲要素 巣穴で独り言
Twitter
https://twitter.com/aimu_53
Soundcloud
https://soundcloud.com/aimu-53

", + "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" +} diff --git a/test/fixtures/tesla_mock/misskey.io_8vs6wxufd0.json b/test/fixtures/tesla_mock/misskey.io_8vs6wxufd0.json new file mode 100644 index 000000000..323ca10ed --- /dev/null +++ b/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": "

Fantiaこれできないように過去のやつは従量課金だった気がする

", + "_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": [] +} diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 4c4831af3..12154cb05 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/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."} = - Fetcher.fetch_object_from_id(@ap_id, depth: 1) + assert {:error, :allowed_depth} = 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, - {"Object has been deleted", "https://mastodon.example.org/users/userisgone", 410}} == + assert {:error, :not_found} == Fetcher.fetch_and_contain_remote_object_from_id( "https://mastodon.example.org/users/userisgone" ) end test "handle HTTP 404 response" do - assert {:error, - {"Object has been deleted", "https://mastodon.example.org/users/userisgone404", 404}} == + assert {:error, :not_found} == Fetcher.fetch_and_contain_remote_object_from_id( "https://mastodon.example.org/users/userisgone404" ) diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index e473ae659..dd7977593 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/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 diff --git a/test/pleroma/web/twitter_api/remote_follow_controller_test.exs b/test/pleroma/web/twitter_api/remote_follow_controller_test.exs index d2bc7840f..5a94e4396 100644 --- a/test/pleroma/web/twitter_api/remote_follow_controller_test.exs +++ b/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 diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs new file mode 100644 index 000000000..c30e773d4 --- /dev/null +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -0,0 +1,69 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2023 Pleroma Authors +# 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 diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index e487d2e6b..042e4110e 100644 --- a/test/support/http_request_mock.ex +++ b/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