From 905efc57e9f2a96519bf1ac84b56f88d1818cca3 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 13 Apr 2021 11:15:52 -0500 Subject: [PATCH 1/5] Initial test validating the AdminAPI issue --- .../controllers/config_controller_test.exs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/pleroma/web/admin_api/controllers/config_controller_test.exs b/test/pleroma/web/admin_api/controllers/config_controller_test.exs index c4d07d61c..d26fd3150 100644 --- a/test/pleroma/web/admin_api/controllers/config_controller_test.exs +++ b/test/pleroma/web/admin_api/controllers/config_controller_test.exs @@ -1452,6 +1452,41 @@ test "custom instance thumbnail", %{conn: conn} do assert res = %{"thumbnail" => "https://example.com/media/new_thumbnail.jpg"} end + + test "Concurrent Limiter", %{conn: conn} do + clear_config([ConcurrentLimiter]) + + params = %{ + "group" => ":pleroma", + "key" => "ConcurrentLimiter", + "value" => [ + %{ + "tuple" => [ + "Pleroma.Web.RichMedia.Helpers", + [ + %{"tuple" => [":max_running", 6]}, + %{"tuple" => [":max_waiting", 6]} + ] + ] + }, + %{ + "tuple" => [ + "Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicy", + [ + %{"tuple" => [":max_running", 7]}, + %{"tuple" => [":max_waiting", 7]} + ] + ] + } + ] + } + + _res = + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/pleroma/admin/config", %{"configs" => [params]}) + |> json_response_and_validate_schema(200) + end end describe "GET /api/pleroma/admin/config/descriptions" do From ee53ad4d7705328a5a583680c6f551c4c3bf2302 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 13 Apr 2021 12:09:18 -0500 Subject: [PATCH 2/5] Add ConcurrentLimiter to module_name?/1 and apply string_to_elixir_types/1 to search_opts keys during update_or_create/1 --- lib/pleroma/config_db.ex | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/config_db.ex b/lib/pleroma/config_db.ex index b874e0e37..03905c06b 100644 --- a/lib/pleroma/config_db.ex +++ b/lib/pleroma/config_db.ex @@ -141,7 +141,9 @@ defp deep_merge(_key, value1, value2) do @spec update_or_create(map()) :: {:ok, ConfigDB.t()} | {:error, Changeset.t()} def update_or_create(params) do params = Map.put(params, :value, to_elixir_types(params[:value])) - search_opts = Map.take(params, [:group, :key]) + + search_opts = + Map.take(params, [:group, :key]) |> Map.update!(:key, &string_to_elixir_types(&1)) with %ConfigDB{} = config <- ConfigDB.get_by_params(search_opts), {_, true, config} <- {:partial_update, can_be_partially_updated?(config), config}, @@ -387,6 +389,6 @@ defp find_valid_delimiter([delimiter | others], pattern, regex_delimiter) do @spec module_name?(String.t()) :: boolean() def module_name?(string) do Regex.match?(~r/^(Pleroma|Phoenix|Tesla|Quack|Ueberauth|Swoosh)\./, string) or - string in ["Oban", "Ueberauth", "ExSyslogger"] + string in ["Oban", "Ueberauth", "ExSyslogger", "ConcurrentLimiter"] end end From 861f1928526930eeb78f79c4840c69cee5c2f215 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 13 Apr 2021 14:39:44 -0500 Subject: [PATCH 3/5] Document fixed ability to save ConcurrentLimiter settings in ConfigDB --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1553245e5..6e13b3875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Try to save exported ConfigDB settings (migrate_from_db) in the system temp directory if default location is not writable. - Uploading custom instance thumbnail via AdminAPI/AdminFE generated invalid URL to the image +- Applying ConcurrentLimiter settings via AdminAPI ## [2.3.0] - 2020-03-01 From c3b8c77967b0c42f93286f864236b7d6f1471c13 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 13 Apr 2021 14:25:15 -0500 Subject: [PATCH 4/5] Improve string_to_elixir_types/1 with guards --- lib/pleroma/config_db.ex | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/config_db.ex b/lib/pleroma/config_db.ex index 03905c06b..eeeb026c1 100644 --- a/lib/pleroma/config_db.ex +++ b/lib/pleroma/config_db.ex @@ -327,7 +327,7 @@ def to_elixir_types(entity), do: entity @spec string_to_elixir_types(String.t()) :: atom() | Regex.t() | module() | String.t() | no_return() - def string_to_elixir_types("~r" <> _pattern = regex) do + def string_to_elixir_types("~r" <> _pattern = regex) when is_binary(regex) do pattern = ~r/^~r(?'delimiter'[\/|"'([{<]{1})(?'pattern'.+)[\/|"')\]}>]{1}(?'modifier'[uismxfU]*)/u @@ -341,9 +341,9 @@ def string_to_elixir_types("~r" <> _pattern = regex) do end end - def string_to_elixir_types(":" <> atom), do: String.to_atom(atom) + def string_to_elixir_types(":" <> atom) when is_binary(atom), do: String.to_atom(atom) - def string_to_elixir_types(value) do + def string_to_elixir_types(value) when is_binary(value) do if module_name?(value) do String.to_existing_atom("Elixir." <> value) else @@ -351,6 +351,8 @@ def string_to_elixir_types(value) do end end + def string_to_elixir_types(value) when is_atom(value), do: value + defp parse_host("localhost"), do: :localhost defp parse_host(host) do From f95b52255b2d7373a3e0bf4adff81f83c080b2ef Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 14 Apr 2021 09:39:57 -0500 Subject: [PATCH 5/5] Revert guards on string_to_elixir_types/1, remove unnecessary assignment in test --- lib/pleroma/config_db.ex | 12 ++++-------- .../admin_api/controllers/config_controller_test.exs | 9 ++++----- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/config_db.ex b/lib/pleroma/config_db.ex index eeeb026c1..cb57673e3 100644 --- a/lib/pleroma/config_db.ex +++ b/lib/pleroma/config_db.ex @@ -141,9 +141,7 @@ defp deep_merge(_key, value1, value2) do @spec update_or_create(map()) :: {:ok, ConfigDB.t()} | {:error, Changeset.t()} def update_or_create(params) do params = Map.put(params, :value, to_elixir_types(params[:value])) - - search_opts = - Map.take(params, [:group, :key]) |> Map.update!(:key, &string_to_elixir_types(&1)) + search_opts = Map.take(params, [:group, :key]) with %ConfigDB{} = config <- ConfigDB.get_by_params(search_opts), {_, true, config} <- {:partial_update, can_be_partially_updated?(config), config}, @@ -327,7 +325,7 @@ def to_elixir_types(entity), do: entity @spec string_to_elixir_types(String.t()) :: atom() | Regex.t() | module() | String.t() | no_return() - def string_to_elixir_types("~r" <> _pattern = regex) when is_binary(regex) do + def string_to_elixir_types("~r" <> _pattern = regex) do pattern = ~r/^~r(?'delimiter'[\/|"'([{<]{1})(?'pattern'.+)[\/|"')\]}>]{1}(?'modifier'[uismxfU]*)/u @@ -341,9 +339,9 @@ def string_to_elixir_types("~r" <> _pattern = regex) when is_binary(regex) do end end - def string_to_elixir_types(":" <> atom) when is_binary(atom), do: String.to_atom(atom) + def string_to_elixir_types(":" <> atom), do: String.to_atom(atom) - def string_to_elixir_types(value) when is_binary(value) do + def string_to_elixir_types(value) do if module_name?(value) do String.to_existing_atom("Elixir." <> value) else @@ -351,8 +349,6 @@ def string_to_elixir_types(value) when is_binary(value) do end end - def string_to_elixir_types(value) when is_atom(value), do: value - defp parse_host("localhost"), do: :localhost defp parse_host(host) do diff --git a/test/pleroma/web/admin_api/controllers/config_controller_test.exs b/test/pleroma/web/admin_api/controllers/config_controller_test.exs index d26fd3150..c39c1b1e1 100644 --- a/test/pleroma/web/admin_api/controllers/config_controller_test.exs +++ b/test/pleroma/web/admin_api/controllers/config_controller_test.exs @@ -1481,11 +1481,10 @@ test "Concurrent Limiter", %{conn: conn} do ] } - _res = - assert conn - |> put_req_header("content-type", "application/json") - |> post("/api/pleroma/admin/config", %{"configs" => [params]}) - |> json_response_and_validate_schema(200) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/pleroma/admin/config", %{"configs" => [params]}) + |> json_response_and_validate_schema(200) end end