diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | config/config.exs | 1 | ||||
| -rw-r--r-- | docs/config.md | 1 | ||||
| -rw-r--r-- | lib/pleroma/object.ex | 24 | ||||
| -rw-r--r-- | lib/pleroma/object/fetcher.ex | 8 | ||||
| -rw-r--r-- | lib/pleroma/web/activity_pub/transmogrifier.ex | 123 | ||||
| -rw-r--r-- | lib/pleroma/web/federator/federator.ex | 12 | ||||
| -rw-r--r-- | lib/pleroma/web/ostatus/handlers/note_handler.ex | 13 | ||||
| -rw-r--r-- | lib/pleroma/web/ostatus/ostatus.ex | 22 | ||||
| -rw-r--r-- | test/web/activity_pub/transmogrifier_test.exs | 37 | ||||
| -rw-r--r-- | test/web/ostatus/ostatus_test.exs | 28 |
11 files changed, 187 insertions, 83 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 84a16ff3c..63ab1a6e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] ### Added - MRF: Support for priming the mediaproxy cache (`Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicy`) +Configuration: `federation_incoming_replies_max_depth` option - Mastodon API: Support for the [`tagged` filter](https://github.com/tootsuite/mastodon/pull/9755) in [`GET /api/v1/accounts/:id/statuses`](https://docs.joinmastodon.org/api/rest/accounts/#get-api-v1-accounts-id-statuses) - Admin API: Return users' tags when querying reports - Admin API: Return avatar and display name when querying users diff --git a/config/config.exs b/config/config.exs index e337f00aa..65f239e31 100644 --- a/config/config.exs +++ b/config/config.exs @@ -218,6 +218,7 @@ config :pleroma, :instance, }, registrations_open: true, federating: true, + federation_incoming_replies_max_depth: 100, federation_reachability_timeout_days: 7, federation_publisher_modules: [ Pleroma.Web.ActivityPub.Publisher, diff --git a/docs/config.md b/docs/config.md index 8afccb228..822c34c51 100644 --- a/docs/config.md +++ b/docs/config.md @@ -87,6 +87,7 @@ config :pleroma, Pleroma.Emails.Mailer, * `invites_enabled`: Enable user invitations for admins (depends on `registrations_open: false`). * `account_activation_required`: Require users to confirm their emails before signing in. * `federating`: Enable federation with other instances +* `federation_incoming_replies_max_depth`: Max. depth of reply-to activities fetching on incoming federation, to prevent out-of-memory situations while fetching very long threads. If set to `nil`, threads of any depth will be fetched. Lower this value if you experience out-of-memory crashes. * `federation_reachability_timeout_days`: Timeout (in days) of each external federation target being unreachable prior to pausing federating to it. * `allow_relay`: Enable Pleroma’s Relay, which makes it possible to follow a whole instance * `rewrite_policy`: Message Rewrite Policy, either one or a list. Here are the ones available by default: diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 4b181ec59..b8647dd26 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -44,20 +44,20 @@ defmodule Pleroma.Object do Repo.one(from(object in Object, where: fragment("(?)->>'id' = ?", object.data, ^ap_id))) end - def normalize(_, fetch_remote \\ true) + def normalize(_, fetch_remote \\ true, options \\ []) # If we pass an Activity to Object.normalize(), we can try to use the preloaded object. # Use this whenever possible, especially when walking graphs in an O(N) loop! - def normalize(%Object{} = object, _), do: object - def normalize(%Activity{object: %Object{} = object}, _), do: object + def normalize(%Object{} = object, _, _), do: object + def normalize(%Activity{object: %Object{} = object}, _, _), do: object # A hack for fake activities - def normalize(%Activity{data: %{"object" => %{"fake" => true} = data}}, _) do + def normalize(%Activity{data: %{"object" => %{"fake" => true} = data}}, _, _) do %Object{id: "pleroma:fake_object_id", data: data} end # Catch and log Object.normalize() calls where the Activity's child object is not # preloaded. - def normalize(%Activity{data: %{"object" => %{"id" => ap_id}}}, fetch_remote) do + def normalize(%Activity{data: %{"object" => %{"id" => ap_id}}}, fetch_remote, _) do Logger.debug( "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object!" ) @@ -67,7 +67,7 @@ defmodule Pleroma.Object do normalize(ap_id, fetch_remote) end - def normalize(%Activity{data: %{"object" => ap_id}}, fetch_remote) do + def normalize(%Activity{data: %{"object" => ap_id}}, fetch_remote, _) do Logger.debug( "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object!" ) @@ -78,10 +78,14 @@ defmodule Pleroma.Object do end # Old way, try fetching the object through cache. - def normalize(%{"id" => ap_id}, fetch_remote), do: normalize(ap_id, fetch_remote) - def normalize(ap_id, false) when is_binary(ap_id), do: get_cached_by_ap_id(ap_id) - def normalize(ap_id, true) when is_binary(ap_id), do: Fetcher.fetch_object_from_id!(ap_id) - def normalize(_, _), do: nil + def normalize(%{"id" => ap_id}, fetch_remote, _), do: normalize(ap_id, fetch_remote) + def normalize(ap_id, false, _) when is_binary(ap_id), do: get_cached_by_ap_id(ap_id) + + def normalize(ap_id, true, options) when is_binary(ap_id) do + Fetcher.fetch_object_from_id!(ap_id, options) + end + + def normalize(_, _, _), do: nil # Owned objects can only be mutated by their owner def authorize_mutation(%Object{data: %{"actor" => actor}}, %User{ap_id: ap_id}), diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index c422490ac..fffbf2bbb 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -22,7 +22,7 @@ defmodule Pleroma.Object.Fetcher do # TODO: # This will create a Create activity, which we need internally at the moment. - def fetch_object_from_id(id) do + def fetch_object_from_id(id, options \\ []) do if object = Object.get_cached_by_ap_id(id) do {:ok, object} else @@ -38,7 +38,7 @@ defmodule Pleroma.Object.Fetcher do "object" => data }, :ok <- Containment.contain_origin(id, params), - {:ok, activity} <- Transmogrifier.handle_incoming(params), + {:ok, activity} <- Transmogrifier.handle_incoming(params, options), {:object, _data, %Object{} = object} <- {:object, data, Object.normalize(activity, false)} do {:ok, object} @@ -63,8 +63,8 @@ defmodule Pleroma.Object.Fetcher do end end - def fetch_object_from_id!(id) do - with {:ok, object} <- fetch_object_from_id(id) do + def fetch_object_from_id!(id, options \\ []) do + with {:ok, object} <- fetch_object_from_id(id, options) do object else _e -> diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 3bb8b40b5..543d4bb7d 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -14,6 +14,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Visibility + alias Pleroma.Web.Federator import Ecto.Query @@ -22,20 +23,20 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do @doc """ Modifies an incoming AP object (mastodon format) to our internal format. """ - def fix_object(object) do + def fix_object(object, options \\ []) do object |> fix_actor |> fix_url |> fix_attachments |> fix_context - |> fix_in_reply_to + |> fix_in_reply_to(options) |> fix_emoji |> fix_tag |> fix_content_map |> fix_likes |> fix_addressing |> fix_summary - |> fix_type + |> fix_type(options) end def fix_summary(%{"summary" => nil} = object) do @@ -164,7 +165,9 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do object end - def fix_in_reply_to(%{"inReplyTo" => in_reply_to} = object) + def fix_in_reply_to(object, options \\ []) + + def fix_in_reply_to(%{"inReplyTo" => in_reply_to} = object, options) when not is_nil(in_reply_to) do in_reply_to_id = cond do @@ -182,28 +185,34 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do "" end - case get_obj_helper(in_reply_to_id) do - {:ok, replied_object} -> - with %Activity{} = _activity <- - Activity.get_create_by_object_ap_id(replied_object.data["id"]) do - object - |> Map.put("inReplyTo", replied_object.data["id"]) - |> Map.put("inReplyToAtomUri", object["inReplyToAtomUri"] || in_reply_to_id) - |> Map.put("conversation", replied_object.data["context"] || object["conversation"]) - |> Map.put("context", replied_object.data["context"] || object["conversation"]) - else - e -> - Logger.error("Couldn't fetch \"#{inspect(in_reply_to_id)}\", error: #{inspect(e)}") + object = Map.put(object, "inReplyToAtomUri", in_reply_to_id) + + if Federator.allowed_incoming_reply_depth?(options[:depth]) do + case get_obj_helper(in_reply_to_id, options) do + {:ok, replied_object} -> + with %Activity{} = _activity <- + Activity.get_create_by_object_ap_id(replied_object.data["id"]) do object - end + |> Map.put("inReplyTo", replied_object.data["id"]) + |> Map.put("inReplyToAtomUri", object["inReplyToAtomUri"] || in_reply_to_id) + |> Map.put("conversation", replied_object.data["context"] || object["conversation"]) + |> Map.put("context", replied_object.data["context"] || object["conversation"]) + else + e -> + Logger.error("Couldn't fetch \"#{inspect(in_reply_to_id)}\", error: #{inspect(e)}") + object + end - e -> - Logger.error("Couldn't fetch \"#{inspect(in_reply_to_id)}\", error: #{inspect(e)}") - object + e -> + Logger.error("Couldn't fetch \"#{inspect(in_reply_to_id)}\", error: #{inspect(e)}") + object + end + else + object end end - def fix_in_reply_to(object), do: object + def fix_in_reply_to(object, _options), do: object def fix_context(object) do context = object["context"] || object["conversation"] || Utils.generate_context_id() @@ -336,8 +345,13 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do def fix_content_map(object), do: object - def fix_type(%{"inReplyTo" => reply_id} = object) when is_binary(reply_id) do - reply = Object.normalize(reply_id) + def fix_type(object, options \\ []) + + def fix_type(%{"inReplyTo" => reply_id} = object, options) when is_binary(reply_id) do + reply = + if Federator.allowed_incoming_reply_depth?(options[:depth]) do + Object.normalize(reply_id, true) + end if reply && (reply.data["type"] == "Question" and object["name"]) do Map.put(object, "type", "Answer") @@ -346,7 +360,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end end - def fix_type(object), do: object + def fix_type(object, _), do: object defp mastodon_follow_hack(%{"id" => id, "actor" => follower_id}, followed) do with true <- id =~ "follows", @@ -374,9 +388,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end end + def handle_incoming(data, options \\ []) + # Flag objects are placed ahead of the ID check because Mastodon 2.8 and earlier send them # with nil ID. - def handle_incoming(%{"type" => "Flag", "object" => objects, "actor" => actor} = data) do + def handle_incoming(%{"type" => "Flag", "object" => objects, "actor" => actor} = data, _options) do with context <- data["context"] || Utils.generate_context_id(), content <- data["content"] || "", %User{} = actor <- User.get_cached_by_ap_id(actor), @@ -409,15 +425,19 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end # disallow objects with bogus IDs - def handle_incoming(%{"id" => nil}), do: :error - def handle_incoming(%{"id" => ""}), do: :error + def handle_incoming(%{"id" => nil}, _options), do: :error + def handle_incoming(%{"id" => ""}, _options), do: :error # length of https:// = 8, should validate better, but good enough for now. - def handle_incoming(%{"id" => id}) when not (is_binary(id) and length(id) > 8), do: :error + def handle_incoming(%{"id" => id}, _options) when not (is_binary(id) and length(id) > 8), + do: :error # TODO: validate those with a Ecto scheme # - tags # - emoji - def handle_incoming(%{"type" => "Create", "object" => %{"type" => objtype} = object} = data) + def handle_incoming( + %{"type" => "Create", "object" => %{"type" => objtype} = object} = data, + options + ) when objtype in ["Article", "Note", "Video", "Page", "Question", "Answer"] do actor = Containment.get_actor(data) @@ -427,7 +447,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do with nil <- Activity.get_create_by_object_ap_id(object["id"]), {:ok, %User{} = user} <- User.get_or_fetch_by_ap_id(data["actor"]) do - object = fix_object(data["object"]) + options = Keyword.put(options, :depth, (options[:depth] || 0) + 1) + object = fix_object(data["object"], options) params = %{ to: data["to"], @@ -452,7 +473,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end def handle_incoming( - %{"type" => "Follow", "object" => followed, "actor" => follower, "id" => id} = data + %{"type" => "Follow", "object" => followed, "actor" => follower, "id" => id} = data, + _options ) do with %User{local: true} = followed <- User.get_cached_by_ap_id(followed), {:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(follower), @@ -503,7 +525,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end def handle_incoming( - %{"type" => "Accept", "object" => follow_object, "actor" => _actor, "id" => _id} = data + %{"type" => "Accept", "object" => follow_object, "actor" => _actor, "id" => _id} = data, + _options ) do with actor <- Containment.get_actor(data), {:ok, %User{} = followed} <- User.get_or_fetch_by_ap_id(actor), @@ -524,7 +547,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end def handle_incoming( - %{"type" => "Reject", "object" => follow_object, "actor" => _actor, "id" => _id} = data + %{"type" => "Reject", "object" => follow_object, "actor" => _actor, "id" => _id} = data, + _options ) do with actor <- Containment.get_actor(data), {:ok, %User{} = followed} <- User.get_or_fetch_by_ap_id(actor), @@ -548,7 +572,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end def handle_incoming( - %{"type" => "Like", "object" => object_id, "actor" => _actor, "id" => id} = data + %{"type" => "Like", "object" => object_id, "actor" => _actor, "id" => id} = data, + _options ) do with actor <- Containment.get_actor(data), {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor), @@ -561,7 +586,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end def handle_incoming( - %{"type" => "Announce", "object" => object_id, "actor" => _actor, "id" => id} = data + %{"type" => "Announce", "object" => object_id, "actor" => _actor, "id" => id} = data, + _options ) do with actor <- Containment.get_actor(data), {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor), @@ -576,7 +602,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do def handle_incoming( %{"type" => "Update", "object" => %{"type" => object_type} = object, "actor" => actor_id} = - data + data, + _options ) when object_type in ["Person", "Application", "Service", "Organization"] do with %User{ap_id: ^actor_id} = actor <- User.get_cached_by_ap_id(object["id"]) do @@ -614,7 +641,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do # an error or a tombstone. This would allow us to verify that a deletion actually took # place. def handle_incoming( - %{"type" => "Delete", "object" => object_id, "actor" => _actor, "id" => _id} = data + %{"type" => "Delete", "object" => object_id, "actor" => _actor, "id" => _id} = data, + _options ) do object_id = Utils.get_ap_id(object_id) @@ -635,7 +663,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do "object" => %{"type" => "Announce", "object" => object_id}, "actor" => _actor, "id" => id - } = data + } = data, + _options ) do with actor <- Containment.get_actor(data), {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor), @@ -653,7 +682,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do "object" => %{"type" => "Follow", "object" => followed}, "actor" => follower, "id" => id - } = _data + } = _data, + _options ) do with %User{local: true} = followed <- User.get_cached_by_ap_id(followed), {:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(follower), @@ -671,7 +701,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do "object" => %{"type" => "Block", "object" => blocked}, "actor" => blocker, "id" => id - } = _data + } = _data, + _options ) do with true <- Pleroma.Config.get([:activitypub, :accept_blocks]), %User{local: true} = blocked <- User.get_cached_by_ap_id(blocked), @@ -685,7 +716,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end def handle_incoming( - %{"type" => "Block", "object" => blocked, "actor" => blocker, "id" => id} = _data + %{"type" => "Block", "object" => blocked, "actor" => blocker, "id" => id} = _data, + _options ) do with true <- Pleroma.Config.get([:activitypub, :accept_blocks]), %User{local: true} = blocked = User.get_cached_by_ap_id(blocked), @@ -705,7 +737,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do "object" => %{"type" => "Like", "object" => object_id}, "actor" => _actor, "id" => id - } = data + } = data, + _options ) do with actor <- Containment.get_actor(data), {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor), @@ -717,10 +750,10 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end end - def handle_incoming(_), do: :error + def handle_incoming(_, _), do: :error - def get_obj_helper(id) do - if object = Object.normalize(id), do: {:ok, object}, else: nil + def get_obj_helper(id, options \\ []) do + if object = Object.normalize(id, true, options), do: {:ok, object}, else: nil end def set_reply_to_uri(%{"inReplyTo" => in_reply_to} = object) when is_binary(in_reply_to) do diff --git a/lib/pleroma/web/federator/federator.ex b/lib/pleroma/web/federator/federator.ex index f4c9fe284..f4f9e83e0 100644 --- a/lib/pleroma/web/federator/federator.ex +++ b/lib/pleroma/web/federator/federator.ex @@ -22,6 +22,18 @@ defmodule Pleroma.Web.Federator do refresh_subscriptions() end + @doc "Addresses [memory leaks on recursive replies fetching](https://git.pleroma.social/pleroma/pleroma/issues/161)" + # credo:disable-for-previous-line Credo.Check.Readability.MaxLineLength + def allowed_incoming_reply_depth?(depth) do + max_replies_depth = Pleroma.Config.get([:instance, :federation_incoming_replies_max_depth]) + + if max_replies_depth do + (depth || 1) <= max_replies_depth + else + true + end + end + # Client API def incoming_doc(doc) do diff --git a/lib/pleroma/web/ostatus/handlers/note_handler.ex b/lib/pleroma/web/ostatus/handlers/note_handler.ex index ec6e5cfaf..8e0adad91 100644 --- a/lib/pleroma/web/ostatus/handlers/note_handler.ex +++ b/lib/pleroma/web/ostatus/handlers/note_handler.ex @@ -10,6 +10,7 @@ defmodule Pleroma.Web.OStatus.NoteHandler do alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.CommonAPI + alias Pleroma.Web.Federator alias Pleroma.Web.OStatus alias Pleroma.Web.XML @@ -88,14 +89,15 @@ defmodule Pleroma.Web.OStatus.NoteHandler do Map.put(note, "external_url", url) end - def fetch_replied_to_activity(entry, in_reply_to) do + def fetch_replied_to_activity(entry, in_reply_to, options \\ []) do with %Activity{} = activity <- Activity.get_create_by_object_ap_id(in_reply_to) do activity else _e -> - with in_reply_to_href when not is_nil(in_reply_to_href) <- + with true <- Federator.allowed_incoming_reply_depth?(options[:depth]), + in_reply_to_href when not is_nil(in_reply_to_href) <- XML.string_from_xpath("//thr:in-reply-to[1]/@href", entry), - {:ok, [activity | _]} <- OStatus.fetch_activity_from_url(in_reply_to_href) do + {:ok, [activity | _]} <- OStatus.fetch_activity_from_url(in_reply_to_href, options) do activity else _e -> nil @@ -104,7 +106,7 @@ defmodule Pleroma.Web.OStatus.NoteHandler do end # TODO: Clean this up a bit. - def handle_note(entry, doc \\ nil) do + def handle_note(entry, doc \\ nil, options \\ []) do with id <- XML.string_from_xpath("//id", entry), activity when is_nil(activity) <- Activity.get_create_by_object_ap_id_with_object(id), [author] <- :xmerl_xpath.string('//author[1]', doc), @@ -112,7 +114,8 @@ defmodule Pleroma.Web.OStatus.NoteHandler do content_html <- OStatus.get_content(entry), cw <- OStatus.get_cw(entry), in_reply_to <- XML.string_from_xpath("//thr:in-reply-to[1]/@ref", entry), - in_reply_to_activity <- fetch_replied_to_activity(entry, in_reply_to), + options <- Keyword.put(options, :depth, (options[:depth] || 0) + 1), + in_reply_to_activity <- fetch_replied_to_activity(entry, in_reply_to, options), in_reply_to_object <- (in_reply_to_activity && Object.normalize(in_reply_to_activity)) || nil, in_reply_to <- (in_reply_to_object && in_reply_to_object.data["id"]) || in_reply_to, diff --git a/lib/pleroma/web/ostatus/ostatus.ex b/lib/pleroma/web/ostatus/ostatus.ex index 6ed089d84..502410c83 100644 --- a/lib/pleroma/web/ostatus/ostatus.ex +++ b/lib/pleroma/web/ostatus/ostatus.ex @@ -54,7 +54,7 @@ defmodule Pleroma.Web.OStatus do "#{Web.base_url()}/ostatus_subscribe?acct={uri}" end - def handle_incoming(xml_string) do + def handle_incoming(xml_string, options \\ []) do with doc when doc != :error <- parse_document(xml_string) do with {:ok, actor_user} <- find_make_or_update_user(doc), do: Pleroma.Instances.set_reachable(actor_user.ap_id) @@ -91,10 +91,12 @@ defmodule Pleroma.Web.OStatus do _ -> case object_type do 'http://activitystrea.ms/schema/1.0/note' -> - with {:ok, activity} <- NoteHandler.handle_note(entry, doc), do: activity + with {:ok, activity} <- NoteHandler.handle_note(entry, doc, options), + do: activity 'http://activitystrea.ms/schema/1.0/comment' -> - with {:ok, activity} <- NoteHandler.handle_note(entry, doc), do: activity + with {:ok, activity} <- NoteHandler.handle_note(entry, doc, options), + do: activity _ -> Logger.error("Couldn't parse incoming document") @@ -359,7 +361,7 @@ defmodule Pleroma.Web.OStatus do end end - def fetch_activity_from_atom_url(url) do + def fetch_activity_from_atom_url(url, options \\ []) do with true <- String.starts_with?(url, "http"), {:ok, %{body: body, status: code}} when code in 200..299 <- HTTP.get( @@ -367,7 +369,7 @@ defmodule Pleroma.Web.OStatus do [{:Accept, "application/atom+xml"}] ) do Logger.debug("Got document from #{url}, handling...") - handle_incoming(body) + handle_incoming(body, options) else e -> Logger.debug("Couldn't get #{url}: #{inspect(e)}") @@ -375,13 +377,13 @@ defmodule Pleroma.Web.OStatus do end end - def fetch_activity_from_html_url(url) do + def fetch_activity_from_html_url(url, options \\ []) do Logger.debug("Trying to fetch #{url}") with true <- String.starts_with?(url, "http"), {:ok, %{body: body}} <- HTTP.get(url, []), {:ok, atom_url} <- get_atom_url(body) do - fetch_activity_from_atom_url(atom_url) + fetch_activity_from_atom_url(atom_url, options) else e -> Logger.debug("Couldn't get #{url}: #{inspect(e)}") @@ -389,11 +391,11 @@ defmodule Pleroma.Web.OStatus do end end - def fetch_activity_from_url(url) do - with {:ok, [_ | _] = activities} <- fetch_activity_from_atom_url(url) do + def fetch_activity_from_url(url, options \\ []) do + with {:ok, [_ | _] = activities} <- fetch_activity_from_atom_url(url, options) do {:ok, activities} else - _e -> fetch_activity_from_html_url(url) + _e -> fetch_activity_from_html_url(url, options) end rescue e -> diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 68ec03c33..a914d3c4c 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -11,12 +11,13 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Transmogrifier + alias Pleroma.Web.CommonAPI alias Pleroma.Web.OStatus alias Pleroma.Web.Websub.WebsubClientSubscription + import Mock import Pleroma.Factory import ExUnit.CaptureLog - alias Pleroma.Web.CommonAPI setup_all do Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end) @@ -46,12 +47,10 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do data["object"] |> Map.put("inReplyTo", "https://shitposter.club/notice/2827873") - data = - data - |> Map.put("object", object) - + data = Map.put(data, "object", object) {:ok, returned_activity} = Transmogrifier.handle_incoming(data) - returned_object = Object.normalize(returned_activity.data["object"]) + + returned_object = Object.normalize(returned_activity.data["object"], false) assert activity = Activity.get_create_by_object_ap_id( @@ -61,6 +60,32 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert returned_object.data["inReplyToAtomUri"] == "https://shitposter.club/notice/2827873" end + test "it does not fetch replied-to activities beyond max_replies_depth" do + data = + File.read!("test/fixtures/mastodon-post-activity.json") + |> Poison.decode!() + + object = + data["object"] + |> Map.put("inReplyTo", "https://shitposter.club/notice/2827873") + + data = Map.put(data, "object", object) + + with_mock Pleroma.Web.Federator, + allowed_incoming_reply_depth?: fn _ -> false end do + {:ok, returned_activity} = Transmogrifier.handle_incoming(data) + + returned_object = Object.normalize(returned_activity.data["object"], false) + + refute Activity.get_create_by_object_ap_id( + "tag:shitposter.club,2017-05-05:noticeId=2827873:objectType=comment" + ) + + assert returned_object.data["inReplyToAtomUri"] == + "https://shitposter.club/notice/2827873" + end + end + test "it does not crash if the object in inReplyTo can't be fetched" do data = File.read!("test/fixtures/mastodon-post-activity.json") diff --git a/test/web/ostatus/ostatus_test.exs b/test/web/ostatus/ostatus_test.exs index f6be16862..acce33008 100644 --- a/test/web/ostatus/ostatus_test.exs +++ b/test/web/ostatus/ostatus_test.exs @@ -11,8 +11,10 @@ defmodule Pleroma.Web.OStatusTest do alias Pleroma.User alias Pleroma.Web.OStatus alias Pleroma.Web.XML - import Pleroma.Factory + import ExUnit.CaptureLog + import Mock + import Pleroma.Factory setup_all do Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end) @@ -266,10 +268,13 @@ defmodule Pleroma.Web.OStatusTest do assert favorited_activity.local end - test "handle incoming replies" do + test_with_mock "handle incoming replies, fetching replied-to activities if we don't have them", + OStatus, + [:passthrough], + [] do incoming = File.read!("test/fixtures/incoming_note_activity_answer.xml") {:ok, [activity]} = OStatus.handle_incoming(incoming) - object = Object.normalize(activity.data["object"]) + object = Object.normalize(activity.data["object"], false) assert activity.data["type"] == "Create" assert object.data["type"] == "Note" @@ -282,6 +287,23 @@ defmodule Pleroma.Web.OStatusTest do assert object.data["id"] == "tag:gs.example.org:4040,2017-04-25:noticeId=55:objectType=note" assert "https://www.w3.org/ns/activitystreams#Public" in activity.data["to"] + + assert called(OStatus.fetch_activity_from_url(object.data["inReplyTo"], :_)) + end + + test_with_mock "handle incoming replies, not fetching replied-to activities beyond max_replies_depth", + OStatus, + [:passthrough], + [] do + incoming = File.read!("test/fixtures/incoming_note_activity_answer.xml") + + with_mock Pleroma.Web.Federator, + allowed_incoming_reply_depth?: fn _ -> false end do + {:ok, [activity]} = OStatus.handle_incoming(incoming) + object = Object.normalize(activity.data["object"], false) + + refute called(OStatus.fetch_activity_from_url(object.data["inReplyTo"], :_)) + end end test "handle incoming follows" do |