From fa347b9c2f416cd2c402e3e6eebb561dfc0ee8a8 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@FreeBSD.org>
Date: Wed, 26 Aug 2020 13:32:03 -0500
Subject: [PATCH 1/6] Fix uploading webp image files when Exiftool Upload
 Filter is enabled

---
 CHANGELOG.md                          |  1 +
 lib/pleroma/upload/filter/exiftool.ex | 17 +++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 46f1e859b..cfe0651c8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -138,6 +138,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Fix whole_word always returning false on filter get requests
 - Migrations not working on OTP releases if the database was connected over ssl
 - Fix relay following
+- Fixed uploading webp images when the Exiftool Upload Filter is enabled
 
 ## [2.0.7] - 2020-06-13
 
diff --git a/lib/pleroma/upload/filter/exiftool.ex b/lib/pleroma/upload/filter/exiftool.ex
index ea8798fe3..a91bd5e24 100644
--- a/lib/pleroma/upload/filter/exiftool.ex
+++ b/lib/pleroma/upload/filter/exiftool.ex
@@ -10,9 +10,20 @@ defmodule Pleroma.Upload.Filter.Exiftool do
   @behaviour Pleroma.Upload.Filter
 
   @spec filter(Pleroma.Upload.t()) :: :ok | {:error, String.t()}
-  def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do
+  def filter(%Pleroma.Upload{name: file, tempfile: path, content_type: "image" <> _}) do
+    # webp is not compatible with exiftool at this time
+    if Regex.match?(~r/\.(webp)$/i, file) do
+      :ok
+    else
+      strip_exif(path)
+    end
+  end
+
+  def filter(_), do: :ok
+
+  defp strip_exif(path) do
     try do
-      case System.cmd("exiftool", ["-overwrite_original", "-gps:all=", file], parallelism: true) do
+      case System.cmd("exiftool", ["-overwrite_original", "-gps:all=", path], parallelism: true) do
         {_response, 0} -> :ok
         {error, 1} -> {:error, error}
       end
@@ -21,6 +32,4 @@ def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do
         {:error, "exiftool command not found"}
     end
   end
-
-  def filter(_), do: :ok
 end

From 2165a249744a1ad4386a9d237871abe88e298942 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@FreeBSD.org>
Date: Fri, 4 Sep 2020 17:40:59 -0500
Subject: [PATCH 2/6] Improve upload filter return values so we can identify
 when filters make no changes to the input

---
 lib/pleroma/upload/filter.ex                    | 13 ++++++++++---
 lib/pleroma/upload/filter/anonymize_filename.ex |  4 +++-
 lib/pleroma/upload/filter/dedupe.ex             |  4 ++--
 lib/pleroma/upload/filter/exiftool.ex           |  8 ++++----
 lib/pleroma/upload/filter/mogrifun.ex           |  6 +++---
 lib/pleroma/upload/filter/mogrify.ex            |  6 +++---
 test/upload/filter/anonymize_filename_test.exs  |  6 +++---
 test/upload/filter/dedupe_test.exs              |  1 +
 test/upload/filter/exiftool_test.exs            |  2 +-
 test/upload/filter/mogrifun_test.exs            |  2 +-
 test/upload/filter/mogrify_test.exs             |  2 +-
 11 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/lib/pleroma/upload/filter.ex b/lib/pleroma/upload/filter.ex
index dbdadc97f..661135634 100644
--- a/lib/pleroma/upload/filter.ex
+++ b/lib/pleroma/upload/filter.ex
@@ -15,7 +15,11 @@ defmodule Pleroma.Upload.Filter do
 
   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()}
 
@@ -25,10 +29,13 @@ def filter([], upload) do
 
   def filter([filter | rest], upload) do
     case filter.filter(upload) do
-      :ok ->
+      {:ok, :filtered} ->
         filter(rest, upload)
 
-      {:ok, upload} ->
+      {:ok, :filtered, upload} ->
+        filter(rest, upload)
+
+      {:ok, :noop} ->
         filter(rest, upload)
 
       error ->
diff --git a/lib/pleroma/upload/filter/anonymize_filename.ex b/lib/pleroma/upload/filter/anonymize_filename.ex
index 07ead8203..fc456e4f4 100644
--- a/lib/pleroma/upload/filter/anonymize_filename.ex
+++ b/lib/pleroma/upload/filter/anonymize_filename.ex
@@ -16,9 +16,11 @@ defmodule Pleroma.Upload.Filter.AnonymizeFilename do
   def filter(%Upload{name: name} = upload) do
     extension = List.last(String.split(name, "."))
     name = predefined_name(extension) || random(extension)
-    {:ok, %Upload{upload | name: name}}
+    {:ok, :filtered, %Upload{upload | name: name}}
   end
 
+  def filter(_), do: {:ok, :noop}
+
   @spec predefined_name(String.t()) :: String.t() | nil
   defp predefined_name(extension) do
     with name when not is_nil(name) <- Config.get([__MODULE__, :text]),
diff --git a/lib/pleroma/upload/filter/dedupe.ex b/lib/pleroma/upload/filter/dedupe.ex
index 41218a918..86cbc8996 100644
--- a/lib/pleroma/upload/filter/dedupe.ex
+++ b/lib/pleroma/upload/filter/dedupe.ex
@@ -17,8 +17,8 @@ def filter(%Upload{name: name, tempfile: tempfile} = upload) do
       |> Base.encode16(case: :lower)
 
     filename = shasum <> "." <> extension
-    {:ok, %Upload{upload | id: shasum, path: filename}}
+    {:ok, :filtered, %Upload{upload | id: shasum, path: filename}}
   end
 
-  def filter(_), do: :ok
+  def filter(_), do: {:ok, :noop}
 end
diff --git a/lib/pleroma/upload/filter/exiftool.ex b/lib/pleroma/upload/filter/exiftool.ex
index a91bd5e24..94d12c01b 100644
--- a/lib/pleroma/upload/filter/exiftool.ex
+++ b/lib/pleroma/upload/filter/exiftool.ex
@@ -9,22 +9,22 @@ defmodule Pleroma.Upload.Filter.Exiftool do
   """
   @behaviour Pleroma.Upload.Filter
 
-  @spec filter(Pleroma.Upload.t()) :: :ok | {:error, String.t()}
+  @spec filter(Pleroma.Upload.t()) :: {:ok, any()} | {:error, String.t()}
   def filter(%Pleroma.Upload{name: file, tempfile: path, content_type: "image" <> _}) do
     # webp is not compatible with exiftool at this time
     if Regex.match?(~r/\.(webp)$/i, file) do
-      :ok
+      {:ok, :noop}
     else
       strip_exif(path)
     end
   end
 
-  def filter(_), do: :ok
+  def filter(_), do: {:ok, :noop}
 
   defp strip_exif(path) do
     try do
       case System.cmd("exiftool", ["-overwrite_original", "-gps:all=", path], parallelism: true) do
-        {_response, 0} -> :ok
+        {_response, 0} -> {:ok, :filtered}
         {error, 1} -> {:error, error}
       end
     rescue
diff --git a/lib/pleroma/upload/filter/mogrifun.ex b/lib/pleroma/upload/filter/mogrifun.ex
index c8fa7b190..363e5cf0f 100644
--- a/lib/pleroma/upload/filter/mogrifun.ex
+++ b/lib/pleroma/upload/filter/mogrifun.ex
@@ -38,16 +38,16 @@ defmodule Pleroma.Upload.Filter.Mogrifun do
     [{"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
     try do
       Filter.Mogrify.do_filter(file, [Enum.random(@filters)])
-      :ok
+      {:ok, :filtered}
     rescue
       _e in ErlangError ->
         {:error, "mogrify command not found"}
     end
   end
 
-  def filter(_), do: :ok
+  def filter(_), do: {:ok, :noop}
 end
diff --git a/lib/pleroma/upload/filter/mogrify.ex b/lib/pleroma/upload/filter/mogrify.ex
index 7a45add5a..71968fd9c 100644
--- a/lib/pleroma/upload/filter/mogrify.ex
+++ b/lib/pleroma/upload/filter/mogrify.ex
@@ -8,18 +8,18 @@ defmodule Pleroma.Upload.Filter.Mogrify do
   @type conversion :: action :: String.t() | {action :: String.t(), opts :: String.t()}
   @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
     try do
       do_filter(file, Pleroma.Config.get!([__MODULE__, :args]))
-      :ok
+      {:ok, :filtered}
     rescue
       _e in ErlangError ->
         {:error, "mogrify command not found"}
     end
   end
 
-  def filter(_), do: :ok
+  def filter(_), do: {:ok, :noop}
 
   def do_filter(file, filters) do
     file
diff --git a/test/upload/filter/anonymize_filename_test.exs b/test/upload/filter/anonymize_filename_test.exs
index adff70f57..19b915cc8 100644
--- a/test/upload/filter/anonymize_filename_test.exs
+++ b/test/upload/filter/anonymize_filename_test.exs
@@ -24,18 +24,18 @@ defmodule Pleroma.Upload.Filter.AnonymizeFilenameTest do
 
   test "it replaces filename on pre-defined text", %{upload_file: upload_file} do
     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"
   end
 
   test "it replaces filename on pre-defined text expression", %{upload_file: upload_file} do
     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"
   end
 
   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
     refute name == "an… image.jpg"
   end
diff --git a/test/upload/filter/dedupe_test.exs b/test/upload/filter/dedupe_test.exs
index 966c353f7..75c7198e1 100644
--- a/test/upload/filter/dedupe_test.exs
+++ b/test/upload/filter/dedupe_test.exs
@@ -25,6 +25,7 @@ test "adds shasum" do
 
     assert {
              :ok,
+             :filtered,
              %Pleroma.Upload{id: @shasum, path: @shasum <> ".jpg"}
            } = Dedupe.filter(upload)
   end
diff --git a/test/upload/filter/exiftool_test.exs b/test/upload/filter/exiftool_test.exs
index 8ed7d650b..094253a25 100644
--- a/test/upload/filter/exiftool_test.exs
+++ b/test/upload/filter/exiftool_test.exs
@@ -21,7 +21,7 @@ test "apply exiftool filter" do
       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_filtered, 0} = System.cmd("exiftool", ["test/fixtures/DSCN0010_tmp.jpg"])
diff --git a/test/upload/filter/mogrifun_test.exs b/test/upload/filter/mogrifun_test.exs
index 2426a8496..dc1e9e78f 100644
--- a/test/upload/filter/mogrifun_test.exs
+++ b/test/upload/filter/mogrifun_test.exs
@@ -36,7 +36,7 @@ test "apply mogrify filter" do
          save: fn _f, _o -> :ok end
        ]}
     ]) do
-      assert Filter.Mogrifun.filter(upload) == :ok
+      assert Filter.Mogrifun.filter(upload) == {:ok, :filtered}
     end
 
     Task.await(task)
diff --git a/test/upload/filter/mogrify_test.exs b/test/upload/filter/mogrify_test.exs
index 62ca30487..bf64b96b3 100644
--- a/test/upload/filter/mogrify_test.exs
+++ b/test/upload/filter/mogrify_test.exs
@@ -33,7 +33,7 @@ test "apply mogrify filter" do
       custom: fn _m, _a -> :ok end,
       custom: fn m, a, o -> send(task.pid, {:apply_filter, {m, a, o}}) end,
       save: fn _f, _o -> :ok end do
-      assert Filter.Mogrify.filter(upload) == :ok
+      assert Filter.Mogrify.filter(upload) == {:ok, :filtered}
     end
 
     Task.await(task)

From 3a98960c2684229435c5d04e4f3e0611098ceea2 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@FreeBSD.org>
Date: Fri, 4 Sep 2020 17:50:16 -0500
Subject: [PATCH 3/6] Verify webp files are not processed with exiftool

---
 test/upload/filter/exiftool_test.exs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/test/upload/filter/exiftool_test.exs b/test/upload/filter/exiftool_test.exs
index 094253a25..fe24036d9 100644
--- a/test/upload/filter/exiftool_test.exs
+++ b/test/upload/filter/exiftool_test.exs
@@ -30,4 +30,15 @@ test "apply exiftool filter" do
     assert String.match?(exif_original, ~r/GPS/)
     refute String.match?(exif_filtered, ~r/GPS/)
   end
+
+  test "verify webp files are skipped" do
+    upload = %Pleroma.Upload{
+      name: "sample.webp",
+      content_type: "image/webp",
+      path: Path.absname("/dev/null"),
+      tempfile: Path.absname("/dev/null")
+    }
+
+    assert Filter.Exiftool.filter(upload) == {:ok, :noop}
+  end
 end

From 216c84a8f4d82649110ffaa2bc9d02b879805c5f Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@FreeBSD.org>
Date: Fri, 4 Sep 2020 17:56:05 -0500
Subject: [PATCH 4/6] Bypass the filter based on content-type as well in case a
 webp image is uploaded with the wrong file extension.

---
 lib/pleroma/upload/filter/exiftool.ex |  5 ++++-
 test/upload/filter/exiftool_test.exs  | 10 +++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/pleroma/upload/filter/exiftool.ex b/lib/pleroma/upload/filter/exiftool.ex
index 94d12c01b..b07a671ac 100644
--- a/lib/pleroma/upload/filter/exiftool.ex
+++ b/lib/pleroma/upload/filter/exiftool.ex
@@ -10,8 +10,11 @@ defmodule Pleroma.Upload.Filter.Exiftool do
   @behaviour Pleroma.Upload.Filter
 
   @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{name: file, tempfile: path, content_type: "image" <> _}) do
-    # webp is not compatible with exiftool at this time
     if Regex.match?(~r/\.(webp)$/i, file) do
       {:ok, :noop}
     else
diff --git a/test/upload/filter/exiftool_test.exs b/test/upload/filter/exiftool_test.exs
index fe24036d9..84a3f8b30 100644
--- a/test/upload/filter/exiftool_test.exs
+++ b/test/upload/filter/exiftool_test.exs
@@ -34,11 +34,15 @@ test "apply exiftool filter" do
   test "verify webp files are skipped" do
     upload = %Pleroma.Upload{
       name: "sample.webp",
-      content_type: "image/webp",
-      path: Path.absname("/dev/null"),
-      tempfile: Path.absname("/dev/null")
+      content_type: "image/webp"
+    }
+
+    bad_type = %Pleroma.Upload{
+      name: "sample.webp",
+      content_type: "image/jpeg"
     }
 
     assert Filter.Exiftool.filter(upload) == {:ok, :noop}
+    assert Filter.Exiftool.filter(bad_type) == {:ok, :noop}
   end
 end

From 4ea07f74e9416da8f97a12cfdc24da82e1c00d91 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@FreeBSD.org>
Date: Fri, 4 Sep 2020 22:18:01 -0500
Subject: [PATCH 5/6] Revert/simplify.

We only need to check the content-type. There's no chance a webp file
will get mismatched as another image type.
---
 lib/pleroma/upload/filter/exiftool.ex | 16 ++++------------
 test/upload/filter/exiftool_test.exs  |  6 ------
 2 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/lib/pleroma/upload/filter/exiftool.ex b/lib/pleroma/upload/filter/exiftool.ex
index b07a671ac..1fd0cfdaa 100644
--- a/lib/pleroma/upload/filter/exiftool.ex
+++ b/lib/pleroma/upload/filter/exiftool.ex
@@ -14,19 +14,9 @@ defmodule Pleroma.Upload.Filter.Exiftool do
   # webp is not compatible with exiftool at this time
   def filter(%Pleroma.Upload{content_type: "image/webp"}), do: {:ok, :noop}
 
-  def filter(%Pleroma.Upload{name: file, tempfile: path, content_type: "image" <> _}) do
-    if Regex.match?(~r/\.(webp)$/i, file) do
-      {:ok, :noop}
-    else
-      strip_exif(path)
-    end
-  end
-
-  def filter(_), do: {:ok, :noop}
-
-  defp strip_exif(path) do
+  def filter(%Pleroma.Upload{tempfile: file, content_type: "image" <> _}) do
     try do
-      case System.cmd("exiftool", ["-overwrite_original", "-gps:all=", path], parallelism: true) do
+      case System.cmd("exiftool", ["-overwrite_original", "-gps:all=", file], parallelism: true) do
         {_response, 0} -> {:ok, :filtered}
         {error, 1} -> {:error, error}
       end
@@ -35,4 +25,6 @@ defp strip_exif(path) do
         {:error, "exiftool command not found"}
     end
   end
+
+  def filter(_), do: {:ok, :noop}
 end
diff --git a/test/upload/filter/exiftool_test.exs b/test/upload/filter/exiftool_test.exs
index 84a3f8b30..d4cd4ba11 100644
--- a/test/upload/filter/exiftool_test.exs
+++ b/test/upload/filter/exiftool_test.exs
@@ -37,12 +37,6 @@ test "verify webp files are skipped" do
       content_type: "image/webp"
     }
 
-    bad_type = %Pleroma.Upload{
-      name: "sample.webp",
-      content_type: "image/jpeg"
-    }
-
     assert Filter.Exiftool.filter(upload) == {:ok, :noop}
-    assert Filter.Exiftool.filter(bad_type) == {:ok, :noop}
   end
 end

From 0f27211dd0b9177abf0eb8b10665c6351f07756a Mon Sep 17 00:00:00 2001
From: rinpatch <rinpatch@sdf.org>
Date: Tue, 8 Sep 2020 12:28:38 +0300
Subject: [PATCH 6/6] CHANGELOG.md: move the exiftool webp entry to a proper
 section

Also clarify how it was fixed
---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cfe0651c8..527df8438 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Fixed
 
 - Possible OOM errors with the default HTTP adapter
+- 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.
   Reduced to just rich media timeout.
@@ -138,7 +139,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Fix whole_word always returning false on filter get requests
 - Migrations not working on OTP releases if the database was connected over ssl
 - Fix relay following
-- Fixed uploading webp images when the Exiftool Upload Filter is enabled
 
 ## [2.0.7] - 2020-06-13