diff --git a/changelog.d/mediaproxy-content-type.fix b/changelog.d/mediaproxy-content-type.fix new file mode 100644 index 000000000..f36dd6283 --- /dev/null +++ b/changelog.d/mediaproxy-content-type.fix @@ -0,0 +1 @@ +Detect image content types for extensionless MediaProxy responses. diff --git a/lib/pleroma/reverse_proxy.ex b/lib/pleroma/reverse_proxy.ex index c7ee47c6e..061b2de9b 100644 --- a/lib/pleroma/reverse_proxy.ex +++ b/lib/pleroma/reverse_proxy.ex @@ -17,6 +17,7 @@ defmodule Pleroma.ReverseProxy do @max_read_duration :timer.seconds(30) @max_body_length :infinity @failed_request_ttl :timer.seconds(60) + @sniff_bytes 8 * 1024 @methods ~w(GET HEAD) @allowed_mime_types Pleroma.Config.get([Pleroma.Upload, :allowed_mime_types], []) @@ -59,7 +60,12 @@ defmodule Pleroma.ReverseProxy do * `inline_content_types`: * `true` will not alter `content-disposition` (up to the upstream), * `false` will add `content-disposition: attachment` to any request, - * a list of whitelisted content types + * a list of whitelisted content types for which `content-disposition: inline` + is always set (overriding any upstream header) so the media can be embedded + in pages; the filename is derived from the content type + + * `sniff_content_type` (default `false`): detects an image MIME type from the first + response chunk when the upstream type is missing or `application/octet-stream`. * `req_headers`, `resp_headers` additional headers. @@ -69,11 +75,15 @@ defmodule Pleroma.ReverseProxy do @default_options [pool: :media] @inline_content_types [ + "image/apng", + "image/avif", + "image/bmp", "image/gif", "image/jpeg", "image/jpg", "image/png", "image/svg+xml", + "image/webp", "audio/mpeg", "audio/mp3", "video/webm", @@ -92,6 +102,7 @@ defmodule Pleroma.ReverseProxy do | {:req_headers, [{String.t(), String.t()}]} | {:resp_headers, [{String.t(), String.t()}]} | {:inline_content_types, boolean() | list(String.t())} + | {:sniff_content_type, boolean()} | {:redirect_on_failure, boolean()} @spec call(Plug.Conn.t(), String.t(), list(option())) :: Plug.Conn.t() @@ -184,12 +195,14 @@ defmodule Pleroma.ReverseProxy do defp response(conn, client, url, status, headers, opts) do Logger.debug("#{__MODULE__} #{status} #{url} #{inspect(headers)}") + {headers, client, prefetched} = maybe_prefetch_for_content_type(headers, client, opts) + result = conn |> put_resp_headers(build_resp_headers(headers, opts)) |> streaming_compat |> send_chunked(status) - |> chunk_reply(client, opts) + |> chunk_reply(client, opts, prefetched) case result do {:ok, conn} -> @@ -209,18 +222,28 @@ defmodule Pleroma.ReverseProxy do end end - defp chunk_reply(conn, client, opts) do + defp chunk_reply(conn, client, opts, :none) 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) + defp chunk_reply(conn, _client, _opts, :done), do: {:ok, conn} + defp chunk_reply(conn, _client, _opts, {:error, error}), do: {:error, error, conn} + + defp chunk_reply(conn, client, opts, {:ok, data, duration}) do + with :ok <- + body_size_constraint( + byte_size(data), + Keyword.get(opts, :max_body_length, @max_body_length) ), - {:ok, data, client} <- client().stream_body(client), - {:ok, duration} <- increase_read_duration(duration), + {:ok, conn} <- chunk(conn, data) do + chunk_reply(conn, client, opts, byte_size(data), duration) + else + {:error, error} -> {:error, error, conn} + end + end + + defp chunk_reply(conn, client, opts, sent_so_far, duration) do + with {:ok, data, client, duration} <- read_chunk(client, duration, opts), sent_so_far = sent_so_far + byte_size(data), :ok <- body_size_constraint( @@ -235,6 +258,66 @@ defmodule Pleroma.ReverseProxy do end end + defp read_chunk(client, duration, opts) do + with {:ok, timer} <- + 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(timer) do + {:ok, data, client, duration} + end + end + + defp maybe_prefetch_for_content_type(headers, client, opts) do + if Keyword.get(opts, :sniff_content_type, false) and generic_content_type?(headers) do + case read_chunk(client, 0, opts) do + {:ok, data, client, duration} -> + {maybe_put_image_content_type(headers, data), client, {:ok, data, duration}} + + :done -> + {headers, client, :done} + + {:error, error} -> + {headers, client, {:error, error}} + end + else + {headers, client, :none} + end + end + + defp generic_content_type?(headers) do + headers + |> get_content_type() + |> String.trim() + |> String.downcase() + |> then(&(&1 in ["", "application/octet-stream"])) + end + + defp maybe_put_image_content_type(headers, data) do + with false <- data == "", + {:ok, %{mime_type: "image/" <> _ = content_type}} <- + Majic.perform({:bytes, binary_part(data, 0, min(byte_size(data), @sniff_bytes))}, + pool: Pleroma.MajicPool + ) do + [ + {"content-type", content_type} + | Enum.reject(headers, fn {key, _} -> key == "content-type" end) + ] + else + _ -> headers + end + rescue + error -> + Logger.debug("#{__MODULE__}: content-type sniffing failed: #{Exception.message(error)}") + headers + catch + kind, reason -> + Logger.debug("#{__MODULE__}: content-type sniffing failed: #{kind}: #{inspect(reason)}") + headers + end + defp head_response(conn, url, code, headers, opts) do Logger.debug("#{__MODULE__} #{code} #{url} #{inspect(headers)}") @@ -281,17 +364,34 @@ defmodule Pleroma.ReverseProxy do |> 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, [])) + |> merge_headers(Keyword.get(opts, :req_headers, [])) + |> maybe_force_identity_encoding(opts) + end + + defp merge_headers(headers, extra_headers) do + Enum.reduce(extra_headers, headers, fn {key, value}, headers -> + List.keystore(headers, String.downcase(key), 0, {String.downcase(key), value}) + end) + end + + defp maybe_force_identity_encoding(headers, opts) do + if Keyword.get(opts, :sniff_content_type, false) do + List.keystore(headers, "accept-encoding", 0, {"accept-encoding", "identity"}) + else + headers + end end # Disable content-encoding if any @range_headers are requested (see #1823). defp build_req_range_or_encoding_header(headers, _opts) do range? = Enum.any?(headers, fn {header, _} -> Enum.member?(@range_headers, header) end) - if range? && List.keymember?(headers, "accept-encoding", 0) do - List.keydelete(headers, "accept-encoding", 0) - else - headers + cond do + range? && List.keymember?(headers, "accept-encoding", 0) -> + List.keydelete(headers, "accept-encoding", 0) + + true -> + headers end end @@ -384,9 +484,33 @@ defmodule Pleroma.ReverseProxy do disposition = "attachment; filename=\"#{name}\"" - List.keystore(headers, "content-disposition", 0, {"content-disposition", disposition}) + replace_header(headers, "content-disposition", disposition) else - headers + if opt == true do + headers + else + name = inline_filename(content_type) + + disposition = + if name do + "inline; filename=\"#{name}\"" + else + "inline" + end + + replace_header(headers, "content-disposition", disposition) + end + end + end + + defp replace_header(headers, key, value) do + [{key, value} | Enum.reject(headers, fn {header, _} -> header == key end)] + end + + defp inline_filename(content_type) do + case MIME.extensions(content_type) do + [ext | _] when ext != "" -> "inline.#{ext}" + _ -> nil end end @@ -421,7 +545,9 @@ defmodule Pleroma.ReverseProxy do end end - defp check_read_duration(_, _), do: {:ok, :no_duration_limit, :no_duration_limit} + defp check_read_duration(_, _), do: {:ok, :no_duration_limit} + + defp increase_read_duration(:no_duration_limit), do: {:ok, :no_duration_limit} defp increase_read_duration({previous_duration, started}) when is_integer(previous_duration) and is_integer(started) do diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 6949c5f2d..725dc2ce7 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -620,6 +620,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do String.contains?(media_type, "image") -> "image" String.contains?(media_type, "video") -> "video" String.contains?(media_type, "audio") -> "audio" + attachment["type"] == "Image" -> "image" true -> "unknown" end diff --git a/lib/pleroma/web/media_proxy/media_proxy_controller.ex b/lib/pleroma/web/media_proxy/media_proxy_controller.ex index a0aafc32e..93f692008 100644 --- a/lib/pleroma/web/media_proxy/media_proxy_controller.ex +++ b/lib/pleroma/web/media_proxy/media_proxy_controller.ex @@ -208,6 +208,7 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do defp media_proxy_opts do Config.get([:media_proxy, :proxy_opts], []) + |> Keyword.put_new(:sniff_content_type, true) end defp sandbox(conn, _params) do diff --git a/test/pleroma/reverse_proxy_test.exs b/test/pleroma/reverse_proxy_test.exs index ec4470379..ad1d75f33 100644 --- a/test/pleroma/reverse_proxy_test.exs +++ b/test/pleroma/reverse_proxy_test.exs @@ -222,6 +222,56 @@ defmodule Pleroma.ReverseProxyTest do assert Conn.get_resp_header(conn, "content-type") == ["application/octet-stream"] end + test "sniffs and preserves an image body with a generic content type", %{conn: conn} do + body = File.read!("test/fixtures/image.jpg") + + ClientMock + |> expect(:request, fn :get, "/extensionless", headers, _, _ -> + assert {"accept-encoding", "identity"} in headers + {:ok, 200, [{"content-type", "application/octet-stream"}], %{body: body}} + end) + |> expect(:stream_body, fn %{body: ^body} = client -> + {:ok, body, Map.delete(client, :body)} + end) + |> expect(:stream_body, fn %{} -> :done end) + + conn = + ReverseProxy.call(conn, "/extensionless", + sniff_content_type: true, + max_read_duration: :infinity, + req_headers: [{"accept-encoding", "gzip"}] + ) + + assert conn.resp_body == body + assert Conn.get_resp_header(conn, "content-type") == ["image/jpeg"] + + assert Conn.get_resp_header(conn, "content-disposition") == [ + "inline; filename=\"inline.jpg\"" + ] + end + + test "preserves a generic content type for a non-image body", %{conn: conn} do + body = "not an image" + + ClientMock + |> expect(:request, fn :get, "/extensionless", _, _, _ -> + {:ok, 200, [], %{body: body}} + end) + |> expect(:stream_body, fn %{body: ^body} = client -> + {:ok, body, Map.delete(client, :body)} + end) + |> expect(:stream_body, fn %{} -> :done end) + + conn = ReverseProxy.call(conn, "/extensionless", sniff_content_type: true) + + assert conn.resp_body == body + assert Conn.get_resp_header(conn, "content-type") == ["application/octet-stream"] + + assert Conn.get_resp_header(conn, "content-disposition") == [ + "attachment; filename=\"extensionless\"" + ] + end + defp headers_mock(_) do ClientMock |> expect(:request, fn :get, "/headers", headers, _, _ -> @@ -328,6 +378,24 @@ defmodule Pleroma.ReverseProxyTest do conn = ReverseProxy.call(conn, "/disposition") assert {"content-type", "image/gif"} in conn.resp_headers + assert {"content-disposition", "inline; filename=\"inline.gif\""} in conn.resp_headers + end + + test "forces inline for inline content types overriding upstream attachment", %{ + conn: conn + } do + disposition_headers_mock([ + {"content-type", "image/png"}, + {"content-disposition", "attachment; filename=\"filename.png\""}, + {"content-disposition", "attachment; filename=\"duplicate.png\""}, + {"content-length", "0"} + ]) + + conn = ReverseProxy.call(conn, "/disposition") + + [disposition] = Conn.get_resp_header(conn, "content-disposition") + assert String.starts_with?(disposition, "inline") + refute String.starts_with?(disposition, "attachment") end test "with content-disposition header", %{conn: conn} do @@ -340,6 +408,50 @@ defmodule Pleroma.ReverseProxyTest do assert {"content-disposition", "attachment; filename=\"filename.jpg\""} in conn.resp_headers end + + test "with inline_content_types: true leaves upstream headers untouched", %{ + conn: conn + } do + # opt == true: the proxy must not synthesise or rewrite content-disposition. + disposition_headers_mock([ + {"content-type", "image/png"}, + {"content-disposition", "attachment; filename=\"upstream.png\""}, + {"content-length", "0"} + ]) + + conn = ReverseProxy.call(conn, "/disposition", inline_content_types: true) + + assert {"content-disposition", "attachment; filename=\"upstream.png\""} in conn.resp_headers + end + + test "forces bare inline for a whitelisted type with no MIME extension", %{ + conn: conn + } do + # image/x-foo-bar has no entry in MIME's database, so inline_filename/1 + # returns nil and the disposition should be the bare token "inline". + disposition_headers_mock([ + {"content-type", "image/x-foo-bar"}, + {"content-length", "0"} + ]) + + conn = ReverseProxy.call(conn, "/disposition", inline_content_types: ["image/x-foo-bar"]) + + [disposition] = Conn.get_resp_header(conn, "content-disposition") + assert disposition == "inline" + end + + test "serves modern browser image types inline", %{conn: conn} do + disposition_headers_mock([ + {"content-type", "image/webp"}, + {"content-length", "0"} + ]) + + conn = ReverseProxy.call(conn, "/disposition") + + assert Conn.get_resp_header(conn, "content-disposition") == [ + "inline; filename=\"inline.webp\"" + ] + end end describe "content-type sanitisation" do diff --git a/test/pleroma/web/mastodon_api/views/status_view_test.exs b/test/pleroma/web/mastodon_api/views/status_view_test.exs index 76123cd0f..faeca050e 100644 --- a/test/pleroma/web/mastodon_api/views/status_view_test.exs +++ b/test/pleroma/web/mastodon_api/views/status_view_test.exs @@ -668,6 +668,23 @@ defmodule Pleroma.Web.MastodonAPI.StatusViewTest do assert expected == StatusView.render("attachment.json", %{attachment: object}) assert_schema(expected, "Attachment", api_spec) end + + test "uses the ActivityStreams image type when the MIME type is generic" do + object = %{ + "type" => "Image", + "url" => [ + %{ + "mediaType" => "application/octet-stream", + "href" => "https://example.com/extensionless" + } + ] + } + + attachment = StatusView.render("attachment.json", %{attachment: object}) + + assert attachment.type == "image" + assert attachment.pleroma.mime_type == "application/octet-stream" + end end test "put the url advertised in the Activity in to the url attribute" do diff --git a/test/pleroma/web/media_proxy/media_proxy_controller_test.exs b/test/pleroma/web/media_proxy/media_proxy_controller_test.exs index f7e52483c..800159c21 100644 --- a/test/pleroma/web/media_proxy/media_proxy_controller_test.exs +++ b/test/pleroma/web/media_proxy/media_proxy_controller_test.exs @@ -71,7 +71,10 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyControllerTest do test "it performs ReverseProxy.call with valid signature", %{conn: conn, url: url} do with_mock Pleroma.ReverseProxy, - call: fn _conn, _url, _opts -> %Conn{status: :success} end do + call: fn _conn, _url, opts -> + assert opts[:sniff_content_type] + %Conn{status: :success} + end do assert %Conn{status: :success} = get(conn, url) end end