commit: b3c3bd99c390a4e5081d411011688e38285547b0
parent da7394f33b4402325b220dcd2957945464517f8b
Author: Mark Felder <feld@feld.me>
Date: Tue, 30 May 2023 16:56:09 -0400
Switch from serving a 400 to a 302
Diffstat:
5 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/changelog.d/3896.add b/changelog.d/3896.add
@@ -1 +1 @@
-Validate Host header for MediaProxy and Uploads
+Validate Host header for MediaProxy and Uploads and return a 302 if the base_url has changed
diff --git a/lib/pleroma/web/media_proxy/media_proxy_controller.ex b/lib/pleroma/web/media_proxy/media_proxy_controller.ex
@@ -207,12 +207,25 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
end
defp validate_host(conn, _params) do
- proxy_host = MediaProxy.base_url() |> URI.parse() |> Map.get(:host)
+ %{scheme: proxy_scheme, host: proxy_host, port: proxy_port} =
+ MediaProxy.base_url() |> URI.parse()
if match?(^proxy_host, conn.host) do
conn
else
- send_resp(conn, 400, Conn.Status.reason_phrase(400))
+ redirect_url =
+ %URI{
+ scheme: proxy_scheme,
+ host: proxy_host,
+ port: proxy_port,
+ path: conn.request_path,
+ query: conn.query_string
+ }
+ |> URI.to_string()
+ |> String.trim_trailing("?")
+
+ conn
+ |> Phoenix.Controller.redirect(external: redirect_url)
|> halt()
end
end
diff --git a/lib/pleroma/web/plugs/uploaded_media.ex b/lib/pleroma/web/plugs/uploaded_media.ex
@@ -46,7 +46,8 @@ defmodule Pleroma.Web.Plugs.UploadedMedia do
config = Pleroma.Config.get(Pleroma.Upload)
- media_host = Pleroma.Upload.base_url() |> URI.parse() |> Map.get(:host)
+ %{scheme: media_scheme, host: media_host, port: media_port} =
+ Pleroma.Upload.base_url() |> URI.parse()
with {:valid_host, true} <- {:valid_host, match?(^media_host, conn.host)},
uploader <- Keyword.fetch!(config, :uploader),
@@ -56,7 +57,19 @@ defmodule Pleroma.Web.Plugs.UploadedMedia do
get_media(conn, get_method, proxy_remote, opts)
else
{:valid_host, false} ->
- send_resp(conn, 400, Plug.Conn.Status.reason_phrase(400))
+ redirect_url =
+ %URI{
+ scheme: media_scheme,
+ host: media_host,
+ port: media_port,
+ path: conn.request_path,
+ query: conn.query_string
+ }
+ |> URI.to_string()
+ |> String.trim_trailing("?")
+
+ conn
+ |> Phoenix.Controller.redirect(external: redirect_url)
|> halt()
_ ->
diff --git a/test/pleroma/web/media_proxy/media_proxy_controller_test.exs b/test/pleroma/web/media_proxy/media_proxy_controller_test.exs
@@ -54,20 +54,37 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyControllerTest do
} = get(conn, "/proxy/hhgfh/eeee/fff")
end
- test "it returns a 400 for invalid host", %{conn: conn} do
- clear_config([:media_proxy, :base_url], "http://mp.localhost/")
+ test "it returns a 302 for invalid host", %{conn: conn} do
+ new_proxy_base = "http://mp.localhost/"
- url =
+ %{scheme: new_proxy_scheme, host: new_proxy_host, port: new_proxy_port} =
+ URI.parse(new_proxy_base)
+
+ clear_config([:media_proxy, :base_url], new_proxy_base)
+
+ proxy_url =
MediaProxy.encode_url("https://pleroma.social/logo.jpeg")
|> URI.parse()
|> Map.put(:host, "wronghost")
|> URI.to_string()
+ expected_url =
+ URI.parse(proxy_url)
+ |> Map.put(:host, new_proxy_host)
+ |> Map.put(:port, new_proxy_port)
+ |> Map.put(:scheme, new_proxy_scheme)
+ |> URI.to_string()
+
with_mock Pleroma.ReverseProxy,
call: fn _conn, _url, _opts -> %Conn{status: :success} end do
- %{status: status} = get(conn, url)
+ %{resp_headers: resp_headers, status: status} = get(conn, proxy_url)
+
+ assert status == 302
- assert status == 400
+ assert Enum.any?(
+ resp_headers,
+ &(&1 == {"location", expected_url})
+ )
end
end
diff --git a/test/pleroma/web/plugs/uploaded_media_plug_test.exs b/test/pleroma/web/plugs/uploaded_media_plug_test.exs
@@ -48,10 +48,27 @@ defmodule Pleroma.Web.Plugs.UploadedMediaPlugTest do
assert conn.status == 200
- clear_config([Pleroma.Upload, :base_url], "http://media.localhost/")
+ new_media_base = "http://media.localhost:8080"
+
+ %{scheme: new_media_scheme, host: new_media_host, port: new_media_port} =
+ URI.parse(new_media_base)
+
+ clear_config([Pleroma.Upload, :base_url], new_media_base)
conn = get(build_conn(), attachment_url)
- assert conn.status == 400
+ expected_url =
+ URI.parse(attachment_url)
+ |> Map.put(:host, new_media_host)
+ |> Map.put(:port, new_media_port)
+ |> Map.put(:scheme, new_media_scheme)
+ |> URI.to_string()
+
+ assert conn.status == 302
+
+ assert Enum.any?(
+ conn.resp_headers,
+ &(&1 == {"location", expected_url})
+ )
end
end