Merge branch 'fix/upload-filter-exiftool-webp' into 'develop'

Skip processing webp image files because we can't strip EXIF

Closes #2080

See merge request pleroma/pleroma!2924
This commit is contained in:
rinpatch 2020-09-08 12:21:33 +00:00
parent e430e98e7d
commit 7490b76bbf
12 changed files with 46 additions and 22 deletions

View File

@ -20,7 +20,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Fixed ### Fixed
- Default HTTP adapter not respecting pool setting, leading to possible OOM. - Default HTTP adapter not respecting pool setting, leading to possible OOM.
- Mastodon API: Search parameter `following` now correctly returns the followings rather than the followers. - Fixed uploading webp images when the Exiftool Upload Filter is enabled by skipping them
- Mastodon API: Search parameter `following` now correctly returns the followings rather than the followers
- Mastodon API: Timelines hanging for (`number of posts with links * rich media timeout`) in the worst case. - Mastodon API: Timelines hanging for (`number of posts with links * rich media timeout`) in the worst case.
Reduced to just rich media timeout. Reduced to just rich media timeout.
- Mastodon API: Cards being wrong for preview statuses due to cache key collision. - Mastodon API: Cards being wrong for preview statuses due to cache key collision.

View File

@ -15,7 +15,11 @@ defmodule Pleroma.Upload.Filter do
require Logger require Logger
@callback filter(Pleroma.Upload.t()) :: :ok | {:ok, Pleroma.Upload.t()} | {:error, any()} @callback filter(Pleroma.Upload.t()) ::
{:ok, :filtered}
| {:ok, :noop}
| {:ok, :filtered, Pleroma.Upload.t()}
| {:error, any()}
@spec filter([module()], Pleroma.Upload.t()) :: {:ok, Pleroma.Upload.t()} | {:error, any()} @spec filter([module()], Pleroma.Upload.t()) :: {:ok, Pleroma.Upload.t()} | {:error, any()}
@ -25,10 +29,13 @@ def filter([], upload) do
def filter([filter | rest], upload) do def filter([filter | rest], upload) do
case filter.filter(upload) do case filter.filter(upload) do
:ok -> {:ok, :filtered} ->
filter(rest, upload) filter(rest, upload)
{:ok, upload} -> {:ok, :filtered, upload} ->
filter(rest, upload)
{:ok, :noop} ->
filter(rest, upload) filter(rest, upload)
error -> error ->

View File

@ -16,9 +16,11 @@ defmodule Pleroma.Upload.Filter.AnonymizeFilename do
def filter(%Upload{name: name} = upload) do def filter(%Upload{name: name} = upload) do
extension = List.last(String.split(name, ".")) extension = List.last(String.split(name, "."))
name = predefined_name(extension) || random(extension) name = predefined_name(extension) || random(extension)
{:ok, %Upload{upload | name: name}} {:ok, :filtered, %Upload{upload | name: name}}
end end
def filter(_), do: {:ok, :noop}
@spec predefined_name(String.t()) :: String.t() | nil @spec predefined_name(String.t()) :: String.t() | nil
defp predefined_name(extension) do defp predefined_name(extension) do
with name when not is_nil(name) <- Config.get([__MODULE__, :text]), with name when not is_nil(name) <- Config.get([__MODULE__, :text]),

View File

@ -17,8 +17,8 @@ def filter(%Upload{name: name, tempfile: tempfile} = upload) do
|> Base.encode16(case: :lower) |> Base.encode16(case: :lower)
filename = shasum <> "." <> extension filename = shasum <> "." <> extension
{:ok, %Upload{upload | id: shasum, path: filename}} {:ok, :filtered, %Upload{upload | id: shasum, path: filename}}
end end
def filter(_), do: :ok def filter(_), do: {:ok, :noop}
end end

View File

@ -9,11 +9,15 @@ defmodule Pleroma.Upload.Filter.Exiftool do
""" """
@behaviour Pleroma.Upload.Filter @behaviour Pleroma.Upload.Filter
@spec filter(Pleroma.Upload.t()) :: :ok | {:error, String.t()} @spec filter(Pleroma.Upload.t()) :: {:ok, any()} | {:error, String.t()}
# webp is not compatible with exiftool at this time
def filter(%Pleroma.Upload{content_type: "image/webp"}), do: {:ok, :noop}
def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do
try do try do
case System.cmd("exiftool", ["-overwrite_original", "-gps:all=", file], parallelism: true) do case System.cmd("exiftool", ["-overwrite_original", "-gps:all=", file], parallelism: true) do
{_response, 0} -> :ok {_response, 0} -> {:ok, :filtered}
{error, 1} -> {:error, error} {error, 1} -> {:error, error}
end end
rescue rescue
@ -22,5 +26,5 @@ def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do
end end
end end
def filter(_), do: :ok def filter(_), do: {:ok, :noop}
end end

View File

@ -38,16 +38,16 @@ defmodule Pleroma.Upload.Filter.Mogrifun do
[{"fill", "yellow"}, {"tint", "40"}] [{"fill", "yellow"}, {"tint", "40"}]
] ]
@spec filter(Pleroma.Upload.t()) :: :ok | {:error, String.t()} @spec filter(Pleroma.Upload.t()) :: {:ok, atom()} | {:error, String.t()}
def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do
try do try do
Filter.Mogrify.do_filter(file, [Enum.random(@filters)]) Filter.Mogrify.do_filter(file, [Enum.random(@filters)])
:ok {:ok, :filtered}
rescue rescue
_e in ErlangError -> _e in ErlangError ->
{:error, "mogrify command not found"} {:error, "mogrify command not found"}
end end
end end
def filter(_), do: :ok def filter(_), do: {:ok, :noop}
end end

View File

@ -8,18 +8,18 @@ defmodule Pleroma.Upload.Filter.Mogrify do
@type conversion :: action :: String.t() | {action :: String.t(), opts :: String.t()} @type conversion :: action :: String.t() | {action :: String.t(), opts :: String.t()}
@type conversions :: conversion() | [conversion()] @type conversions :: conversion() | [conversion()]
@spec filter(Pleroma.Upload.t()) :: :ok | {:error, String.t()} @spec filter(Pleroma.Upload.t()) :: {:ok, :atom} | {:error, String.t()}
def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do
try do try do
do_filter(file, Pleroma.Config.get!([__MODULE__, :args])) do_filter(file, Pleroma.Config.get!([__MODULE__, :args]))
:ok {:ok, :filtered}
rescue rescue
_e in ErlangError -> _e in ErlangError ->
{:error, "mogrify command not found"} {:error, "mogrify command not found"}
end end
end end
def filter(_), do: :ok def filter(_), do: {:ok, :noop}
def do_filter(file, filters) do def do_filter(file, filters) do
file file

View File

@ -24,18 +24,18 @@ defmodule Pleroma.Upload.Filter.AnonymizeFilenameTest do
test "it replaces filename on pre-defined text", %{upload_file: upload_file} do test "it replaces filename on pre-defined text", %{upload_file: upload_file} do
Config.put([Upload.Filter.AnonymizeFilename, :text], "custom-file.png") Config.put([Upload.Filter.AnonymizeFilename, :text], "custom-file.png")
{:ok, %Upload{name: name}} = Upload.Filter.AnonymizeFilename.filter(upload_file) {:ok, :filtered, %Upload{name: name}} = Upload.Filter.AnonymizeFilename.filter(upload_file)
assert name == "custom-file.png" assert name == "custom-file.png"
end end
test "it replaces filename on pre-defined text expression", %{upload_file: upload_file} do test "it replaces filename on pre-defined text expression", %{upload_file: upload_file} do
Config.put([Upload.Filter.AnonymizeFilename, :text], "custom-file.{extension}") Config.put([Upload.Filter.AnonymizeFilename, :text], "custom-file.{extension}")
{:ok, %Upload{name: name}} = Upload.Filter.AnonymizeFilename.filter(upload_file) {:ok, :filtered, %Upload{name: name}} = Upload.Filter.AnonymizeFilename.filter(upload_file)
assert name == "custom-file.jpg" assert name == "custom-file.jpg"
end end
test "it replaces filename on random text", %{upload_file: upload_file} do test "it replaces filename on random text", %{upload_file: upload_file} do
{:ok, %Upload{name: name}} = Upload.Filter.AnonymizeFilename.filter(upload_file) {:ok, :filtered, %Upload{name: name}} = Upload.Filter.AnonymizeFilename.filter(upload_file)
assert <<_::bytes-size(14)>> <> ".jpg" = name assert <<_::bytes-size(14)>> <> ".jpg" = name
refute name == "an… image.jpg" refute name == "an… image.jpg"
end end

View File

@ -25,6 +25,7 @@ test "adds shasum" do
assert { assert {
:ok, :ok,
:filtered,
%Pleroma.Upload{id: @shasum, path: @shasum <> ".jpg"} %Pleroma.Upload{id: @shasum, path: @shasum <> ".jpg"}
} = Dedupe.filter(upload) } = Dedupe.filter(upload)
end end

View File

@ -21,7 +21,7 @@ test "apply exiftool filter" do
tempfile: Path.absname("test/fixtures/DSCN0010_tmp.jpg") tempfile: Path.absname("test/fixtures/DSCN0010_tmp.jpg")
} }
assert Filter.Exiftool.filter(upload) == :ok assert Filter.Exiftool.filter(upload) == {:ok, :filtered}
{exif_original, 0} = System.cmd("exiftool", ["test/fixtures/DSCN0010.jpg"]) {exif_original, 0} = System.cmd("exiftool", ["test/fixtures/DSCN0010.jpg"])
{exif_filtered, 0} = System.cmd("exiftool", ["test/fixtures/DSCN0010_tmp.jpg"]) {exif_filtered, 0} = System.cmd("exiftool", ["test/fixtures/DSCN0010_tmp.jpg"])
@ -30,4 +30,13 @@ test "apply exiftool filter" do
assert String.match?(exif_original, ~r/GPS/) assert String.match?(exif_original, ~r/GPS/)
refute String.match?(exif_filtered, ~r/GPS/) refute String.match?(exif_filtered, ~r/GPS/)
end end
test "verify webp files are skipped" do
upload = %Pleroma.Upload{
name: "sample.webp",
content_type: "image/webp"
}
assert Filter.Exiftool.filter(upload) == {:ok, :noop}
end
end end

View File

@ -36,7 +36,7 @@ test "apply mogrify filter" do
save: fn _f, _o -> :ok end save: fn _f, _o -> :ok end
]} ]}
]) do ]) do
assert Filter.Mogrifun.filter(upload) == :ok assert Filter.Mogrifun.filter(upload) == {:ok, :filtered}
end end
Task.await(task) Task.await(task)

View File

@ -33,7 +33,7 @@ test "apply mogrify filter" do
custom: fn _m, _a -> :ok end, custom: fn _m, _a -> :ok end,
custom: fn m, a, o -> send(task.pid, {:apply_filter, {m, a, o}}) end, custom: fn m, a, o -> send(task.pid, {:apply_filter, {m, a, o}}) end,
save: fn _f, _o -> :ok end do save: fn _f, _o -> :ok end do
assert Filter.Mogrify.filter(upload) == :ok assert Filter.Mogrify.filter(upload) == {:ok, :filtered}
end end
Task.await(task) Task.await(task)