mrf(media_proxy_warming): avoid adapter-level redirects
Drop follow_redirect/force_redirect from the HTTP options used when warming MediaProxy, relying on Tesla middleware instead (Hackney redirect handling can crash behind CONNECT proxies). Also add a regression assertion in the policy test and document the upstream Hackney issues in ReverseProxy redirect handling.
This commit is contained in:
parent
029994aa75
commit
0b813b9c4a
3 changed files with 21 additions and 16 deletions
|
|
@ -5,9 +5,12 @@
|
|||
defmodule Pleroma.ReverseProxy.Client.Hackney do
|
||||
@behaviour Pleroma.ReverseProxy.Client
|
||||
|
||||
# redirect handler from Pleb, slightly modified to work with Hackney
|
||||
# In-app redirect handler to avoid Hackney redirect bugs:
|
||||
# - https://github.com/benoitc/hackney/issues/527 (relative/protocol-less redirects can crash Hackney)
|
||||
# - https://github.com/benoitc/hackney/issues/273 (redirects not followed when using HTTP proxy)
|
||||
#
|
||||
# Based on a redirect handler from Pleb, slightly modified to work with Hackney:
|
||||
# https://declin.eu/objects/d4f38e62-5429-4614-86d1-e8fc16e6bf33
|
||||
# https://github.com/benoitc/hackney/issues/273
|
||||
@redirect_statuses [301, 302, 303, 307, 308]
|
||||
defp absolute_redirect_url(original_url, resp_headers) do
|
||||
location =
|
||||
|
|
|
|||
|
|
@ -27,7 +27,14 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicy do
|
|||
end
|
||||
|
||||
defp fetch(url) do
|
||||
http_client_opts = Pleroma.Config.get([:media_proxy, :proxy_opts, :http], pool: :media)
|
||||
# This module uses Tesla (Pleroma.HTTP) to fetch the MediaProxy URL.
|
||||
# Redirect following is handled by Tesla middleware, so we must not enable
|
||||
# adapter-level redirect logic (Hackney can crash on relative redirects when proxied).
|
||||
http_client_opts =
|
||||
[:media_proxy, :proxy_opts, :http]
|
||||
|> Pleroma.Config.get(pool: :media)
|
||||
|> Keyword.drop([:follow_redirect, :force_redirect])
|
||||
|
||||
HTTP.get(url, [], http_client_opts)
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -54,14 +54,17 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicyTest do
|
|||
setup do: clear_config([:media_proxy, :enabled], true)
|
||||
|
||||
test "it prefetches media proxy URIs" do
|
||||
Tesla.Mock.mock(fn %{method: :get, url: "http://example.com/image.jpg"} ->
|
||||
{:ok, %Tesla.Env{status: 200, body: ""}}
|
||||
end)
|
||||
|
||||
with_mock HTTP, get: fn _, _, _ -> {:ok, []} end do
|
||||
with_mock HTTP,
|
||||
get: fn _, _, opts ->
|
||||
send(self(), {:prefetch_opts, opts})
|
||||
{:ok, []}
|
||||
end do
|
||||
MediaProxyWarmingPolicy.filter(@message)
|
||||
|
||||
assert called(HTTP.get(:_, :_, :_))
|
||||
assert_receive {:prefetch_opts, opts}
|
||||
refute Keyword.has_key?(opts, :follow_redirect)
|
||||
refute Keyword.has_key?(opts, :force_redirect)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -81,10 +84,6 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicyTest do
|
|||
end
|
||||
|
||||
test "history-aware" do
|
||||
Tesla.Mock.mock(fn %{method: :get, url: "http://example.com/image.jpg"} ->
|
||||
{:ok, %Tesla.Env{status: 200, body: ""}}
|
||||
end)
|
||||
|
||||
with_mock HTTP, get: fn _, _, _ -> {:ok, []} end do
|
||||
MRF.filter_one(MediaProxyWarmingPolicy, @message_with_history)
|
||||
|
||||
|
|
@ -93,10 +92,6 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicyTest do
|
|||
end
|
||||
|
||||
test "works with Updates" do
|
||||
Tesla.Mock.mock(fn %{method: :get, url: "http://example.com/image.jpg"} ->
|
||||
{:ok, %Tesla.Env{status: 200, body: ""}}
|
||||
end)
|
||||
|
||||
with_mock HTTP, get: fn _, _, _ -> {:ok, []} end do
|
||||
MRF.filter_one(MediaProxyWarmingPolicy, @message_with_history |> Map.put("type", "Update"))
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue