From 2b9d914089755297f6ac24ffbb81934cf3c70cdd Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 30 Jun 2019 15:58:50 +0300 Subject: [PATCH] [#161] Refactoring, documentation. --- CHANGELOG.md | 1 + config/config.exs | 1 + docs/config.md | 1 + lib/pleroma/web/activity_pub/transmogrifier.ex | 6 ++---- lib/pleroma/web/federator/federator.ex | 12 +++++++++--- lib/pleroma/web/ostatus/handlers/note_handler.ex | 2 +- test/web/activity_pub/transmogrifier_test.exs | 2 +- test/web/ostatus/ostatus_test.exs | 2 +- 8 files changed, 17 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6ec8674d..85d077e3f 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`) +- Federation: Support for restricting max. reply-to depth on fetching ## [1.0.0] - 2019-06-29 ### Security 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 feef43ba9..b40147481 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 memory leakage on extremely nested incoming threads). If set to `nil`, threads of any depth will be fetched. * `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/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index d5ced2d1d..543d4bb7d 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -187,7 +187,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do object = Map.put(object, "inReplyToAtomUri", in_reply_to_id) - if (options[:depth] || 1) <= Federator.max_replies_depth() do + 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 <- @@ -349,10 +349,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do def fix_type(%{"inReplyTo" => reply_id} = object, options) when is_binary(reply_id) do reply = - if (options[:depth] || 1) <= Federator.max_replies_depth() do + if Federator.allowed_incoming_reply_depth?(options[:depth]) do Object.normalize(reply_id, true) - else - nil end if reply && (reply.data["type"] == "Question" and object["name"]) do diff --git a/lib/pleroma/web/federator/federator.ex b/lib/pleroma/web/federator/federator.ex index 7c13ff323..f4f9e83e0 100644 --- a/lib/pleroma/web/federator/federator.ex +++ b/lib/pleroma/web/federator/federator.ex @@ -22,11 +22,17 @@ defmodule Pleroma.Web.Federator do refresh_subscriptions() end - @max_replies_depth 100 - @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 max_replies_depth, do: @max_replies_depth + 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 diff --git a/lib/pleroma/web/ostatus/handlers/note_handler.ex b/lib/pleroma/web/ostatus/handlers/note_handler.ex index 6f8f3ddcb..8e0adad91 100644 --- a/lib/pleroma/web/ostatus/handlers/note_handler.ex +++ b/lib/pleroma/web/ostatus/handlers/note_handler.ex @@ -94,7 +94,7 @@ defmodule Pleroma.Web.OStatus.NoteHandler do activity else _e -> - with true <- (options[:depth] || 1) <= Federator.max_replies_depth(), + 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, options) do diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index fc8703ca9..a914d3c4c 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -72,7 +72,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do data = Map.put(data, "object", object) with_mock Pleroma.Web.Federator, - max_replies_depth: fn -> 0 end do + allowed_incoming_reply_depth?: fn _ -> false end do {:ok, returned_activity} = Transmogrifier.handle_incoming(data) returned_object = Object.normalize(returned_activity.data["object"], false) diff --git a/test/web/ostatus/ostatus_test.exs b/test/web/ostatus/ostatus_test.exs index 12627356c..acce33008 100644 --- a/test/web/ostatus/ostatus_test.exs +++ b/test/web/ostatus/ostatus_test.exs @@ -298,7 +298,7 @@ defmodule Pleroma.Web.OStatusTest do incoming = File.read!("test/fixtures/incoming_note_activity_answer.xml") with_mock Pleroma.Web.Federator, - max_replies_depth: fn -> 0 end do + allowed_incoming_reply_depth?: fn _ -> false end do {:ok, [activity]} = OStatus.handle_incoming(incoming) object = Object.normalize(activity.data["object"], false)