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
346014b897
commit
e7d2d9bd89
3 changed files with 21 additions and 16 deletions
|
|
@ -5,9 +5,12 @@
|
||||||
defmodule Pleroma.ReverseProxy.Client.Hackney do
|
defmodule Pleroma.ReverseProxy.Client.Hackney do
|
||||||
@behaviour Pleroma.ReverseProxy.Client
|
@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://declin.eu/objects/d4f38e62-5429-4614-86d1-e8fc16e6bf33
|
||||||
# https://github.com/benoitc/hackney/issues/273
|
|
||||||
@redirect_statuses [301, 302, 303, 307, 308]
|
@redirect_statuses [301, 302, 303, 307, 308]
|
||||||
defp absolute_redirect_url(original_url, resp_headers) do
|
defp absolute_redirect_url(original_url, resp_headers) do
|
||||||
location =
|
location =
|
||||||
|
|
|
||||||
|
|
@ -27,7 +27,14 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicy do
|
||||||
end
|
end
|
||||||
|
|
||||||
defp fetch(url) do
|
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)
|
HTTP.get(url, [], http_client_opts)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -54,14 +54,17 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicyTest do
|
||||||
setup do: clear_config([:media_proxy, :enabled], true)
|
setup do: clear_config([:media_proxy, :enabled], true)
|
||||||
|
|
||||||
test "it prefetches media proxy URIs" do
|
test "it prefetches media proxy URIs" do
|
||||||
Tesla.Mock.mock(fn %{method: :get, url: "http://example.com/image.jpg"} ->
|
with_mock HTTP,
|
||||||
{:ok, %Tesla.Env{status: 200, body: ""}}
|
get: fn _, _, opts ->
|
||||||
end)
|
send(self(), {:prefetch_opts, opts})
|
||||||
|
{:ok, []}
|
||||||
with_mock HTTP, get: fn _, _, _ -> {:ok, []} end do
|
end do
|
||||||
MediaProxyWarmingPolicy.filter(@message)
|
MediaProxyWarmingPolicy.filter(@message)
|
||||||
|
|
||||||
assert called(HTTP.get(:_, :_, :_))
|
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
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
@ -81,10 +84,6 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicyTest do
|
||||||
end
|
end
|
||||||
|
|
||||||
test "history-aware" do
|
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
|
with_mock HTTP, get: fn _, _, _ -> {:ok, []} end do
|
||||||
MRF.filter_one(MediaProxyWarmingPolicy, @message_with_history)
|
MRF.filter_one(MediaProxyWarmingPolicy, @message_with_history)
|
||||||
|
|
||||||
|
|
@ -93,10 +92,6 @@ defmodule Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicyTest do
|
||||||
end
|
end
|
||||||
|
|
||||||
test "works with Updates" do
|
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
|
with_mock HTTP, get: fn _, _, _ -> {:ok, []} end do
|
||||||
MRF.filter_one(MediaProxyWarmingPolicy, @message_with_history |> Map.put("type", "Update"))
|
MRF.filter_one(MediaProxyWarmingPolicy, @message_with_history |> Map.put("type", "Update"))
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue