[#2497] Media preview proxy for images: fixes, tweaks, refactoring, tests adjustments.

3 files changed, 64 insertions, 35 deletions

diff --git a/lib/pleroma/reverse_proxy/reverse_proxy.ex b/lib/pleroma/reverse_proxy/reverse_proxy.ex
index 4bbeb493c..aeaf9bd39 100644
--- a/lib/pleroma/reverse_proxy/reverse_proxy.ex
+++ b/lib/pleroma/reverse_proxy/reverse_proxy.ex
@@ -16,6 +16,8 @@ defmodule Pleroma.ReverseProxy do
   @failed_request_ttl :timer.seconds(60)
   @methods ~w(GET HEAD)
 
+  def max_read_duration_default, do: @max_read_duration
+
   @moduledoc """
   A reverse proxy.
 
@@ -370,6 +372,8 @@ defmodule Pleroma.ReverseProxy do
 
   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
diff --git a/lib/pleroma/web/media_proxy/media_proxy.ex b/lib/pleroma/web/media_proxy/media_proxy.ex
index f4791c758..4e01c14e4 100644
--- a/lib/pleroma/web/media_proxy/media_proxy.ex
+++ b/lib/pleroma/web/media_proxy/media_proxy.ex
@@ -13,26 +13,32 @@ defmodule Pleroma.Web.MediaProxy do
   def url("/" <> _ = url), do: url
 
   def url(url) do
-    if disabled?() or local?(url) or whitelisted?(url) do
+    if not enabled?() or local?(url) or whitelisted?(url) do
       url
     else
       encode_url(url)
     end
   end
 
+  # Note: routing all URLs to preview handler (even local and whitelisted).
+  #   Preview handler will call url/1 on decoded URLs, and applicable ones will detour media proxy.
   def preview_url(url) do
-    if disabled?() or whitelisted?(url) do
-      url
-    else
+    if preview_enabled?() do
       encode_preview_url(url)
+    else
+      url
     end
   end
 
-  defp disabled?, do: !Config.get([:media_proxy, :enabled], false)
+  def enabled?, do: Config.get([:media_proxy, :enabled], false)
 
-  defp local?(url), do: String.starts_with?(url, Pleroma.Web.base_url())
+  # Note: media proxy must be enabled for media preview proxy in order to load all
+  #   non-local non-whitelisted URLs through it and be sure that body size constraint is preserved.
+  def preview_enabled?, do: enabled?() and Config.get([:media_preview_proxy, :enabled], false)
 
-  defp whitelisted?(url) do
+  def local?(url), do: String.starts_with?(url, Pleroma.Web.base_url())
+
+  def whitelisted?(url) do
     %{host: domain} = URI.parse(url)
 
     mediaproxy_whitelist = Config.get([:media_proxy, :whitelist])
@@ -111,17 +117,24 @@ defmodule Pleroma.Web.MediaProxy do
     proxy_url("proxy/preview", sig_base64, url_base64, filename)
   end
 
-  def filename_matches(%{"filename" => _} = _, path, url) do
+  def verify_request_path_and_url(
+        %Plug.Conn{params: %{"filename" => _}, request_path: request_path},
+        url
+      ) do
+    verify_request_path_and_url(request_path, url)
+  end
+
+  def verify_request_path_and_url(request_path, url) when is_binary(request_path) do
     filename = filename(url)
 
-    if filename && not basename_matches?(path, filename) do
+    if filename && not basename_matches?(request_path, filename) do
       {:wrong_filename, filename}
     else
       :ok
     end
   end
 
-  def filename_matches(_, _, _), do: :ok
+  def verify_request_path_and_url(_, _), do: :ok
 
   defp basename_matches?(path, filename) do
     basename = Path.basename(path)
diff --git a/lib/pleroma/web/media_proxy/media_proxy_controller.ex b/lib/pleroma/web/media_proxy/media_proxy_controller.ex
index fe3f61c18..157365e08 100644
--- a/lib/pleroma/web/media_proxy/media_proxy_controller.ex
+++ b/lib/pleroma/web/media_proxy/media_proxy_controller.ex
@@ -10,14 +10,12 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
   alias Pleroma.ReverseProxy
   alias Pleroma.Web.MediaProxy
 
-  @default_proxy_opts [max_body_length: 25 * 1_048_576, http: [follow_redirect: true]]
-
-  def remote(conn, %{"sig" => sig64, "url" => url64} = params) do
-    with config <- Config.get([:media_proxy], []),
-         {_, true} <- {:enabled, Keyword.get(config, :enabled, false)},
+  def remote(conn, %{"sig" => sig64, "url" => url64}) do
+    with {_, true} <- {:enabled, MediaProxy.enabled?()},
          {:ok, url} <- MediaProxy.decode_url(sig64, url64),
-         :ok <- MediaProxy.filename_matches(params, conn.request_path, url) do
-      ReverseProxy.call(conn, url, Keyword.get(config, :proxy_opts, @default_proxy_opts))
+         :ok <- MediaProxy.verify_request_path_and_url(conn, url) do
+      proxy_opts = Config.get([:media_proxy, :proxy_opts], [])
+      ReverseProxy.call(conn, url, proxy_opts)
     else
       {:enabled, false} ->
         send_resp(conn, 404, Plug.Conn.Status.reason_phrase(404))
@@ -30,10 +28,10 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
     end
   end
 
-  def preview(conn, %{"sig" => sig64, "url" => url64} = params) do
-    with {_, true} <- {:enabled, Config.get([:media_preview_proxy, :enabled], false)},
+  def preview(conn, %{"sig" => sig64, "url" => url64}) do
+    with {_, true} <- {:enabled, MediaProxy.preview_enabled?()},
          {:ok, url} <- MediaProxy.decode_url(sig64, url64),
-         :ok <- MediaProxy.filename_matches(params, conn.request_path, url) do
+         :ok <- MediaProxy.verify_request_path_and_url(conn, url) do
       handle_preview(conn, url)
     else
       {:enabled, false} ->
@@ -48,21 +46,27 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
   end
 
   defp handle_preview(conn, url) do
-    with {:ok, %{status: status} = head_response} when status in 200..299 <- Tesla.head(url),
-         {_, true} <- {:acceptable_content_length, acceptable_body_length?(head_response)} do
+    with {:ok, %{status: status} = head_response} when status in 200..299 <-
+           Tesla.head(url, opts: [adapter: [timeout: preview_head_request_timeout()]]) do
       content_type = Tesla.get_header(head_response, "content-type")
       handle_preview(content_type, conn, url)
     else
       {_, %{status: status}} ->
         send_resp(conn, :failed_dependency, "Can't fetch HTTP headers (HTTP #{status}).")
 
-      {:acceptable_content_length, false} ->
-        send_resp(conn, :unprocessable_entity, "Source file size exceeds limit.")
+      {:error, :recv_response_timeout} ->
+        send_resp(conn, :failed_dependency, "HEAD request timeout.")
+
+      _ ->
+        send_resp(conn, :failed_dependency, "Can't fetch HTTP headers.")
     end
   end
 
-  defp handle_preview("image/" <> _, %{params: params} = conn, url) do
-    with {:ok, %{status: status, body: body}} when status in 200..299 <- Tesla.get(url),
+  defp handle_preview("image/" <> _ = content_type, %{params: params} = conn, url) do
+    with {:ok, %{status: status, body: body}} when status in 200..299 <-
+           url
+           |> MediaProxy.url()
+           |> Tesla.get(opts: [adapter: [timeout: preview_timeout()]]),
          {:ok, path} <- MogrifyHelper.store_as_temporary_file(url, body),
          resize_dimensions <-
            Map.get(
@@ -70,12 +74,19 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
              "limit_dimensions",
              Config.get([:media_preview_proxy, :limit_dimensions])
            ),
-         %Mogrify.Image{} <- MogrifyHelper.in_place_resize_to_limit(path, resize_dimensions) do
-      send_file(conn, 200, path)
+         %Mogrify.Image{} <- MogrifyHelper.in_place_resize_to_limit(path, resize_dimensions),
+         {:ok, image_binary} <- File.read(path),
+         _ <- File.rm(path) do
+      conn
+      |> put_resp_header("content-type", content_type)
+      |> send_resp(200, image_binary)
     else
       {_, %{status: _}} ->
         send_resp(conn, :failed_dependency, "Can't fetch the image.")
 
+      {:error, :recv_response_timeout} ->
+        send_resp(conn, :failed_dependency, "Downstream timeout.")
+
       _ ->
         send_resp(conn, :failed_dependency, "Can't handle image preview.")
     end
@@ -85,13 +96,14 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
     send_resp(conn, :unprocessable_entity, "Unsupported content type: #{content_type}.")
   end
 
-  defp acceptable_body_length?(head_response) do
-    max_body_length = Config.get([:media_preview_proxy, :max_body_length], nil)
-    content_length = Tesla.get_header(head_response, "content-length")
-    content_length = with {int, _} <- Integer.parse(content_length), do: int
+  defp preview_head_request_timeout do
+    Config.get([:media_preview_proxy, :proxy_opts, :head_request_max_read_duration]) ||
+      preview_timeout()
+  end
 
-    content_length == :error or
-      max_body_length in [nil, :infinity] or
-      content_length <= max_body_length
+  defp preview_timeout do
+    Config.get([:media_preview_proxy, :proxy_opts, :max_read_duration]) ||
+      Config.get([:media_proxy, :proxy_opts, :max_read_duration]) ||
+      ReverseProxy.max_read_duration_default()
   end
 end