From 364b6969eb7c79e57ed02345ddff4f48519e6b0a Mon Sep 17 00:00:00 2001 From: floatingghost Date: Mon, 4 Jul 2022 16:30:38 +0000 Subject: [PATCH] Use finch everywhere (#33) Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/33 --- config/benchmark.exs | 2 - config/config.exs | 56 +-- config/description.exs | 145 -------- config/test.exs | 6 +- lib/mix/pleroma.ex | 21 +- lib/mix/tasks/pleroma/benchmark.ex | 36 -- lib/pleroma/application.ex | 59 +--- lib/pleroma/config/deprecation_warnings.ex | 46 --- lib/pleroma/config/transfer_task.ex | 5 +- lib/pleroma/emoji/pack.ex | 2 +- lib/pleroma/frontend.ex | 2 +- lib/pleroma/helpers/media_helper.ex | 4 +- lib/pleroma/http.ex | 27 +- lib/pleroma/http/adapter_helper.ex | 14 +- lib/pleroma/http/ex_aws.ex | 2 - lib/pleroma/http/request_builder.ex | 23 +- lib/pleroma/http/tzdata.ex | 4 - lib/pleroma/instances/instance.ex | 2 +- lib/pleroma/reverse_proxy.ex | 172 +++------- lib/pleroma/telemetry/logger.ex | 110 +++--- lib/pleroma/uploaders/s3.ex | 23 +- .../mrf/media_proxy_warming_policy.ex | 1 - .../web/media_proxy/media_proxy_controller.ex | 2 +- lib/pleroma/web/plugs/uploaded_media.ex | 21 +- lib/pleroma/web/rel_me.ex | 1 - lib/pleroma/web/rich_media/helpers.ex | 1 - mix.exs | 1 - .../config/deprecation_warnings_test.exs | 44 --- test/pleroma/gun/connection_pool_test.exs | 100 ------ test/pleroma/reverse_proxy_test.exs | 320 +++++++----------- test/test_helper.exs | 3 - 31 files changed, 261 insertions(+), 994 deletions(-) delete mode 100644 test/pleroma/gun/connection_pool_test.exs diff --git a/config/benchmark.exs b/config/benchmark.exs index 9a7ea5669..b6a0115c4 100644 --- a/config/benchmark.exs +++ b/config/benchmark.exs @@ -70,8 +70,6 @@ config :pleroma, :http_security, report_uri: "https://endpoint.com" -config :pleroma, :http, send_user_agent: false - rum_enabled = System.get_env("RUM_ENABLED") == "true" config :pleroma, :database, rum_enabled: rum_enabled IO.puts("RUM enabled: #{rum_enabled}") diff --git a/config/config.exs b/config/config.exs index 914b5db61..06c946b2a 100644 --- a/config/config.exs +++ b/config/config.exs @@ -175,12 +175,11 @@ "application/ld+json" => ["activity+json"] } -config :tesla, adapter: Tesla.Adapter.Hackney +config :tesla, :adapter, {Tesla.Adapter.Finch, name: MyFinch} # Configures http settings, upstream proxy etc. config :pleroma, :http, proxy_url: nil, - send_user_agent: true, user_agent: :default, adapter: [] @@ -440,11 +439,7 @@ redirect_on_failure: false, max_body_length: 25 * 1_048_576, # Note: max_read_duration defaults to Pleroma.ReverseProxy.max_read_duration_default/1 - max_read_duration: 30_000, - http: [ - follow_redirect: true, - pool: :media - ] + max_read_duration: 30_000 ], whitelist: [] @@ -765,51 +760,6 @@ parameters: [gin_fuzzy_search_limit: "500"], prepare: :unnamed -config :pleroma, :connections_pool, - reclaim_multiplier: 0.1, - connection_acquisition_wait: 250, - connection_acquisition_retries: 5, - max_connections: 250, - max_idle_time: 30_000, - retry: 0, - connect_timeout: 5_000 - -config :pleroma, :pools, - federation: [ - size: 50, - max_waiting: 10, - recv_timeout: 10_000 - ], - media: [ - size: 50, - max_waiting: 20, - recv_timeout: 15_000 - ], - upload: [ - size: 25, - max_waiting: 5, - recv_timeout: 15_000 - ], - default: [ - size: 10, - max_waiting: 2, - recv_timeout: 5_000 - ] - -config :pleroma, :hackney_pools, - federation: [ - max_connections: 50, - timeout: 150_000 - ], - media: [ - max_connections: 50, - timeout: 150_000 - ], - upload: [ - max_connections: 25, - timeout: 300_000 - ] - config :pleroma, :majic_pool, size: 2 private_instance? = :if_instance_is_private @@ -826,8 +776,6 @@ transparency: true, transparency_exclusions: [] -config :tzdata, :http_client, Pleroma.HTTP.Tzdata - config :ex_aws, http_client: Pleroma.HTTP.ExAws config :web_push_encryption, http_client: Pleroma.HTTP.WebPush diff --git a/config/description.exs b/config/description.exs index 3777905a3..d8eaa34a2 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2625,10 +2625,6 @@ description: "Proxy URL", suggestions: ["localhost:9020", {:socks5, :localhost, 3090}] }, - %{ - key: :send_user_agent, - type: :boolean - }, %{ key: :user_agent, type: [:string, :atom], @@ -2941,147 +2937,6 @@ } ] }, - %{ - group: :pleroma, - key: :connections_pool, - type: :group, - description: "Advanced settings for `Gun` connections pool", - children: [ - %{ - key: :connection_acquisition_wait, - type: :integer, - description: - "Timeout to acquire a connection from pool. The total max time is this value multiplied by the number of retries. Default: 250ms.", - suggestions: [250] - }, - %{ - key: :connection_acquisition_retries, - type: :integer, - description: - "Number of attempts to acquire the connection from the pool if it is overloaded. Default: 5", - suggestions: [5] - }, - %{ - key: :max_connections, - type: :integer, - description: "Maximum number of connections in the pool. Default: 250 connections.", - suggestions: [250] - }, - %{ - key: :connect_timeout, - type: :integer, - description: "Timeout while `gun` will wait until connection is up. Default: 5000ms.", - suggestions: [5000] - }, - %{ - key: :reclaim_multiplier, - type: :integer, - description: - "Multiplier for the number of idle connection to be reclaimed if the pool is full. For example if the pool maxes out at 250 connections and this setting is set to 0.3, the pool will reclaim at most 75 idle connections if it's overloaded. Default: 0.1", - suggestions: [0.1] - } - ] - }, - %{ - group: :pleroma, - key: :pools, - type: :group, - description: "Advanced settings for `Gun` workers pools", - children: - Enum.map([:federation, :media, :upload, :default], fn pool_name -> - %{ - key: pool_name, - type: :keyword, - description: "Settings for #{pool_name} pool.", - children: [ - %{ - key: :size, - type: :integer, - description: "Maximum number of concurrent requests in the pool.", - suggestions: [50] - }, - %{ - key: :max_waiting, - type: :integer, - description: - "Maximum number of requests waiting for other requests to finish. After this number is reached, the pool will start returning errrors when a new request is made", - suggestions: [10] - }, - %{ - key: :recv_timeout, - type: :integer, - description: "Timeout for the pool while gun will wait for response", - suggestions: [10_000] - } - ] - } - end) - }, - %{ - group: :pleroma, - key: :hackney_pools, - type: :group, - description: "Advanced settings for `Hackney` connections pools", - children: [ - %{ - key: :federation, - type: :keyword, - description: "Settings for federation pool.", - children: [ - %{ - key: :max_connections, - type: :integer, - description: "Number workers in the pool.", - suggestions: [50] - }, - %{ - key: :timeout, - type: :integer, - description: "Timeout while `hackney` will wait for response.", - suggestions: [150_000] - } - ] - }, - %{ - key: :media, - type: :keyword, - description: "Settings for media pool.", - children: [ - %{ - key: :max_connections, - type: :integer, - description: "Number workers in the pool.", - suggestions: [50] - }, - %{ - key: :timeout, - type: :integer, - description: "Timeout while `hackney` will wait for response.", - suggestions: [150_000] - } - ] - }, - %{ - key: :upload, - type: :keyword, - description: "Settings for upload pool.", - children: [ - %{ - key: :max_connections, - type: :integer, - description: "Number workers in the pool.", - suggestions: [25] - }, - %{ - key: :timeout, - type: :integer, - description: "Timeout while `hackney` will wait for response.", - suggestions: [300_000] - } - ] - } - ] - }, %{ group: :pleroma, key: :restrict_unauthenticated, diff --git a/config/test.exs b/config/test.exs index 7fbababdf..93d07ff19 100644 --- a/config/test.exs +++ b/config/test.exs @@ -45,7 +45,7 @@ adapter: Ecto.Adapters.Postgres, username: "postgres", password: "postgres", - database: "pleroma_test", + database: "akkoma_test", hostname: System.get_env("DB_HOST") || "localhost", pool: Ecto.Adapters.SQL.Sandbox, pool_size: 50, @@ -104,12 +104,8 @@ config :joken, default_signer: "yU8uHKq+yyAkZ11Hx//jcdacWc8yQ1bxAAGrplzB0Zwwjkp35v0RK9SO8WTPr6QZ" -config :pleroma, Pleroma.ReverseProxy.Client, Pleroma.ReverseProxy.ClientMock - config :pleroma, :modules, runtime_dir: "test/fixtures/modules" -config :pleroma, Pleroma.Gun, Pleroma.GunMock - config :pleroma, Pleroma.Emails.NewUsersDigestEmail, enabled: true config :pleroma, Pleroma.Web.Plugs.RemoteIp, enabled: false diff --git a/lib/mix/pleroma.ex b/lib/mix/pleroma.ex index 02c40850a..4eff37859 100644 --- a/lib/mix/pleroma.ex +++ b/lib/mix/pleroma.ex @@ -23,21 +23,14 @@ def start_pleroma do Pleroma.Config.Oban.warn() Pleroma.Application.limiters_setup() Application.put_env(:phoenix, :serve_endpoints, false, persistent: true) + Finch.start_link(name: MyFinch) + unless System.get_env("DEBUG") do Logger.remove_backend(:console) end - adapter = Application.get_env(:tesla, :adapter) - - apps = - if adapter == Tesla.Adapter.Gun do - [:gun | @apps] - else - [:hackney | @apps] - end - - Enum.each(apps, &Application.ensure_all_started/1) + Enum.each(@apps, &Application.ensure_all_started/1) oban_config = [ crontab: [], @@ -57,7 +50,6 @@ def start_pleroma do {Majic.Pool, [name: Pleroma.MajicPool, pool_size: Pleroma.Config.get([:majic_pool, :size], 2)]} ] ++ - http_children(adapter) ++ elasticsearch_children() cachex_children = Enum.map(@cachex_children, &Pleroma.Application.build_cachex(&1, [])) @@ -131,13 +123,6 @@ def escape_sh_path(path) do ~S(') <> String.replace(path, ~S('), ~S(\')) <> ~S(') end - defp http_children(Tesla.Adapter.Gun) do - Pleroma.Gun.ConnectionPool.children() ++ - [{Task, &Pleroma.HTTP.AdapterHelper.Gun.limiter_setup/0}] - end - - defp http_children(_), do: [] - def elasticsearch_children do config = Pleroma.Config.get([Pleroma.Search, :module]) diff --git a/lib/mix/tasks/pleroma/benchmark.ex b/lib/mix/tasks/pleroma/benchmark.ex index fdf99747a..339bf6927 100644 --- a/lib/mix/tasks/pleroma/benchmark.ex +++ b/lib/mix/tasks/pleroma/benchmark.ex @@ -74,40 +74,4 @@ def run(["render_timeline", nickname | _] = args) do inputs: inputs ) end - - def run(["adapters"]) do - start_pleroma() - - :ok = - Pleroma.Gun.Conn.open( - "https://httpbin.org/stream-bytes/1500", - :gun_connections - ) - - Process.sleep(1_500) - - Benchee.run( - %{ - "Without conn and without pool" => fn -> - {:ok, %Tesla.Env{}} = - Pleroma.HTTP.get("https://httpbin.org/stream-bytes/1500", [], - pool: :no_pool, - receive_conn: false - ) - end, - "Without conn and with pool" => fn -> - {:ok, %Tesla.Env{}} = - Pleroma.HTTP.get("https://httpbin.org/stream-bytes/1500", [], receive_conn: false) - end, - "With reused conn and without pool" => fn -> - {:ok, %Tesla.Env{}} = - Pleroma.HTTP.get("https://httpbin.org/stream-bytes/1500", [], pool: :no_pool) - end, - "With reused conn and with pool" => fn -> - {:ok, %Tesla.Env{}} = Pleroma.HTTP.get("https://httpbin.org/stream-bytes/1500") - end - }, - parallel: 10 - ) - end end diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index b709e737b..b55d980e9 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -59,34 +59,8 @@ def start(_type, _args) do Pleroma.Docs.JSON.compile() limiters_setup() - adapter = Application.get_env(:tesla, :adapter) - - if match?({Tesla.Adapter.Finch, _}, adapter) do - Logger.info("Starting Finch") - Finch.start_link(name: MyFinch) - end - - if adapter == Tesla.Adapter.Gun do - if version = Pleroma.OTPVersion.version() do - [major, minor] = - version - |> String.split(".") - |> Enum.map(&String.to_integer/1) - |> Enum.take(2) - - if (major == 22 and minor < 2) or major < 22 do - raise " - !!!OTP VERSION WARNING!!! - You are using gun adapter with OTP version #{version}, which doesn't support correct handling of unordered certificates chains. Please update your Erlang/OTP to at least 22.2. - " - end - else - raise " - !!!OTP VERSION WARNING!!! - To support correct handling of unordered certificates chains - OTP version must be > 22.2. - " - end - end + Logger.info("Starting Finch") + Finch.start_link(name: MyFinch) # Define workers and child supervisors to be supervised children = @@ -97,7 +71,6 @@ def start(_type, _args) do Pleroma.Web.Plugs.RateLimiter.Supervisor ] ++ cachex_children() ++ - http_children(adapter, @mix_env) ++ [ Pleroma.Stats, Pleroma.JobQueueMonitor, @@ -276,34 +249,6 @@ defp task_children(_) do ] end - # start hackney and gun pools in tests - defp http_children(_, :test) do - http_children(Tesla.Adapter.Hackney, nil) ++ http_children(Tesla.Adapter.Gun, nil) - end - - defp http_children(Tesla.Adapter.Hackney, _) do - pools = [:federation, :media] - - pools = - if Config.get([Pleroma.Upload, :proxy_remote]) do - [:upload | pools] - else - pools - end - - for pool <- pools do - options = Config.get([:hackney_pools, pool]) - :hackney_pool.child_spec(pool, options) - end - end - - defp http_children(Tesla.Adapter.Gun, _) do - Pleroma.Gun.ConnectionPool.children() ++ - [{Task, &Pleroma.HTTP.AdapterHelper.Gun.limiter_setup/0}] - end - - defp http_children(_, _), do: [] - def elasticsearch_children do config = Config.get([Pleroma.Search, :module]) diff --git a/lib/pleroma/config/deprecation_warnings.ex b/lib/pleroma/config/deprecation_warnings.ex index 029ee8b65..911357841 100644 --- a/lib/pleroma/config/deprecation_warnings.ex +++ b/lib/pleroma/config/deprecation_warnings.ex @@ -173,7 +173,6 @@ def warn do check_old_mrf_config(), check_media_proxy_whitelist_config(), check_welcome_message_config(), - check_gun_pool_options(), check_activity_expiration_config(), check_remote_ip_plug_name(), check_uploders_s3_public_endpoint(), @@ -257,51 +256,6 @@ def check_media_proxy_whitelist_config do end end - def check_gun_pool_options do - pool_config = Config.get(:connections_pool) - - if timeout = pool_config[:await_up_timeout] do - Logger.warn(""" - !!!DEPRECATION WARNING!!! - Your config is using old setting `config :pleroma, :connections_pool, await_up_timeout`. Please change to `config :pleroma, :connections_pool, connect_timeout` to ensure compatibility with future releases. - """) - - Config.put(:connections_pool, Keyword.put_new(pool_config, :connect_timeout, timeout)) - end - - pools_configs = Config.get(:pools) - - warning_preface = """ - !!!DEPRECATION WARNING!!! - Your config is using old setting name `timeout` instead of `recv_timeout` in pool settings. Setting should work for now, but you are advised to change format to scheme with port to prevent possible issues later. - """ - - updated_config = - Enum.reduce(pools_configs, [], fn {pool_name, config}, acc -> - if timeout = config[:timeout] do - Keyword.put(acc, pool_name, Keyword.put_new(config, :recv_timeout, timeout)) - else - acc - end - end) - - if updated_config != [] do - pool_warnings = - updated_config - |> Keyword.keys() - |> Enum.map(fn pool_name -> - "\n* `:timeout` options in #{pool_name} pool is now `:recv_timeout`" - end) - - Logger.warn(Enum.join([warning_preface | pool_warnings])) - - Config.put(:pools, updated_config) - :error - else - :ok - end - end - @spec check_activity_expiration_config() :: :ok | nil def check_activity_expiration_config do warning_preface = """ diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex index 99d49f995..4676429ae 100644 --- a/lib/pleroma/config/transfer_task.ex +++ b/lib/pleroma/config/transfer_task.ex @@ -15,14 +15,11 @@ defmodule Pleroma.Config.TransferTask do defp reboot_time_keys, do: [ - {:pleroma, :hackney_pools}, {:pleroma, :shout}, {:pleroma, Oban}, {:pleroma, :rate_limit}, {:pleroma, :markup}, - {:pleroma, :streamer}, - {:pleroma, :pools}, - {:pleroma, :connections_pool} + {:pleroma, :streamer} ] defp reboot_time_subkeys, diff --git a/lib/pleroma/emoji/pack.ex b/lib/pleroma/emoji/pack.ex index 09bfcc868..8d233d5e4 100644 --- a/lib/pleroma/emoji/pack.ex +++ b/lib/pleroma/emoji/pack.ex @@ -542,7 +542,7 @@ defp get_filename(pack, shortcode) do defp http_get(%URI{} = url), do: url |> to_string() |> http_get() defp http_get(url) do - with {:ok, %{body: body}} <- Pleroma.HTTP.get(url, [], pool: :default) do + with {:ok, %{body: body}} <- Pleroma.HTTP.get(url, [], []) do Jason.decode(body) end end diff --git a/lib/pleroma/frontend.ex b/lib/pleroma/frontend.ex index c9e51bdc1..adda71eef 100644 --- a/lib/pleroma/frontend.ex +++ b/lib/pleroma/frontend.ex @@ -93,7 +93,7 @@ defp download_build(frontend_info, dest) do url = String.replace(frontend_info["build_url"], "${ref}", frontend_info["ref"]) with {:ok, %{status: 200, body: zip_body}} <- - Pleroma.HTTP.get(url, [], pool: :media, recv_timeout: 120_000) do + Pleroma.HTTP.get(url, [], recv_timeout: 120_000) do unzip(zip_body, dest) else {:error, e} -> {:error, e} diff --git a/lib/pleroma/helpers/media_helper.ex b/lib/pleroma/helpers/media_helper.ex index 738adfcaa..d0c3ab5cc 100644 --- a/lib/pleroma/helpers/media_helper.ex +++ b/lib/pleroma/helpers/media_helper.ex @@ -24,7 +24,7 @@ def missing_dependencies do def image_resize(url, options) do with executable when is_binary(executable) <- System.find_executable("convert"), {:ok, args} <- prepare_image_resize_args(options), - {:ok, env} <- HTTP.get(url, [], pool: :media), + {:ok, env} <- HTTP.get(url, [], []), {:ok, fifo_path} <- mkfifo() do args = List.flatten([fifo_path, args]) run_fifo(fifo_path, env, executable, args) @@ -73,7 +73,7 @@ defp prepare_image_resize_args(_), do: {:error, :missing_options} # Note: video thumbnail is intentionally not resized (always has original dimensions) def video_framegrab(url) do with executable when is_binary(executable) <- System.find_executable("ffmpeg"), - {:ok, env} <- HTTP.get(url, [], pool: :media), + {:ok, env} <- HTTP.get(url, [], []), {:ok, fifo_path} <- mkfifo(), args = [ "-y", diff --git a/lib/pleroma/http.ex b/lib/pleroma/http.ex index 07b3ab0ae..01f307d17 100644 --- a/lib/pleroma/http.ex +++ b/lib/pleroma/http.ex @@ -66,17 +66,9 @@ def request(method, url, body, headers, options) when is_binary(url) do params = options[:params] || [] request = build_request(method, headers, options, url, body, params) - adapter = Application.get_env(:tesla, :adapter) + client = Tesla.client([Tesla.Middleware.FollowRedirects]) - client = Tesla.client(adapter_middlewares(adapter), adapter) - - maybe_limit( - fn -> - request(client, request) - end, - adapter, - adapter_opts - ) + request(client, request) end @spec request(Client.t(), keyword()) :: {:ok, Env.t()} | {:error, any()} @@ -92,19 +84,4 @@ defp build_request(method, headers, options, url, body, params) 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 - - defp adapter_middlewares(Tesla.Adapter.Gun) do - [Tesla.Middleware.FollowRedirects, Pleroma.Tesla.Middleware.ConnectionPool] - end - - defp adapter_middlewares(_), do: [] end diff --git a/lib/pleroma/http/adapter_helper.ex b/lib/pleroma/http/adapter_helper.ex index c667afd25..f9b489616 100644 --- a/lib/pleroma/http/adapter_helper.ex +++ b/lib/pleroma/http/adapter_helper.ex @@ -6,7 +6,7 @@ defmodule Pleroma.HTTP.AdapterHelper do @moduledoc """ Configure Tesla.Client with default and customized adapter options. """ - @defaults [pool: :federation, connect_timeout: 5_000, recv_timeout: 5_000] + @defaults [name: MyFinch, connect_timeout: 5_000, recv_timeout: 5_000] @type proxy_type() :: :socks4 | :socks5 @type host() :: charlist() | :inet.ip_address() @@ -43,17 +43,7 @@ def maybe_add_proxy(opts, proxy), do: Keyword.put_new(opts, :proxy, proxy) def options(%URI{} = uri, opts \\ []) do @defaults |> Keyword.merge(opts) - |> adapter_helper().options(uri) - end - - 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 + |> AdapterHelper.Default.options(uri) end @spec parse_proxy(String.t() | tuple() | nil) :: diff --git a/lib/pleroma/http/ex_aws.ex b/lib/pleroma/http/ex_aws.ex index 283590b18..ac2d911aa 100644 --- a/lib/pleroma/http/ex_aws.ex +++ b/lib/pleroma/http/ex_aws.ex @@ -11,8 +11,6 @@ defmodule Pleroma.HTTP.ExAws do @impl true def request(method, url, body \\ "", headers \\ [], http_opts \\ []) do - http_opts = Keyword.put_new(http_opts, :pool, :upload) - case HTTP.request(method, url, body, headers, http_opts) do {:ok, env} -> {:ok, %{status_code: env.status, headers: env.headers, body: env.body}} diff --git a/lib/pleroma/http/request_builder.ex b/lib/pleroma/http/request_builder.ex index 631c927af..4cd75d3a0 100644 --- a/lib/pleroma/http/request_builder.ex +++ b/lib/pleroma/http/request_builder.ex @@ -10,6 +10,8 @@ defmodule Pleroma.HTTP.RequestBuilder do alias Pleroma.HTTP.Request alias Tesla.Multipart + @mix_env Mix.env() + @doc """ Creates new request """ @@ -33,14 +35,7 @@ def url(request, u), do: %{request | url: u} """ @spec headers(Request.t(), Request.headers()) :: Request.t() def headers(request, headers) do - headers_list = - with true <- Pleroma.Config.get([:http, :send_user_agent]), - nil <- Enum.find(headers, fn {key, _val} -> String.downcase(key) == "user-agent" end) do - [{"user-agent", Pleroma.Application.user_agent()} | headers] - else - _ -> - headers - end + headers_list = maybe_add_user_agent(headers, @mix_env) %{request | headers: headers_list} end @@ -92,4 +87,16 @@ def convert_to_keyword(request) do |> Map.from_struct() |> Enum.into([]) end + + defp maybe_add_user_agent(headers, :test) do + with true <- Pleroma.Config.get([:http, :send_user_agent]) do + [{"user-agent", Pleroma.Application.user_agent()} | headers] + else + _ -> + headers + end + end + + defp maybe_add_user_agent(headers, _), + do: [{"user-agent", Pleroma.Application.user_agent()} | headers] end diff --git a/lib/pleroma/http/tzdata.ex b/lib/pleroma/http/tzdata.ex index 77e1b537e..a208b9be3 100644 --- a/lib/pleroma/http/tzdata.ex +++ b/lib/pleroma/http/tzdata.ex @@ -11,8 +11,6 @@ defmodule Pleroma.HTTP.Tzdata do @impl true def get(url, headers, options) do - options = Keyword.put_new(options, :pool, :default) - with {:ok, %Tesla.Env{} = env} <- HTTP.get(url, headers, options) do {:ok, {env.status, env.headers, env.body}} end @@ -20,8 +18,6 @@ def get(url, headers, options) do @impl true def head(url, headers, options) do - options = Keyword.put_new(options, :pool, :default) - with {:ok, %Tesla.Env{} = env} <- HTTP.head(url, headers, options) do {:ok, {env.status, env.headers}} end diff --git a/lib/pleroma/instances/instance.ex b/lib/pleroma/instances/instance.ex index 2f338b3e2..533dbbb82 100644 --- a/lib/pleroma/instances/instance.ex +++ b/lib/pleroma/instances/instance.ex @@ -170,7 +170,7 @@ defp scrape_favicon(%URI{} = instance_uri) do try do with {_, true} <- {:reachable, reachable?(instance_uri.host)}, {:ok, %Tesla.Env{body: html}} <- - Pleroma.HTTP.get(to_string(instance_uri), [{"accept", "text/html"}], pool: :media), + Pleroma.HTTP.get(to_string(instance_uri), [{"accept", "text/html"}], []), {_, [favicon_rel | _]} when is_binary(favicon_rel) <- {:parse, html |> Floki.parse_document!() |> Floki.attribute("link[rel=icon]", "href")}, diff --git a/lib/pleroma/reverse_proxy.ex b/lib/pleroma/reverse_proxy.ex index ec69a1779..51f9609cb 100644 --- a/lib/pleroma/reverse_proxy.ex +++ b/lib/pleroma/reverse_proxy.ex @@ -9,7 +9,7 @@ defmodule Pleroma.ReverseProxy do @resp_cache_headers ~w(etag date last-modified) @keep_resp_headers @resp_cache_headers ++ ~w(content-length content-type content-disposition content-encoding) ++ - ~w(content-range accept-ranges vary) + ~w(content-range accept-ranges vary expires) @default_cache_control_header "public, max-age=1209600" @valid_resp_codes [200, 206, 304] @max_read_duration :timer.seconds(30) @@ -59,11 +59,7 @@ def default_cache_control_header, do: @default_cache_control_header * `req_headers`, `resp_headers` additional headers. - * `http`: options for [hackney](https://github.com/benoitc/hackney) or [gun](https://github.com/ninenines/gun). - """ - @default_options [pool: :media] - @inline_content_types [ "image/gif", "image/jpeg", @@ -94,7 +90,7 @@ def default_cache_control_header, do: @default_cache_control_header def call(_conn, _url, _opts \\ []) def call(conn = %{method: method}, url, opts) when method in @methods do - client_opts = Keyword.merge(@default_options, Keyword.get(opts, :http, [])) + client_opts = Keyword.get(opts, :http, []) req_headers = build_req_headers(conn.req_headers, opts) @@ -106,32 +102,39 @@ def call(conn = %{method: method}, url, opts) when method in @methods do end with {:ok, nil} <- @cachex.get(:failed_proxy_url_cache, url), - {:ok, code, headers, client} <- request(method, url, req_headers, client_opts), + {:ok, status, headers, body} <- request(method, url, req_headers, client_opts), :ok <- header_length_constraint( headers, Keyword.get(opts, :max_body_length, @max_body_length) ) do - response(conn, client, url, code, headers, opts) + conn + |> put_private(:proxied_url, url) + |> response(body, status, headers, opts) else {:ok, true} -> conn - |> error_or_redirect(url, 500, "Request failed", opts) + |> error_or_redirect(500, "Request failed", opts) |> halt() - {:ok, code, headers} -> - head_response(conn, url, code, headers, opts) + {:ok, status, headers} -> + conn + |> put_private(:proxied_url, url) + |> head_response(status, headers, opts) |> halt() - {:error, {:invalid_http_response, code}} -> - Logger.error("#{__MODULE__}: request to #{inspect(url)} failed with HTTP status #{code}") - track_failed_url(url, code, opts) + {:error, {:invalid_http_response, status}} -> + Logger.error( + "#{__MODULE__}: request to #{inspect(url)} failed with HTTP status #{status}" + ) + + track_failed_url(url, status, opts) conn + |> put_private(:proxied_url, url) |> error_or_redirect( - url, - code, - "Request failed: " <> Plug.Conn.Status.reason_phrase(code), + status, + "Request failed: " <> Plug.Conn.Status.reason_phrase(status), opts ) |> halt() @@ -141,7 +144,8 @@ def call(conn = %{method: method}, url, opts) when method in @methods do track_failed_url(url, error, opts) conn - |> error_or_redirect(url, 500, "Request failed", opts) + |> put_private(:proxied_url, url) + |> error_or_redirect(500, "Request failed", opts) |> halt() end end @@ -156,93 +160,48 @@ defp request(method, url, headers, opts) do Logger.debug("#{__MODULE__} #{method} #{url} #{inspect(headers)}") method = method |> String.downcase() |> String.to_existing_atom() - case client().request(method, url, headers, "", opts) do - {:ok, code, headers, client} when code in @valid_resp_codes -> - {:ok, code, downcase_headers(headers), client} + opts = opts ++ [receive_timeout: @max_read_duration] - {:ok, code, headers} when code in @valid_resp_codes -> - {:ok, code, downcase_headers(headers)} + case Pleroma.HTTP.request(method, url, "", headers, opts) do + {:ok, %Tesla.Env{status: status, headers: headers, body: body}} + when status in @valid_resp_codes -> + {:ok, status, downcase_headers(headers), body} - {:ok, code, _, _} -> - {:error, {:invalid_http_response, code}} + {:ok, %Tesla.Env{status: status, headers: headers}} when status in @valid_resp_codes -> + {:ok, status, downcase_headers(headers)} - {:ok, code, _} -> - {:error, {:invalid_http_response, code}} + {:ok, %Tesla.Env{status: status}} -> + {:error, {:invalid_http_response, status}} {:error, error} -> {:error, error} end end - defp response(conn, client, url, status, headers, opts) do - Logger.debug("#{__MODULE__} #{status} #{url} #{inspect(headers)}") - - result = - conn - |> put_resp_headers(build_resp_headers(headers, opts)) - |> send_chunked(status) - |> chunk_reply(client, opts) - - case result do - {:ok, conn} -> - halt(conn) - - {:error, :closed, conn} -> - client().close(client) - halt(conn) - - {:error, error, conn} -> - Logger.warn( - "#{__MODULE__} request to #{url} failed while reading/chunking: #{inspect(error)}" - ) - - client().close(client) - halt(conn) - end - end - - defp chunk_reply(conn, client, opts) do - chunk_reply(conn, client, opts, 0, 0) - end - - defp chunk_reply(conn, client, opts, sent_so_far, duration) do - with {:ok, duration} <- - check_read_duration( - duration, - Keyword.get(opts, :max_read_duration, @max_read_duration) - ), - {:ok, data, client} <- client().stream_body(client), - {:ok, duration} <- increase_read_duration(duration), - sent_so_far = sent_so_far + byte_size(data), - :ok <- - body_size_constraint( - sent_so_far, - Keyword.get(opts, :max_body_length, @max_body_length) - ), - {:ok, conn} <- chunk(conn, data) do - chunk_reply(conn, client, opts, sent_so_far, duration) - else - :done -> {:ok, conn} - {:error, error} -> {:error, error, conn} - end - end - - defp head_response(conn, url, code, headers, opts) do - Logger.debug("#{__MODULE__} #{code} #{url} #{inspect(headers)}") + defp response(conn, body, status, headers, opts) do + Logger.debug("#{__MODULE__} #{status} #{conn.private[:proxied_url]} #{inspect(headers)}") conn |> put_resp_headers(build_resp_headers(headers, opts)) - |> send_resp(code, "") + |> send_resp(status, body) end - defp error_or_redirect(conn, url, code, body, opts) do + defp head_response(conn, status, headers, opts) do + Logger.debug("#{__MODULE__} #{status} #{conn.private[:proxied_url]} #{inspect(headers)}") + + conn + |> put_resp_headers(build_resp_headers(headers, opts)) + |> send_resp(status, "") + end + + defp error_or_redirect(conn, status, body, opts) do if Keyword.get(opts, :redirect_on_failure, false) do conn - |> Phoenix.Controller.redirect(external: url) + |> Phoenix.Controller.redirect(external: conn.private[:proxied_url]) |> halt() else conn - |> send_resp(code, body) + |> send_resp(status, body) |> halt end end @@ -272,7 +231,6 @@ defp build_req_headers(headers, opts) do |> downcase_headers() |> Enum.filter(fn {k, _} -> k in @keep_req_headers end) |> build_req_range_or_encoding_header(opts) - |> build_req_user_agent_header(opts) |> Keyword.merge(Keyword.get(opts, :req_headers, [])) end @@ -287,15 +245,6 @@ defp build_req_range_or_encoding_header(headers, _opts) do end end - defp build_req_user_agent_header(headers, _opts) do - List.keystore( - headers, - "user-agent", - 0, - {"user-agent", Pleroma.Application.user_agent()} - ) - end - defp build_resp_headers(headers, opts) do headers |> Enum.filter(fn {k, _} -> k in @keep_resp_headers end) @@ -382,37 +331,6 @@ defp header_length_constraint(headers, limit) when is_integer(limit) and limit > defp header_length_constraint(_, _), do: :ok - defp body_size_constraint(size, limit) when is_integer(limit) and limit > 0 and size >= limit do - {:error, :body_too_large} - end - - defp body_size_constraint(_, _), do: :ok - - defp check_read_duration(nil = _duration, max), do: check_read_duration(@max_read_duration, max) - - defp check_read_duration(duration, max) - when is_integer(duration) and is_integer(max) and max > 0 do - if duration > max do - {:error, :read_duration_exceeded} - else - {:ok, {duration, :erlang.system_time(:millisecond)}} - end - end - - defp check_read_duration(_, _), do: {:ok, :no_duration_limit, :no_duration_limit} - - defp increase_read_duration({previous_duration, started}) - when is_integer(previous_duration) and is_integer(started) do - duration = :erlang.system_time(:millisecond) - started - {:ok, previous_duration + duration} - end - - defp increase_read_duration(_) do - {:ok, :no_duration_limit, :no_duration_limit} - end - - defp client, do: Pleroma.ReverseProxy.Client.Wrapper - defp track_failed_url(url, error, opts) do ttl = unless error in [:body_too_large, 400, 204] do diff --git a/lib/pleroma/telemetry/logger.ex b/lib/pleroma/telemetry/logger.ex index 50f7fcf2a..4c781c504 100644 --- a/lib/pleroma/telemetry/logger.ex +++ b/lib/pleroma/telemetry/logger.ex @@ -8,11 +8,7 @@ defmodule Pleroma.Telemetry.Logger do require Logger @events [ - [:pleroma, :connection_pool, :reclaim, :start], - [:pleroma, :connection_pool, :reclaim, :stop], - [:pleroma, :connection_pool, :provision_failure], - [:pleroma, :connection_pool, :client, :dead], - [:pleroma, :connection_pool, :client, :add] + [:pleroma, :repo, :query] ] def attach do :telemetry.attach_many( @@ -28,68 +24,62 @@ def attach do # out anyway due to higher log level configured def handle_event( - [:pleroma, :connection_pool, :reclaim, :start], - _, - %{max_connections: max_connections, reclaim_max: reclaim_max}, - _ + [:pleroma, :repo, :query] = _name, + %{query_time: query_time} = measurements, + %{source: source} = metadata, + config ) 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) + logging_config = Pleroma.Config.get([:telemetry, :slow_queries_logging], []) + + if logging_config[:enabled] && + logging_config[:min_duration] && + query_time > logging_config[:min_duration] and + (is_nil(logging_config[:exclude_sources]) or + source not in logging_config[:exclude_sources]) do + log_slow_query(measurements, metadata, config) + else + :ok + 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 + defp log_slow_query( + %{query_time: query_time} = _measurements, + %{source: _source, query: query, params: query_params, repo: repo} = _metadata, + _config + ) do + sql_explain = + with {:ok, %{rows: explain_result_rows}} <- + repo.query("EXPLAIN " <> query, query_params, log: false) do + Enum.map_join(explain_result_rows, "\n", & &1) + 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 + {:current_stacktrace, stacktrace} = Process.info(self(), :current_stacktrace) - 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 + pleroma_stacktrace = + Enum.filter(stacktrace, fn + {__MODULE__, _, _, _} -> + false + + {mod, _, _, _} -> + mod + |> to_string() + |> String.starts_with?("Elixir.Pleroma.") + end) - def handle_event( - [:pleroma, :connection_pool, :client, :dead], - %{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)}" + """ + Slow query! + + Total time: #{round(query_time / 1_000)} ms + + #{query} + + #{inspect(query_params, limit: :infinity)} + + #{sql_explain} + + #{Exception.format_stacktrace(pleroma_stacktrace)} + """ end) end - - def handle_event( - [:pleroma, :connection_pool, :client, :add], - %{clients: [_, _ | _] = clients}, - %{key: key, protocol: :http}, - _ - ) do - Logger.info(fn -> - "Pool worker for #{key}: #{length(clients)} clients are using an HTTP1 connection at the same time, head-of-line blocking might occur." - end) - end - - def handle_event([:pleroma, :connection_pool, :client, :add], _, _, _), do: :ok end diff --git a/lib/pleroma/uploaders/s3.ex b/lib/pleroma/uploaders/s3.ex index d85c8cb2f..481153fe8 100644 --- a/lib/pleroma/uploaders/s3.ex +++ b/lib/pleroma/uploaders/s3.ex @@ -30,23 +30,12 @@ def put_file(%Pleroma.Upload{} = upload) do op = if streaming do - op = - upload.tempfile - |> ExAws.S3.Upload.stream_file() - |> ExAws.S3.upload(bucket, s3_name, [ - {:acl, :public_read}, - {:content_type, upload.content_type} - ]) - - if Application.get_env(:tesla, :adapter) == Tesla.Adapter.Gun do - # set s3 upload timeout to respect :upload pool timeout - # timeout should be slightly larger, so s3 can retry upload on fail - timeout = Pleroma.HTTP.AdapterHelper.Gun.pool_timeout(:upload) + 1_000 - opts = Keyword.put(op.opts, :timeout, timeout) - Map.put(op, :opts, opts) - else - op - end + upload.tempfile + |> ExAws.S3.Upload.stream_file() + |> ExAws.S3.upload(bucket, s3_name, [ + {:acl, :public_read}, + {:content_type, upload.content_type} + ]) else {:ok, file_data} = File.read(upload.tempfile) diff --git a/lib/pleroma/web/activity_pub/mrf/media_proxy_warming_policy.ex b/lib/pleroma/web/activity_pub/mrf/media_proxy_warming_policy.ex index 25289d3a4..f60a76adf 100644 --- a/lib/pleroma/web/activity_pub/mrf/media_proxy_warming_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/media_proxy_warming_policy.ex @@ -12,7 +12,6 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicy do require Logger @adapter_options [ - pool: :media, recv_timeout: 10_000 ] diff --git a/lib/pleroma/web/media_proxy/media_proxy_controller.ex b/lib/pleroma/web/media_proxy/media_proxy_controller.ex index c74eaaf93..b3a92d75a 100644 --- a/lib/pleroma/web/media_proxy/media_proxy_controller.ex +++ b/lib/pleroma/web/media_proxy/media_proxy_controller.ex @@ -54,7 +54,7 @@ defp handle_preview(conn, url) do media_proxy_url = MediaProxy.url(url) with {:ok, %{status: status} = head_response} when status in 200..299 <- - Pleroma.HTTP.request("head", media_proxy_url, [], [], pool: :media) do + Pleroma.HTTP.request("head", media_proxy_url, [], [], name: MyFinch) do content_type = Tesla.get_header(head_response, "content-type") content_length = Tesla.get_header(head_response, "content-length") content_length = content_length && String.to_integer(content_length) diff --git a/lib/pleroma/web/plugs/uploaded_media.ex b/lib/pleroma/web/plugs/uploaded_media.ex index 2378e98d2..7b87d8f17 100644 --- a/lib/pleroma/web/plugs/uploaded_media.ex +++ b/lib/pleroma/web/plugs/uploaded_media.ex @@ -47,10 +47,9 @@ def call(%{request_path: <<"/", @path, "/", file::binary>>} = conn, opts) do config = Pleroma.Config.get(Pleroma.Upload) with uploader <- Keyword.fetch!(config, :uploader), - proxy_remote = Keyword.get(config, :proxy_remote, false), {:ok, get_method} <- uploader.get_file(file), false <- media_is_banned(conn, get_method) do - get_media(conn, get_method, proxy_remote, opts) + get_media(conn, get_method, opts) else _ -> conn @@ -69,7 +68,7 @@ defp media_is_banned(_, {:url, url}), do: MediaProxy.in_banned_urls(url) defp media_is_banned(_, _), do: false - defp get_media(conn, {:static_dir, directory}, _, opts) do + defp get_media(conn, {:static_dir, directory}, opts) do static_opts = Map.get(opts, :static_plug_opts) |> Map.put(:at, [@path]) @@ -86,25 +85,13 @@ defp get_media(conn, {:static_dir, directory}, _, opts) do end end - defp get_media(conn, {:url, url}, true, _) do - proxy_opts = [ - http: [ - follow_redirect: true, - pool: :upload - ] - ] - - conn - |> Pleroma.ReverseProxy.call(url, proxy_opts) - end - - defp get_media(conn, {:url, url}, _, _) do + defp get_media(conn, {:url, url}, _) do conn |> Phoenix.Controller.redirect(external: url) |> halt() end - defp get_media(conn, unknown, _, _) do + defp get_media(conn, unknown, _) do Logger.error("#{__MODULE__}: Unknown get startegy: #{inspect(unknown)}") conn diff --git a/lib/pleroma/web/rel_me.ex b/lib/pleroma/web/rel_me.ex index 7e745d07e..da92b5754 100644 --- a/lib/pleroma/web/rel_me.ex +++ b/lib/pleroma/web/rel_me.ex @@ -4,7 +4,6 @@ defmodule Pleroma.Web.RelMe do @options [ - pool: :media, max_body: 2_000_000, recv_timeout: 2_000 ] diff --git a/lib/pleroma/web/rich_media/helpers.ex b/lib/pleroma/web/rich_media/helpers.ex index 566fc8c8a..ba3524307 100644 --- a/lib/pleroma/web/rich_media/helpers.ex +++ b/lib/pleroma/web/rich_media/helpers.ex @@ -10,7 +10,6 @@ defmodule Pleroma.Web.RichMedia.Helpers do alias Pleroma.Web.RichMedia.Parser @options [ - pool: :media, max_body: 2_000_000, recv_timeout: 2_000 ] diff --git a/mix.exs b/mix.exs index 6b4b4289c..4259f20b3 100644 --- a/mix.exs +++ b/mix.exs @@ -215,7 +215,6 @@ defp deps do {:mock, "~> 0.3.5", only: :test}, # temporary downgrade for excoveralls, hackney until hackney max_connections bug will be fixed {:excoveralls, "0.12.3", only: :test}, - {:hackney, "~> 1.18.0", override: true}, {:mox, "~> 1.0", only: :test}, {:websocket_client, git: "https://github.com/jeremyong/websocket_client.git", only: :test} ] ++ oauth_deps() diff --git a/test/pleroma/config/deprecation_warnings_test.exs b/test/pleroma/config/deprecation_warnings_test.exs index c5e2b20f4..12597506b 100644 --- a/test/pleroma/config/deprecation_warnings_test.exs +++ b/test/pleroma/config/deprecation_warnings_test.exs @@ -280,50 +280,6 @@ test "check_uploders_s3_public_endpoint/0" do "Your config is using the old setting for controlling the URL of media uploaded to your S3 bucket." end - describe "check_gun_pool_options/0" do - test "await_up_timeout" do - config = Config.get(:connections_pool) - clear_config(:connections_pool, Keyword.put(config, :await_up_timeout, 5_000)) - - assert capture_log(fn -> - DeprecationWarnings.check_gun_pool_options() - end) =~ - "Your config is using old setting `config :pleroma, :connections_pool, await_up_timeout`." - end - - test "pool timeout" do - old_config = [ - federation: [ - size: 50, - max_waiting: 10, - timeout: 10_000 - ], - media: [ - size: 50, - max_waiting: 10, - timeout: 10_000 - ], - upload: [ - size: 25, - max_waiting: 5, - timeout: 15_000 - ], - default: [ - size: 10, - max_waiting: 2, - timeout: 5_000 - ] - ] - - clear_config(:pools, old_config) - - assert capture_log(fn -> - DeprecationWarnings.check_gun_pool_options() - end) =~ - "Your config is using old setting name `timeout` instead of `recv_timeout` in pool settings" - end - end - test "check_old_chat_shoutbox/0" do clear_config([:instance, :chat_limit], 1_000) clear_config([:chat, :enabled], true) diff --git a/test/pleroma/gun/connection_pool_test.exs b/test/pleroma/gun/connection_pool_test.exs deleted file mode 100644 index 51637f541..000000000 --- a/test/pleroma/gun/connection_pool_test.exs +++ /dev/null @@ -1,100 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2021 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Gun.ConnectionPoolTest do - use Pleroma.DataCase - - import Mox - import ExUnit.CaptureLog - alias Pleroma.Gun.ConnectionPool - - defp gun_mock(_) do - Pleroma.GunMock - |> stub(:open, fn _, _, _ -> Task.start_link(fn -> Process.sleep(100) end) end) - |> stub(:await_up, fn _, _ -> {:ok, :http} end) - |> stub(:set_owner, fn _, _ -> :ok end) - - :ok - end - - setup :gun_mock - - test "gives the same connection to 2 concurrent requests" do - Enum.map( - [ - "http://www.korean-books.com.kp/KBMbooks/en/periodic/pictorial/20200530163914.pdf", - "http://www.korean-books.com.kp/KBMbooks/en/periodic/pictorial/20200528183427.pdf" - ], - fn uri -> - uri = URI.parse(uri) - task_parent = self() - - Task.start_link(fn -> - {:ok, conn} = ConnectionPool.get_conn(uri, []) - ConnectionPool.release_conn(conn) - send(task_parent, conn) - end) - end - ) - - [pid, pid] = - for _ <- 1..2 do - receive do - pid -> pid - end - end - end - - @tag :erratic - test "connection limit is respected with concurrent requests" do - clear_config([:connections_pool, :max_connections]) do - clear_config([:connections_pool, :max_connections], 1) - # The supervisor needs a reboot to apply the new config setting - Process.exit(Process.whereis(Pleroma.Gun.ConnectionPool.WorkerSupervisor), :kill) - - on_exit(fn -> - Process.exit(Process.whereis(Pleroma.Gun.ConnectionPool.WorkerSupervisor), :kill) - end) - end - - capture_log(fn -> - Enum.map( - [ - "https://ninenines.eu/", - "https://youtu.be/PFGwMiDJKNY" - ], - fn uri -> - uri = URI.parse(uri) - task_parent = self() - - Task.start_link(fn -> - result = ConnectionPool.get_conn(uri, []) - # Sleep so that we don't end up with a situation, - # where request from the second process gets processed - # only after the first process already released the connection - Process.sleep(50) - - case result do - {:ok, pid} -> - ConnectionPool.release_conn(pid) - - _ -> - nil - end - - send(task_parent, result) - end) - end - ) - - [{:error, :pool_full}, {:ok, _pid}] = - for _ <- 1..2 do - receive do - result -> result - end - end - |> Enum.sort() - end) - end -end diff --git a/test/pleroma/reverse_proxy_test.exs b/test/pleroma/reverse_proxy_test.exs index 49ddf251d..9cbe06f21 100644 --- a/test/pleroma/reverse_proxy_test.exs +++ b/test/pleroma/reverse_proxy_test.exs @@ -5,112 +5,57 @@ defmodule Pleroma.ReverseProxyTest do use Pleroma.Web.ConnCase import ExUnit.CaptureLog - import Mox alias Pleroma.ReverseProxy - alias Pleroma.ReverseProxy.ClientMock alias Plug.Conn - setup_all do - {:ok, _} = Registry.start_link(keys: :unique, name: ClientMock) - :ok - end - - setup :verify_on_exit! - - defp request_mock(invokes) do - ClientMock - |> expect(:request, fn :get, url, headers, _body, _opts -> - Registry.register(ClientMock, url, 0) - body = headers |> Enum.into(%{}) |> Jason.encode!() - - {:ok, 200, - [ - {"content-type", "application/json"}, - {"content-length", byte_size(body) |> to_string()} - ], %{url: url, body: body}} - end) - |> expect(:stream_body, invokes, fn %{url: url, body: body} = client -> - case Registry.lookup(ClientMock, url) do - [{_, 0}] -> - Registry.update_value(ClientMock, url, &(&1 + 1)) - {:ok, body, client} - - [{_, 1}] -> - Registry.unregister(ClientMock, url) - :done - end - end) - end - describe "reverse proxy" do test "do not track successful request", %{conn: conn} do - request_mock(2) url = "/success" + Tesla.Mock.mock(fn %{url: ^url} -> + %Tesla.Env{ + status: 200, + body: "" + } + end) + conn = ReverseProxy.call(conn, url) - assert conn.status == 200 + assert response(conn, 200) assert Cachex.get(:failed_proxy_url_cache, url) == {:ok, nil} end - end - test "use Pleroma's user agent in the request; don't pass the client's", %{conn: conn} do - request_mock(2) + test "use Pleroma's user agent in the request; don't pass the client's", %{conn: conn} do + clear_config([:http, :send_user_agent], true) + # Mock will fail if the client's user agent isn't filtered + wanted_headers = [{"user-agent", Pleroma.Application.user_agent()}] - conn = - conn - |> Plug.Conn.put_req_header("user-agent", "fake/1.0") - |> ReverseProxy.call("/user-agent") + Tesla.Mock.mock(fn %{url: "/user-agent", headers: ^wanted_headers} -> + %Tesla.Env{ + status: 200, + body: "" + } + end) - assert json_response(conn, 200) == %{"user-agent" => Pleroma.Application.user_agent()} - end + conn = + conn + |> Plug.Conn.put_req_header("user-agent", "fake/1.0") + |> ReverseProxy.call("/user-agent") - test "closed connection", %{conn: conn} do - ClientMock - |> expect(:request, fn :get, "/closed", _, _, _ -> {:ok, 200, [], %{}} end) - |> expect(:stream_body, fn _ -> {:error, :closed} end) - |> expect(:close, fn _ -> :ok end) - - conn = ReverseProxy.call(conn, "/closed") - assert conn.halted - end - - defp stream_mock(invokes, with_close? \\ false) do - ClientMock - |> expect(:request, fn :get, "/stream-bytes/" <> length, _, _, _ -> - Registry.register(ClientMock, "/stream-bytes/" <> length, 0) - - {:ok, 200, [{"content-type", "application/octet-stream"}], - %{url: "/stream-bytes/" <> length}} - end) - |> expect(:stream_body, invokes, fn %{url: "/stream-bytes/" <> length} = client -> - max = String.to_integer(length) - - case Registry.lookup(ClientMock, "/stream-bytes/" <> length) do - [{_, current}] when current < max -> - Registry.update_value( - ClientMock, - "/stream-bytes/" <> length, - &(&1 + 10) - ) - - {:ok, "0123456789", client} - - [{_, ^max}] -> - Registry.unregister(ClientMock, "/stream-bytes/" <> length) - :done - end - end) - - if with_close? do - expect(ClientMock, :close, fn _ -> :ok end) + assert response(conn, 200) end end describe "max_body" do test "length returns error if content-length more than option", %{conn: conn} do - request_mock(0) + Tesla.Mock.mock(fn %{url: "/huge-file"} -> + %Tesla.Env{ + status: 200, + headers: [{"content-length", "100"}], + body: "This body is too large." + } + end) assert capture_log(fn -> ReverseProxy.call(conn, "/huge-file", max_body_length: 4) @@ -123,22 +68,16 @@ test "length returns error if content-length more than option", %{conn: conn} do ReverseProxy.call(conn, "/huge-file", max_body_length: 4) end) == "" end - - test "max_body_length returns error if streaming body more than that option", %{conn: conn} do - stream_mock(3, true) - - assert capture_log(fn -> - ReverseProxy.call(conn, "/stream-bytes/50", max_body_length: 30) - end) =~ - "Elixir.Pleroma.ReverseProxy request to /stream-bytes/50 failed while reading/chunking: :body_too_large" - end end describe "HEAD requests" do test "common", %{conn: conn} do - ClientMock - |> expect(:request, fn :head, "/head", _, _, _ -> - {:ok, 200, [{"content-type", "text/html; charset=utf-8"}]} + Tesla.Mock.mock(fn %{method: :head, url: "/head"} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "text/html; charset=utf-8"}], + body: "" + } end) conn = ReverseProxy.call(Map.put(conn, :method, "HEAD"), "/head") @@ -146,18 +85,17 @@ test "common", %{conn: conn} do end end - defp error_mock(status) when is_integer(status) do - ClientMock - |> expect(:request, fn :get, "/status/" <> _, _, _, _ -> - {:error, status} - end) - end - describe "returns error on" do test "500", %{conn: conn} do - error_mock(500) url = "/status/500" + Tesla.Mock.mock(fn %{url: ^url} -> + %Tesla.Env{ + status: 500, + body: "" + } + end) + capture_log(fn -> ReverseProxy.call(conn, url) end) =~ "[error] Elixir.Pleroma.ReverseProxy: request to /status/500 failed with HTTP status 500" @@ -168,9 +106,15 @@ test "500", %{conn: conn} do end test "400", %{conn: conn} do - error_mock(400) url = "/status/400" + Tesla.Mock.mock(fn %{url: ^url} -> + %Tesla.Env{ + status: 400, + body: "" + } + end) + capture_log(fn -> ReverseProxy.call(conn, url) end) =~ "[error] Elixir.Pleroma.ReverseProxy: request to /status/400 failed with HTTP status 400" @@ -179,9 +123,15 @@ test "400", %{conn: conn} do end test "403", %{conn: conn} do - error_mock(403) url = "/status/403" + Tesla.Mock.mock(fn %{url: ^url} -> + %Tesla.Env{ + status: 403, + body: "" + } + end) + capture_log(fn -> ReverseProxy.call(conn, url, failed_request_ttl: :timer.seconds(120)) end) =~ @@ -190,57 +140,17 @@ test "403", %{conn: conn} do {:ok, ttl} = Cachex.ttl(:failed_proxy_url_cache, url) assert ttl > 100_000 end - - test "204", %{conn: conn} do - url = "/status/204" - expect(ClientMock, :request, fn :get, _url, _, _, _ -> {:ok, 204, [], %{}} end) - - capture_log(fn -> - conn = ReverseProxy.call(conn, url) - assert conn.resp_body == "Request failed: No Content" - assert conn.halted - end) =~ - "[error] Elixir.Pleroma.ReverseProxy: request to \"/status/204\" failed with HTTP status 204" - - assert Cachex.get(:failed_proxy_url_cache, url) == {:ok, true} - assert Cachex.ttl(:failed_proxy_url_cache, url) == {:ok, nil} - end - end - - test "streaming", %{conn: conn} do - stream_mock(21) - conn = ReverseProxy.call(conn, "/stream-bytes/200") - assert conn.state == :chunked - assert byte_size(conn.resp_body) == 200 - assert Conn.get_resp_header(conn, "content-type") == ["application/octet-stream"] - end - - defp headers_mock(_) do - ClientMock - |> expect(:request, fn :get, "/headers", headers, _, _ -> - Registry.register(ClientMock, "/headers", 0) - {:ok, 200, [{"content-type", "application/json"}], %{url: "/headers", headers: headers}} - end) - |> expect(:stream_body, 2, fn %{url: url, headers: headers} = client -> - case Registry.lookup(ClientMock, url) do - [{_, 0}] -> - Registry.update_value(ClientMock, url, &(&1 + 1)) - headers = for {k, v} <- headers, into: %{}, do: {String.capitalize(k), v} - {:ok, Jason.encode!(%{headers: headers}), client} - - [{_, 1}] -> - Registry.unregister(ClientMock, url) - :done - end - end) - - :ok end describe "keep request headers" do - setup [:headers_mock] - test "header passes", %{conn: conn} do + Tesla.Mock.mock(fn %{url: "/headers"} -> + %Tesla.Env{ + status: 200, + body: "" + } + end) + conn = Conn.put_req_header( conn, @@ -249,68 +159,76 @@ test "header passes", %{conn: conn} do ) |> ReverseProxy.call("/headers") - %{"headers" => headers} = json_response(conn, 200) - assert headers["Accept"] == "text/html" + assert response(conn, 200) + assert {"accept", "text/html"} in conn.req_headers end test "header is filtered", %{conn: conn} do + # Mock will fail if the accept-language header isn't filtered + wanted_headers = [{"accept-encoding", "*"}] + + Tesla.Mock.mock(fn %{url: "/headers", headers: ^wanted_headers} -> + %Tesla.Env{ + status: 200, + body: "" + } + end) + conn = - Conn.put_req_header( - conn, - "accept-language", - "en-US" - ) + conn + |> Conn.put_req_header("accept-language", "en-US") + |> Conn.put_req_header("accept-encoding", "*") |> ReverseProxy.call("/headers") - %{"headers" => headers} = json_response(conn, 200) - refute headers["Accept-Language"] + assert response(conn, 200) end end test "returns 400 on non GET, HEAD requests", %{conn: conn} do + Tesla.Mock.mock(fn %{url: "/ip"} -> + %Tesla.Env{ + status: 200, + body: "" + } + end) + conn = ReverseProxy.call(Map.put(conn, :method, "POST"), "/ip") - assert conn.status == 400 + assert response(conn, 400) end - describe "cache resp headers" do + describe "cache resp headers not filtered" do test "add cache-control", %{conn: conn} do - ClientMock - |> expect(:request, fn :get, "/cache", _, _, _ -> - {:ok, 200, [{"ETag", "some ETag"}], %{}} + Tesla.Mock.mock(fn %{url: "/cache"} -> + %Tesla.Env{ + status: 200, + headers: [ + {"cache-control", "public, max-age=1209600"}, + {"etag", "some ETag"}, + {"expires", "Wed, 21 Oct 2015 07:28:00 GMT"} + ], + body: "" + } end) - |> expect(:stream_body, fn _ -> :done end) conn = ReverseProxy.call(conn, "/cache") assert {"cache-control", "public, max-age=1209600"} in conn.resp_headers + assert {"etag", "some ETag"} in conn.resp_headers + assert {"expires", "Wed, 21 Oct 2015 07:28:00 GMT"} in conn.resp_headers end end - defp disposition_headers_mock(headers) do - ClientMock - |> expect(:request, fn :get, "/disposition", _, _, _ -> - Registry.register(ClientMock, "/disposition", 0) - - {:ok, 200, headers, %{url: "/disposition"}} - end) - |> expect(:stream_body, 2, fn %{url: "/disposition"} = client -> - case Registry.lookup(ClientMock, "/disposition") do - [{_, 0}] -> - Registry.update_value(ClientMock, "/disposition", &(&1 + 1)) - {:ok, "", client} - - [{_, 1}] -> - Registry.unregister(ClientMock, "/disposition") - :done - end - end) - end - describe "response content disposition header" do - test "not atachment", %{conn: conn} do - disposition_headers_mock([ - {"content-type", "image/gif"}, - {"content-length", "0"} - ]) + test "not attachment", %{conn: conn} do + Tesla.Mock.mock(fn %{url: "/disposition"} -> + %Tesla.Env{ + status: 200, + headers: [ + {"content-type", "image/gif"}, + {"content-length", "0"} + ], + body: "" + } + end) conn = ReverseProxy.call(conn, "/disposition") @@ -318,10 +236,16 @@ test "not atachment", %{conn: conn} do end test "with content-disposition header", %{conn: conn} do - disposition_headers_mock([ - {"content-disposition", "attachment; filename=\"filename.jpg\""}, - {"content-length", "0"} - ]) + Tesla.Mock.mock(fn %{url: "/disposition"} -> + %Tesla.Env{ + status: 200, + headers: [ + {"content-disposition", "attachment; filename=\"filename.jpg\""}, + {"content-length", "0"} + ], + body: "" + } + end) conn = ReverseProxy.call(conn, "/disposition") diff --git a/test/test_helper.exs b/test/test_helper.exs index 9fb41e985..0fc7a86b9 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -7,9 +7,6 @@ Ecto.Adapters.SQL.Sandbox.mode(Pleroma.Repo, :manual) -Mox.defmock(Pleroma.ReverseProxy.ClientMock, for: Pleroma.ReverseProxy.Client) -Mox.defmock(Pleroma.GunMock, for: Pleroma.Gun) - {:ok, _} = Application.ensure_all_started(:ex_machina) ExUnit.after_suite(fn _results ->