From 9eea80002673eb1359a2d4369c65a89c42c2f707 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 28 May 2020 10:16:09 -0500 Subject: Refactor notification settings --- lib/pleroma/notification.ex | 29 +++++++++-------------------- lib/pleroma/user/notification_setting.ex | 14 ++++++-------- lib/pleroma/web/api_spec/schemas/account.ex | 14 ++++++-------- 3 files changed, 21 insertions(+), 36 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 7eca55ac9..ca556f0bb 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -459,10 +459,9 @@ defmodule Pleroma.Notification do def skip?(%Activity{} = activity, %User{} = user) do [ :self, - :followers, - :follows, - :non_followers, - :non_follows, + :from_followers, + :from_following, + :from_strangers, :recently_followed ] |> Enum.find(&skip?(&1, activity, user)) @@ -476,9 +475,9 @@ defmodule Pleroma.Notification do end def skip?( - :followers, + :from_followers, %Activity{} = activity, - %User{notification_settings: %{followers: false}} = user + %User{notification_settings: %{from_followers: false}} = user ) do actor = activity.data["actor"] follower = User.get_cached_by_ap_id(actor) @@ -486,9 +485,9 @@ defmodule Pleroma.Notification do end def skip?( - :non_followers, + :from_strangers, %Activity{} = activity, - %User{notification_settings: %{non_followers: false}} = user + %User{notification_settings: %{from_strangers: false}} = user ) do actor = activity.data["actor"] follower = User.get_cached_by_ap_id(actor) @@ -496,25 +495,15 @@ defmodule Pleroma.Notification do end def skip?( - :follows, + :from_following, %Activity{} = activity, - %User{notification_settings: %{follows: false}} = user + %User{notification_settings: %{from_following: false}} = user ) do actor = activity.data["actor"] followed = User.get_cached_by_ap_id(actor) User.following?(user, followed) end - def skip?( - :non_follows, - %Activity{} = activity, - %User{notification_settings: %{non_follows: false}} = user - ) do - actor = activity.data["actor"] - followed = User.get_cached_by_ap_id(actor) - !User.following?(user, followed) - end - # To do: consider defining recency in hours and checking FollowingRelationship with a single SQL def skip?(:recently_followed, %Activity{data: %{"type" => "Follow"}} = activity, %User{} = user) do actor = activity.data["actor"] diff --git a/lib/pleroma/user/notification_setting.ex b/lib/pleroma/user/notification_setting.ex index 4bd55e139..e47ac4cab 100644 --- a/lib/pleroma/user/notification_setting.ex +++ b/lib/pleroma/user/notification_setting.ex @@ -10,20 +10,18 @@ defmodule Pleroma.User.NotificationSetting do @primary_key false embedded_schema do - field(:followers, :boolean, default: true) - field(:follows, :boolean, default: true) - field(:non_follows, :boolean, default: true) - field(:non_followers, :boolean, default: true) + field(:from_followers, :boolean, default: true) + field(:from_following, :boolean, default: true) + field(:from_strangers, :boolean, default: true) field(:privacy_option, :boolean, default: false) end def changeset(schema, params) do schema |> cast(prepare_attrs(params), [ - :followers, - :follows, - :non_follows, - :non_followers, + :from_followers, + :from_following, + :from_strangers, :privacy_option ]) end diff --git a/lib/pleroma/web/api_spec/schemas/account.ex b/lib/pleroma/web/api_spec/schemas/account.ex index d54e2158d..ed90ef3db 100644 --- a/lib/pleroma/web/api_spec/schemas/account.ex +++ b/lib/pleroma/web/api_spec/schemas/account.ex @@ -57,10 +57,9 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Account do notification_settings: %Schema{ type: :object, properties: %{ - followers: %Schema{type: :boolean}, - follows: %Schema{type: :boolean}, - non_followers: %Schema{type: :boolean}, - non_follows: %Schema{type: :boolean}, + from_followers: %Schema{type: :boolean}, + from_following: %Schema{type: :boolean}, + from_strangers: %Schema{type: :boolean}, privacy_option: %Schema{type: :boolean} } }, @@ -123,10 +122,9 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Account do "unread_conversation_count" => 0, "tags" => [], "notification_settings" => %{ - "followers" => true, - "follows" => true, - "non_followers" => true, - "non_follows" => true, + "from_followers" => true, + "from_following" => true, + "from_strangers" => true, "privacy_option" => false }, "relationship" => %{ -- cgit v1.2.3 From fd5e797379155e5a85deb88dc79f8fbca483948e Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 26 Jun 2020 11:24:28 -0500 Subject: Simplify notification filtering settings further --- lib/pleroma/notification.ex | 28 +++------------------------- lib/pleroma/user/notification_setting.ex | 8 ++------ lib/pleroma/web/api_spec/schemas/account.ex | 8 ++------ 3 files changed, 7 insertions(+), 37 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 9d09cf082..8a28a1821 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -550,9 +550,7 @@ defmodule Pleroma.Notification do [ :self, :invisible, - :from_followers, - :from_following, - :from_strangers, + :block_from_strangers, :recently_followed ] |> Enum.find(&skip?(&1, activity, user)) @@ -572,35 +570,15 @@ defmodule Pleroma.Notification do end def skip?( - :from_followers, + :block_from_strangers, %Activity{} = activity, - %User{notification_settings: %{from_followers: false}} = user - ) do - actor = activity.data["actor"] - follower = User.get_cached_by_ap_id(actor) - User.following?(follower, user) - end - - def skip?( - :from_strangers, - %Activity{} = activity, - %User{notification_settings: %{from_strangers: false}} = user + %User{notification_settings: %{block_from_strangers: true}} = user ) do actor = activity.data["actor"] follower = User.get_cached_by_ap_id(actor) !User.following?(follower, user) end - def skip?( - :from_following, - %Activity{} = activity, - %User{notification_settings: %{from_following: false}} = user - ) do - actor = activity.data["actor"] - followed = User.get_cached_by_ap_id(actor) - User.following?(user, followed) - end - # To do: consider defining recency in hours and checking FollowingRelationship with a single SQL def skip?(:recently_followed, %Activity{data: %{"type" => "Follow"}} = activity, %User{} = user) do actor = activity.data["actor"] diff --git a/lib/pleroma/user/notification_setting.ex b/lib/pleroma/user/notification_setting.ex index e47ac4cab..ffe9860de 100644 --- a/lib/pleroma/user/notification_setting.ex +++ b/lib/pleroma/user/notification_setting.ex @@ -10,18 +10,14 @@ defmodule Pleroma.User.NotificationSetting do @primary_key false embedded_schema do - field(:from_followers, :boolean, default: true) - field(:from_following, :boolean, default: true) - field(:from_strangers, :boolean, default: true) + field(:block_from_strangers, :boolean, default: false) field(:privacy_option, :boolean, default: false) end def changeset(schema, params) do schema |> cast(prepare_attrs(params), [ - :from_followers, - :from_following, - :from_strangers, + :block_from_strangers, :privacy_option ]) end diff --git a/lib/pleroma/web/api_spec/schemas/account.ex b/lib/pleroma/web/api_spec/schemas/account.ex index ed90ef3db..91bb1ba88 100644 --- a/lib/pleroma/web/api_spec/schemas/account.ex +++ b/lib/pleroma/web/api_spec/schemas/account.ex @@ -57,9 +57,7 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Account do notification_settings: %Schema{ type: :object, properties: %{ - from_followers: %Schema{type: :boolean}, - from_following: %Schema{type: :boolean}, - from_strangers: %Schema{type: :boolean}, + block_from_strangers: %Schema{type: :boolean}, privacy_option: %Schema{type: :boolean} } }, @@ -122,9 +120,7 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Account do "unread_conversation_count" => 0, "tags" => [], "notification_settings" => %{ - "from_followers" => true, - "from_following" => true, - "from_strangers" => true, + "block_from_strangers" => false, "privacy_option" => false }, "relationship" => %{ -- cgit v1.2.3 From 69848d5c97c9e5d4c14fb5613eb174cb81d5026d Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 26 Jun 2020 12:45:46 -0500 Subject: Rename notification "privacy_option" setting --- lib/mix/tasks/pleroma/notification_settings.ex | 18 +++++++++--------- lib/pleroma/user/notification_setting.ex | 4 ++-- lib/pleroma/web/api_spec/schemas/account.ex | 4 ++-- lib/pleroma/web/push/impl.ex | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) (limited to 'lib') diff --git a/lib/mix/tasks/pleroma/notification_settings.ex b/lib/mix/tasks/pleroma/notification_settings.ex index 7d65f0587..00f5ba7bf 100644 --- a/lib/mix/tasks/pleroma/notification_settings.ex +++ b/lib/mix/tasks/pleroma/notification_settings.ex @@ -3,8 +3,8 @@ defmodule Mix.Tasks.Pleroma.NotificationSettings do @moduledoc """ Example: - > mix pleroma.notification_settings --privacy-option=false --nickname-users="parallel588" # set false only for parallel588 user - > mix pleroma.notification_settings --privacy-option=true # set true for all users + > mix pleroma.notification_settings --hide-notification-contents=false --nickname-users="parallel588" # set false only for parallel588 user + > mix pleroma.notification_settings --hide-notification-contents=true # set true for all users """ @@ -19,16 +19,16 @@ defmodule Mix.Tasks.Pleroma.NotificationSettings do OptionParser.parse( args, strict: [ - privacy_option: :boolean, + hide_notification_contents: :boolean, email_users: :string, nickname_users: :string ] ) - privacy_option = Keyword.get(options, :privacy_option) + hide_notification_contents = Keyword.get(options, :hide_notification_contents) - if not is_nil(privacy_option) do - privacy_option + if not is_nil(hide_notification_contents) do + hide_notification_contents |> build_query(options) |> Pleroma.Repo.update_all([]) end @@ -36,15 +36,15 @@ defmodule Mix.Tasks.Pleroma.NotificationSettings do shell_info("Done") end - defp build_query(privacy_option, options) do + defp build_query(hide_notification_contents, options) do query = from(u in Pleroma.User, update: [ set: [ notification_settings: fragment( - "jsonb_set(notification_settings, '{privacy_option}', ?)", - ^privacy_option + "jsonb_set(notification_settings, '{hide_notification_contents}', ?)", + ^hide_notification_contents ) ] ] diff --git a/lib/pleroma/user/notification_setting.ex b/lib/pleroma/user/notification_setting.ex index ffe9860de..7d9e8a000 100644 --- a/lib/pleroma/user/notification_setting.ex +++ b/lib/pleroma/user/notification_setting.ex @@ -11,14 +11,14 @@ defmodule Pleroma.User.NotificationSetting do embedded_schema do field(:block_from_strangers, :boolean, default: false) - field(:privacy_option, :boolean, default: false) + field(:hide_notification_contents, :boolean, default: false) end def changeset(schema, params) do schema |> cast(prepare_attrs(params), [ :block_from_strangers, - :privacy_option + :hide_notification_contents ]) end diff --git a/lib/pleroma/web/api_spec/schemas/account.ex b/lib/pleroma/web/api_spec/schemas/account.ex index 91bb1ba88..71d402b18 100644 --- a/lib/pleroma/web/api_spec/schemas/account.ex +++ b/lib/pleroma/web/api_spec/schemas/account.ex @@ -58,7 +58,7 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Account do type: :object, properties: %{ block_from_strangers: %Schema{type: :boolean}, - privacy_option: %Schema{type: :boolean} + hide_notification_contents: %Schema{type: :boolean} } }, relationship: AccountRelationship, @@ -121,7 +121,7 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Account do "tags" => [], "notification_settings" => %{ "block_from_strangers" => false, - "privacy_option" => false + "hide_notification_contents" => false }, "relationship" => %{ "blocked_by" => false, diff --git a/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex index cdb827e76..16368485e 100644 --- a/lib/pleroma/web/push/impl.ex +++ b/lib/pleroma/web/push/impl.ex @@ -104,7 +104,7 @@ defmodule Pleroma.Web.Push.Impl do def build_content( %{ - user: %{notification_settings: %{privacy_option: true}} + user: %{notification_settings: %{hide_notification_contents: true}} } = notification, _actor, _object, -- cgit v1.2.3 From b3764423251c963a5ca007517189f556bfe95155 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Sat, 11 Jul 2020 10:36:36 +0300 Subject: MediaProxy whitelist setting now supports hosts with scheme added deprecation warning about using bare domains --- lib/pleroma/config/deprecation_warnings.ex | 15 +++++++++- lib/pleroma/plugs/http_security_plug.ex | 47 ++++++++++++++++++++---------- lib/pleroma/web/media_proxy/media_proxy.ex | 26 ++++++++++------- 3 files changed, 62 insertions(+), 26 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/config/deprecation_warnings.ex b/lib/pleroma/config/deprecation_warnings.ex index 0a6c724fb..026871c4f 100644 --- a/lib/pleroma/config/deprecation_warnings.ex +++ b/lib/pleroma/config/deprecation_warnings.ex @@ -54,6 +54,7 @@ defmodule Pleroma.Config.DeprecationWarnings do check_hellthread_threshold() mrf_user_allowlist() check_old_mrf_config() + check_media_proxy_whitelist_config() end def check_old_mrf_config do @@ -65,7 +66,7 @@ defmodule Pleroma.Config.DeprecationWarnings do move_namespace_and_warn(@mrf_config_map, warning_preface) end - @spec move_namespace_and_warn([config_map()], String.t()) :: :ok + @spec move_namespace_and_warn([config_map()], String.t()) :: :ok | nil def move_namespace_and_warn(config_map, warning_preface) do warning = Enum.reduce(config_map, "", fn @@ -84,4 +85,16 @@ defmodule Pleroma.Config.DeprecationWarnings do Logger.warn(warning_preface <> warning) end end + + @spec check_media_proxy_whitelist_config() :: :ok | nil + def check_media_proxy_whitelist_config do + whitelist = Config.get([:media_proxy, :whitelist]) + + if Enum.any?(whitelist, &(not String.starts_with?(&1, "http"))) do + Logger.warn(""" + !!!DEPRECATION WARNING!!! + Your config is using old format (only domain) for MediaProxy whitelist option. Setting should work for now, but you are advised to change format to scheme with port to prevent possible issues later. + """) + end + end end diff --git a/lib/pleroma/plugs/http_security_plug.ex b/lib/pleroma/plugs/http_security_plug.ex index 7d65cf078..c363b193b 100644 --- a/lib/pleroma/plugs/http_security_plug.ex +++ b/lib/pleroma/plugs/http_security_plug.ex @@ -108,31 +108,48 @@ defmodule Pleroma.Plugs.HTTPSecurityPlug do |> :erlang.iolist_to_binary() end - defp build_csp_multimedia_source_list do - media_proxy_whitelist = - Enum.reduce(Config.get([:media_proxy, :whitelist]), [], fn host, acc -> - add_source(acc, host) - end) + defp build_csp_from_whitelist([], acc), do: acc - media_proxy_base_url = build_csp_param(Config.get([:media_proxy, :base_url])) + defp build_csp_from_whitelist([last], acc) do + [build_csp_param_from_whitelist(last) | acc] + end - upload_base_url = build_csp_param(Config.get([Pleroma.Upload, :base_url])) + defp build_csp_from_whitelist([head | tail], acc) do + build_csp_from_whitelist(tail, [[?\s, build_csp_param_from_whitelist(head)] | acc]) + end - s3_endpoint = build_csp_param(Config.get([Pleroma.Uploaders.S3, :public_endpoint])) + # TODO: use `build_csp_param/1` after removing support bare domains for media proxy whitelist + defp build_csp_param_from_whitelist("http" <> _ = url) do + build_csp_param(url) + end - captcha_method = Config.get([Pleroma.Captcha, :method]) + defp build_csp_param_from_whitelist(url), do: url - captcha_endpoint = build_csp_param(Config.get([captcha_method, :endpoint])) + defp build_csp_multimedia_source_list do + media_proxy_whitelist = + [:media_proxy, :whitelist] + |> Config.get() + |> build_csp_from_whitelist([]) - [] - |> add_source(media_proxy_base_url) - |> add_source(upload_base_url) - |> add_source(s3_endpoint) + captcha_method = Config.get([Pleroma.Captcha, :method]) + captcha_endpoint = Config.get([captcha_method, :endpoint]) + + base_endpoints = + [ + [:media_proxy, :base_url], + [Pleroma.Upload, :base_url], + [Pleroma.Uploaders.S3, :public_endpoint] + ] + |> Enum.map(&Config.get/1) + + [captcha_endpoint | base_endpoints] + |> Enum.map(&build_csp_param/1) + |> Enum.reduce([], &add_source(&2, &1)) |> add_source(media_proxy_whitelist) - |> add_source(captcha_endpoint) end defp add_source(iodata, nil), do: iodata + defp add_source(iodata, []), do: iodata defp add_source(iodata, source), do: [[?\s, source] | iodata] defp add_csp_param(csp_iodata, nil), do: csp_iodata diff --git a/lib/pleroma/web/media_proxy/media_proxy.ex b/lib/pleroma/web/media_proxy/media_proxy.ex index 6f35826da..dfbfcea6b 100644 --- a/lib/pleroma/web/media_proxy/media_proxy.ex +++ b/lib/pleroma/web/media_proxy/media_proxy.ex @@ -60,22 +60,28 @@ defmodule Pleroma.Web.MediaProxy do defp whitelisted?(url) do %{host: domain} = URI.parse(url) - mediaproxy_whitelist = Config.get([:media_proxy, :whitelist]) - - upload_base_url_domain = - if !is_nil(Config.get([Upload, :base_url])) do - [URI.parse(Config.get([Upload, :base_url])).host] + mediaproxy_whitelist_domains = + [:media_proxy, :whitelist] + |> Config.get() + |> Enum.map(&maybe_get_domain_from_url/1) + + whitelist_domains = + if base_url = Config.get([Upload, :base_url]) do + %{host: base_domain} = URI.parse(base_url) + [base_domain | mediaproxy_whitelist_domains] else - [] + mediaproxy_whitelist_domains end - whitelist = mediaproxy_whitelist ++ upload_base_url_domain + domain in whitelist_domains + end - Enum.any?(whitelist, fn pattern -> - String.equivalent?(domain, pattern) - end) + defp maybe_get_domain_from_url("http" <> _ = url) do + URI.parse(url).host end + defp maybe_get_domain_from_url(domain), do: domain + def encode_url(url) do base64 = Base.url_encode64(url, @base64_opts) -- cgit v1.2.3 From b221b640a2dd443e3c2274b16ed5b62566329d09 Mon Sep 17 00:00:00 2001 From: = <=> Date: Mon, 13 Jul 2020 22:19:13 +0300 Subject: Transmogrifier: filtering weirdness in address fields --- lib/pleroma/web/activity_pub/transmogrifier.ex | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 884646ceb..f37bcab3e 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -62,15 +62,17 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do def fix_summary(object), do: Map.put(object, "summary", "") def fix_addressing_list(map, field) do + addrs = map[field] + cond do - is_binary(map[field]) -> - Map.put(map, field, [map[field]]) + is_list(addrs) -> + Map.put(map, field, Enum.filter(addrs, &is_binary/1)) - is_nil(map[field]) -> - Map.put(map, field, []) + is_binary(addrs) -> + Map.put(map, field, [addrs]) true -> - map + Map.put(map, field, []) end end -- cgit v1.2.3 From cf3f8cb72a46f0c8c798d4022cff442fae4ab401 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 19 Jul 2020 21:35:57 +0300 Subject: [#1940] Reinstated OAuth-less `admin_token` authentication. Refactored UserIsAdminPlug (freed from checking admin scopes presence). --- .../plugs/admin_secret_authentication_plug.ex | 12 +++++++++-- lib/pleroma/plugs/user_is_admin_plug.ex | 25 +++------------------- 2 files changed, 13 insertions(+), 24 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/admin_secret_authentication_plug.ex b/lib/pleroma/plugs/admin_secret_authentication_plug.ex index b4b47a31f..ff0328d4a 100644 --- a/lib/pleroma/plugs/admin_secret_authentication_plug.ex +++ b/lib/pleroma/plugs/admin_secret_authentication_plug.ex @@ -4,7 +4,9 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do import Plug.Conn + alias Pleroma.User + alias Pleroma.Plugs.OAuthScopesPlug def init(options) do options @@ -26,7 +28,7 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do def authenticate(%{params: %{"admin_token" => admin_token}} = conn) do if admin_token == secret_token() do - assign(conn, :user, %User{is_admin: true}) + assign_admin_user(conn) else conn end @@ -36,8 +38,14 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do token = secret_token() case get_req_header(conn, "x-admin-token") do - [^token] -> assign(conn, :user, %User{is_admin: true}) + [^token] -> assign_admin_user(conn) _ -> conn end end + + defp assign_admin_user(conn) do + conn + |> assign(:user, %User{is_admin: true}) + |> OAuthScopesPlug.skip_plug() + end end diff --git a/lib/pleroma/plugs/user_is_admin_plug.ex b/lib/pleroma/plugs/user_is_admin_plug.ex index 2748102df..488a61d1d 100644 --- a/lib/pleroma/plugs/user_is_admin_plug.ex +++ b/lib/pleroma/plugs/user_is_admin_plug.ex @@ -7,37 +7,18 @@ defmodule Pleroma.Plugs.UserIsAdminPlug do import Plug.Conn alias Pleroma.User - alias Pleroma.Web.OAuth def init(options) do options end - def call(%{assigns: %{user: %User{is_admin: true}} = assigns} = conn, _) do - token = assigns[:token] - - cond do - not Pleroma.Config.enforce_oauth_admin_scope_usage?() -> - conn - - token && OAuth.Scopes.contains_admin_scopes?(token.scopes) -> - # Note: checking for _any_ admin scope presence, not necessarily fitting requested action. - # Thus, controller must explicitly invoke OAuthScopesPlug to verify scope requirements. - # Admin might opt out of admin scope for some apps to block any admin actions from them. - conn - - true -> - fail(conn) - end + def call(%{assigns: %{user: %User{is_admin: true}}} = conn, _) do + conn end def call(conn, _) do - fail(conn) - end - - defp fail(conn) do conn - |> render_error(:forbidden, "User is not an admin or OAuth admin scope is not granted.") + |> render_error(:forbidden, "User is not an admin.") |> halt() end end -- cgit v1.2.3 From 9b225db7d86289fb9d9c51f62e6ec29f6c07f60d Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 14 Jul 2020 11:58:41 +0300 Subject: [#1940] Applied rate limit for requests with bad `admin_token`. Added doc warnings on `admin_token` setting. --- lib/pleroma/plugs/admin_secret_authentication_plug.ex | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/admin_secret_authentication_plug.ex b/lib/pleroma/plugs/admin_secret_authentication_plug.ex index ff0328d4a..2e54df47a 100644 --- a/lib/pleroma/plugs/admin_secret_authentication_plug.ex +++ b/lib/pleroma/plugs/admin_secret_authentication_plug.ex @@ -5,15 +5,19 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do import Plug.Conn - alias Pleroma.User alias Pleroma.Plugs.OAuthScopesPlug + alias Pleroma.Plugs.RateLimiter + alias Pleroma.User def init(options) do options end def secret_token do - Pleroma.Config.get(:admin_token) + case Pleroma.Config.get(:admin_token) do + blank when blank in [nil, ""] -> nil + token -> token + end end def call(%{assigns: %{user: %User{}}} = conn, _), do: conn @@ -30,7 +34,7 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do if admin_token == secret_token() do assign_admin_user(conn) else - conn + handle_bad_token(conn) end end @@ -38,8 +42,9 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do token = secret_token() case get_req_header(conn, "x-admin-token") do + blank when blank in [[], [""]] -> conn [^token] -> assign_admin_user(conn) - _ -> conn + _ -> handle_bad_token(conn) end end @@ -48,4 +53,8 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do |> assign(:user, %User{is_admin: true}) |> OAuthScopesPlug.skip_plug() end + + defp handle_bad_token(conn) do + RateLimiter.call(conn, name: :authentication) + end end -- cgit v1.2.3 From 858d9fc7e8e722604676c90cf2707f0209f935ec Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Mon, 13 Jul 2020 15:47:13 +0200 Subject: MRF Policies: Return a {:reject, reason} instead of {:reject, nil} --- .../web/activity_pub/mrf/anti_followbot_policy.ex | 2 +- .../web/activity_pub/mrf/anti_link_spam_policy.ex | 7 +++---- lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex | 4 ++-- lib/pleroma/web/activity_pub/mrf/keyword_policy.ex | 7 ++++--- lib/pleroma/web/activity_pub/mrf/mention_policy.ex | 5 +++-- lib/pleroma/web/activity_pub/mrf/object_age_policy.ex | 8 +++----- lib/pleroma/web/activity_pub/mrf/reject_non_public.ex | 2 +- lib/pleroma/web/activity_pub/mrf/simple_policy.ex | 16 ++++++++++------ lib/pleroma/web/activity_pub/mrf/tag_policy.ex | 7 ++++--- .../web/activity_pub/mrf/user_allow_list_policy.ex | 2 +- lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex | 18 +++++++++++------- 11 files changed, 43 insertions(+), 35 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex b/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex index 0270b96ae..b96388489 100644 --- a/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex @@ -60,7 +60,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.AntiFollowbotPolicy do if score < 0.8 do {:ok, message} else - {:reject, nil} + {:reject, "[AntiFollowbotPolicy] Scored #{actor_id} as #{score}"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex b/lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex index a7e187b5e..b22464111 100644 --- a/lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex @@ -39,14 +39,13 @@ defmodule Pleroma.Web.ActivityPub.MRF.AntiLinkSpamPolicy do {:ok, message} {:old_user, false} -> - {:reject, nil} + {:reject, "[AntiLinkSpamPolicy] User has no posts nor followers"} {:error, _} -> - {:reject, nil} + {:reject, "[AntiLinkSpamPolicy] Failed to get or fetch user by ap_id"} e -> - Logger.warn("[MRF anti-link-spam] WTF: unhandled error #{inspect(e)}") - {:reject, nil} + {:reject, "[AntiLinkSpamPolicy] Unhandled error #{inspect(e)}"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex index f6b2c4415..9ba07b4e3 100644 --- a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex @@ -43,7 +43,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do defp reject_message(message, threshold) when threshold > 0 do with {_, recipients} <- get_recipient_count(message) do if recipients > threshold do - {:reject, nil} + {:reject, "[HellthreadPolicy] #{recipients} recipients is over the limit of #{threshold}"} else {:ok, message} end @@ -87,7 +87,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do {:ok, message} <- delist_message(message, delist_threshold) do {:ok, message} else - _e -> {:reject, nil} + e -> e end end diff --git a/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex b/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex index 88b0d2b39..15e09dcf0 100644 --- a/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex @@ -24,7 +24,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.KeywordPolicy do if Enum.any?(Pleroma.Config.get([:mrf_keyword, :reject]), fn pattern -> string_matches?(content, pattern) or string_matches?(summary, pattern) end) do - {:reject, nil} + {:reject, "[KeywordPolicy] Matches with rejected keyword"} else {:ok, message} end @@ -89,8 +89,9 @@ defmodule Pleroma.Web.ActivityPub.MRF.KeywordPolicy do {:ok, message} <- check_replace(message) do {:ok, message} else - _e -> - {:reject, nil} + {:reject, nil} -> {:reject, "[KeywordPolicy] "} + {:reject, _} = e -> e + _e -> {:reject, "[KeywordPolicy] "} end end diff --git a/lib/pleroma/web/activity_pub/mrf/mention_policy.ex b/lib/pleroma/web/activity_pub/mrf/mention_policy.ex index 06f003921..7910ca131 100644 --- a/lib/pleroma/web/activity_pub/mrf/mention_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/mention_policy.ex @@ -12,8 +12,9 @@ defmodule Pleroma.Web.ActivityPub.MRF.MentionPolicy do reject_actors = Pleroma.Config.get([:mrf_mention, :actors], []) recipients = (message["to"] || []) ++ (message["cc"] || []) - if Enum.any?(recipients, fn recipient -> Enum.member?(reject_actors, recipient) end) do - {:reject, nil} + if rejected_mention = + Enum.find(recipients, fn recipient -> Enum.member?(reject_actors, recipient) end) do + {:reject, "[MentionPolicy] Rejected for mention of #{rejected_mention}"} else {:ok, message} end diff --git a/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex b/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex index a62914135..5f111c72f 100644 --- a/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex @@ -28,7 +28,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy do defp check_reject(message, actions) do if :reject in actions do - {:reject, nil} + {:reject, "[ObjectAgePolicy]"} else {:ok, message} end @@ -47,9 +47,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy do {:ok, message} else - # Unhandleable error: somebody is messing around, just drop the message. _e -> - {:reject, nil} + {:reject, "[ObjectAgePolicy] Unhandled error"} end else {:ok, message} @@ -69,9 +68,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy do {:ok, message} else - # Unhandleable error: somebody is messing around, just drop the message. _e -> - {:reject, nil} + {:reject, "[ObjectAgePolicy] Unhandled error"} end else {:ok, message} diff --git a/lib/pleroma/web/activity_pub/mrf/reject_non_public.ex b/lib/pleroma/web/activity_pub/mrf/reject_non_public.ex index 4fd63106d..0b9ed2224 100644 --- a/lib/pleroma/web/activity_pub/mrf/reject_non_public.ex +++ b/lib/pleroma/web/activity_pub/mrf/reject_non_public.ex @@ -38,7 +38,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.RejectNonPublic do {:ok, object} true -> - {:reject, nil} + {:reject, "[RejectNonPublic] visibility: #{visibility}"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex index 70a2ca053..b77b8c7b4 100644 --- a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex @@ -21,7 +21,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicy do accepts == [] -> {:ok, object} actor_host == Config.get([Pleroma.Web.Endpoint, :url, :host]) -> {:ok, object} MRF.subdomain_match?(accepts, actor_host) -> {:ok, object} - true -> {:reject, nil} + true -> {:reject, "[SimplePolicy] host not in accept list"} end end @@ -31,7 +31,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicy do |> MRF.subdomains_regex() if MRF.subdomain_match?(rejects, actor_host) do - {:reject, nil} + {:reject, "[SimplePolicy] host in reject list"} else {:ok, object} end @@ -114,7 +114,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicy do |> MRF.subdomains_regex() if MRF.subdomain_match?(report_removal, actor_host) do - {:reject, nil} + {:reject, "[SimplePolicy] host in report_removal list"} else {:ok, object} end @@ -159,7 +159,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicy do |> MRF.subdomains_regex() if MRF.subdomain_match?(reject_deletes, actor_host) do - {:reject, nil} + {:reject, "[SimplePolicy] host in reject_deletes list"} else {:ok, object} end @@ -177,7 +177,9 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicy do {:ok, object} <- check_report_removal(actor_info, object) do {:ok, object} else - _e -> {:reject, nil} + {:reject, nil} -> {:reject, "[SimplePolicy]"} + {:reject, _} = e -> e + _ -> {:reject, "[SimplePolicy]"} end end @@ -191,7 +193,9 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicy do {:ok, object} <- check_banner_removal(actor_info, object) do {:ok, object} else - _e -> {:reject, nil} + {:reject, nil} -> {:reject, "[SimplePolicy]"} + {:reject, _} = e -> e + _ -> {:reject, "[SimplePolicy]"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/tag_policy.ex b/lib/pleroma/web/activity_pub/mrf/tag_policy.ex index c310462cb..febabda08 100644 --- a/lib/pleroma/web/activity_pub/mrf/tag_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/tag_policy.ex @@ -134,12 +134,13 @@ defmodule Pleroma.Web.ActivityPub.MRF.TagPolicy do if user.local == true do {:ok, message} else - {:reject, nil} + {:reject, + "[TagPolicy] Follow from #{actor} tagged with mrf_tag:disable-remote-subscription"} end end - defp process_tag("mrf_tag:disable-any-subscription", %{"type" => "Follow"}), - do: {:reject, nil} + defp process_tag("mrf_tag:disable-any-subscription", %{"type" => "Follow", "actor" => actor}), + do: {:reject, "[TagPolicy] Follow from #{actor} tagged with mrf_tag:disable-any-subscription"} defp process_tag(_, message), do: {:ok, message} diff --git a/lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex b/lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex index 651aed70f..1a28f2ba2 100644 --- a/lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex @@ -14,7 +14,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.UserAllowListPolicy do if actor in allow_list do {:ok, object} else - {:reject, nil} + {:reject, "[UserAllowListPolicy] #{actor} not in the list"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex b/lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex index 6167a74e2..a6c545570 100644 --- a/lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex @@ -11,22 +11,26 @@ defmodule Pleroma.Web.ActivityPub.MRF.VocabularyPolicy do with {:ok, _} <- filter(child_message) do {:ok, message} else - {:reject, nil} -> - {:reject, nil} + {:reject, _} = e -> e end end def filter(%{"type" => message_type} = message) do with accepted_vocabulary <- Pleroma.Config.get([:mrf_vocabulary, :accept]), rejected_vocabulary <- Pleroma.Config.get([:mrf_vocabulary, :reject]), - true <- - Enum.empty?(accepted_vocabulary) || Enum.member?(accepted_vocabulary, message_type), - false <- - length(rejected_vocabulary) > 0 && Enum.member?(rejected_vocabulary, message_type), + {_, true} <- + {:accepted, + Enum.empty?(accepted_vocabulary) || Enum.member?(accepted_vocabulary, message_type)}, + {_, false} <- + {:rejected, + length(rejected_vocabulary) > 0 && Enum.member?(rejected_vocabulary, message_type)}, {:ok, _} <- filter(message["object"]) do {:ok, message} else - _ -> {:reject, nil} + {:reject, _} = e -> e + {:accepted, _} -> {:reject, "[VocabularyPolicy] #{message_type} not in accept list"} + {:rejected, _} -> {:reject, "[VocabularyPolicy] #{message_type} in reject list"} + _ -> {:reject, "[VocabularyPolicy]"} end end -- cgit v1.2.3 From e6ccc2556568f2180c3ce1945bdc7a0cba97e924 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 14 Jul 2020 11:41:30 +0300 Subject: Fix in-db configuration in dev environment Previously, in-db configuration only worked when `warnings_as_errors` was disabled because re-compiling scrubbers on application restart created a warning about module conflicts. This patch fixes that by enabling `ignore_module_conflict` option of the compiler at runtime, and enables `warnings_as_errors` in prod since there is no reason to keep it disabled anymore. --- lib/pleroma/application.ex | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib') diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index b68a373a4..3282c6882 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -35,6 +35,10 @@ defmodule Pleroma.Application do # See http://elixir-lang.org/docs/stable/elixir/Application.html # for more information on OTP Applications def start(_type, _args) do + # Scrubbers are compiled at runtime and therefore will cause a conflict + # every time the application is restarted, so we disable module + # conflicts at runtime + Code.compiler_options(ignore_module_conflict: true) Config.Holder.save_default() Pleroma.HTML.compile_scrubbers() Config.DeprecationWarnings.warn() -- cgit v1.2.3 From 124b4709dcf12a417f5164e53ef3ba67e538d4c7 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 14 Jul 2020 19:31:05 +0300 Subject: [#1940] Added `admin_token` param (as `admin_api_params/0`) to existing Admin API OpenAPI operations. --- lib/pleroma/web/api_spec/helpers.ex | 4 ++++ lib/pleroma/web/api_spec/operations/admin/config_operation.ex | 3 +++ lib/pleroma/web/api_spec/operations/admin/invite_operation.ex | 4 ++++ .../web/api_spec/operations/admin/media_proxy_cache_operation.ex | 3 +++ lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex | 6 ++++-- lib/pleroma/web/api_spec/operations/admin/relay_operation.ex | 3 +++ lib/pleroma/web/api_spec/operations/admin/report_operation.ex | 7 +++++-- lib/pleroma/web/api_spec/operations/admin/status_operation.ex | 7 ++++--- 8 files changed, 30 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/api_spec/helpers.ex b/lib/pleroma/web/api_spec/helpers.ex index a258e8421..2a7f1a706 100644 --- a/lib/pleroma/web/api_spec/helpers.ex +++ b/lib/pleroma/web/api_spec/helpers.ex @@ -29,6 +29,10 @@ defmodule Pleroma.Web.ApiSpec.Helpers do } end + def admin_api_params do + [Operation.parameter(:admin_token, :query, :string, "Allows authorization via admin token.")] + end + def pagination_params do [ Operation.parameter(:max_id, :query, :string, "Return items older than this ID"), diff --git a/lib/pleroma/web/api_spec/operations/admin/config_operation.ex b/lib/pleroma/web/api_spec/operations/admin/config_operation.ex index 7b38a2ef4..3a8380797 100644 --- a/lib/pleroma/web/api_spec/operations/admin/config_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/config_operation.ex @@ -26,6 +26,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.ConfigOperation do %Schema{type: :boolean, default: false}, "Get only saved in database settings" ) + | admin_api_params() ], security: [%{"oAuth" => ["read"]}], responses: %{ @@ -41,6 +42,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.ConfigOperation do summary: "Update config settings", operationId: "AdminAPI.ConfigController.update", security: [%{"oAuth" => ["write"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", %Schema{ type: :object, @@ -73,6 +75,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.ConfigOperation do summary: "Get JSON with config descriptions.", operationId: "AdminAPI.ConfigController.descriptions", security: [%{"oAuth" => ["read"]}], + parameters: admin_api_params(), responses: %{ 200 => Operation.response("Config Descriptions", "application/json", %Schema{ diff --git a/lib/pleroma/web/api_spec/operations/admin/invite_operation.ex b/lib/pleroma/web/api_spec/operations/admin/invite_operation.ex index d3af9db49..801024d75 100644 --- a/lib/pleroma/web/api_spec/operations/admin/invite_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/invite_operation.ex @@ -20,6 +20,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.InviteOperation do summary: "Get a list of generated invites", operationId: "AdminAPI.InviteController.index", security: [%{"oAuth" => ["read:invites"]}], + parameters: admin_api_params(), responses: %{ 200 => Operation.response("Invites", "application/json", %Schema{ @@ -51,6 +52,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.InviteOperation do summary: "Create an account registration invite token", operationId: "AdminAPI.InviteController.create", security: [%{"oAuth" => ["write:invites"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", %Schema{ type: :object, @@ -71,6 +73,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.InviteOperation do summary: "Revoke invite by token", operationId: "AdminAPI.InviteController.revoke", security: [%{"oAuth" => ["write:invites"]}], + parameters: admin_api_params(), requestBody: request_body( "Parameters", @@ -97,6 +100,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.InviteOperation do summary: "Sends registration invite via email", operationId: "AdminAPI.InviteController.email", security: [%{"oAuth" => ["write:invites"]}], + parameters: admin_api_params(), requestBody: request_body( "Parameters", diff --git a/lib/pleroma/web/api_spec/operations/admin/media_proxy_cache_operation.ex b/lib/pleroma/web/api_spec/operations/admin/media_proxy_cache_operation.ex index 0358cfbad..20d033f66 100644 --- a/lib/pleroma/web/api_spec/operations/admin/media_proxy_cache_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/media_proxy_cache_operation.ex @@ -33,6 +33,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.MediaProxyCacheOperation do %Schema{type: :integer, default: 50}, "Number of statuses to return" ) + | admin_api_params() ], responses: %{ 200 => success_response() @@ -46,6 +47,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.MediaProxyCacheOperation do summary: "Remove a banned MediaProxy URL from Cachex", operationId: "AdminAPI.MediaProxyCacheController.delete", security: [%{"oAuth" => ["write:media_proxy_caches"]}], + parameters: admin_api_params(), requestBody: request_body( "Parameters", @@ -71,6 +73,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.MediaProxyCacheOperation do summary: "Purge and optionally ban a MediaProxy URL", operationId: "AdminAPI.MediaProxyCacheController.purge", security: [%{"oAuth" => ["write:media_proxy_caches"]}], + parameters: admin_api_params(), requestBody: request_body( "Parameters", diff --git a/lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex b/lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex index fbc9f80d7..a75f3e622 100644 --- a/lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex @@ -36,6 +36,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.OAuthAppOperation do %Schema{type: :integer, default: 50}, "Number of apps to return" ) + | admin_api_params() ], responses: %{ 200 => @@ -72,6 +73,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.OAuthAppOperation do summary: "Create OAuth App", operationId: "AdminAPI.OAuthAppController.create", requestBody: request_body("Parameters", create_request()), + parameters: admin_api_params(), security: [%{"oAuth" => ["write"]}], responses: %{ 200 => Operation.response("App", "application/json", oauth_app()), @@ -85,7 +87,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.OAuthAppOperation do tags: ["Admin", "oAuth Apps"], summary: "Update OAuth App", operationId: "AdminAPI.OAuthAppController.update", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["write"]}], requestBody: request_body("Parameters", update_request()), responses: %{ @@ -103,7 +105,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.OAuthAppOperation do tags: ["Admin", "oAuth Apps"], summary: "Delete OAuth App", operationId: "AdminAPI.OAuthAppController.delete", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["write"]}], responses: %{ 204 => no_content_response(), diff --git a/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex b/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex index 7672cb467..67ee5eee0 100644 --- a/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex @@ -19,6 +19,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.RelayOperation do summary: "List Relays", operationId: "AdminAPI.RelayController.index", security: [%{"oAuth" => ["read"]}], + parameters: admin_api_params(), responses: %{ 200 => Operation.response("Response", "application/json", %Schema{ @@ -41,6 +42,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.RelayOperation do summary: "Follow a Relay", operationId: "AdminAPI.RelayController.follow", security: [%{"oAuth" => ["write:follows"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", %Schema{ type: :object, @@ -64,6 +66,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.RelayOperation do summary: "Unfollow a Relay", operationId: "AdminAPI.RelayController.unfollow", security: [%{"oAuth" => ["write:follows"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", %Schema{ type: :object, diff --git a/lib/pleroma/web/api_spec/operations/admin/report_operation.ex b/lib/pleroma/web/api_spec/operations/admin/report_operation.ex index 15e78bfaf..3bb7ec49e 100644 --- a/lib/pleroma/web/api_spec/operations/admin/report_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/report_operation.ex @@ -48,6 +48,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.ReportOperation do %Schema{type: :integer, default: 50}, "Number number of log entries per page" ) + | admin_api_params() ], responses: %{ 200 => @@ -71,7 +72,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.ReportOperation do tags: ["Admin", "Reports"], summary: "Get an individual report", operationId: "AdminAPI.ReportController.show", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["read:reports"]}], responses: %{ 200 => Operation.response("Report", "application/json", report()), @@ -86,6 +87,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.ReportOperation do summary: "Change the state of one or multiple reports", operationId: "AdminAPI.ReportController.update", security: [%{"oAuth" => ["write:reports"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", update_request(), required: true), responses: %{ 204 => no_content_response(), @@ -100,7 +102,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.ReportOperation do tags: ["Admin", "Reports"], summary: "Create report note", operationId: "AdminAPI.ReportController.notes_create", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], requestBody: request_body("Parameters", %Schema{ type: :object, @@ -124,6 +126,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.ReportOperation do parameters: [ Operation.parameter(:report_id, :path, :string, "Report ID"), Operation.parameter(:id, :path, :string, "Note ID") + | admin_api_params() ], security: [%{"oAuth" => ["write:reports"]}], responses: %{ diff --git a/lib/pleroma/web/api_spec/operations/admin/status_operation.ex b/lib/pleroma/web/api_spec/operations/admin/status_operation.ex index 745399b4b..c105838a4 100644 --- a/lib/pleroma/web/api_spec/operations/admin/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/status_operation.ex @@ -55,6 +55,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.StatusOperation do %Schema{type: :integer, default: 50}, "Number of statuses to return" ) + | admin_api_params() ], responses: %{ 200 => @@ -71,7 +72,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.StatusOperation do tags: ["Admin", "Statuses"], summary: "Show Status", operationId: "AdminAPI.StatusController.show", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["read:statuses"]}], responses: %{ 200 => Operation.response("Status", "application/json", status()), @@ -85,7 +86,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.StatusOperation do tags: ["Admin", "Statuses"], summary: "Change the scope of an individual reported status", operationId: "AdminAPI.StatusController.update", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["write:statuses"]}], requestBody: request_body("Parameters", update_request(), required: true), responses: %{ @@ -100,7 +101,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.StatusOperation do tags: ["Admin", "Statuses"], summary: "Delete an individual reported status", operationId: "AdminAPI.StatusController.delete", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["write:statuses"]}], responses: %{ 200 => empty_object_response(), -- cgit v1.2.3 From 1dd767b8c7b7565ad94ccb85324e97fa9885923e Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Tue, 14 Jul 2020 21:44:08 +0300 Subject: Include port in host for signatures --- lib/pleroma/web/activity_pub/publisher.ex | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index b70cbd043..d88f7f3ee 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -49,7 +49,8 @@ defmodule Pleroma.Web.ActivityPub.Publisher do """ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = params) do Logger.debug("Federating #{id} to #{inbox}") - %{host: host, path: path} = URI.parse(inbox) + + uri = URI.parse(inbox) digest = "SHA-256=" <> (:crypto.hash(:sha256, json) |> Base.encode64()) @@ -57,8 +58,8 @@ defmodule Pleroma.Web.ActivityPub.Publisher do signature = Pleroma.Signature.sign(actor, %{ - "(request-target)": "post #{path}", - host: host, + "(request-target)": "post #{uri.path}", + host: signature_host(uri), "content-length": byte_size(json), digest: digest, date: date @@ -76,8 +77,9 @@ defmodule Pleroma.Web.ActivityPub.Publisher do {"digest", digest} ] ) do - if !Map.has_key?(params, :unreachable_since) || params[:unreachable_since], - do: Instances.set_reachable(inbox) + if not Map.has_key?(params, :unreachable_since) || params[:unreachable_since] do + Instances.set_reachable(inbox) + end result else @@ -96,6 +98,14 @@ defmodule Pleroma.Web.ActivityPub.Publisher do |> publish_one() end + defp signature_host(%URI{port: port, scheme: scheme, host: host}) do + if port == URI.default_port(scheme) do + host + else + "#{host}:#{port}" + end + end + defp should_federate?(inbox, public) do if public do true -- cgit v1.2.3 From 58a4f350a8bc361d793cb96442f856362c18f195 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 6 May 2020 01:51:10 +0300 Subject: Refactor gun pooling and simplify adapter option insertion This patch refactors gun pooling to use Elixir process registry and simplifies adapter option insertion. Having the pool use process registry instead of a GenServer has a number of advantages: - Simpler code: the initial implementation adds about half the lines of code it deletes - Concurrency: unlike a GenServer, ETS-based registry can handle multiple checkout/checkin requests at the same time - Precise and easy idle connection clousure: current proposal for closing idle connections in the GenServer-based pool needs to filter through all connections once a minute and compare their last active time with closing time. With Elixir process registry this can be done by just using `Process.send_after`/`Process.cancel_timer` in the worker process. - Lower memory footprint: In my tests `gun-memory-leak` branch uses about 290mb on peak load (250 connections) and 235mb on idle (5-10 connections). Registry-based pool uses 210mb on idle and 240mb on peak load --- lib/pleroma/application.ex | 8 +- lib/pleroma/gun/conn.ex | 78 +------- lib/pleroma/gun/connection_pool.ex | 129 +++++++++++++ lib/pleroma/gun/connection_pool/worker.ex | 95 ++++++++++ lib/pleroma/http/adapter_helper.ex | 133 ++++++++++++-- lib/pleroma/http/adapter_helper/default.ex | 17 ++ lib/pleroma/http/adapter_helper/gun.ex | 32 +--- lib/pleroma/http/adapter_helper/hackney.ex | 3 + lib/pleroma/http/connection.ex | 124 ------------- lib/pleroma/http/http.ex | 53 ++---- lib/pleroma/pool/connections.ex | 283 ----------------------------- lib/pleroma/pool/pool.ex | 22 --- lib/pleroma/pool/request.ex | 65 ------- lib/pleroma/pool/supervisor.ex | 42 ----- lib/pleroma/reverse_proxy/client/tesla.ex | 2 +- 15 files changed, 400 insertions(+), 686 deletions(-) create mode 100644 lib/pleroma/gun/connection_pool.ex create mode 100644 lib/pleroma/gun/connection_pool/worker.ex create mode 100644 lib/pleroma/http/adapter_helper/default.ex delete mode 100644 lib/pleroma/http/connection.ex delete mode 100644 lib/pleroma/pool/connections.ex delete mode 100644 lib/pleroma/pool/pool.ex delete mode 100644 lib/pleroma/pool/request.ex delete mode 100644 lib/pleroma/pool/supervisor.ex (limited to 'lib') diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index 3282c6882..be14c1f9f 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -223,9 +223,7 @@ defmodule Pleroma.Application do # start hackney and gun pools in tests defp http_children(_, :test) do - hackney_options = Config.get([:hackney_pools, :federation]) - hackney_pool = :hackney_pool.child_spec(:federation, hackney_options) - [hackney_pool, Pleroma.Pool.Supervisor] + http_children(Tesla.Adapter.Hackney, nil) ++ http_children(Tesla.Adapter.Gun, nil) end defp http_children(Tesla.Adapter.Hackney, _) do @@ -244,7 +242,9 @@ defmodule Pleroma.Application do end end - defp http_children(Tesla.Adapter.Gun, _), do: [Pleroma.Pool.Supervisor] + defp http_children(Tesla.Adapter.Gun, _) do + [{Registry, keys: :unique, name: Pleroma.Gun.ConnectionPool}] + end defp http_children(_, _), do: [] end diff --git a/lib/pleroma/gun/conn.ex b/lib/pleroma/gun/conn.ex index cd25a2e74..77f78c7ff 100644 --- a/lib/pleroma/gun/conn.ex +++ b/lib/pleroma/gun/conn.ex @@ -3,40 +3,11 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Gun.Conn do - @moduledoc """ - Struct for gun connection data - """ alias Pleroma.Gun - alias Pleroma.Pool.Connections require Logger - @type gun_state :: :up | :down - @type conn_state :: :active | :idle - - @type t :: %__MODULE__{ - conn: pid(), - gun_state: gun_state(), - conn_state: conn_state(), - used_by: [pid()], - last_reference: pos_integer(), - crf: float(), - retries: pos_integer() - } - - defstruct conn: nil, - gun_state: :open, - conn_state: :init, - used_by: [], - last_reference: 0, - crf: 1, - retries: 0 - - @spec open(String.t() | URI.t(), atom(), keyword()) :: :ok | nil - def open(url, name, opts \\ []) - def open(url, name, opts) when is_binary(url), do: open(URI.parse(url), name, opts) - - def open(%URI{} = uri, name, opts) do + def open(%URI{} = uri, opts) do pool_opts = Pleroma.Config.get([:connections_pool], []) opts = @@ -45,30 +16,10 @@ defmodule Pleroma.Gun.Conn do |> Map.put_new(:retry, pool_opts[:retry] || 1) |> Map.put_new(:retry_timeout, pool_opts[:retry_timeout] || 1000) |> Map.put_new(:await_up_timeout, pool_opts[:await_up_timeout] || 5_000) + |> Map.put_new(:supervise, false) |> maybe_add_tls_opts(uri) - key = "#{uri.scheme}:#{uri.host}:#{uri.port}" - - max_connections = pool_opts[:max_connections] || 250 - - conn_pid = - if Connections.count(name) < max_connections do - do_open(uri, opts) - else - close_least_used_and_do_open(name, uri, opts) - end - - if is_pid(conn_pid) do - conn = %Pleroma.Gun.Conn{ - conn: conn_pid, - gun_state: :up, - conn_state: :active, - last_reference: :os.system_time(:second) - } - - :ok = Gun.set_owner(conn_pid, Process.whereis(name)) - Connections.add_conn(name, key, conn) - end + do_open(uri, opts) end defp maybe_add_tls_opts(opts, %URI{scheme: "http"}), do: opts @@ -81,7 +32,7 @@ defmodule Pleroma.Gun.Conn do reuse_sessions: false, verify_fun: {&:ssl_verify_hostname.verify_fun/3, - [check_hostname: Pleroma.HTTP.Connection.format_host(host)]} + [check_hostname: Pleroma.HTTP.AdapterHelper.format_host(host)]} ] tls_opts = @@ -105,7 +56,7 @@ defmodule Pleroma.Gun.Conn do {:ok, _} <- Gun.await_up(conn, opts[:await_up_timeout]), stream <- Gun.connect(conn, connect_opts), {:response, :fin, 200, _} <- Gun.await(conn, stream) do - conn + {:ok, conn} else error -> Logger.warn( @@ -141,7 +92,7 @@ defmodule Pleroma.Gun.Conn do with {:ok, conn} <- Gun.open(proxy_host, proxy_port, opts), {:ok, _} <- Gun.await_up(conn, opts[:await_up_timeout]) do - conn + {:ok, conn} else error -> Logger.warn( @@ -155,11 +106,11 @@ defmodule Pleroma.Gun.Conn do end defp do_open(%URI{host: host, port: port} = uri, opts) do - host = Pleroma.HTTP.Connection.parse_host(host) + host = Pleroma.HTTP.AdapterHelper.parse_host(host) with {:ok, conn} <- Gun.open(host, port, opts), {:ok, _} <- Gun.await_up(conn, opts[:await_up_timeout]) do - conn + {:ok, conn} else error -> Logger.warn( @@ -171,7 +122,7 @@ defmodule Pleroma.Gun.Conn do end defp destination_opts(%URI{host: host, port: port}) do - host = Pleroma.HTTP.Connection.parse_host(host) + host = Pleroma.HTTP.AdapterHelper.parse_host(host) %{host: host, port: port} end @@ -181,17 +132,6 @@ defmodule Pleroma.Gun.Conn do defp add_http2_opts(opts, _, _), do: opts - defp close_least_used_and_do_open(name, uri, opts) do - with [{key, conn} | _conns] <- Connections.get_unused_conns(name), - :ok <- Gun.close(conn.conn) do - Connections.remove_conn(name, key) - - do_open(uri, opts) - else - [] -> {:error, :pool_overflowed} - end - end - def compose_uri_log(%URI{scheme: scheme, host: host, path: path}) do "#{scheme}://#{host}#{path}" end diff --git a/lib/pleroma/gun/connection_pool.ex b/lib/pleroma/gun/connection_pool.ex new file mode 100644 index 000000000..e6abee69c --- /dev/null +++ b/lib/pleroma/gun/connection_pool.ex @@ -0,0 +1,129 @@ +defmodule Pleroma.Gun.ConnectionPool do + @registry __MODULE__ + + def get_conn(uri, opts) do + case enforce_pool_limits() do + :ok -> + key = "#{uri.scheme}:#{uri.host}:#{uri.port}" + + case Registry.lookup(@registry, key) do + # The key has already been registered, but connection is not up yet + [{worker_pid, {nil, _used_by, _crf, _last_reference}}] -> + get_gun_pid_from_worker(worker_pid) + + [{worker_pid, {gun_pid, _used_by, _crf, _last_reference}}] -> + GenServer.cast(worker_pid, {:add_client, self(), false}) + {:ok, gun_pid} + + [] -> + # :gun.set_owner fails in :connected state for whatevever reason, + # so we open the connection in the process directly and send it's pid back + # We trust gun to handle timeouts by itself + case GenServer.start(Pleroma.Gun.ConnectionPool.Worker, [uri, key, opts, self()], + timeout: :infinity + ) do + {:ok, _worker_pid} -> + receive do + {:conn_pid, pid} -> {:ok, pid} + end + + {:error, {:error, {:already_registered, worker_pid}}} -> + get_gun_pid_from_worker(worker_pid) + + err -> + err + end + end + + :error -> + {:error, :pool_full} + end + end + + @enforcer_key "enforcer" + defp enforce_pool_limits() do + max_connections = Pleroma.Config.get([:connections_pool, :max_connections]) + + if Registry.count(@registry) >= max_connections do + case Registry.lookup(@registry, @enforcer_key) do + [] -> + pid = + spawn(fn -> + {:ok, _pid} = Registry.register(@registry, @enforcer_key, nil) + + reclaim_max = + [:connections_pool, :reclaim_multiplier] + |> Pleroma.Config.get() + |> Kernel.*(max_connections) + |> round + |> max(1) + + unused_conns = + Registry.select( + @registry, + [ + {{:_, :"$1", {:_, :"$2", :"$3", :"$4"}}, [{:==, :"$2", []}], + [{{:"$1", :"$3", :"$4"}}]} + ] + ) + + case unused_conns do + [] -> + exit(:pool_full) + + unused_conns -> + unused_conns + |> Enum.sort(fn {_pid1, crf1, last_reference1}, + {_pid2, crf2, last_reference2} -> + crf1 <= crf2 and last_reference1 <= last_reference2 + end) + |> Enum.take(reclaim_max) + |> Enum.each(fn {pid, _, _} -> GenServer.call(pid, :idle_close) end) + end + end) + + wait_for_enforcer_finish(pid) + + [{pid, _}] -> + wait_for_enforcer_finish(pid) + end + else + :ok + end + end + + defp wait_for_enforcer_finish(pid) do + ref = Process.monitor(pid) + + receive do + {:DOWN, ^ref, :process, ^pid, :pool_full} -> + :error + + {:DOWN, ^ref, :process, ^pid, :normal} -> + :ok + end + end + + defp get_gun_pid_from_worker(worker_pid) do + # GenServer.call will block the process for timeout length if + # the server crashes on startup (which will happen if gun fails to connect) + # so instead we use cast + monitor + + ref = Process.monitor(worker_pid) + GenServer.cast(worker_pid, {:add_client, self(), true}) + + receive do + {:conn_pid, pid} -> {:ok, pid} + {:DOWN, ^ref, :process, ^worker_pid, reason} -> reason + end + end + + def release_conn(conn_pid) do + [worker_pid] = + Registry.select(@registry, [ + {{:_, :"$1", {:"$2", :_, :_, :_}}, [{:==, :"$2", conn_pid}], [:"$1"]} + ]) + + GenServer.cast(worker_pid, {:remove_client, self()}) + end +end diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex new file mode 100644 index 000000000..ebde4bbf6 --- /dev/null +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -0,0 +1,95 @@ +defmodule Pleroma.Gun.ConnectionPool.Worker do + alias Pleroma.Gun + use GenServer + + @registry Pleroma.Gun.ConnectionPool + + @impl true + def init([uri, key, opts, client_pid]) do + time = :os.system_time(:second) + # Register before opening connection to prevent race conditions + with {:ok, _owner} <- Registry.register(@registry, key, {nil, [client_pid], 1, time}), + {:ok, conn_pid} <- Gun.Conn.open(uri, opts), + Process.link(conn_pid) do + {_, _} = + Registry.update_value(@registry, key, fn {_, used_by, crf, last_reference} -> + {conn_pid, used_by, crf, last_reference} + end) + + send(client_pid, {:conn_pid, conn_pid}) + {:ok, %{key: key, timer: nil}, :hibernate} + else + err -> {:stop, err} + end + end + + @impl true + def handle_cast({:add_client, client_pid, send_pid_back}, %{key: key} = state) do + time = :os.system_time(:second) + + {{conn_pid, _, _, _}, _} = + Registry.update_value(@registry, key, fn {conn_pid, used_by, crf, last_reference} -> + {conn_pid, [client_pid | used_by], crf(time - last_reference, crf), time} + end) + + if send_pid_back, do: send(client_pid, {:conn_pid, conn_pid}) + + state = + if state.timer != nil do + Process.cancel_timer(state[:timer]) + %{state | timer: nil} + else + state + end + + {:noreply, state, :hibernate} + end + + @impl true + def handle_cast({:remove_client, client_pid}, %{key: key} = state) do + {{_conn_pid, used_by, _crf, _last_reference}, _} = + Registry.update_value(@registry, key, fn {conn_pid, used_by, crf, last_reference} -> + {conn_pid, List.delete(used_by, client_pid), crf, last_reference} + end) + + timer = + if used_by == [] do + max_idle = Pleroma.Config.get([:connections_pool, :max_idle_time], 30_000) + Process.send_after(self(), :idle_close, max_idle) + else + nil + end + + {:noreply, %{state | timer: timer}, :hibernate} + end + + @impl true + def handle_info(:idle_close, state) do + # Gun monitors the owner process, and will close the connection automatically + # when it's terminated + {:stop, :normal, state} + end + + # Gracefully shutdown if the connection got closed without any streams left + @impl true + def handle_info({:gun_down, _pid, _protocol, _reason, []}, state) do + {:stop, :normal, state} + end + + # Otherwise, shutdown with an error + @impl true + def handle_info({:gun_down, _pid, _protocol, _reason, _killed_streams} = down_message, state) do + {:stop, {:error, down_message}, state} + end + + @impl true + def handle_call(:idle_close, _, %{key: key} = state) do + Registry.unregister(@registry, key) + {:stop, :normal, state} + end + + # LRFU policy: https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.55.1478 + defp crf(time_delta, prev_crf) do + 1 + :math.pow(0.5, time_delta / 100) * prev_crf + end +end diff --git a/lib/pleroma/http/adapter_helper.ex b/lib/pleroma/http/adapter_helper.ex index 510722ff9..0532ea31d 100644 --- a/lib/pleroma/http/adapter_helper.ex +++ b/lib/pleroma/http/adapter_helper.ex @@ -3,7 +3,21 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.HTTP.AdapterHelper do - alias Pleroma.HTTP.Connection + @moduledoc """ + Configure Tesla.Client with default and customized adapter options. + """ + @defaults [pool: :federation] + + @type ip_address :: ipv4_address() | ipv6_address() + @type ipv4_address :: {0..255, 0..255, 0..255, 0..255} + @type ipv6_address :: + {0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535} + @type proxy_type() :: :socks4 | :socks5 + @type host() :: charlist() | ip_address() + + alias Pleroma.Config + alias Pleroma.HTTP.AdapterHelper + require Logger @type proxy :: {Connection.host(), pos_integer()} @@ -11,24 +25,13 @@ defmodule Pleroma.HTTP.AdapterHelper do @callback options(keyword(), URI.t()) :: keyword() @callback after_request(keyword()) :: :ok - - @spec options(keyword(), URI.t()) :: keyword() - def options(opts, _uri) do - proxy = Pleroma.Config.get([:http, :proxy_url], nil) - maybe_add_proxy(opts, format_proxy(proxy)) - end - - @spec maybe_get_conn(URI.t(), keyword()) :: keyword() - def maybe_get_conn(_uri, opts), do: opts - - @spec after_request(keyword()) :: :ok - def after_request(_opts), do: :ok + @callback get_conn(URI.t(), keyword()) :: {:ok, term()} | {:error, term()} @spec format_proxy(String.t() | tuple() | nil) :: proxy() | nil def format_proxy(nil), do: nil def format_proxy(proxy_url) do - case Connection.parse_proxy(proxy_url) do + case parse_proxy(proxy_url) do {:ok, host, port} -> {host, port} {:ok, type, host, port} -> {type, host, port} _ -> nil @@ -38,4 +41,106 @@ defmodule Pleroma.HTTP.AdapterHelper do @spec maybe_add_proxy(keyword(), proxy() | nil) :: keyword() def maybe_add_proxy(opts, nil), do: opts def maybe_add_proxy(opts, proxy), do: Keyword.put_new(opts, :proxy, proxy) + + @doc """ + Merge default connection & adapter options with received ones. + """ + + @spec options(URI.t(), keyword()) :: keyword() + def options(%URI{} = uri, opts \\ []) do + @defaults + |> pool_timeout() + |> Keyword.merge(opts) + |> adapter_helper().options(uri) + end + + defp pool_timeout(opts) do + {config_key, default} = + if adapter() == Tesla.Adapter.Gun do + {:pools, Config.get([:pools, :default, :timeout])} + else + {:hackney_pools, 10_000} + end + + timeout = Config.get([config_key, opts[:pool], :timeout], default) + + Keyword.merge(opts, timeout: timeout) + end + + @spec after_request(keyword()) :: :ok + def after_request(opts), do: adapter_helper().after_request(opts) + + def get_conn(uri, opts), do: adapter_helper().get_conn(uri, opts) + defp adapter, do: Application.get_env(:tesla, :adapter) + + defp adapter_helper do + case adapter() do + Tesla.Adapter.Gun -> AdapterHelper.Gun + Tesla.Adapter.Hackney -> AdapterHelper.Hackney + _ -> AdapterHelper.Default + end + end + + @spec parse_proxy(String.t() | tuple() | nil) :: + {:ok, host(), pos_integer()} + | {:ok, proxy_type(), host(), pos_integer()} + | {:error, atom()} + | nil + + def parse_proxy(nil), do: nil + + def parse_proxy(proxy) when is_binary(proxy) do + with [host, port] <- String.split(proxy, ":"), + {port, ""} <- Integer.parse(port) do + {:ok, parse_host(host), port} + else + {_, _} -> + Logger.warn("Parsing port failed #{inspect(proxy)}") + {:error, :invalid_proxy_port} + + :error -> + Logger.warn("Parsing port failed #{inspect(proxy)}") + {:error, :invalid_proxy_port} + + _ -> + Logger.warn("Parsing proxy failed #{inspect(proxy)}") + {:error, :invalid_proxy} + end + end + + def parse_proxy(proxy) when is_tuple(proxy) do + with {type, host, port} <- proxy do + {:ok, type, parse_host(host), port} + else + _ -> + Logger.warn("Parsing proxy failed #{inspect(proxy)}") + {:error, :invalid_proxy} + end + end + + @spec parse_host(String.t() | atom() | charlist()) :: charlist() | ip_address() + def parse_host(host) when is_list(host), do: host + def parse_host(host) when is_atom(host), do: to_charlist(host) + + def parse_host(host) when is_binary(host) do + host = to_charlist(host) + + case :inet.parse_address(host) do + {:error, :einval} -> host + {:ok, ip} -> ip + end + end + + @spec format_host(String.t()) :: charlist() + def format_host(host) do + host_charlist = to_charlist(host) + + case :inet.parse_address(host_charlist) do + {:error, :einval} -> + :idna.encode(host_charlist) + + {:ok, _ip} -> + host_charlist + end + end end diff --git a/lib/pleroma/http/adapter_helper/default.ex b/lib/pleroma/http/adapter_helper/default.ex new file mode 100644 index 000000000..218cfacc0 --- /dev/null +++ b/lib/pleroma/http/adapter_helper/default.ex @@ -0,0 +1,17 @@ +defmodule Pleroma.HTTP.AdapterHelper.Default do + alias Pleroma.HTTP.AdapterHelper + + @behaviour Pleroma.HTTP.AdapterHelper + + @spec options(keyword(), URI.t()) :: keyword() + def options(opts, _uri) do + proxy = Pleroma.Config.get([:http, :proxy_url], nil) + AdapterHelper.maybe_add_proxy(opts, AdapterHelper.format_proxy(proxy)) + end + + @spec after_request(keyword()) :: :ok + def after_request(_opts), do: :ok + + @spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} + def get_conn(_uri, opts), do: {:ok, opts} +end diff --git a/lib/pleroma/http/adapter_helper/gun.ex b/lib/pleroma/http/adapter_helper/gun.ex index ead7cdc6b..6f7cc9784 100644 --- a/lib/pleroma/http/adapter_helper/gun.ex +++ b/lib/pleroma/http/adapter_helper/gun.ex @@ -5,8 +5,8 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do @behaviour Pleroma.HTTP.AdapterHelper + alias Pleroma.Gun.ConnectionPool alias Pleroma.HTTP.AdapterHelper - alias Pleroma.Pool.Connections require Logger @@ -31,13 +31,13 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do |> Keyword.merge(config_opts) |> add_scheme_opts(uri) |> AdapterHelper.maybe_add_proxy(proxy) - |> maybe_get_conn(uri, incoming_opts) + |> Keyword.merge(incoming_opts) end @spec after_request(keyword()) :: :ok def after_request(opts) do if opts[:conn] && opts[:body_as] != :chunks do - Connections.checkout(opts[:conn], self(), :gun_connections) + ConnectionPool.release_conn(opts[:conn]) end :ok @@ -51,27 +51,11 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do |> Keyword.put(:tls_opts, log_level: :warning) end - defp maybe_get_conn(adapter_opts, uri, incoming_opts) do - {receive_conn?, opts} = - adapter_opts - |> Keyword.merge(incoming_opts) - |> Keyword.pop(:receive_conn, true) - - if Connections.alive?(:gun_connections) and receive_conn? do - checkin_conn(uri, opts) - else - opts - end - end - - defp checkin_conn(uri, opts) do - case Connections.checkin(uri, :gun_connections) do - nil -> - Task.start(Pleroma.Gun.Conn, :open, [uri, :gun_connections, opts]) - opts - - conn when is_pid(conn) -> - Keyword.merge(opts, conn: conn, close_conn: false) + @spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} | {:error, atom()} + def get_conn(uri, opts) do + case ConnectionPool.get_conn(uri, opts) do + {:ok, conn_pid} -> {:ok, Keyword.merge(opts, conn: conn_pid, close_conn: false)} + err -> err end end end diff --git a/lib/pleroma/http/adapter_helper/hackney.ex b/lib/pleroma/http/adapter_helper/hackney.ex index 3972a03a9..42d552740 100644 --- a/lib/pleroma/http/adapter_helper/hackney.ex +++ b/lib/pleroma/http/adapter_helper/hackney.ex @@ -25,4 +25,7 @@ defmodule Pleroma.HTTP.AdapterHelper.Hackney do defp add_scheme_opts(opts, _), do: opts def after_request(_), do: :ok + + @spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} + def get_conn(_uri, opts), do: {:ok, opts} end diff --git a/lib/pleroma/http/connection.ex b/lib/pleroma/http/connection.ex deleted file mode 100644 index ebacf7902..000000000 --- a/lib/pleroma/http/connection.ex +++ /dev/null @@ -1,124 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.HTTP.Connection do - @moduledoc """ - Configure Tesla.Client with default and customized adapter options. - """ - - alias Pleroma.Config - alias Pleroma.HTTP.AdapterHelper - - require Logger - - @defaults [pool: :federation] - - @type ip_address :: ipv4_address() | ipv6_address() - @type ipv4_address :: {0..255, 0..255, 0..255, 0..255} - @type ipv6_address :: - {0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535} - @type proxy_type() :: :socks4 | :socks5 - @type host() :: charlist() | ip_address() - - @doc """ - Merge default connection & adapter options with received ones. - """ - - @spec options(URI.t(), keyword()) :: keyword() - def options(%URI{} = uri, opts \\ []) do - @defaults - |> pool_timeout() - |> Keyword.merge(opts) - |> adapter_helper().options(uri) - end - - defp pool_timeout(opts) do - {config_key, default} = - if adapter() == Tesla.Adapter.Gun do - {:pools, Config.get([:pools, :default, :timeout])} - else - {:hackney_pools, 10_000} - end - - timeout = Config.get([config_key, opts[:pool], :timeout], default) - - Keyword.merge(opts, timeout: timeout) - end - - @spec after_request(keyword()) :: :ok - def after_request(opts), do: adapter_helper().after_request(opts) - - defp adapter, do: Application.get_env(:tesla, :adapter) - - defp adapter_helper do - case adapter() do - Tesla.Adapter.Gun -> AdapterHelper.Gun - Tesla.Adapter.Hackney -> AdapterHelper.Hackney - _ -> AdapterHelper - end - end - - @spec parse_proxy(String.t() | tuple() | nil) :: - {:ok, host(), pos_integer()} - | {:ok, proxy_type(), host(), pos_integer()} - | {:error, atom()} - | nil - - def parse_proxy(nil), do: nil - - def parse_proxy(proxy) when is_binary(proxy) do - with [host, port] <- String.split(proxy, ":"), - {port, ""} <- Integer.parse(port) do - {:ok, parse_host(host), port} - else - {_, _} -> - Logger.warn("Parsing port failed #{inspect(proxy)}") - {:error, :invalid_proxy_port} - - :error -> - Logger.warn("Parsing port failed #{inspect(proxy)}") - {:error, :invalid_proxy_port} - - _ -> - Logger.warn("Parsing proxy failed #{inspect(proxy)}") - {:error, :invalid_proxy} - end - end - - def parse_proxy(proxy) when is_tuple(proxy) do - with {type, host, port} <- proxy do - {:ok, type, parse_host(host), port} - else - _ -> - Logger.warn("Parsing proxy failed #{inspect(proxy)}") - {:error, :invalid_proxy} - end - end - - @spec parse_host(String.t() | atom() | charlist()) :: charlist() | ip_address() - def parse_host(host) when is_list(host), do: host - def parse_host(host) when is_atom(host), do: to_charlist(host) - - def parse_host(host) when is_binary(host) do - host = to_charlist(host) - - case :inet.parse_address(host) do - {:error, :einval} -> host - {:ok, ip} -> ip - end - end - - @spec format_host(String.t()) :: charlist() - def format_host(host) do - host_charlist = to_charlist(host) - - case :inet.parse_address(host_charlist) do - {:error, :einval} -> - :idna.encode(host_charlist) - - {:ok, _ip} -> - host_charlist - end - end -end diff --git a/lib/pleroma/http/http.ex b/lib/pleroma/http/http.ex index 66ca75367..8ded76601 100644 --- a/lib/pleroma/http/http.ex +++ b/lib/pleroma/http/http.ex @@ -7,7 +7,7 @@ defmodule Pleroma.HTTP do Wrapper for `Tesla.request/2`. """ - alias Pleroma.HTTP.Connection + alias Pleroma.HTTP.AdapterHelper alias Pleroma.HTTP.Request alias Pleroma.HTTP.RequestBuilder, as: Builder alias Tesla.Client @@ -60,49 +60,26 @@ defmodule Pleroma.HTTP do {:ok, Env.t()} | {:error, any()} def request(method, url, body, headers, options) when is_binary(url) do uri = URI.parse(url) - adapter_opts = Connection.options(uri, options[:adapter] || []) - options = put_in(options[:adapter], adapter_opts) - params = options[:params] || [] - request = build_request(method, headers, options, url, body, params) + adapter_opts = AdapterHelper.options(uri, options[:adapter] || []) - adapter = Application.get_env(:tesla, :adapter) - client = Tesla.client([Tesla.Middleware.FollowRedirects], adapter) + case AdapterHelper.get_conn(uri, adapter_opts) do + {:ok, adapter_opts} -> + options = put_in(options[:adapter], adapter_opts) + params = options[:params] || [] + request = build_request(method, headers, options, url, body, params) - pid = Process.whereis(adapter_opts[:pool]) + adapter = Application.get_env(:tesla, :adapter) + client = Tesla.client([Tesla.Middleware.FollowRedirects], adapter) - pool_alive? = - if adapter == Tesla.Adapter.Gun && pid do - Process.alive?(pid) - else - false - end + response = request(client, request) - request_opts = - adapter_opts - |> Enum.into(%{}) - |> Map.put(:env, Pleroma.Config.get([:env])) - |> Map.put(:pool_alive?, pool_alive?) + AdapterHelper.after_request(adapter_opts) - response = request(client, request, request_opts) + response - Connection.after_request(adapter_opts) - - response - end - - @spec request(Client.t(), keyword(), map()) :: {:ok, Env.t()} | {:error, any()} - def request(%Client{} = client, request, %{env: :test}), do: request(client, request) - - def request(%Client{} = client, request, %{body_as: :chunks}), do: request(client, request) - - def request(%Client{} = client, request, %{pool_alive?: false}), do: request(client, request) - - def request(%Client{} = client, request, %{pool: pool, timeout: timeout}) do - :poolboy.transaction( - pool, - &Pleroma.Pool.Request.execute(&1, client, request, timeout), - timeout - ) + err -> + err + end end @spec request(Client.t(), keyword()) :: {:ok, Env.t()} | {:error, any()} diff --git a/lib/pleroma/pool/connections.ex b/lib/pleroma/pool/connections.ex deleted file mode 100644 index acafe1bea..000000000 --- a/lib/pleroma/pool/connections.ex +++ /dev/null @@ -1,283 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Pool.Connections do - use GenServer - - alias Pleroma.Config - alias Pleroma.Gun - - require Logger - - @type domain :: String.t() - @type conn :: Pleroma.Gun.Conn.t() - - @type t :: %__MODULE__{ - conns: %{domain() => conn()}, - opts: keyword() - } - - defstruct conns: %{}, opts: [] - - @spec start_link({atom(), keyword()}) :: {:ok, pid()} - def start_link({name, opts}) do - GenServer.start_link(__MODULE__, opts, name: name) - end - - @impl true - def init(opts), do: {:ok, %__MODULE__{conns: %{}, opts: opts}} - - @spec checkin(String.t() | URI.t(), atom()) :: pid() | nil - def checkin(url, name) - def checkin(url, name) when is_binary(url), do: checkin(URI.parse(url), name) - - def checkin(%URI{} = uri, name) do - timeout = Config.get([:connections_pool, :checkin_timeout], 250) - - GenServer.call(name, {:checkin, uri}, timeout) - end - - @spec alive?(atom()) :: boolean() - def alive?(name) do - if pid = Process.whereis(name) do - Process.alive?(pid) - else - false - end - end - - @spec get_state(atom()) :: t() - def get_state(name) do - GenServer.call(name, :state) - end - - @spec count(atom()) :: pos_integer() - def count(name) do - GenServer.call(name, :count) - end - - @spec get_unused_conns(atom()) :: [{domain(), conn()}] - def get_unused_conns(name) do - GenServer.call(name, :unused_conns) - end - - @spec checkout(pid(), pid(), atom()) :: :ok - def checkout(conn, pid, name) do - GenServer.cast(name, {:checkout, conn, pid}) - end - - @spec add_conn(atom(), String.t(), Pleroma.Gun.Conn.t()) :: :ok - def add_conn(name, key, conn) do - GenServer.cast(name, {:add_conn, key, conn}) - end - - @spec remove_conn(atom(), String.t()) :: :ok - def remove_conn(name, key) do - GenServer.cast(name, {:remove_conn, key}) - end - - @impl true - def handle_cast({:add_conn, key, conn}, state) do - state = put_in(state.conns[key], conn) - - Process.monitor(conn.conn) - {:noreply, state} - end - - @impl true - def handle_cast({:checkout, conn_pid, pid}, state) do - state = - with true <- Process.alive?(conn_pid), - {key, conn} <- find_conn(state.conns, conn_pid), - used_by <- List.keydelete(conn.used_by, pid, 0) do - conn_state = if used_by == [], do: :idle, else: conn.conn_state - - put_in(state.conns[key], %{conn | conn_state: conn_state, used_by: used_by}) - else - false -> - Logger.debug("checkout for closed conn #{inspect(conn_pid)}") - state - - nil -> - Logger.debug("checkout for alive conn #{inspect(conn_pid)}, but is not in state") - state - end - - {:noreply, state} - end - - @impl true - def handle_cast({:remove_conn, key}, state) do - state = put_in(state.conns, Map.delete(state.conns, key)) - {:noreply, state} - end - - @impl true - def handle_call({:checkin, uri}, from, state) do - key = "#{uri.scheme}:#{uri.host}:#{uri.port}" - - case state.conns[key] do - %{conn: pid, gun_state: :up} = conn -> - time = :os.system_time(:second) - last_reference = time - conn.last_reference - crf = crf(last_reference, 100, conn.crf) - - state = - put_in(state.conns[key], %{ - conn - | last_reference: time, - crf: crf, - conn_state: :active, - used_by: [from | conn.used_by] - }) - - {:reply, pid, state} - - %{gun_state: :down} -> - {:reply, nil, state} - - nil -> - {:reply, nil, state} - end - end - - @impl true - def handle_call(:state, _from, state), do: {:reply, state, state} - - @impl true - def handle_call(:count, _from, state) do - {:reply, Enum.count(state.conns), state} - end - - @impl true - def handle_call(:unused_conns, _from, state) do - unused_conns = - state.conns - |> Enum.filter(&filter_conns/1) - |> Enum.sort(&sort_conns/2) - - {:reply, unused_conns, state} - end - - defp filter_conns({_, %{conn_state: :idle, used_by: []}}), do: true - defp filter_conns(_), do: false - - defp sort_conns({_, c1}, {_, c2}) do - c1.crf <= c2.crf and c1.last_reference <= c2.last_reference - end - - @impl true - def handle_info({:gun_up, conn_pid, _protocol}, state) do - %{origin_host: host, origin_scheme: scheme, origin_port: port} = Gun.info(conn_pid) - - host = - case :inet.ntoa(host) do - {:error, :einval} -> host - ip -> ip - end - - key = "#{scheme}:#{host}:#{port}" - - state = - with {key, conn} <- find_conn(state.conns, conn_pid, key), - {true, key} <- {Process.alive?(conn_pid), key} do - put_in(state.conns[key], %{ - conn - | gun_state: :up, - conn_state: :active, - retries: 0 - }) - else - {false, key} -> - put_in( - state.conns, - Map.delete(state.conns, key) - ) - - nil -> - :ok = Gun.close(conn_pid) - - state - end - - {:noreply, state} - end - - @impl true - def handle_info({:gun_down, conn_pid, _protocol, _reason, _killed}, state) do - retries = Config.get([:connections_pool, :retry], 1) - # we can't get info on this pid, because pid is dead - state = - with {key, conn} <- find_conn(state.conns, conn_pid), - {true, key} <- {Process.alive?(conn_pid), key} do - if conn.retries == retries do - :ok = Gun.close(conn.conn) - - put_in( - state.conns, - Map.delete(state.conns, key) - ) - else - put_in(state.conns[key], %{ - conn - | gun_state: :down, - retries: conn.retries + 1 - }) - end - else - {false, key} -> - put_in( - state.conns, - Map.delete(state.conns, key) - ) - - nil -> - Logger.debug(":gun_down for conn which isn't found in state") - - state - end - - {:noreply, state} - end - - @impl true - def handle_info({:DOWN, _ref, :process, conn_pid, reason}, state) do - Logger.debug("received DOWN message for #{inspect(conn_pid)} reason -> #{inspect(reason)}") - - state = - with {key, conn} <- find_conn(state.conns, conn_pid) do - Enum.each(conn.used_by, fn {pid, _ref} -> - Process.exit(pid, reason) - end) - - put_in( - state.conns, - Map.delete(state.conns, key) - ) - else - nil -> - Logger.debug(":DOWN for conn which isn't found in state") - - state - end - - {:noreply, state} - end - - defp find_conn(conns, conn_pid) do - Enum.find(conns, fn {_key, conn} -> - conn.conn == conn_pid - end) - end - - defp find_conn(conns, conn_pid, conn_key) do - Enum.find(conns, fn {key, conn} -> - key == conn_key and conn.conn == conn_pid - end) - end - - def crf(current, steps, crf) do - 1 + :math.pow(0.5, current / steps) * crf - end -end diff --git a/lib/pleroma/pool/pool.ex b/lib/pleroma/pool/pool.ex deleted file mode 100644 index 21a6fbbc5..000000000 --- a/lib/pleroma/pool/pool.ex +++ /dev/null @@ -1,22 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Pool do - def child_spec(opts) do - poolboy_opts = - opts - |> Keyword.put(:worker_module, Pleroma.Pool.Request) - |> Keyword.put(:name, {:local, opts[:name]}) - |> Keyword.put(:size, opts[:size]) - |> Keyword.put(:max_overflow, opts[:max_overflow]) - - %{ - id: opts[:id] || {__MODULE__, make_ref()}, - start: {:poolboy, :start_link, [poolboy_opts, [name: opts[:name]]]}, - restart: :permanent, - shutdown: 5000, - type: :worker - } - end -end diff --git a/lib/pleroma/pool/request.ex b/lib/pleroma/pool/request.ex deleted file mode 100644 index 3fb930db7..000000000 --- a/lib/pleroma/pool/request.ex +++ /dev/null @@ -1,65 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Pool.Request do - use GenServer - - require Logger - - def start_link(args) do - GenServer.start_link(__MODULE__, args) - end - - @impl true - def init(_), do: {:ok, []} - - @spec execute(pid() | atom(), Tesla.Client.t(), keyword(), pos_integer()) :: - {:ok, Tesla.Env.t()} | {:error, any()} - def execute(pid, client, request, timeout) do - GenServer.call(pid, {:execute, client, request}, timeout) - end - - @impl true - def handle_call({:execute, client, request}, _from, state) do - response = Pleroma.HTTP.request(client, request) - - {:reply, response, state} - end - - @impl true - def handle_info({:gun_data, _conn, _stream, _, _}, state) do - {:noreply, state} - end - - @impl true - def handle_info({:gun_up, _conn, _protocol}, state) do - {:noreply, state} - end - - @impl true - def handle_info({:gun_down, _conn, _protocol, _reason, _killed}, state) do - {:noreply, state} - end - - @impl true - def handle_info({:gun_error, _conn, _stream, _error}, state) do - {:noreply, state} - end - - @impl true - def handle_info({:gun_push, _conn, _stream, _new_stream, _method, _uri, _headers}, state) do - {:noreply, state} - end - - @impl true - def handle_info({:gun_response, _conn, _stream, _, _status, _headers}, state) do - {:noreply, state} - end - - @impl true - def handle_info(msg, state) do - Logger.warn("Received unexpected message #{inspect(__MODULE__)} #{inspect(msg)}") - {:noreply, state} - end -end diff --git a/lib/pleroma/pool/supervisor.ex b/lib/pleroma/pool/supervisor.ex deleted file mode 100644 index faf646cb2..000000000 --- a/lib/pleroma/pool/supervisor.ex +++ /dev/null @@ -1,42 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Pool.Supervisor do - use Supervisor - - alias Pleroma.Config - alias Pleroma.Pool - - def start_link(args) do - Supervisor.start_link(__MODULE__, args, name: __MODULE__) - end - - def init(_) do - conns_child = %{ - id: Pool.Connections, - start: - {Pool.Connections, :start_link, [{:gun_connections, Config.get([:connections_pool])}]} - } - - Supervisor.init([conns_child | pools()], strategy: :one_for_one) - end - - defp pools do - pools = Config.get(:pools) - - pools = - if Config.get([Pleroma.Upload, :proxy_remote]) == false do - Keyword.delete(pools, :upload) - else - pools - end - - for {pool_name, pool_opts} <- pools do - pool_opts - |> Keyword.put(:id, {Pool, pool_name}) - |> Keyword.put(:name, pool_name) - |> Pool.child_spec() - end - end -end diff --git a/lib/pleroma/reverse_proxy/client/tesla.ex b/lib/pleroma/reverse_proxy/client/tesla.ex index e81ea8bde..65785445d 100644 --- a/lib/pleroma/reverse_proxy/client/tesla.ex +++ b/lib/pleroma/reverse_proxy/client/tesla.ex @@ -48,7 +48,7 @@ defmodule Pleroma.ReverseProxy.Client.Tesla do # if there were redirects we need to checkout old conn conn = opts[:old_conn] || opts[:conn] - if conn, do: :ok = Pleroma.Pool.Connections.checkout(conn, self(), :gun_connections) + if conn, do: :ok = Pleroma.Gun.ConnectionPool.release_conn(conn) :done end -- cgit v1.2.3 From fffbcffb8c9ce1e96de5d1a5e15005e271deacd4 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 6 May 2020 21:41:34 +0300 Subject: Connection Pool: don't enforce pool limits if no new connection needs to be opened --- lib/pleroma/gun/connection_pool.ex | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool.ex b/lib/pleroma/gun/connection_pool.ex index e6abee69c..ed7ddff81 100644 --- a/lib/pleroma/gun/connection_pool.ex +++ b/lib/pleroma/gun/connection_pool.ex @@ -2,20 +2,20 @@ defmodule Pleroma.Gun.ConnectionPool do @registry __MODULE__ def get_conn(uri, opts) do - case enforce_pool_limits() do - :ok -> - key = "#{uri.scheme}:#{uri.host}:#{uri.port}" + key = "#{uri.scheme}:#{uri.host}:#{uri.port}" - case Registry.lookup(@registry, key) do - # The key has already been registered, but connection is not up yet - [{worker_pid, {nil, _used_by, _crf, _last_reference}}] -> - get_gun_pid_from_worker(worker_pid) + case Registry.lookup(@registry, key) do + # The key has already been registered, but connection is not up yet + [{worker_pid, {nil, _used_by, _crf, _last_reference}}] -> + get_gun_pid_from_worker(worker_pid) - [{worker_pid, {gun_pid, _used_by, _crf, _last_reference}}] -> - GenServer.cast(worker_pid, {:add_client, self(), false}) - {:ok, gun_pid} + [{worker_pid, {gun_pid, _used_by, _crf, _last_reference}}] -> + GenServer.cast(worker_pid, {:add_client, self(), false}) + {:ok, gun_pid} - [] -> + [] -> + case enforce_pool_limits() do + :ok -> # :gun.set_owner fails in :connected state for whatevever reason, # so we open the connection in the process directly and send it's pid back # We trust gun to handle timeouts by itself @@ -33,10 +33,10 @@ defmodule Pleroma.Gun.ConnectionPool do err -> err end - end - :error -> - {:error, :pool_full} + :error -> + {:error, :pool_full} + end end end -- cgit v1.2.3 From d08b1576990ca33ac4178fb757ec03a777c55b5b Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 6 May 2020 21:51:10 +0300 Subject: Connection pool: check that there actually is a result Sometimes connections died before being released to the pool, resulting in MatchErrors --- lib/pleroma/gun/connection_pool.ex | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool.ex b/lib/pleroma/gun/connection_pool.ex index ed7ddff81..0daf1da44 100644 --- a/lib/pleroma/gun/connection_pool.ex +++ b/lib/pleroma/gun/connection_pool.ex @@ -119,11 +119,17 @@ defmodule Pleroma.Gun.ConnectionPool do end def release_conn(conn_pid) do - [worker_pid] = + query_result = Registry.select(@registry, [ {{:_, :"$1", {:"$2", :_, :_, :_}}, [{:==, :"$2", conn_pid}], [:"$1"]} ]) - GenServer.cast(worker_pid, {:remove_client, self()}) + case query_result do + [worker_pid] -> + GenServer.cast(worker_pid, {:remove_client, self()}) + + [] -> + :ok + end end end -- cgit v1.2.3 From ec9d0d146b4ec6752f8f2896ace9bb5585469773 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 6 May 2020 23:14:24 +0300 Subject: Connection pool: Fix race conditions in limit enforcement Fixes race conditions in limit enforcement by putting worker processes in a DynamicSupervisor --- lib/pleroma/application.ex | 2 +- lib/pleroma/gun/connection_pool.ex | 105 +++++---------------- lib/pleroma/gun/connection_pool/worker.ex | 12 +-- .../gun/connection_pool/worker_supervisor.ex | 91 ++++++++++++++++++ 4 files changed, 118 insertions(+), 92 deletions(-) create mode 100644 lib/pleroma/gun/connection_pool/worker_supervisor.ex (limited to 'lib') diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index be14c1f9f..cfdaf1770 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -243,7 +243,7 @@ defmodule Pleroma.Application do end defp http_children(Tesla.Adapter.Gun, _) do - [{Registry, keys: :unique, name: Pleroma.Gun.ConnectionPool}] + Pleroma.Gun.ConnectionPool.children() end defp http_children(_, _), do: [] diff --git a/lib/pleroma/gun/connection_pool.ex b/lib/pleroma/gun/connection_pool.ex index 0daf1da44..545bfaf7f 100644 --- a/lib/pleroma/gun/connection_pool.ex +++ b/lib/pleroma/gun/connection_pool.ex @@ -1,6 +1,15 @@ defmodule Pleroma.Gun.ConnectionPool do @registry __MODULE__ + alias Pleroma.Gun.ConnectionPool.WorkerSupervisor + + def children do + [ + {Registry, keys: :unique, name: @registry}, + Pleroma.Gun.ConnectionPool.WorkerSupervisor + ] + end + def get_conn(uri, opts) do key = "#{uri.scheme}:#{uri.host}:#{uri.port}" @@ -14,93 +23,21 @@ defmodule Pleroma.Gun.ConnectionPool do {:ok, gun_pid} [] -> - case enforce_pool_limits() do - :ok -> - # :gun.set_owner fails in :connected state for whatevever reason, - # so we open the connection in the process directly and send it's pid back - # We trust gun to handle timeouts by itself - case GenServer.start(Pleroma.Gun.ConnectionPool.Worker, [uri, key, opts, self()], - timeout: :infinity - ) do - {:ok, _worker_pid} -> - receive do - {:conn_pid, pid} -> {:ok, pid} - end - - {:error, {:error, {:already_registered, worker_pid}}} -> - get_gun_pid_from_worker(worker_pid) - - err -> - err + # :gun.set_owner fails in :connected state for whatevever reason, + # so we open the connection in the process directly and send it's pid back + # We trust gun to handle timeouts by itself + case WorkerSupervisor.start_worker([uri, key, opts, self()]) do + {:ok, _worker_pid} -> + receive do + {:conn_pid, pid} -> {:ok, pid} end - :error -> - {:error, :pool_full} - end - end - end - - @enforcer_key "enforcer" - defp enforce_pool_limits() do - max_connections = Pleroma.Config.get([:connections_pool, :max_connections]) - - if Registry.count(@registry) >= max_connections do - case Registry.lookup(@registry, @enforcer_key) do - [] -> - pid = - spawn(fn -> - {:ok, _pid} = Registry.register(@registry, @enforcer_key, nil) - - reclaim_max = - [:connections_pool, :reclaim_multiplier] - |> Pleroma.Config.get() - |> Kernel.*(max_connections) - |> round - |> max(1) - - unused_conns = - Registry.select( - @registry, - [ - {{:_, :"$1", {:_, :"$2", :"$3", :"$4"}}, [{:==, :"$2", []}], - [{{:"$1", :"$3", :"$4"}}]} - ] - ) + {:error, {:error, {:already_registered, worker_pid}}} -> + get_gun_pid_from_worker(worker_pid) - case unused_conns do - [] -> - exit(:pool_full) - - unused_conns -> - unused_conns - |> Enum.sort(fn {_pid1, crf1, last_reference1}, - {_pid2, crf2, last_reference2} -> - crf1 <= crf2 and last_reference1 <= last_reference2 - end) - |> Enum.take(reclaim_max) - |> Enum.each(fn {pid, _, _} -> GenServer.call(pid, :idle_close) end) - end - end) - - wait_for_enforcer_finish(pid) - - [{pid, _}] -> - wait_for_enforcer_finish(pid) - end - else - :ok - end - end - - defp wait_for_enforcer_finish(pid) do - ref = Process.monitor(pid) - - receive do - {:DOWN, ^ref, :process, ^pid, :pool_full} -> - :error - - {:DOWN, ^ref, :process, ^pid, :normal} -> - :ok + err -> + err + end end end diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex index ebde4bbf6..25fafc64c 100644 --- a/lib/pleroma/gun/connection_pool/worker.ex +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -1,9 +1,13 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do alias Pleroma.Gun - use GenServer + use GenServer, restart: :temporary @registry Pleroma.Gun.ConnectionPool + def start_link(opts) do + GenServer.start_link(__MODULE__, opts) + end + @impl true def init([uri, key, opts, client_pid]) do time = :os.system_time(:second) @@ -82,12 +86,6 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do {:stop, {:error, down_message}, state} end - @impl true - def handle_call(:idle_close, _, %{key: key} = state) do - Registry.unregister(@registry, key) - {:stop, :normal, state} - end - # LRFU policy: https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.55.1478 defp crf(time_delta, prev_crf) do 1 + :math.pow(0.5, time_delta / 100) * prev_crf diff --git a/lib/pleroma/gun/connection_pool/worker_supervisor.ex b/lib/pleroma/gun/connection_pool/worker_supervisor.ex new file mode 100644 index 000000000..5b546bd87 --- /dev/null +++ b/lib/pleroma/gun/connection_pool/worker_supervisor.ex @@ -0,0 +1,91 @@ +defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do + @doc "Supervisor for pool workers. Does not do anything except enforce max connection limit" + + use DynamicSupervisor + + def start_link(opts) do + DynamicSupervisor.start_link(__MODULE__, opts, name: __MODULE__) + end + + def init(_opts) do + DynamicSupervisor.init( + strategy: :one_for_one, + max_children: Pleroma.Config.get([:connections_pool, :max_connections]) + ) + end + + def start_worker(opts) do + case DynamicSupervisor.start_child(__MODULE__, {Pleroma.Gun.ConnectionPool.Worker, opts}) do + {:error, :max_children} -> + case free_pool() do + :ok -> start_worker(opts) + :error -> {:error, :pool_full} + end + + res -> + res + end + end + + @registry Pleroma.Gun.ConnectionPool + @enforcer_key "enforcer" + defp free_pool do + case Registry.lookup(@registry, @enforcer_key) do + [] -> + pid = + spawn(fn -> + {:ok, _pid} = Registry.register(@registry, @enforcer_key, nil) + + max_connections = Pleroma.Config.get([:connections_pool, :max_connections]) + + reclaim_max = + [:connections_pool, :reclaim_multiplier] + |> Pleroma.Config.get() + |> Kernel.*(max_connections) + |> round + |> max(1) + + unused_conns = + Registry.select( + @registry, + [ + {{:_, :"$1", {:_, :"$2", :"$3", :"$4"}}, [{:==, :"$2", []}], + [{{:"$1", :"$3", :"$4"}}]} + ] + ) + + case unused_conns do + [] -> + exit(:no_unused_conns) + + unused_conns -> + unused_conns + |> Enum.sort(fn {_pid1, crf1, last_reference1}, {_pid2, crf2, last_reference2} -> + crf1 <= crf2 and last_reference1 <= last_reference2 + end) + |> Enum.take(reclaim_max) + |> Enum.each(fn {pid, _, _} -> + DynamicSupervisor.terminate_child(__MODULE__, pid) + end) + end + end) + + wait_for_enforcer_finish(pid) + + [{pid, _}] -> + wait_for_enforcer_finish(pid) + end + end + + defp wait_for_enforcer_finish(pid) do + ref = Process.monitor(pid) + + receive do + {:DOWN, ^ref, :process, ^pid, :no_unused_conns} -> + :error + + {:DOWN, ^ref, :process, ^pid, :normal} -> + :ok + end + end +end -- cgit v1.2.3 From 0ffde499b8a8f31c82183253bdd692c75733ca2f Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 15 Jul 2020 15:24:47 +0300 Subject: Connection Pool: register workers using :via --- lib/pleroma/gun/connection_pool.ex | 8 +++++--- lib/pleroma/gun/connection_pool/worker.ex | 17 ++++++++--------- lib/pleroma/gun/connection_pool/worker_supervisor.ex | 3 +-- 3 files changed, 14 insertions(+), 14 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool.ex b/lib/pleroma/gun/connection_pool.ex index 545bfaf7f..e951872fe 100644 --- a/lib/pleroma/gun/connection_pool.ex +++ b/lib/pleroma/gun/connection_pool.ex @@ -15,7 +15,7 @@ defmodule Pleroma.Gun.ConnectionPool do case Registry.lookup(@registry, key) do # The key has already been registered, but connection is not up yet - [{worker_pid, {nil, _used_by, _crf, _last_reference}}] -> + [{worker_pid, nil}] -> get_gun_pid_from_worker(worker_pid) [{worker_pid, {gun_pid, _used_by, _crf, _last_reference}}] -> @@ -26,13 +26,13 @@ defmodule Pleroma.Gun.ConnectionPool do # :gun.set_owner fails in :connected state for whatevever reason, # so we open the connection in the process directly and send it's pid back # We trust gun to handle timeouts by itself - case WorkerSupervisor.start_worker([uri, key, opts, self()]) do + case WorkerSupervisor.start_worker([key, uri, opts, self()]) do {:ok, _worker_pid} -> receive do {:conn_pid, pid} -> {:ok, pid} end - {:error, {:error, {:already_registered, worker_pid}}} -> + {:error, {:already_started, worker_pid}} -> get_gun_pid_from_worker(worker_pid) err -> @@ -56,6 +56,8 @@ defmodule Pleroma.Gun.ConnectionPool do end def release_conn(conn_pid) do + # :ets.fun2ms(fn {_, {worker_pid, {gun_pid, _, _, _}}} when gun_pid == conn_pid -> + # worker_pid end) query_result = Registry.select(@registry, [ {{:_, :"$1", {:"$2", :_, :_, :_}}, [{:==, :"$2", conn_pid}], [:"$1"]} diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex index 25fafc64c..0a94f16a2 100644 --- a/lib/pleroma/gun/connection_pool/worker.ex +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -4,20 +4,19 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do @registry Pleroma.Gun.ConnectionPool - def start_link(opts) do - GenServer.start_link(__MODULE__, opts) + def start_link([key | _] = opts) do + GenServer.start_link(__MODULE__, opts, name: {:via, Registry, {@registry, key}}) end @impl true - def init([uri, key, opts, client_pid]) do - time = :os.system_time(:second) - # Register before opening connection to prevent race conditions - with {:ok, _owner} <- Registry.register(@registry, key, {nil, [client_pid], 1, time}), - {:ok, conn_pid} <- Gun.Conn.open(uri, opts), + def init([key, uri, opts, client_pid]) do + with {:ok, conn_pid} <- Gun.Conn.open(uri, opts), Process.link(conn_pid) do + time = :os.system_time(:second) + {_, _} = - Registry.update_value(@registry, key, fn {_, used_by, crf, last_reference} -> - {conn_pid, used_by, crf, last_reference} + Registry.update_value(@registry, key, fn _ -> + {conn_pid, [client_pid], 1, time} end) send(client_pid, {:conn_pid, conn_pid}) diff --git a/lib/pleroma/gun/connection_pool/worker_supervisor.ex b/lib/pleroma/gun/connection_pool/worker_supervisor.ex index 5b546bd87..d090c034e 100644 --- a/lib/pleroma/gun/connection_pool/worker_supervisor.ex +++ b/lib/pleroma/gun/connection_pool/worker_supervisor.ex @@ -1,5 +1,5 @@ defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do - @doc "Supervisor for pool workers. Does not do anything except enforce max connection limit" + @moduledoc "Supervisor for pool workers. Does not do anything except enforce max connection limit" use DynamicSupervisor @@ -35,7 +35,6 @@ defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do pid = spawn(fn -> {:ok, _pid} = Registry.register(@registry, @enforcer_key, nil) - max_connections = Pleroma.Config.get([:connections_pool, :max_connections]) reclaim_max = -- cgit v1.2.3 From 7738fbbaf5a6fcd6a10b4ef0a2dcea731a3d4192 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 15 Jul 2020 15:26:25 +0300 Subject: Connection pool: implement logging and telemetry events --- lib/pleroma/application.ex | 1 + .../gun/connection_pool/worker_supervisor.ex | 44 ++++++++++++--- lib/pleroma/telemetry/logger.ex | 62 ++++++++++++++++++++++ 3 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 lib/pleroma/telemetry/logger.ex (limited to 'lib') diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index cfdaf1770..37fcdf293 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -39,6 +39,7 @@ defmodule Pleroma.Application do # every time the application is restarted, so we disable module # conflicts at runtime Code.compiler_options(ignore_module_conflict: true) + Pleroma.Telemetry.Logger.attach() Config.Holder.save_default() Pleroma.HTML.compile_scrubbers() Config.DeprecationWarnings.warn() diff --git a/lib/pleroma/gun/connection_pool/worker_supervisor.ex b/lib/pleroma/gun/connection_pool/worker_supervisor.ex index d090c034e..4b5d10d2a 100644 --- a/lib/pleroma/gun/connection_pool/worker_supervisor.ex +++ b/lib/pleroma/gun/connection_pool/worker_supervisor.ex @@ -18,8 +18,12 @@ defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do case DynamicSupervisor.start_child(__MODULE__, {Pleroma.Gun.ConnectionPool.Worker, opts}) do {:error, :max_children} -> case free_pool() do - :ok -> start_worker(opts) - :error -> {:error, :pool_full} + :ok -> + start_worker(opts) + + :error -> + :telemetry.execute([:pleroma, :connection_pool, :provision_failure], %{opts: opts}) + {:error, :pool_full} end res -> @@ -44,6 +48,14 @@ defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do |> round |> max(1) + :telemetry.execute([:pleroma, :connection_pool, :reclaim, :start], %{}, %{ + max_connections: max_connections, + reclaim_max: reclaim_max + }) + + # :ets.fun2ms( + # fn {_, {worker_pid, {_, used_by, crf, last_reference}}} when used_by == [] -> + # {worker_pid, crf, last_reference} end) unused_conns = Registry.select( @registry, @@ -55,17 +67,35 @@ defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do case unused_conns do [] -> + :telemetry.execute( + [:pleroma, :connection_pool, :reclaim, :stop], + %{reclaimed_count: 0}, + %{ + max_connections: max_connections + } + ) + exit(:no_unused_conns) unused_conns -> - unused_conns - |> Enum.sort(fn {_pid1, crf1, last_reference1}, {_pid2, crf2, last_reference2} -> - crf1 <= crf2 and last_reference1 <= last_reference2 - end) - |> Enum.take(reclaim_max) + reclaimed = + unused_conns + |> Enum.sort(fn {_pid1, crf1, last_reference1}, + {_pid2, crf2, last_reference2} -> + crf1 <= crf2 and last_reference1 <= last_reference2 + end) + |> Enum.take(reclaim_max) + + reclaimed |> Enum.each(fn {pid, _, _} -> DynamicSupervisor.terminate_child(__MODULE__, pid) end) + + :telemetry.execute( + [:pleroma, :connection_pool, :reclaim, :stop], + %{reclaimed_count: Enum.count(reclaimed)}, + %{max_connections: max_connections} + ) end end) diff --git a/lib/pleroma/telemetry/logger.ex b/lib/pleroma/telemetry/logger.ex new file mode 100644 index 000000000..d76dd37b5 --- /dev/null +++ b/lib/pleroma/telemetry/logger.ex @@ -0,0 +1,62 @@ +defmodule Pleroma.Telemetry.Logger do + @moduledoc "Transforms Pleroma telemetry events to logs" + + require Logger + + @events [ + [:pleroma, :connection_pool, :reclaim, :start], + [:pleroma, :connection_pool, :reclaim, :stop], + [:pleroma, :connection_pool, :provision_failure] + ] + def attach do + :telemetry.attach_many("pleroma-logger", @events, &handle_event/4, []) + end + + # Passing anonymous functions instead of strings to logger is intentional, + # that way strings won't be concatenated if the message is going to be thrown + # out anyway due to higher log level configured + + def handle_event( + [:pleroma, :connection_pool, :reclaim, :start], + _, + %{max_connections: max_connections, reclaim_max: reclaim_max}, + _ + ) do + Logger.debug(fn -> + "Connection pool is exhausted (reached #{max_connections} connections). Starting idle connection cleanup to reclaim as much as #{ + reclaim_max + } connections" + end) + end + + def handle_event( + [:pleroma, :connection_pool, :reclaim, :stop], + %{reclaimed_count: 0}, + _, + _ + ) do + Logger.error(fn -> + "Connection pool failed to reclaim any connections due to all of them being in use. It will have to drop requests for opening connections to new hosts" + end) + end + + def handle_event( + [:pleroma, :connection_pool, :reclaim, :stop], + %{reclaimed_count: reclaimed_count}, + _, + _ + ) do + Logger.debug(fn -> "Connection pool cleaned up #{reclaimed_count} idle connections" end) + end + + def handle_event( + [:pleroma, :connection_pool, :provision_failure], + %{opts: [key | _]}, + _, + _ + ) do + Logger.error(fn -> + "Connection pool had to refuse opening a connection to #{key} due to connection limit exhaustion" + end) + end +end -- cgit v1.2.3 From e94ba05e523d735cd7a357a3aa30e433f60ef9a3 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 7 May 2020 16:11:48 +0300 Subject: Connection pool: Fix a possible infinite recursion if the pool is exhausted --- lib/pleroma/gun/connection_pool/worker_supervisor.ex | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool/worker_supervisor.ex b/lib/pleroma/gun/connection_pool/worker_supervisor.ex index 4b5d10d2a..5cb8d488a 100644 --- a/lib/pleroma/gun/connection_pool/worker_supervisor.ex +++ b/lib/pleroma/gun/connection_pool/worker_supervisor.ex @@ -14,16 +14,14 @@ defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do ) end - def start_worker(opts) do + def start_worker(opts, retry \\ false) do case DynamicSupervisor.start_child(__MODULE__, {Pleroma.Gun.ConnectionPool.Worker, opts}) do {:error, :max_children} -> - case free_pool() do - :ok -> - start_worker(opts) - - :error -> - :telemetry.execute([:pleroma, :connection_pool, :provision_failure], %{opts: opts}) - {:error, :pool_full} + if retry or free_pool() == :error do + :telemetry.execute([:pleroma, :connection_pool, :provision_failure], %{opts: opts}) + {:error, :pool_full} + else + start_worker(opts, true) end res -> -- cgit v1.2.3 From 1b15cb066c612c72d106e7e7026819ea14e0ceab Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 8 May 2020 18:18:59 +0300 Subject: Connection pool: Add client death tracking While running this in production I noticed a number of ghost processes with all their clients dead before they released the connection, so let's track them to log it and remove them from clients --- lib/pleroma/gun/connection_pool/worker.ex | 31 ++++++++++++++++++++++++++++++- lib/pleroma/telemetry/logger.ex | 16 +++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex index 0a94f16a2..8467325f3 100644 --- a/lib/pleroma/gun/connection_pool/worker.ex +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -20,7 +20,10 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do end) send(client_pid, {:conn_pid, conn_pid}) - {:ok, %{key: key, timer: nil}, :hibernate} + + {:ok, + %{key: key, timer: nil, client_monitors: %{client_pid => Process.monitor(client_pid)}}, + :hibernate} else err -> {:stop, err} end @@ -45,6 +48,9 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do state end + ref = Process.monitor(client_pid) + + state = put_in(state.client_monitors[client_pid], ref) {:noreply, state, :hibernate} end @@ -55,6 +61,9 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do {conn_pid, List.delete(used_by, client_pid), crf, last_reference} end) + {ref, state} = pop_in(state.client_monitors[client_pid]) + Process.demonitor(ref) + timer = if used_by == [] do max_idle = Pleroma.Config.get([:connections_pool, :max_idle_time], 30_000) @@ -85,6 +94,26 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do {:stop, {:error, down_message}, state} end + @impl true + def handle_info({:DOWN, _ref, :process, pid, reason}, state) do + # Sometimes the client is dead before we demonitor it in :remove_client, so the message + # arrives anyway + + case state.client_monitors[pid] do + nil -> + {:noreply, state, :hibernate} + + _ref -> + :telemetry.execute( + [:pleroma, :connection_pool, :client_death], + %{client_pid: pid, reason: reason}, + %{key: state.key} + ) + + handle_cast({:remove_client, pid}, state) + end + end + # LRFU policy: https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.55.1478 defp crf(time_delta, prev_crf) do 1 + :math.pow(0.5, time_delta / 100) * prev_crf diff --git a/lib/pleroma/telemetry/logger.ex b/lib/pleroma/telemetry/logger.ex index d76dd37b5..4cacae02f 100644 --- a/lib/pleroma/telemetry/logger.ex +++ b/lib/pleroma/telemetry/logger.ex @@ -6,7 +6,8 @@ defmodule Pleroma.Telemetry.Logger do @events [ [:pleroma, :connection_pool, :reclaim, :start], [:pleroma, :connection_pool, :reclaim, :stop], - [:pleroma, :connection_pool, :provision_failure] + [:pleroma, :connection_pool, :provision_failure], + [:pleroma, :connection_pool, :client_death] ] def attach do :telemetry.attach_many("pleroma-logger", @events, &handle_event/4, []) @@ -59,4 +60,17 @@ defmodule Pleroma.Telemetry.Logger do "Connection pool had to refuse opening a connection to #{key} due to connection limit exhaustion" end) end + + def handle_event( + [:pleroma, :connection_pool, :client_death], + %{client_pid: client_pid, reason: reason}, + %{key: key}, + _ + ) do + Logger.warn(fn -> + "Pool worker for #{key}: Client #{inspect(client_pid)} died before releasing the connection with #{ + inspect(reason) + }" + end) + end end -- cgit v1.2.3 From 281ddd5e371c5698489774e703106bd7c3ccb56b Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 8 May 2020 19:57:11 +0300 Subject: Connection pool: fix connections being supervised by gun_sup --- lib/pleroma/gun/api.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/gun/api.ex b/lib/pleroma/gun/api.ex index f51cd7db8..09be74392 100644 --- a/lib/pleroma/gun/api.ex +++ b/lib/pleroma/gun/api.ex @@ -19,7 +19,8 @@ defmodule Pleroma.Gun.API do :tls_opts, :tcp_opts, :socks_opts, - :ws_opts + :ws_opts, + :supervise ] @impl Gun -- cgit v1.2.3 From 94c8f3cfafb92c6d092549b24bb69f3870e1c0d8 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 16 May 2020 11:49:19 +0300 Subject: Use a custom pool-aware FollowRedirects middleware --- lib/pleroma/http/adapter_helper.ex | 4 - lib/pleroma/http/adapter_helper/default.ex | 3 - lib/pleroma/http/adapter_helper/gun.ex | 9 -- lib/pleroma/http/adapter_helper/hackney.ex | 2 - lib/pleroma/http/http.ex | 9 +- lib/pleroma/tesla/middleware/follow_redirects.ex | 106 +++++++++++++++++++++++ 6 files changed, 109 insertions(+), 24 deletions(-) create mode 100644 lib/pleroma/tesla/middleware/follow_redirects.ex (limited to 'lib') diff --git a/lib/pleroma/http/adapter_helper.ex b/lib/pleroma/http/adapter_helper.ex index 0532ea31d..bcb9b2b1e 100644 --- a/lib/pleroma/http/adapter_helper.ex +++ b/lib/pleroma/http/adapter_helper.ex @@ -24,7 +24,6 @@ defmodule Pleroma.HTTP.AdapterHelper do | {Connection.proxy_type(), Connection.host(), pos_integer()} @callback options(keyword(), URI.t()) :: keyword() - @callback after_request(keyword()) :: :ok @callback get_conn(URI.t(), keyword()) :: {:ok, term()} | {:error, term()} @spec format_proxy(String.t() | tuple() | nil) :: proxy() | nil @@ -67,9 +66,6 @@ defmodule Pleroma.HTTP.AdapterHelper do Keyword.merge(opts, timeout: timeout) end - @spec after_request(keyword()) :: :ok - def after_request(opts), do: adapter_helper().after_request(opts) - def get_conn(uri, opts), do: adapter_helper().get_conn(uri, opts) defp adapter, do: Application.get_env(:tesla, :adapter) diff --git a/lib/pleroma/http/adapter_helper/default.ex b/lib/pleroma/http/adapter_helper/default.ex index 218cfacc0..e13441316 100644 --- a/lib/pleroma/http/adapter_helper/default.ex +++ b/lib/pleroma/http/adapter_helper/default.ex @@ -9,9 +9,6 @@ defmodule Pleroma.HTTP.AdapterHelper.Default do AdapterHelper.maybe_add_proxy(opts, AdapterHelper.format_proxy(proxy)) end - @spec after_request(keyword()) :: :ok - def after_request(_opts), do: :ok - @spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} def get_conn(_uri, opts), do: {:ok, opts} end diff --git a/lib/pleroma/http/adapter_helper/gun.ex b/lib/pleroma/http/adapter_helper/gun.ex index 6f7cc9784..5b4629978 100644 --- a/lib/pleroma/http/adapter_helper/gun.ex +++ b/lib/pleroma/http/adapter_helper/gun.ex @@ -34,15 +34,6 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do |> Keyword.merge(incoming_opts) end - @spec after_request(keyword()) :: :ok - def after_request(opts) do - if opts[:conn] && opts[:body_as] != :chunks do - ConnectionPool.release_conn(opts[:conn]) - end - - :ok - end - defp add_scheme_opts(opts, %{scheme: "http"}), do: opts defp add_scheme_opts(opts, %{scheme: "https"}) do diff --git a/lib/pleroma/http/adapter_helper/hackney.ex b/lib/pleroma/http/adapter_helper/hackney.ex index 42d552740..cd569422b 100644 --- a/lib/pleroma/http/adapter_helper/hackney.ex +++ b/lib/pleroma/http/adapter_helper/hackney.ex @@ -24,8 +24,6 @@ defmodule Pleroma.HTTP.AdapterHelper.Hackney do defp add_scheme_opts(opts, _), do: opts - def after_request(_), do: :ok - @spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} def get_conn(_uri, opts), do: {:ok, opts} end diff --git a/lib/pleroma/http/http.ex b/lib/pleroma/http/http.ex index 8ded76601..afcb4d738 100644 --- a/lib/pleroma/http/http.ex +++ b/lib/pleroma/http/http.ex @@ -69,14 +69,11 @@ defmodule Pleroma.HTTP do request = build_request(method, headers, options, url, body, params) adapter = Application.get_env(:tesla, :adapter) - client = Tesla.client([Tesla.Middleware.FollowRedirects], adapter) + client = Tesla.client([Pleroma.HTTP.Middleware.FollowRedirects], adapter) - response = request(client, request) - - AdapterHelper.after_request(adapter_opts) - - response + request(client, request) + # Connection release is handled in a custom FollowRedirects middleware err -> err end diff --git a/lib/pleroma/tesla/middleware/follow_redirects.ex b/lib/pleroma/tesla/middleware/follow_redirects.ex new file mode 100644 index 000000000..f2c502c69 --- /dev/null +++ b/lib/pleroma/tesla/middleware/follow_redirects.ex @@ -0,0 +1,106 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2015-2020 Tymon Tobolski +# Copyright © 2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.HTTP.Middleware.FollowRedirects do + @moduledoc """ + Pool-aware version of https://github.com/teamon/tesla/blob/master/lib/tesla/middleware/follow_redirects.ex + + Follow 3xx redirects + ## Options + - `:max_redirects` - limit number of redirects (default: `5`) + """ + + alias Pleroma.Gun.ConnectionPool + + @behaviour Tesla.Middleware + + @max_redirects 5 + @redirect_statuses [301, 302, 303, 307, 308] + + @impl Tesla.Middleware + def call(env, next, opts \\ []) do + max = Keyword.get(opts, :max_redirects, @max_redirects) + + redirect(env, next, max) + end + + defp redirect(env, next, left) do + opts = env.opts[:adapter] + + case Tesla.run(env, next) do + {:ok, %{status: status} = res} when status in @redirect_statuses and left > 0 -> + release_conn(opts) + + case Tesla.get_header(res, "location") do + nil -> + {:ok, res} + + location -> + location = parse_location(location, res) + + case get_conn(location, opts) do + {:ok, opts} -> + %{env | opts: Keyword.put(env.opts, :adapter, opts)} + |> new_request(res.status, location) + |> redirect(next, left - 1) + + e -> + e + end + end + + {:ok, %{status: status}} when status in @redirect_statuses -> + release_conn(opts) + {:error, {__MODULE__, :too_many_redirects}} + + other -> + unless opts[:body_as] == :chunks do + release_conn(opts) + end + + other + end + end + + defp get_conn(location, opts) do + uri = URI.parse(location) + + case ConnectionPool.get_conn(uri, opts) do + {:ok, conn} -> + {:ok, Keyword.merge(opts, conn: conn)} + + e -> + e + end + end + + defp release_conn(opts) do + ConnectionPool.release_conn(opts[:conn]) + end + + # The 303 (See Other) redirect was added in HTTP/1.1 to indicate that the originally + # requested resource is not available, however a related resource (or another redirect) + # available via GET is available at the specified location. + # https://tools.ietf.org/html/rfc7231#section-6.4.4 + defp new_request(env, 303, location), do: %{env | url: location, method: :get, query: []} + + # The 307 (Temporary Redirect) status code indicates that the target + # resource resides temporarily under a different URI and the user agent + # MUST NOT change the request method (...) + # https://tools.ietf.org/html/rfc7231#section-6.4.7 + defp new_request(env, 307, location), do: %{env | url: location} + + defp new_request(env, _, location), do: %{env | url: location, query: []} + + defp parse_location("https://" <> _rest = location, _env), do: location + defp parse_location("http://" <> _rest = location, _env), do: location + + defp parse_location(location, env) do + env.url + |> URI.parse() + |> URI.merge(location) + |> URI.to_string() + end +end -- cgit v1.2.3 From 4128e3a84a2b6d75a8f92759e65ee673b47cec01 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sun, 17 May 2020 22:16:02 +0300 Subject: HTTP: Implement max request limits --- lib/pleroma/application.ex | 3 ++- lib/pleroma/http/adapter_helper/gun.ex | 21 +++++++++++++++++++++ lib/pleroma/http/http.ex | 17 ++++++++++++++++- 3 files changed, 39 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index 37fcdf293..0ffb55358 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -244,7 +244,8 @@ defmodule Pleroma.Application do end defp http_children(Tesla.Adapter.Gun, _) do - Pleroma.Gun.ConnectionPool.children() + Pleroma.Gun.ConnectionPool.children() ++ + [{Task, &Pleroma.HTTP.AdapterHelper.Gun.limiter_setup/0}] end defp http_children(_, _), do: [] diff --git a/lib/pleroma/http/adapter_helper/gun.ex b/lib/pleroma/http/adapter_helper/gun.ex index 5b4629978..883f7f6f7 100644 --- a/lib/pleroma/http/adapter_helper/gun.ex +++ b/lib/pleroma/http/adapter_helper/gun.ex @@ -49,4 +49,25 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do err -> err end end + + @prefix Pleroma.Gun.ConnectionPool + def limiter_setup do + wait = Pleroma.Config.get([:connections_pool, :connection_acquisition_wait]) + retries = Pleroma.Config.get([:connections_pool, :connection_acquisition_retries]) + + :pools + |> Pleroma.Config.get([]) + |> Enum.each(fn {name, opts} -> + max_running = Keyword.get(opts, :size, 50) + max_waiting = Keyword.get(opts, :max_waiting, 10) + + :ok = + ConcurrentLimiter.new(:"#{@prefix}.#{name}", max_running, max_waiting, + wait: wait, + max_retries: retries + ) + end) + + :ok + end end diff --git a/lib/pleroma/http/http.ex b/lib/pleroma/http/http.ex index afcb4d738..6128bc4cf 100644 --- a/lib/pleroma/http/http.ex +++ b/lib/pleroma/http/http.ex @@ -71,7 +71,13 @@ defmodule Pleroma.HTTP do adapter = Application.get_env(:tesla, :adapter) client = Tesla.client([Pleroma.HTTP.Middleware.FollowRedirects], adapter) - request(client, request) + maybe_limit( + fn -> + request(client, request) + end, + adapter, + adapter_opts + ) # Connection release is handled in a custom FollowRedirects middleware err -> @@ -92,4 +98,13 @@ defmodule Pleroma.HTTP do |> Builder.add_param(:query, :query, params) |> Builder.convert_to_keyword() end + + @prefix Pleroma.Gun.ConnectionPool + defp maybe_limit(fun, Tesla.Adapter.Gun, opts) do + ConcurrentLimiter.limit(:"#{@prefix}.#{opts[:pool] || :default}", fun) + end + + defp maybe_limit(fun, _, _) do + fun.() + end end -- cgit v1.2.3 From 00926a63fb174a8bcb2f496921c5d17e04e44b1d Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 16 Jun 2020 16:20:28 +0300 Subject: Adapter Helper: Use built-in ip address type --- lib/pleroma/http/adapter_helper.ex | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/http/adapter_helper.ex b/lib/pleroma/http/adapter_helper.ex index bcb9b2b1e..8ca433732 100644 --- a/lib/pleroma/http/adapter_helper.ex +++ b/lib/pleroma/http/adapter_helper.ex @@ -8,12 +8,8 @@ defmodule Pleroma.HTTP.AdapterHelper do """ @defaults [pool: :federation] - @type ip_address :: ipv4_address() | ipv6_address() - @type ipv4_address :: {0..255, 0..255, 0..255, 0..255} - @type ipv6_address :: - {0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535, 0..65_535} @type proxy_type() :: :socks4 | :socks5 - @type host() :: charlist() | ip_address() + @type host() :: charlist() | :inet.ip_address() alias Pleroma.Config alias Pleroma.HTTP.AdapterHelper @@ -114,7 +110,7 @@ defmodule Pleroma.HTTP.AdapterHelper do end end - @spec parse_host(String.t() | atom() | charlist()) :: charlist() | ip_address() + @spec parse_host(String.t() | atom() | charlist()) :: charlist() | :inet.ip_address() def parse_host(host) when is_list(host), do: host def parse_host(host) when is_atom(host), do: to_charlist(host) -- cgit v1.2.3 From 7882f28569bfaee2996d059990eec279415f0785 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 17 Jun 2020 12:54:13 +0300 Subject: Use erlang monotonic time for CRF calculation --- lib/pleroma/gun/connection_pool/worker.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex index 8467325f3..418cb18c1 100644 --- a/lib/pleroma/gun/connection_pool/worker.ex +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -12,7 +12,7 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do def init([key, uri, opts, client_pid]) do with {:ok, conn_pid} <- Gun.Conn.open(uri, opts), Process.link(conn_pid) do - time = :os.system_time(:second) + time = :erlang.monotonic_time() {_, _} = Registry.update_value(@registry, key, fn _ -> @@ -31,7 +31,7 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do @impl true def handle_cast({:add_client, client_pid, send_pid_back}, %{key: key} = state) do - time = :os.system_time(:second) + time = :erlang.monotonic_time() {{conn_pid, _, _, _}, _} = Registry.update_value(@registry, key, fn {conn_pid, used_by, crf, last_reference} -> -- cgit v1.2.3 From 007843b75e0c7087dad1ef932224b21327d81793 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 23 Jun 2020 15:38:45 +0300 Subject: Add documentation for new connection pool settings and remove some `:retry_timeout` and `:retry` got removed because reconnecting on failure is something the new pool intentionally doesn't do. `:max_overflow` had to go in favor of `:max_waiting`, I didn't reuse the key because the settings are very different in their behaviour. `:checkin_timeout` got removed in favor of `:connection_acquisition_wait`, I didn't reuse the key because the settings are somewhat different. I didn't do any migrations/deprecation warnings/changelog entries because these settings were never in stable. --- lib/pleroma/gun/conn.ex | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/conn.ex b/lib/pleroma/gun/conn.ex index 77f78c7ff..9dc8880db 100644 --- a/lib/pleroma/gun/conn.ex +++ b/lib/pleroma/gun/conn.ex @@ -13,8 +13,6 @@ defmodule Pleroma.Gun.Conn do opts = opts |> Enum.into(%{}) - |> Map.put_new(:retry, pool_opts[:retry] || 1) - |> Map.put_new(:retry_timeout, pool_opts[:retry_timeout] || 1000) |> Map.put_new(:await_up_timeout, pool_opts[:await_up_timeout] || 5_000) |> Map.put_new(:supervise, false) |> maybe_add_tls_opts(uri) -- cgit v1.2.3 From 37f1e781cb19594a6534efbc4d28e793d5960915 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 23 Jun 2020 20:36:21 +0300 Subject: Gun adapter helper: fix wildcard cert issues on OTP 23 See https://bugs.erlang.org/browse/ERL-1260 for more info. The ssl match function is basically copied from mint, except that `:string.lowercase/1` was replaced by `:string.casefold`. It was a TODO in mint's code, so might as well do it since we don't need to support OTP <20. Closes #1834 --- lib/pleroma/http/adapter_helper/gun.ex | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/http/adapter_helper/gun.ex b/lib/pleroma/http/adapter_helper/gun.ex index 883f7f6f7..07aaed7f6 100644 --- a/lib/pleroma/http/adapter_helper/gun.ex +++ b/lib/pleroma/http/adapter_helper/gun.ex @@ -39,9 +39,36 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do defp add_scheme_opts(opts, %{scheme: "https"}) do opts |> Keyword.put(:certificates_verification, true) - |> Keyword.put(:tls_opts, log_level: :warning) + |> Keyword.put(:tls_opts, + log_level: :warning, + customize_hostname_check: [match_fun: &ssl_match_fun/2] + ) end + # ssl_match_fun is adapted from [Mint](https://github.com/elixir-mint/mint) + # Copyright 2018 Eric Meadows-Jönsson and Andrea Leopardi + + # Wildcard domain handling for DNS ID entries in the subjectAltName X.509 + # extension. Note that this is a subset of the wildcard patterns implemented + # by OTP when matching against the subject CN attribute, but this is the only + # wildcard usage defined by the CA/Browser Forum's Baseline Requirements, and + # therefore the only pattern used in commercially issued certificates. + defp ssl_match_fun({:dns_id, reference}, {:dNSName, [?*, ?. | presented]}) do + case domain_without_host(reference) do + '' -> + :default + + domain -> + :string.casefold(domain) == :string.casefold(presented) + end + end + + defp ssl_match_fun(_reference, _presented), do: :default + + defp domain_without_host([]), do: [] + defp domain_without_host([?. | domain]), do: domain + defp domain_without_host([_ | more]), do: domain_without_host(more) + @spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} | {:error, atom()} def get_conn(uri, opts) do case ConnectionPool.get_conn(uri, opts) do -- cgit v1.2.3 From 12fa5541f01ca5cfe082a62dac3317da78043e8f Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 30 Jun 2020 15:58:53 +0300 Subject: FollowRedirects: Unconditionally release the connection if there is an error There is no need for streaming the body if there is no body --- lib/pleroma/tesla/middleware/follow_redirects.ex | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib') diff --git a/lib/pleroma/tesla/middleware/follow_redirects.ex b/lib/pleroma/tesla/middleware/follow_redirects.ex index f2c502c69..5a7032215 100644 --- a/lib/pleroma/tesla/middleware/follow_redirects.ex +++ b/lib/pleroma/tesla/middleware/follow_redirects.ex @@ -55,6 +55,10 @@ defmodule Pleroma.HTTP.Middleware.FollowRedirects do release_conn(opts) {:error, {__MODULE__, :too_many_redirects}} + {:error, _} = e -> + release_conn(opts) + e + other -> unless opts[:body_as] == :chunks do release_conn(opts) -- cgit v1.2.3 From 9b73c35ca8b051316815461247b802bc8567854f Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 30 Jun 2020 18:35:15 +0300 Subject: Request limiter setup: consider {:error, :existing} a success When the application restarts (which happens after certain config changes), the limiters are not destroyed, so `ConcurrentLimiter.new` will produce {:error, :existing} --- lib/pleroma/http/adapter_helper/gun.ex | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/http/adapter_helper/gun.ex b/lib/pleroma/http/adapter_helper/gun.ex index 07aaed7f6..b8c4cc59c 100644 --- a/lib/pleroma/http/adapter_helper/gun.ex +++ b/lib/pleroma/http/adapter_helper/gun.ex @@ -88,11 +88,17 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do max_running = Keyword.get(opts, :size, 50) max_waiting = Keyword.get(opts, :max_waiting, 10) - :ok = + result = ConcurrentLimiter.new(:"#{@prefix}.#{name}", max_running, max_waiting, wait: wait, max_retries: retries ) + + case result do + :ok -> :ok + {:error, :existing} -> :ok + e -> raise e + end end) :ok -- cgit v1.2.3 From a705637dcf7ffe063c9c0f3f190f57e44aaa63f2 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 2 Jul 2020 01:53:27 +0300 Subject: Connection Pool: fix LRFU implementation to not actually be LRU The numbers of the native time unit were so small the CRF was always 1, making it an LRU. This commit switches the time to miliseconds and changes the time delta multiplier to the one yielding mostly highest hit rates according to the paper --- lib/pleroma/gun/connection_pool/worker.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex index 418cb18c1..ec0502621 100644 --- a/lib/pleroma/gun/connection_pool/worker.ex +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -12,7 +12,7 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do def init([key, uri, opts, client_pid]) do with {:ok, conn_pid} <- Gun.Conn.open(uri, opts), Process.link(conn_pid) do - time = :erlang.monotonic_time() + time = :erlang.monotonic_time(:millisecond) {_, _} = Registry.update_value(@registry, key, fn _ -> @@ -31,7 +31,7 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do @impl true def handle_cast({:add_client, client_pid, send_pid_back}, %{key: key} = state) do - time = :erlang.monotonic_time() + time = :erlang.monotonic_time(:millisecond) {{conn_pid, _, _, _}, _} = Registry.update_value(@registry, key, fn {conn_pid, used_by, crf, last_reference} -> @@ -116,6 +116,6 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do # LRFU policy: https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.55.1478 defp crf(time_delta, prev_crf) do - 1 + :math.pow(0.5, time_delta / 100) * prev_crf + 1 + :math.pow(0.5, 0.0001 * time_delta) * prev_crf end end -- cgit v1.2.3 From 46dd276d686e49676101e2af743aad61393f4b70 Mon Sep 17 00:00:00 2001 From: href Date: Tue, 7 Jul 2020 18:56:17 +0200 Subject: ConnectionPool.Worker: Open gun conn in continue instead of init --- lib/pleroma/gun/connection_pool/worker.ex | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex index ec0502621..6ee622fb0 100644 --- a/lib/pleroma/gun/connection_pool/worker.ex +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -9,7 +9,12 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do end @impl true - def init([key, uri, opts, client_pid]) do + def init([_key, _uri, _opts, _client_pid] = opts) do + {:ok, nil, {:continue, {:connect, opts}}} + end + + @impl true + def handle_continue({:connect, [key, uri, opts, client_pid]}, _) do with {:ok, conn_pid} <- Gun.Conn.open(uri, opts), Process.link(conn_pid) do time = :erlang.monotonic_time(:millisecond) @@ -21,7 +26,7 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do send(client_pid, {:conn_pid, conn_pid}) - {:ok, + {:noreply, %{key: key, timer: nil, client_monitors: %{client_pid => Process.monitor(client_pid)}}, :hibernate} else -- cgit v1.2.3 From 6a0f2bdf8ceb4127678cc55406a02d41c7fb0ed7 Mon Sep 17 00:00:00 2001 From: href Date: Wed, 8 Jul 2020 13:01:02 +0200 Subject: Ensure connections error get known by the caller --- lib/pleroma/gun/connection_pool.ex | 22 ++++++++++++---------- lib/pleroma/gun/connection_pool/worker.ex | 3 ++- lib/pleroma/http/adapter_helper/gun.ex | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool.ex b/lib/pleroma/gun/connection_pool.ex index e951872fe..d3eead7d8 100644 --- a/lib/pleroma/gun/connection_pool.ex +++ b/lib/pleroma/gun/connection_pool.ex @@ -16,7 +16,7 @@ defmodule Pleroma.Gun.ConnectionPool do case Registry.lookup(@registry, key) do # The key has already been registered, but connection is not up yet [{worker_pid, nil}] -> - get_gun_pid_from_worker(worker_pid) + get_gun_pid_from_worker(worker_pid, true) [{worker_pid, {gun_pid, _used_by, _crf, _last_reference}}] -> GenServer.cast(worker_pid, {:add_client, self(), false}) @@ -27,13 +27,11 @@ defmodule Pleroma.Gun.ConnectionPool do # so we open the connection in the process directly and send it's pid back # We trust gun to handle timeouts by itself case WorkerSupervisor.start_worker([key, uri, opts, self()]) do - {:ok, _worker_pid} -> - receive do - {:conn_pid, pid} -> {:ok, pid} - end + {:ok, worker_pid} -> + get_gun_pid_from_worker(worker_pid, false) {:error, {:already_started, worker_pid}} -> - get_gun_pid_from_worker(worker_pid) + get_gun_pid_from_worker(worker_pid, true) err -> err @@ -41,17 +39,21 @@ defmodule Pleroma.Gun.ConnectionPool do end end - defp get_gun_pid_from_worker(worker_pid) do + defp get_gun_pid_from_worker(worker_pid, register) do # GenServer.call will block the process for timeout length if # the server crashes on startup (which will happen if gun fails to connect) # so instead we use cast + monitor ref = Process.monitor(worker_pid) - GenServer.cast(worker_pid, {:add_client, self(), true}) + if register, do: GenServer.cast(worker_pid, {:add_client, self(), true}) receive do - {:conn_pid, pid} -> {:ok, pid} - {:DOWN, ^ref, :process, ^worker_pid, reason} -> reason + {:conn_pid, pid} -> + Process.demonitor(ref) + {:ok, pid} + + {:DOWN, ^ref, :process, ^worker_pid, reason} -> + {:error, reason} end end diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex index 6ee622fb0..16a508ad9 100644 --- a/lib/pleroma/gun/connection_pool/worker.ex +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -30,7 +30,8 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do %{key: key, timer: nil, client_monitors: %{client_pid => Process.monitor(client_pid)}}, :hibernate} else - err -> {:stop, err} + err -> + {:stop, err, nil} end end diff --git a/lib/pleroma/http/adapter_helper/gun.ex b/lib/pleroma/http/adapter_helper/gun.ex index b8c4cc59c..74677ddb5 100644 --- a/lib/pleroma/http/adapter_helper/gun.ex +++ b/lib/pleroma/http/adapter_helper/gun.ex @@ -14,7 +14,7 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do connect_timeout: 5_000, domain_lookup_timeout: 5_000, tls_handshake_timeout: 5_000, - retry: 1, + retry: 0, retry_timeout: 1000, await_up_timeout: 5_000 ] -- cgit v1.2.3 From 23d714ed3038a24eb78314d52807c46d8b8de2f3 Mon Sep 17 00:00:00 2001 From: href Date: Wed, 8 Jul 2020 13:22:42 +0200 Subject: Fix race in enforcer/reclaimer start --- lib/pleroma/gun/connection_pool/reclaimer.ex | 85 ++++++++++++++++++++++ .../gun/connection_pool/worker_supervisor.ex | 81 +-------------------- 2 files changed, 89 insertions(+), 77 deletions(-) create mode 100644 lib/pleroma/gun/connection_pool/reclaimer.ex (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool/reclaimer.ex b/lib/pleroma/gun/connection_pool/reclaimer.ex new file mode 100644 index 000000000..1793ac3ee --- /dev/null +++ b/lib/pleroma/gun/connection_pool/reclaimer.ex @@ -0,0 +1,85 @@ +defmodule Pleroma.Gun.ConnectionPool.Reclaimer do + use GenServer, restart: :temporary + + @registry Pleroma.Gun.ConnectionPool + + def start_monitor() do + pid = + case :gen_server.start(__MODULE__, [], name: {:via, Registry, {@registry, "reclaimer"}}) do + {:ok, pid} -> + pid + + {:error, {:already_registered, pid}} -> + pid + end + + {pid, Process.monitor(pid)} + end + + @impl true + def init(_) do + {:ok, nil, {:continue, :reclaim}} + end + + @impl true + def handle_continue(:reclaim, _) do + max_connections = Pleroma.Config.get([:connections_pool, :max_connections]) + + reclaim_max = + [:connections_pool, :reclaim_multiplier] + |> Pleroma.Config.get() + |> Kernel.*(max_connections) + |> round + |> max(1) + + :telemetry.execute([:pleroma, :connection_pool, :reclaim, :start], %{}, %{ + max_connections: max_connections, + reclaim_max: reclaim_max + }) + + # :ets.fun2ms( + # fn {_, {worker_pid, {_, used_by, crf, last_reference}}} when used_by == [] -> + # {worker_pid, crf, last_reference} end) + unused_conns = + Registry.select( + @registry, + [ + {{:_, :"$1", {:_, :"$2", :"$3", :"$4"}}, [{:==, :"$2", []}], [{{:"$1", :"$3", :"$4"}}]} + ] + ) + + case unused_conns do + [] -> + :telemetry.execute( + [:pleroma, :connection_pool, :reclaim, :stop], + %{reclaimed_count: 0}, + %{ + max_connections: max_connections + } + ) + + {:stop, :no_unused_conns, nil} + + unused_conns -> + reclaimed = + unused_conns + |> Enum.sort(fn {_pid1, crf1, last_reference1}, {_pid2, crf2, last_reference2} -> + crf1 <= crf2 and last_reference1 <= last_reference2 + end) + |> Enum.take(reclaim_max) + + reclaimed + |> Enum.each(fn {pid, _, _} -> + DynamicSupervisor.terminate_child(Pleroma.Gun.ConnectionPool.WorkerSupervisor, pid) + end) + + :telemetry.execute( + [:pleroma, :connection_pool, :reclaim, :stop], + %{reclaimed_count: Enum.count(reclaimed)}, + %{max_connections: max_connections} + ) + + {:stop, :normal, nil} + end + end +end diff --git a/lib/pleroma/gun/connection_pool/worker_supervisor.ex b/lib/pleroma/gun/connection_pool/worker_supervisor.ex index 5cb8d488a..39615c956 100644 --- a/lib/pleroma/gun/connection_pool/worker_supervisor.ex +++ b/lib/pleroma/gun/connection_pool/worker_supervisor.ex @@ -29,89 +29,16 @@ defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do end end - @registry Pleroma.Gun.ConnectionPool - @enforcer_key "enforcer" defp free_pool do - case Registry.lookup(@registry, @enforcer_key) do - [] -> - pid = - spawn(fn -> - {:ok, _pid} = Registry.register(@registry, @enforcer_key, nil) - max_connections = Pleroma.Config.get([:connections_pool, :max_connections]) - - reclaim_max = - [:connections_pool, :reclaim_multiplier] - |> Pleroma.Config.get() - |> Kernel.*(max_connections) - |> round - |> max(1) - - :telemetry.execute([:pleroma, :connection_pool, :reclaim, :start], %{}, %{ - max_connections: max_connections, - reclaim_max: reclaim_max - }) - - # :ets.fun2ms( - # fn {_, {worker_pid, {_, used_by, crf, last_reference}}} when used_by == [] -> - # {worker_pid, crf, last_reference} end) - unused_conns = - Registry.select( - @registry, - [ - {{:_, :"$1", {:_, :"$2", :"$3", :"$4"}}, [{:==, :"$2", []}], - [{{:"$1", :"$3", :"$4"}}]} - ] - ) - - case unused_conns do - [] -> - :telemetry.execute( - [:pleroma, :connection_pool, :reclaim, :stop], - %{reclaimed_count: 0}, - %{ - max_connections: max_connections - } - ) - - exit(:no_unused_conns) - - unused_conns -> - reclaimed = - unused_conns - |> Enum.sort(fn {_pid1, crf1, last_reference1}, - {_pid2, crf2, last_reference2} -> - crf1 <= crf2 and last_reference1 <= last_reference2 - end) - |> Enum.take(reclaim_max) - - reclaimed - |> Enum.each(fn {pid, _, _} -> - DynamicSupervisor.terminate_child(__MODULE__, pid) - end) - - :telemetry.execute( - [:pleroma, :connection_pool, :reclaim, :stop], - %{reclaimed_count: Enum.count(reclaimed)}, - %{max_connections: max_connections} - ) - end - end) - - wait_for_enforcer_finish(pid) - - [{pid, _}] -> - wait_for_enforcer_finish(pid) - end + wait_for_reclaimer_finish(Pleroma.Gun.ConnectionPool.Reclaimer.start_monitor()) end - defp wait_for_enforcer_finish(pid) do - ref = Process.monitor(pid) - + defp wait_for_reclaimer_finish({pid, mon}) do receive do - {:DOWN, ^ref, :process, ^pid, :no_unused_conns} -> + {:DOWN, ^mon, :process, ^pid, :no_unused_conns} -> :error - {:DOWN, ^ref, :process, ^pid, :normal} -> + {:DOWN, ^mon, :process, ^pid, :normal} -> :ok end end -- cgit v1.2.3 From 53ba6815b170d7d5c11282933b99730209f526ea Mon Sep 17 00:00:00 2001 From: href Date: Wed, 8 Jul 2020 13:58:38 +0200 Subject: parentheses... --- lib/pleroma/gun/connection_pool/reclaimer.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool/reclaimer.ex b/lib/pleroma/gun/connection_pool/reclaimer.ex index 1793ac3ee..cea800882 100644 --- a/lib/pleroma/gun/connection_pool/reclaimer.ex +++ b/lib/pleroma/gun/connection_pool/reclaimer.ex @@ -3,7 +3,7 @@ defmodule Pleroma.Gun.ConnectionPool.Reclaimer do @registry Pleroma.Gun.ConnectionPool - def start_monitor() do + def start_monitor do pid = case :gen_server.start(__MODULE__, [], name: {:via, Registry, {@registry, "reclaimer"}}) do {:ok, pid} -> -- cgit v1.2.3 From ce1a42bd04bcf352ea1565b411444a98261b0a96 Mon Sep 17 00:00:00 2001 From: href Date: Wed, 8 Jul 2020 15:12:09 +0200 Subject: Simplify TLS opts - `verify_fun` is not useful now - use `customize_check_hostname` (OTP 20+ so OK) - `partial_chain` is useless as of OTP 21.1 (wasn't there, but hackney/.. uses it) --- lib/pleroma/gun/conn.ex | 5 ++--- lib/pleroma/http/adapter_helper/gun.ex | 28 ---------------------------- 2 files changed, 2 insertions(+), 31 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/conn.ex b/lib/pleroma/gun/conn.ex index 9dc8880db..5c12e8153 100644 --- a/lib/pleroma/gun/conn.ex +++ b/lib/pleroma/gun/conn.ex @@ -28,9 +28,8 @@ defmodule Pleroma.Gun.Conn do cacertfile: CAStore.file_path(), depth: 20, reuse_sessions: false, - verify_fun: - {&:ssl_verify_hostname.verify_fun/3, - [check_hostname: Pleroma.HTTP.AdapterHelper.format_host(host)]} + log_level: :warning, + customize_hostname_check: [match_fun: :public_key.pkix_verify_hostname_match_fun(:https)] ] tls_opts = diff --git a/lib/pleroma/http/adapter_helper/gun.ex b/lib/pleroma/http/adapter_helper/gun.ex index 74677ddb5..b4ff8306c 100644 --- a/lib/pleroma/http/adapter_helper/gun.ex +++ b/lib/pleroma/http/adapter_helper/gun.ex @@ -39,36 +39,8 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do defp add_scheme_opts(opts, %{scheme: "https"}) do opts |> Keyword.put(:certificates_verification, true) - |> Keyword.put(:tls_opts, - log_level: :warning, - customize_hostname_check: [match_fun: &ssl_match_fun/2] - ) end - # ssl_match_fun is adapted from [Mint](https://github.com/elixir-mint/mint) - # Copyright 2018 Eric Meadows-Jönsson and Andrea Leopardi - - # Wildcard domain handling for DNS ID entries in the subjectAltName X.509 - # extension. Note that this is a subset of the wildcard patterns implemented - # by OTP when matching against the subject CN attribute, but this is the only - # wildcard usage defined by the CA/Browser Forum's Baseline Requirements, and - # therefore the only pattern used in commercially issued certificates. - defp ssl_match_fun({:dns_id, reference}, {:dNSName, [?*, ?. | presented]}) do - case domain_without_host(reference) do - '' -> - :default - - domain -> - :string.casefold(domain) == :string.casefold(presented) - end - end - - defp ssl_match_fun(_reference, _presented), do: :default - - defp domain_without_host([]), do: [] - defp domain_without_host([?. | domain]), do: domain - defp domain_without_host([_ | more]), do: domain_without_host(more) - @spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} | {:error, atom()} def get_conn(uri, opts) do case ConnectionPool.get_conn(uri, opts) do -- cgit v1.2.3 From afd378f84c4c1b784eba11b35c21e0b6ae3d7915 Mon Sep 17 00:00:00 2001 From: href Date: Wed, 8 Jul 2020 16:02:57 +0200 Subject: host is now useless --- lib/pleroma/gun/conn.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/gun/conn.ex b/lib/pleroma/gun/conn.ex index 5c12e8153..a3f75a4bb 100644 --- a/lib/pleroma/gun/conn.ex +++ b/lib/pleroma/gun/conn.ex @@ -22,7 +22,7 @@ defmodule Pleroma.Gun.Conn do defp maybe_add_tls_opts(opts, %URI{scheme: "http"}), do: opts - defp maybe_add_tls_opts(opts, %URI{scheme: "https", host: host}) do + defp maybe_add_tls_opts(opts, %URI{scheme: "https"}) do tls_opts = [ verify: :verify_peer, cacertfile: CAStore.file_path(), -- cgit v1.2.3 From 6d583bcc3b23c0c16aefa3f34155e7e15b745b01 Mon Sep 17 00:00:00 2001 From: href Date: Mon, 13 Jul 2020 10:44:36 +0200 Subject: Set a default timeout for Gun adapter timeout --- lib/pleroma/http/adapter_helper.ex | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/http/adapter_helper.ex b/lib/pleroma/http/adapter_helper.ex index 8ca433732..9ec3836b0 100644 --- a/lib/pleroma/http/adapter_helper.ex +++ b/lib/pleroma/http/adapter_helper.ex @@ -44,15 +44,17 @@ defmodule Pleroma.HTTP.AdapterHelper do @spec options(URI.t(), keyword()) :: keyword() def options(%URI{} = uri, opts \\ []) do @defaults - |> pool_timeout() + |> put_timeout() |> Keyword.merge(opts) |> adapter_helper().options(uri) end - defp pool_timeout(opts) do + # For Hackney, this is the time a connection can stay idle in the pool. + # For Gun, this is the timeout to receive a message from Gun. + defp put_timeout(opts) do {config_key, default} = if adapter() == Tesla.Adapter.Gun do - {:pools, Config.get([:pools, :default, :timeout])} + {:pools, Config.get([:pools, :default, :timeout], 5_000)} else {:hackney_pools, 10_000} end -- cgit v1.2.3 From 7115c5f82efe1ca1817da3152ba3cbc66e0da1a4 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 15 Jul 2020 15:58:08 +0300 Subject: ConnectionPool.Worker: do not stop with an error when there is a timeout This produced error log messages about GenServer termination every time the connection was not open due to a timeout. Instead we stop with `{:shutdown, }` since shutting down when the connection can't be established is normal behavior. --- lib/pleroma/gun/connection_pool.ex | 5 ++++- lib/pleroma/gun/connection_pool/worker.ex | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/gun/connection_pool.ex b/lib/pleroma/gun/connection_pool.ex index d3eead7d8..8b41a668c 100644 --- a/lib/pleroma/gun/connection_pool.ex +++ b/lib/pleroma/gun/connection_pool.ex @@ -53,7 +53,10 @@ defmodule Pleroma.Gun.ConnectionPool do {:ok, pid} {:DOWN, ^ref, :process, ^worker_pid, reason} -> - {:error, reason} + case reason do + {:shutdown, error} -> error + _ -> {:error, reason} + end end end diff --git a/lib/pleroma/gun/connection_pool/worker.ex b/lib/pleroma/gun/connection_pool/worker.ex index 16a508ad9..f33447cb6 100644 --- a/lib/pleroma/gun/connection_pool/worker.ex +++ b/lib/pleroma/gun/connection_pool/worker.ex @@ -31,7 +31,7 @@ defmodule Pleroma.Gun.ConnectionPool.Worker do :hibernate} else err -> - {:stop, err, nil} + {:stop, {:shutdown, err}, nil} end end -- cgit v1.2.3 From d29b8997f4a3601eac7f2e1e57de27a67df6699c Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Wed, 15 Jul 2020 15:25:33 +0200 Subject: MastoAPI: fix & test giving MRF reject reasons --- lib/pleroma/web/mastodon_api/controllers/status_controller.ex | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib') diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 12be530c9..9bb2ef117 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -172,6 +172,11 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do with_direct_conversation_id: true ) else + {:error, {:reject, message}} -> + conn + |> put_status(:unprocessable_entity) + |> json(%{error: message}) + {:error, message} -> conn |> put_status(:unprocessable_entity) -- cgit v1.2.3