From 338612d72bbb99792497c829c77d6654a6734f10 Mon Sep 17 00:00:00 2001 From: Ilja Date: Mon, 14 Feb 2022 13:14:25 +0100 Subject: [PATCH 1/8] Use EXIF data of image to prefill image description During attachment upload Pleroma returns a "description" field. * This MR allows Pleroma to read the EXIF data during upload and return the description to the FE using this field. * If a description is already present (e.g. because a previous module added it), it will use that * Otherwise it will read from the EXIF data. First it will check -ImageDescription, if that's empty, it will check -iptc:Caption-Abstract * If no description is found, it will simply return nil, which is the default value * When people set up a new instance, they will be asked if they want to read metadata and this module will be activated if so There was an Exiftool module, which has now been renamed to Exiftool.StripLocation --- CHANGELOG.md | 2 + .../docs/administration/CLI_tasks/instance.md | 3 +- docs/docs/configuration/cheatsheet.md | 8 +- .../optional/media_graphics_packages.md | 15 +-- lib/mix/tasks/pleroma/instance.ex | 44 ++++++-- lib/pleroma/application_requirements.ex | 3 +- lib/pleroma/config/deprecation_warnings.ex | 40 ++++++- lib/pleroma/upload.ex | 25 +++-- .../filter/exiftool/read_description.ex | 47 +++++++++ .../strip_location.ex} | 2 +- ...er_exiftool_to_exiftool_strip_location.exs | 37 +++++++ test/fixtures/image_with_caption-abstract.jpg | Bin 0 -> 697 bytes ..._imagedescription_and_caption-abstract.jpg | Bin 0 -> 823 bytes test/fixtures/image_with_no_description.jpg | Bin 0 -> 631 bytes test/mix/tasks/pleroma/instance_test.exs | 9 +- .../config/deprecation_warnings_test.exs | 56 ++++++++++ .../filter/exiftool/read_description_test.exs | 98 ++++++++++++++++++ .../strip_location_test.exs} | 6 +- 18 files changed, 363 insertions(+), 32 deletions(-) create mode 100644 lib/pleroma/upload/filter/exiftool/read_description.ex rename lib/pleroma/upload/filter/{exiftool.ex => exiftool/strip_location.ex} (94%) create mode 100644 priv/repo/migrations/20220220135625_upload_filter_exiftool_to_exiftool_strip_location.exs create mode 100644 test/fixtures/image_with_caption-abstract.jpg create mode 100644 test/fixtures/image_with_imagedescription_and_caption-abstract.jpg create mode 100644 test/fixtures/image_with_no_description.jpg create mode 100644 test/pleroma/upload/filter/exiftool/read_description_test.exs rename test/pleroma/upload/filter/{exiftool_test.exs => exiftool/strip_location_test.exs} (84%) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e4f371f..65980c28c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Added - Officially supported docker release - Ability to remove followers unilaterally without a block +- Uploadfilter `Pleroma.Upload.Filter.Exiftool.ReadDescription` returns description values to the FE so they can pre fill the image description field ## Changes - Follows no longer override domain blocks, a domain block is final - Deletes are now the lowest priority to publish and will be handled after creates +- Uploadfilter `Pleroma.Upload.Filter.Exiftool` has been renamed to `Pleroma.Upload.Filter.Exiftool.StripLocation` ## 2022.10 diff --git a/docs/docs/administration/CLI_tasks/instance.md b/docs/docs/administration/CLI_tasks/instance.md index 5c93cdb30..10872e5b3 100644 --- a/docs/docs/administration/CLI_tasks/instance.md +++ b/docs/docs/administration/CLI_tasks/instance.md @@ -37,7 +37,8 @@ If any of the options are left unspecified, you will be prompted interactively. - `--static-dir ` - the directory custom public files should be read from (custom emojis, frontend bundle overrides, robots.txt, etc.) - `--listen-ip ` - the ip the app should listen to, defaults to 127.0.0.1 - `--listen-port ` - the port the app should listen to, defaults to 4000 -- `--strip-uploads ` - use ExifTool to strip uploads of sensitive location data +- `--strip-uploads-location ` - use ExifTool to strip uploads of sensitive location data +- `--read-uploads-description ` - use ExifTool to read image descriptions from uploads - `--anonymize-uploads ` - randomize uploaded filenames - `--dedupe-uploads ` - store files based on their hash to reduce data storage requirements if duplicates are uploaded with different filenames - `--skip-release-env` - skip generation the release environment file diff --git a/docs/docs/configuration/cheatsheet.md b/docs/docs/configuration/cheatsheet.md index ec8bea0cc..8c402049b 100644 --- a/docs/docs/configuration/cheatsheet.md +++ b/docs/docs/configuration/cheatsheet.md @@ -657,12 +657,18 @@ This filter replaces the filename (not the path) of an upload. For complete obfu No specific configuration. -#### Pleroma.Upload.Filter.Exiftool +#### Pleroma.Upload.Filter.Exiftool.StripLocation This filter only strips the GPS and location metadata with Exiftool leaving color profiles and attributes intact. No specific configuration. +#### Pleroma.Upload.Filter.Exiftool.ReadDescription + +This filter reads the ImageDescription and iptc:Caption-Abstract fields with Exiftool so clients can prefill the media description field. + +No specific configuration. + #### Pleroma.Upload.Filter.Mogrify * `args`: List of actions for the `mogrify` command like `"strip"` or `["strip", "auto-orient", {"implode", "1"}]`. diff --git a/docs/docs/installation/optional/media_graphics_packages.md b/docs/docs/installation/optional/media_graphics_packages.md index cb3d71188..de402d1c4 100644 --- a/docs/docs/installation/optional/media_graphics_packages.md +++ b/docs/docs/installation/optional/media_graphics_packages.md @@ -1,9 +1,9 @@ # Optional software packages needed for specific functionality For specific Pleroma functionality (which is disabled by default) some or all of the below packages are required: - * `ImageMagic` - * `ffmpeg` - * `exiftool` + * `ImageMagic` + * `ffmpeg` + * `exiftool` Please refer to documentation in `docs/installation` on how to install them on specific OS. @@ -14,19 +14,20 @@ Note: the packages are not required with the current default settings of Pleroma `ImageMagick` is a set of tools to create, edit, compose, or convert bitmap images. It is required for the following Pleroma features: - * `Pleroma.Upload.Filters.Mogrify`, `Pleroma.Upload.Filters.Mogrifun` upload filters (related config: `Plaroma.Upload/filters` in `config/config.exs`) - * Media preview proxy for still images (related config: `media_preview_proxy/enabled` in `config/config.exs`) + * `Pleroma.Upload.Filters.Mogrify`, `Pleroma.Upload.Filters.Mogrifun` upload filters (related config: `Plaroma.Upload/filters` in `config/config.exs`) + * Media preview proxy for still images (related config: `media_preview_proxy/enabled` in `config/config.exs`) ## `ffmpeg` `ffmpeg` is software to record, convert and stream audio and video. It is required for the following Pleroma features: - * Media preview proxy for videos (related config: `media_preview_proxy/enabled` in `config/config.exs`) + * Media preview proxy for videos (related config: `media_preview_proxy/enabled` in `config/config.exs`) ## `exiftool` `exiftool` is media files metadata reader/writer. It is required for the following Pleroma features: - * `Pleroma.Upload.Filters.Exiftool` upload filter (related config: `Plaroma.Upload/filters` in `config/config.exs`) + * `Pleroma.Upload.Filters.Exiftool.StripLocation` upload filter (related config: `Plaroma.Upload/filters` in `config/config.exs`) + * `Pleroma.Upload.Filters.Exiftool.ReadDescription` upload filter (related config: `Plaroma.Upload/filters` in `config/config.exs`) diff --git a/lib/mix/tasks/pleroma/instance.ex b/lib/mix/tasks/pleroma/instance.ex index 8954b3b7c..c5354ac43 100644 --- a/lib/mix/tasks/pleroma/instance.ex +++ b/lib/mix/tasks/pleroma/instance.ex @@ -34,7 +34,8 @@ defmodule Mix.Tasks.Pleroma.Instance do static_dir: :string, listen_ip: :string, listen_port: :string, - strip_uploads: :string, + strip_uploads_location: :string, + read_uploads_description: :string, anonymize_uploads: :string, dedupe_uploads: :string ], @@ -161,7 +162,7 @@ defmodule Mix.Tasks.Pleroma.Instance do ) |> Path.expand() - {strip_uploads_message, strip_uploads_default} = + {strip_uploads_location_message, strip_uploads_location_default} = if Pleroma.Utils.command_available?("exiftool") do {"Do you want to strip location (GPS) data from uploaded images? This requires exiftool, it was detected as installed. (y/n)", "y"} @@ -170,12 +171,29 @@ defmodule Mix.Tasks.Pleroma.Instance do "n"} end - strip_uploads = + strip_uploads_location = get_option( options, - :strip_uploads, - strip_uploads_message, - strip_uploads_default + :strip_uploads_location, + strip_uploads_location_message, + strip_uploads_location_default + ) === "y" + + {read_uploads_description_message, read_uploads_description_default} = + if Pleroma.Utils.command_available?("exiftool") do + {"Do you want to read data from uploaded files so clients can use it to prefill fields like image description? This requires exiftool, it was detected as installed. (y/n)", + "y"} + else + {"Do you want to read data from uploaded files so clients can use it to prefill fields like image description? This requires exiftool, it was detected as not installed, please install it if you answer yes. (y/n)", + "n"} + end + + read_uploads_description = + get_option( + options, + :read_uploads_description, + read_uploads_description_message, + read_uploads_description_default ) === "y" anonymize_uploads = @@ -229,7 +247,8 @@ defmodule Mix.Tasks.Pleroma.Instance do listen_port: listen_port, upload_filters: upload_filters(%{ - strip: strip_uploads, + strip_location: strip_uploads_location, + read_description: read_uploads_description, anonymize: anonymize_uploads, dedupe: dedupe_uploads }) @@ -297,12 +316,19 @@ defmodule Mix.Tasks.Pleroma.Instance do defp upload_filters(filters) when is_map(filters) do enabled_filters = - if filters.strip do - [Pleroma.Upload.Filter.Exiftool] + if filters.strip_location do + [Pleroma.Upload.Filter.Exiftool.StripLocation] else [] end + enabled_filters = + if filters.read_description do + enabled_filters ++ [Pleroma.Upload.Filter.Exiftool.ReadDescription] + else + enabled_filters + end + enabled_filters = if filters.anonymize do enabled_filters ++ [Pleroma.Upload.Filter.AnonymizeFilename] diff --git a/lib/pleroma/application_requirements.ex b/lib/pleroma/application_requirements.ex index a56311a65..141f7888a 100644 --- a/lib/pleroma/application_requirements.ex +++ b/lib/pleroma/application_requirements.ex @@ -164,7 +164,8 @@ defmodule Pleroma.ApplicationRequirements do defp check_system_commands!(:ok) do filter_commands_statuses = [ - check_filter(Pleroma.Upload.Filter.Exiftool, "exiftool"), + check_filter(Pleroma.Upload.Filter.Exiftool.StripLocation, "exiftool"), + check_filter(Pleroma.Upload.Filter.Exiftool.ReadDescription, "exiftool"), check_filter(Pleroma.Upload.Filter.Mogrify, "mogrify"), check_filter(Pleroma.Upload.Filter.Mogrifun, "mogrify"), check_filter(Pleroma.Upload.Filter.AnalyzeMetadata, "mogrify"), diff --git a/lib/pleroma/config/deprecation_warnings.ex b/lib/pleroma/config/deprecation_warnings.ex index 8a336c35a..317106bde 100644 --- a/lib/pleroma/config/deprecation_warnings.ex +++ b/lib/pleroma/config/deprecation_warnings.ex @@ -22,6 +22,43 @@ defmodule Pleroma.Config.DeprecationWarnings do "\n* `config :pleroma, :instance, :quarantined_instances` is now covered by `:pleroma, :mrf_simple, :reject`"} ] + def check_exiftool_filter do + filters = Config.get([Pleroma.Upload]) |> Keyword.get(:filters, []) + + if Pleroma.Upload.Filter.Exiftool in filters do + Logger.warn(""" + !!!DEPRECATION WARNING!!! + Your config is using Exiftool as a filter instead of Exiftool.StripLocation. This should work for now, but you are advised to change to the new configuration to prevent possible issues later: + + ``` + config :pleroma, Pleroma.Upload, + filters: [Pleroma.Upload.Filter.Exiftool] + ``` + + Is now + + + ``` + config :pleroma, Pleroma.Upload, + filters: [Pleroma.Upload.Filter.Exiftool.StripLocation] + ``` + """) + + new_config = + filters + |> Enum.map(fn + Pleroma.Upload.Filter.Exiftool -> Pleroma.Upload.Filter.Exiftool.StripLocation + filter -> filter + end) + + Config.put([Pleroma.Upload, :filters], new_config) + + :error + else + :ok + end + end + def check_simple_policy_tuples do has_strings = Config.get([:mrf_simple]) @@ -180,7 +217,8 @@ defmodule Pleroma.Config.DeprecationWarnings do check_uploders_s3_public_endpoint(), check_quarantined_instances_tuples(), check_transparency_exclusions_tuples(), - check_simple_policy_tuples() + check_simple_policy_tuples(), + check_exiftool_filter() ] |> Enum.reduce(:ok, fn :ok, :ok -> :ok diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index 9bf8e03df..7ea7f0e2c 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -61,12 +61,23 @@ defmodule Pleroma.Upload do width: integer(), height: integer(), blurhash: String.t(), + description: String.t(), path: String.t() } - defstruct [:id, :name, :tempfile, :content_type, :width, :height, :blurhash, :path] + defstruct [ + :id, + :name, + :tempfile, + :content_type, + :width, + :height, + :blurhash, + :description, + :path + ] - defp get_description(opts, upload) do - case {opts[:description], Pleroma.Config.get([Pleroma.Upload, :default_description])} do + defp get_description(upload) do + case {upload.description, Pleroma.Config.get([Pleroma.Upload, :default_description])} do {description, _} when is_binary(description) -> description {_, :filename} -> upload.name {_, str} when is_binary(str) -> str @@ -82,7 +93,7 @@ defmodule Pleroma.Upload do with {:ok, upload} <- prepare_upload(upload, opts), upload = %__MODULE__{upload | path: upload.path || "#{upload.id}/#{upload.name}"}, {:ok, upload} <- Pleroma.Upload.Filter.filter(opts.filters, upload), - description = get_description(opts, upload), + description = get_description(upload), {_, true} <- {:description_limit, String.length(description) <= Pleroma.Config.get([:instance, :description_limit])}, @@ -154,7 +165,8 @@ defmodule Pleroma.Upload do id: UUID.generate(), name: file.filename, tempfile: file.path, - content_type: file.content_type + content_type: file.content_type, + description: opts.description }} end end @@ -174,7 +186,8 @@ defmodule Pleroma.Upload do id: UUID.generate(), name: hash <> "." <> ext, tempfile: tmp_path, - content_type: content_type + content_type: content_type, + description: opts.description }} end end diff --git a/lib/pleroma/upload/filter/exiftool/read_description.ex b/lib/pleroma/upload/filter/exiftool/read_description.ex new file mode 100644 index 000000000..3f7b7c798 --- /dev/null +++ b/lib/pleroma/upload/filter/exiftool/read_description.ex @@ -0,0 +1,47 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2021 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Upload.Filter.Exiftool.ReadDescription do + @moduledoc """ + Gets the description from the related EXIF tags and provides them in the response if no description is provided yet. + It will first check ImageDescription, when that's too long or empty, it will check iptc:Caption-Abstract. + When the description is too long (see `:instance, :description_limit`), an empty string is returned. + """ + @behaviour Pleroma.Upload.Filter + + @spec filter(Pleroma.Upload.t()) :: {:ok, any()} | {:error, String.t()} + + def filter(%Pleroma.Upload{description: description}) + when is_binary(description), + do: {:ok, :noop} + + def filter(%Pleroma.Upload{tempfile: file} = upload), + do: {:ok, :filtered, upload |> Map.put(:description, read_description_from_exif_data(file))} + + def filter(_, _), do: {:ok, :noop} + + defp read_description_from_exif_data(file) do + nil + |> read_when_empty(file, "-ImageDescription") + |> read_when_empty(file, "-iptc:Caption-Abstract") + end + + defp read_when_empty(current_description, _, _) when is_binary(current_description), + do: current_description + + defp read_when_empty(_, file, tag) do + try do + {tag_content, 0} = + System.cmd("exiftool", ["-b", "-s3", tag, file], stderr_to_stdout: true, parallelism: true) + + if tag_content != "" and + String.length(tag_content) <= + Pleroma.Config.get([:instance, :description_limit]), + do: tag_content, + else: nil + rescue + _ in ErlangError -> nil + end + end +end diff --git a/lib/pleroma/upload/filter/exiftool.ex b/lib/pleroma/upload/filter/exiftool/strip_location.ex similarity index 94% rename from lib/pleroma/upload/filter/exiftool.ex rename to lib/pleroma/upload/filter/exiftool/strip_location.ex index a2bfbbf61..0a9ec4fb2 100644 --- a/lib/pleroma/upload/filter/exiftool.ex +++ b/lib/pleroma/upload/filter/exiftool/strip_location.ex @@ -2,7 +2,7 @@ # Copyright © 2017-2021 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only -defmodule Pleroma.Upload.Filter.Exiftool do +defmodule Pleroma.Upload.Filter.Exiftool.StripLocation do @moduledoc """ Strips GPS related EXIF tags and overwrites the file in place. Also strips or replaces filesystem metadata e.g., timestamps. diff --git a/priv/repo/migrations/20220220135625_upload_filter_exiftool_to_exiftool_strip_location.exs b/priv/repo/migrations/20220220135625_upload_filter_exiftool_to_exiftool_strip_location.exs new file mode 100644 index 000000000..0878b9699 --- /dev/null +++ b/priv/repo/migrations/20220220135625_upload_filter_exiftool_to_exiftool_strip_location.exs @@ -0,0 +1,37 @@ +defmodule Pleroma.Repo.Migrations.UploadFilterExiftoolToExiftoolStripLocation do + use Ecto.Migration + + alias Pleroma.ConfigDB + + def up, + do: + ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Upload}) + |> update_filtername( + Pleroma.Upload.Filter.Exiftool, + Pleroma.Upload.Filter.Exiftool.StripLocation + ) + + def down, + do: + ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Upload}) + |> update_filtername( + Pleroma.Upload.Filter.Exiftool.StripLocation, + Pleroma.Upload.Filter.Exiftool + ) + + defp update_filtername(%{value: value}, from_filtername, to_filtername) do + new_value = + value + |> Keyword.update(:filters, [], fn filters -> + filters + |> Enum.map(fn + ^from_filtername -> to_filtername + filter -> filter + end) + end) + + ConfigDB.update_or_create(%{group: :pleroma, key: Pleroma.Upload, value: new_value}) + end + + defp update_filtername(_, _, _), do: nil +end diff --git a/test/fixtures/image_with_caption-abstract.jpg b/test/fixtures/image_with_caption-abstract.jpg new file mode 100644 index 0000000000000000000000000000000000000000..f982ffa819f9d18091d50bc22bce9fb780d4ed02 GIT binary patch literal 697 zcmex=DX82;a8aAx3OV`FDyNsy6Qkn#T!26+YsMpmGIL0*Oe zMkZz!RyKAHPA+bsf~^7!OpMITOf1Z-tRRmw)&k`jSOi&x6b&8OgaZ@Vl?p|S8YeE~ zPwh=DOELf4NWZ*Q!{f5ODks=S2uSLPp{yR(6I1`$f)F$ z)U@=B%&g*)(z5c3%Btp;*0%PJ&aO$5r%atTea6gLixw|gx@`H1m8&*w-m-Pu_8mKS z9XfpE=&|D`PM*4S`O4L6*Kgds_3+W-Cr_U}fAR9w$4{TXeEs(Q$Io9Ne=#yJL%ap| z8JfQYf&OA*VPR%r2lxYE#}NLy#lXYN2#h>tK?Zw<@4qw_HQ4{Z2>@SH;I{w( literal 0 HcmV?d00001 diff --git a/test/fixtures/image_with_imagedescription_and_caption-abstract.jpg b/test/fixtures/image_with_imagedescription_and_caption-abstract.jpg new file mode 100644 index 0000000000000000000000000000000000000000..c82a269ef2639b6a6dad4f65366cba8c85a78681 GIT binary patch literal 823 zcmex=p;^d;tf|AU#RE6@4%#u`vg3OB4 z9Izyi0I37o&A|BoErUZqMt(_taYlZDg0Y?fgN2i)FAEDuA%n6EQw4)~VxB@`QgKO9 zVsZ(xQ8FNV8CV$p-)3-T;9z58XJh4HXJ_Z+uA0}~@NGZPCl zD=Wz3jI}^{1{Oh9Aw@$+HsQcTcBMiQqsEB~Ih36?9uy6__(8=usi=vQOH5osQc6`# zT|-mL#MI2(!qUpw#nsK-!_zA`Bs45MA~GsDB{eNQBQvYGq_nKOqOz*FrM0cSqqA$$ zM%n)zEeTL>QL7=~wSXh`@*g^hcWGV+@WL#^(PF6}rMnOeST|r4lSw=>~TvNxu(8R<ECr+Na zbot8FYu9hwy!G(W<0ns_J%91?)yGetzkL1n{m0K=Ab&A3Fhjfr_ZgbM1cClyVqsxs zVF&q(k*OSrnFU!`6%E;h90S=C3x$=88aYIqCNA7~kW<+>=!0ld(M2vX6_bamA3L7B$%azX<@ioX "setup.psql") == generated_setup_psql() assert File.exists?(Path.expand("./test/instance/static/robots.txt")) end diff --git a/test/pleroma/config/deprecation_warnings_test.exs b/test/pleroma/config/deprecation_warnings_test.exs index 053e28207..e6b06c979 100644 --- a/test/pleroma/config/deprecation_warnings_test.exs +++ b/test/pleroma/config/deprecation_warnings_test.exs @@ -11,6 +11,62 @@ defmodule Pleroma.Config.DeprecationWarningsTest do alias Pleroma.Config alias Pleroma.Config.DeprecationWarnings + describe "filter exiftool" do + test "gives warning when still used" do + clear_config( + [Pleroma.Upload, :filters], + [Pleroma.Upload.Filter.Exiftool] + ) + + assert capture_log(fn -> DeprecationWarnings.check_exiftool_filter() end) =~ + """ + !!!DEPRECATION WARNING!!! + Your config is using Exiftool as a filter instead of Exiftool.StripLocation. This should work for now, but you are advised to change to the new configuration to prevent possible issues later: + + ``` + config :pleroma, Pleroma.Upload, + filters: [Pleroma.Upload.Filter.Exiftool] + ``` + + Is now + + + ``` + config :pleroma, Pleroma.Upload, + filters: [Pleroma.Upload.Filter.Exiftool.StripLocation] + ``` + """ + end + + test "changes setting to exiftool strip location" do + clear_config( + [Pleroma.Upload, :filters], + [Pleroma.Upload.Filter.Exiftool, Pleroma.Upload.Filter.Exiftool.ReadDescription] + ) + + expected_config = [ + Pleroma.Upload.Filter.Exiftool.StripLocation, + Pleroma.Upload.Filter.Exiftool.ReadDescription + ] + + capture_log(fn -> DeprecationWarnings.warn() end) + + assert Config.get([Pleroma.Upload]) |> Keyword.get(:filters, []) == expected_config + end + + test "doesn't give a warning with correct config" do + clear_config( + [Pleroma.Upload, :filters], + [ + Pleroma.Upload.Filter.Exiftool.StripLocation, + Pleroma.Upload.Filter.Exiftool.ReadDescription + ] + ) + + assert capture_log(fn -> DeprecationWarnings.check_exiftool_filter() end) == "" + end + end + describe "simple policy tuples" do test "gives warning when there are still strings" do clear_config([:mrf_simple], diff --git a/test/pleroma/upload/filter/exiftool/read_description_test.exs b/test/pleroma/upload/filter/exiftool/read_description_test.exs new file mode 100644 index 000000000..0b358215c --- /dev/null +++ b/test/pleroma/upload/filter/exiftool/read_description_test.exs @@ -0,0 +1,98 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2021 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Upload.Filter.Exiftool.ReadDescriptionTest do + use Pleroma.DataCase, async: true + alias Pleroma.Upload.Filter + + @uploads %Pleroma.Upload{ + name: "image_with_imagedescription_and_caption-abstract.jpg", + content_type: "image/jpeg", + path: Path.absname("test/fixtures/image_with_imagedescription_and_caption-abstract.jpg"), + tempfile: Path.absname("test/fixtures/image_with_imagedescription_and_caption-abstract.jpg"), + description: nil + } + + test "keeps description when not empty" do + uploads = %Pleroma.Upload{ + name: "image_with_imagedescription_and_caption-abstract.jpg", + content_type: "image/jpeg", + path: Path.absname("test/fixtures/image_with_imagedescription_and_caption-abstract.jpg"), + tempfile: + Path.absname("test/fixtures/image_with_imagedescription_and_caption-abstract.jpg"), + description: "Some description" + } + + assert Filter.Exiftool.ReadDescription.filter(uploads) == + {:ok, :noop} + end + + test "otherwise returns ImageDescription when present" do + uploads_after = %Pleroma.Upload{ + name: "image_with_imagedescription_and_caption-abstract.jpg", + content_type: "image/jpeg", + path: Path.absname("test/fixtures/image_with_imagedescription_and_caption-abstract.jpg"), + tempfile: + Path.absname("test/fixtures/image_with_imagedescription_and_caption-abstract.jpg"), + description: "a descriptive white pixel" + } + + assert Filter.Exiftool.ReadDescription.filter(@uploads) == + {:ok, :filtered, uploads_after} + end + + test "otherwise returns iptc:Caption-Abstract when present" do + upload = %Pleroma.Upload{ + name: "image_with_caption-abstract.jpg", + content_type: "image/jpeg", + path: Path.absname("test/fixtures/image_with_caption-abstract.jpg"), + tempfile: Path.absname("test/fixtures/image_with_caption-abstract.jpg"), + description: nil + } + + upload_after = %Pleroma.Upload{ + name: "image_with_caption-abstract.jpg", + content_type: "image/jpeg", + path: Path.absname("test/fixtures/image_with_caption-abstract.jpg"), + tempfile: Path.absname("test/fixtures/image_with_caption-abstract.jpg"), + description: "an abstract white pixel" + } + + assert Filter.Exiftool.ReadDescription.filter(upload) == + {:ok, :filtered, upload_after} + end + + test "otherwise returns nil" do + uploads = %Pleroma.Upload{ + name: "image_with_no_description.jpg", + content_type: "image/jpeg", + path: Path.absname("test/fixtures/image_with_no_description.jpg"), + tempfile: Path.absname("test/fixtures/image_with_no_description.jpg"), + description: nil + } + + assert Filter.Exiftool.ReadDescription.filter(uploads) == + {:ok, :filtered, uploads} + end + + test "Return nil when image description from EXIF data exceeds the maximum length" do + clear_config([:instance, :description_limit], 5) + + assert Filter.Exiftool.ReadDescription.filter(@uploads) == + {:ok, :filtered, @uploads} + end + + test "Return nil when image description from EXIF data can't be read" do + uploads = %Pleroma.Upload{ + name: "non-existant.jpg", + content_type: "image/jpeg", + path: Path.absname("test/fixtures/non-existant.jpg"), + tempfile: Path.absname("test/fixtures/non-existant_tmp.jpg"), + description: nil + } + + assert Filter.Exiftool.ReadDescription.filter(uploads) == + {:ok, :filtered, uploads} + end +end diff --git a/test/pleroma/upload/filter/exiftool_test.exs b/test/pleroma/upload/filter/exiftool/strip_location_test.exs similarity index 84% rename from test/pleroma/upload/filter/exiftool_test.exs rename to test/pleroma/upload/filter/exiftool/strip_location_test.exs index cfbe34be8..fdd0c07e7 100644 --- a/test/pleroma/upload/filter/exiftool_test.exs +++ b/test/pleroma/upload/filter/exiftool/strip_location_test.exs @@ -2,7 +2,7 @@ # Copyright © 2017-2021 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only -defmodule Pleroma.Upload.Filter.ExiftoolTest do +defmodule Pleroma.Upload.Filter.Exiftool.StripLocationTest do use Pleroma.DataCase, async: true alias Pleroma.Upload.Filter @@ -21,7 +21,7 @@ defmodule Pleroma.Upload.Filter.ExiftoolTest do tempfile: Path.absname("test/fixtures/DSCN0010_tmp.jpg") } - assert Filter.Exiftool.filter(upload) == {:ok, :filtered} + assert Filter.Exiftool.StripLocation.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"]) @@ -37,6 +37,6 @@ defmodule Pleroma.Upload.Filter.ExiftoolTest do content_type: "image/webp" } - assert Filter.Exiftool.filter(upload) == {:ok, :noop} + assert Filter.Exiftool.StripLocation.filter(upload) == {:ok, :noop} end end From f50cffd134625cfdda82aab83dc7aa7cd6d82a9d Mon Sep 17 00:00:00 2001 From: Ilja Date: Sat, 21 May 2022 11:31:14 +0200 Subject: [PATCH 2/8] update moduledoc --- lib/pleroma/upload/filter/exiftool/read_description.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/upload/filter/exiftool/read_description.ex b/lib/pleroma/upload/filter/exiftool/read_description.ex index 3f7b7c798..a69f79124 100644 --- a/lib/pleroma/upload/filter/exiftool/read_description.ex +++ b/lib/pleroma/upload/filter/exiftool/read_description.ex @@ -4,9 +4,9 @@ defmodule Pleroma.Upload.Filter.Exiftool.ReadDescription do @moduledoc """ - Gets the description from the related EXIF tags and provides them in the response if no description is provided yet. - It will first check ImageDescription, when that's too long or empty, it will check iptc:Caption-Abstract. - When the description is too long (see `:instance, :description_limit`), an empty string is returned. + Gets a valid description from the related EXIF tags and provides them in the response if no description is provided yet. + It will first check ImageDescription, when that doesn't probide a valid description, it will check iptc:Caption-Abstract. + A valid description means the fields are filled in and not too long (see `:instance, :description_limit`). """ @behaviour Pleroma.Upload.Filter From 66a04cead34b1772fff93ff359471ff3e4ac93dc Mon Sep 17 00:00:00 2001 From: Ilja Date: Fri, 1 Jul 2022 12:31:34 +0200 Subject: [PATCH 3/8] Descriptions from exif data with only whitespeces are considered empty I noticed that pictures taken with Ubuntu-Touch have whitespace in one of the fields This should just be ignored imo --- .../filter/exiftool/read_description.ex | 2 ++ ...ption_and_caption-abstract_whitespaces.jpg | Bin 0 -> 785 bytes .../filter/exiftool/read_description_test.exs | 19 ++++++++++++++++++ 3 files changed, 21 insertions(+) create mode 100644 test/fixtures/image_with_imagedescription_and_caption-abstract_whitespaces.jpg diff --git a/lib/pleroma/upload/filter/exiftool/read_description.ex b/lib/pleroma/upload/filter/exiftool/read_description.ex index a69f79124..03d698a81 100644 --- a/lib/pleroma/upload/filter/exiftool/read_description.ex +++ b/lib/pleroma/upload/filter/exiftool/read_description.ex @@ -35,6 +35,8 @@ defmodule Pleroma.Upload.Filter.Exiftool.ReadDescription do {tag_content, 0} = System.cmd("exiftool", ["-b", "-s3", tag, file], stderr_to_stdout: true, parallelism: true) + tag_content = String.trim(tag_content) + if tag_content != "" and String.length(tag_content) <= Pleroma.Config.get([:instance, :description_limit]), diff --git a/test/fixtures/image_with_imagedescription_and_caption-abstract_whitespaces.jpg b/test/fixtures/image_with_imagedescription_and_caption-abstract_whitespaces.jpg new file mode 100644 index 0000000000000000000000000000000000000000..a232fd2a1a467b86a13a4e63e07cf7e59efe4709 GIT binary patch literal 785 zcmex=`H|qMvW5}awt1(JSZA; z@q>zSQc)8pmzcPOq?D?fx`w8fiK&^ng{76Vi>sTvho@I?NN8AiL}XNQN@`kqMrKxV zNoiSmMP*fUOKV$uM`zch$y26In?7UatVN5LEM2yI#mZHiHgDOwZTpU$yAB;ba`f2o z6DLnyx_ss8wd*%--g@}x@sp>|p1*kc>f@)+U%r0({^RE_kiQrim?7SR`wY!rf#!}nhriyG|z-vj^vYv1_* literal 0 HcmV?d00001 diff --git a/test/pleroma/upload/filter/exiftool/read_description_test.exs b/test/pleroma/upload/filter/exiftool/read_description_test.exs index 0b358215c..7389fda47 100644 --- a/test/pleroma/upload/filter/exiftool/read_description_test.exs +++ b/test/pleroma/upload/filter/exiftool/read_description_test.exs @@ -83,6 +83,25 @@ defmodule Pleroma.Upload.Filter.Exiftool.ReadDescriptionTest do {:ok, :filtered, @uploads} end + test "Ignores content with only whitespace" do + uploads = %Pleroma.Upload{ + name: "non-existant.jpg", + content_type: "image/jpeg", + path: + Path.absname( + "test/fixtures/image_with_imagedescription_and_caption-abstract_whitespaces.jpg" + ), + tempfile: + Path.absname( + "test/fixtures/image_with_imagedescription_and_caption-abstract_whitespaces.jpg" + ), + description: nil + } + + assert Filter.Exiftool.ReadDescription.filter(uploads) == + {:ok, :filtered, uploads} + end + test "Return nil when image description from EXIF data can't be read" do uploads = %Pleroma.Upload{ name: "non-existant.jpg", From 59d32c10d985ea328e291e3c912fd4b352c25945 Mon Sep 17 00:00:00 2001 From: timorl Date: Tue, 16 Apr 2024 08:02:13 +0200 Subject: [PATCH 4/8] Formatting --- lib/pleroma/web/plugs/o_auth_scopes_plug.ex | 4 +- lib/pleroma/web/telemetry.ex | 69 +++++++++---------- test/mix/tasks/pleroma/instance_test.exs | 2 + .../attachment_validator_test.exs | 15 ++-- test/support/factory.ex | 19 +++-- 5 files changed, 53 insertions(+), 56 deletions(-) diff --git a/lib/pleroma/web/plugs/o_auth_scopes_plug.ex b/lib/pleroma/web/plugs/o_auth_scopes_plug.ex index e4d098a7d..f017c8bc7 100644 --- a/lib/pleroma/web/plugs/o_auth_scopes_plug.ex +++ b/lib/pleroma/web/plugs/o_auth_scopes_plug.ex @@ -34,9 +34,7 @@ defmodule Pleroma.Web.Plugs.OAuthScopesPlug do permissions = Enum.join(missing_scopes, " #{op} ") error_message = - dgettext("errors", "Insufficient permissions: %{permissions}.", - permissions: permissions - ) + dgettext("errors", "Insufficient permissions: %{permissions}.", permissions: permissions) conn |> put_resp_content_type("application/json") diff --git a/lib/pleroma/web/telemetry.ex b/lib/pleroma/web/telemetry.ex index 269f9f238..0ddb2c903 100644 --- a/lib/pleroma/web/telemetry.ex +++ b/lib/pleroma/web/telemetry.ex @@ -151,41 +151,40 @@ defmodule Pleroma.Web.Telemetry do # phoenix.router_dispatch.stop.duration # pleroma.repo.query.total_time # pleroma.repo.query.queue_time - dist_metrics = - [ - distribution("phoenix.endpoint.stop.duration.fdist", - event_name: [:phoenix, :endpoint, :stop], - measurement: :duration, - unit: {:native, :millisecond}, - reporter_options: [ - buckets: simple_buckets - ] - ), - distribution("pleroma.repo.query.decode_time.fdist", - event_name: [:pleroma, :repo, :query], - measurement: :decode_time, - unit: {:native, :millisecond}, - reporter_options: [ - buckets: simple_buckets_quick - ] - ), - distribution("pleroma.repo.query.query_time.fdist", - event_name: [:pleroma, :repo, :query], - measurement: :query_time, - unit: {:native, :millisecond}, - reporter_options: [ - buckets: simple_buckets - ] - ), - distribution("pleroma.repo.query.idle_time.fdist", - event_name: [:pleroma, :repo, :query], - measurement: :idle_time, - unit: {:native, :millisecond}, - reporter_options: [ - buckets: simple_buckets - ] - ) - ] + dist_metrics = [ + distribution("phoenix.endpoint.stop.duration.fdist", + event_name: [:phoenix, :endpoint, :stop], + measurement: :duration, + unit: {:native, :millisecond}, + reporter_options: [ + buckets: simple_buckets + ] + ), + distribution("pleroma.repo.query.decode_time.fdist", + event_name: [:pleroma, :repo, :query], + measurement: :decode_time, + unit: {:native, :millisecond}, + reporter_options: [ + buckets: simple_buckets_quick + ] + ), + distribution("pleroma.repo.query.query_time.fdist", + event_name: [:pleroma, :repo, :query], + measurement: :query_time, + unit: {:native, :millisecond}, + reporter_options: [ + buckets: simple_buckets + ] + ), + distribution("pleroma.repo.query.idle_time.fdist", + event_name: [:pleroma, :repo, :query], + measurement: :idle_time, + unit: {:native, :millisecond}, + reporter_options: [ + buckets: simple_buckets + ] + ) + ] vm_metrics = sum_counter_pair("vm.memory.total", diff --git a/test/mix/tasks/pleroma/instance_test.exs b/test/mix/tasks/pleroma/instance_test.exs index a2b73e0c8..d650bdf7c 100644 --- a/test/mix/tasks/pleroma/instance_test.exs +++ b/test/mix/tasks/pleroma/instance_test.exs @@ -93,8 +93,10 @@ defmodule Mix.Tasks.Pleroma.InstanceTest do assert generated_config =~ "password: \"dbpass\"" assert generated_config =~ "configurable_from_database: true" assert generated_config =~ "http: [ip: {127, 0, 0, 1}, port: 4000]" + assert generated_config =~ "filters: [Pleroma.Upload.Filter.Exiftool.StripLocation, Pleroma.Upload.Filter.Exiftool.ReadDescription]" + assert generated_config =~ "base_url: \"https://media.pleroma.social/media\"" assert File.read!(tmp_path() <> "setup.psql") == generated_setup_psql() assert File.exists?(Path.expand("./test/instance/static/robots.txt")) diff --git a/test/pleroma/web/activity_pub/object_validators/attachment_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/attachment_validator_test.exs index f8dec09d3..22a8d0899 100644 --- a/test/pleroma/web/activity_pub/object_validators/attachment_validator_test.exs +++ b/test/pleroma/web/activity_pub/object_validators/attachment_validator_test.exs @@ -12,14 +12,13 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AttachmentValidatorTest do describe "attachments" do test "works with apng" do - attachment = - %{ - "mediaType" => "image/apng", - "name" => "", - "type" => "Document", - "url" => - "https://media.misskeyusercontent.com/io/2859c26e-cd43-4550-848b-b6243bc3fe28.apng" - } + attachment = %{ + "mediaType" => "image/apng", + "name" => "", + "type" => "Document", + "url" => + "https://media.misskeyusercontent.com/io/2859c26e-cd43-4550-848b-b6243bc3fe28.apng" + } assert {:ok, attachment} = AttachmentValidator.cast_and_validate(attachment) diff --git a/test/support/factory.ex b/test/support/factory.ex index e21b8fc1e..2a73a4ae6 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -559,16 +559,15 @@ defmodule Pleroma.Factory do like_activity = attrs[:like_activity] || insert(:like_activity) attrs = Map.drop(attrs, [:like_activity]) - data = - %{ - "id" => Pleroma.Web.ActivityPub.Utils.generate_activity_id(), - "type" => "Undo", - "actor" => like_activity.data["actor"], - "to" => like_activity.data["to"], - "object" => like_activity.data["id"], - "published" => DateTime.utc_now() |> DateTime.to_iso8601(), - "context" => like_activity.data["context"] - } + data = %{ + "id" => Pleroma.Web.ActivityPub.Utils.generate_activity_id(), + "type" => "Undo", + "actor" => like_activity.data["actor"], + "to" => like_activity.data["to"], + "object" => like_activity.data["id"], + "published" => DateTime.utc_now() |> DateTime.to_iso8601(), + "context" => like_activity.data["context"] + } %Pleroma.Activity{ data: data, From cd7af81896d6946d425de6e9436aec23abe905a0 Mon Sep 17 00:00:00 2001 From: timorl Date: Tue, 16 Apr 2024 20:37:00 +0200 Subject: [PATCH 5/8] Rename StripLocation to StripMetadata for temporal-proofing reasons --- CHANGELOG.md | 2 +- docs/docs/administration/CLI_tasks/instance.md | 2 +- docs/docs/configuration/cheatsheet.md | 2 +- .../optional/media_graphics_packages.md | 2 +- lib/mix/tasks/pleroma/instance.ex | 18 +++++++++--------- lib/pleroma/application_requirements.ex | 2 +- lib/pleroma/config/deprecation_warnings.ex | 6 +++--- .../{strip_location.ex => strip_metadata.ex} | 2 +- ...ter_exiftool_to_exiftool_strip_location.exs | 6 +++--- test/mix/tasks/pleroma/instance_test.exs | 4 ++-- .../config/deprecation_warnings_test.exs | 10 +++++----- .../filter/exiftool/strip_location_test.exs | 6 +++--- 12 files changed, 31 insertions(+), 31 deletions(-) rename lib/pleroma/upload/filter/exiftool/{strip_location.ex => strip_metadata.ex} (95%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a226aa71..497c7152b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Uploadfilter `Pleroma.Upload.Filter.Exiftool.ReadDescription` returns description values to the FE so they can pre fill the image description field ## Changed -- Uploadfilter `Pleroma.Upload.Filter.Exiftool` has been renamed to `Pleroma.Upload.Filter.Exiftool.StripLocation` +- Uploadfilter `Pleroma.Upload.Filter.Exiftool` has been renamed to `Pleroma.Upload.Filter.Exiftool.StripMetadata` ## Fixed - Issue preventing fetching anything from IPv6-only instances diff --git a/docs/docs/administration/CLI_tasks/instance.md b/docs/docs/administration/CLI_tasks/instance.md index 10872e5b3..1a3f8153e 100644 --- a/docs/docs/administration/CLI_tasks/instance.md +++ b/docs/docs/administration/CLI_tasks/instance.md @@ -37,7 +37,7 @@ If any of the options are left unspecified, you will be prompted interactively. - `--static-dir ` - the directory custom public files should be read from (custom emojis, frontend bundle overrides, robots.txt, etc.) - `--listen-ip ` - the ip the app should listen to, defaults to 127.0.0.1 - `--listen-port ` - the port the app should listen to, defaults to 4000 -- `--strip-uploads-location ` - use ExifTool to strip uploads of sensitive location data +- `--strip-uploads-metadata ` - use ExifTool to strip uploads of sensitive metadata - `--read-uploads-description ` - use ExifTool to read image descriptions from uploads - `--anonymize-uploads ` - randomize uploaded filenames - `--dedupe-uploads ` - store files based on their hash to reduce data storage requirements if duplicates are uploaded with different filenames diff --git a/docs/docs/configuration/cheatsheet.md b/docs/docs/configuration/cheatsheet.md index 004f6cf70..d2b699a51 100644 --- a/docs/docs/configuration/cheatsheet.md +++ b/docs/docs/configuration/cheatsheet.md @@ -654,7 +654,7 @@ This filter replaces the declared filename (not the path) of an upload. * `text`: Text to replace filenames in links. If empty, `{random}.extension` will be used. You can get the original filename extension by using `{extension}`, for example `custom-file-name.{extension}`. -#### Pleroma.Upload.Filter.Exiftool.StripLocation +#### Pleroma.Upload.Filter.Exiftool.StripMetadata This filter only strips the GPS and location metadata with Exiftool leaving color profiles and attributes intact. diff --git a/docs/docs/installation/optional/media_graphics_packages.md b/docs/docs/installation/optional/media_graphics_packages.md index cbde2a952..d79ac2a07 100644 --- a/docs/docs/installation/optional/media_graphics_packages.md +++ b/docs/docs/installation/optional/media_graphics_packages.md @@ -29,5 +29,5 @@ It is required for the following Akkoma features: `exiftool` is media files metadata reader/writer. It is required for the following Akkoma features: - * `Pleroma.Upload.Filters.Exiftool.StripLocation` upload filter (related config: `Plaroma.Upload/filters` in `config/config.exs`) + * `Pleroma.Upload.Filters.Exiftool.StripMetadata` upload filter (related config: `Plaroma.Upload/filters` in `config/config.exs`) * `Pleroma.Upload.Filters.Exiftool.ReadDescription` upload filter (related config: `Plaroma.Upload/filters` in `config/config.exs`) diff --git a/lib/mix/tasks/pleroma/instance.ex b/lib/mix/tasks/pleroma/instance.ex index 8f89ebe3e..45f3812c9 100644 --- a/lib/mix/tasks/pleroma/instance.ex +++ b/lib/mix/tasks/pleroma/instance.ex @@ -35,7 +35,7 @@ defmodule Mix.Tasks.Pleroma.Instance do static_dir: :string, listen_ip: :string, listen_port: :string, - strip_uploads_location: :string, + strip_uploads_metadata: :string, read_uploads_description: :string, anonymize_uploads: :string ], @@ -170,7 +170,7 @@ defmodule Mix.Tasks.Pleroma.Instance do ) |> Path.expand() - {strip_uploads_location_message, strip_uploads_location_default} = + {strip_uploads_metadata_message, strip_uploads_metadata_default} = if Pleroma.Utils.command_available?("exiftool") do {"Do you want to strip location (GPS) data from uploaded images? This requires exiftool, it was detected as installed. (y/n)", "y"} @@ -179,12 +179,12 @@ defmodule Mix.Tasks.Pleroma.Instance do "n"} end - strip_uploads_location = + strip_uploads_metadata = get_option( options, - :strip_uploads_location, - strip_uploads_location_message, - strip_uploads_location_default + :strip_uploads_metadata, + strip_uploads_metadata_message, + strip_uploads_metadata_default ) === "y" {read_uploads_description_message, read_uploads_description_default} = @@ -248,7 +248,7 @@ defmodule Mix.Tasks.Pleroma.Instance do listen_port: listen_port, upload_filters: upload_filters(%{ - strip_location: strip_uploads_location, + strip_metadata: strip_uploads_metadata, read_description: read_uploads_description, anonymize: anonymize_uploads }) @@ -325,8 +325,8 @@ defmodule Mix.Tasks.Pleroma.Instance do defp upload_filters(filters) when is_map(filters) do enabled_filters = - if filters.strip_location do - [Pleroma.Upload.Filter.Exiftool.StripLocation] + if filters.strip_metadata do + [Pleroma.Upload.Filter.Exiftool.StripMetadata] else [] end diff --git a/lib/pleroma/application_requirements.ex b/lib/pleroma/application_requirements.ex index ee9b1ef82..c3777d8f1 100644 --- a/lib/pleroma/application_requirements.ex +++ b/lib/pleroma/application_requirements.ex @@ -164,7 +164,7 @@ defmodule Pleroma.ApplicationRequirements do defp check_system_commands!(:ok) do filter_commands_statuses = [ - check_filter(Pleroma.Upload.Filter.Exiftool.StripLocation, "exiftool"), + check_filter(Pleroma.Upload.Filter.Exiftool.StripMetadata, "exiftool"), check_filter(Pleroma.Upload.Filter.Exiftool.ReadDescription, "exiftool"), check_filter(Pleroma.Upload.Filter.Mogrify, "mogrify"), check_filter(Pleroma.Upload.Filter.Mogrifun, "mogrify"), diff --git a/lib/pleroma/config/deprecation_warnings.ex b/lib/pleroma/config/deprecation_warnings.ex index 3858bf88e..b62532595 100644 --- a/lib/pleroma/config/deprecation_warnings.ex +++ b/lib/pleroma/config/deprecation_warnings.ex @@ -28,7 +28,7 @@ defmodule Pleroma.Config.DeprecationWarnings do if Pleroma.Upload.Filter.Exiftool in filters do Logger.warning(""" !!!DEPRECATION WARNING!!! - Your config is using Exiftool as a filter instead of Exiftool.StripLocation. This should work for now, but you are advised to change to the new configuration to prevent possible issues later: + Your config is using Exiftool as a filter instead of Exiftool.StripMetadata. This should work for now, but you are advised to change to the new configuration to prevent possible issues later: ``` config :pleroma, Pleroma.Upload, @@ -40,14 +40,14 @@ defmodule Pleroma.Config.DeprecationWarnings do ``` config :pleroma, Pleroma.Upload, - filters: [Pleroma.Upload.Filter.Exiftool.StripLocation] + filters: [Pleroma.Upload.Filter.Exiftool.StripMetadata] ``` """) new_config = filters |> Enum.map(fn - Pleroma.Upload.Filter.Exiftool -> Pleroma.Upload.Filter.Exiftool.StripLocation + Pleroma.Upload.Filter.Exiftool -> Pleroma.Upload.Filter.Exiftool.StripMetadata filter -> filter end) diff --git a/lib/pleroma/upload/filter/exiftool/strip_location.ex b/lib/pleroma/upload/filter/exiftool/strip_metadata.ex similarity index 95% rename from lib/pleroma/upload/filter/exiftool/strip_location.ex rename to lib/pleroma/upload/filter/exiftool/strip_metadata.ex index 9bb39f68c..178b595f3 100644 --- a/lib/pleroma/upload/filter/exiftool/strip_location.ex +++ b/lib/pleroma/upload/filter/exiftool/strip_metadata.ex @@ -2,7 +2,7 @@ # Copyright © 2017-2021 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only -defmodule Pleroma.Upload.Filter.Exiftool.StripLocation do +defmodule Pleroma.Upload.Filter.Exiftool.StripMetadata do @moduledoc """ Strips GPS related EXIF tags and overwrites the file in place. Also strips or replaces filesystem metadata e.g., timestamps. diff --git a/priv/repo/migrations/20220220135625_upload_filter_exiftool_to_exiftool_strip_location.exs b/priv/repo/migrations/20220220135625_upload_filter_exiftool_to_exiftool_strip_location.exs index 0878b9699..0d68a0787 100644 --- a/priv/repo/migrations/20220220135625_upload_filter_exiftool_to_exiftool_strip_location.exs +++ b/priv/repo/migrations/20220220135625_upload_filter_exiftool_to_exiftool_strip_location.exs @@ -1,4 +1,4 @@ -defmodule Pleroma.Repo.Migrations.UploadFilterExiftoolToExiftoolStripLocation do +defmodule Pleroma.Repo.Migrations.UploadFilterExiftoolToExiftoolStripMetadata do use Ecto.Migration alias Pleroma.ConfigDB @@ -8,14 +8,14 @@ defmodule Pleroma.Repo.Migrations.UploadFilterExiftoolToExiftoolStripLocation do ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Upload}) |> update_filtername( Pleroma.Upload.Filter.Exiftool, - Pleroma.Upload.Filter.Exiftool.StripLocation + Pleroma.Upload.Filter.Exiftool.StripMetadata ) def down, do: ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Upload}) |> update_filtername( - Pleroma.Upload.Filter.Exiftool.StripLocation, + Pleroma.Upload.Filter.Exiftool.StripMetadata, Pleroma.Upload.Filter.Exiftool ) diff --git a/test/mix/tasks/pleroma/instance_test.exs b/test/mix/tasks/pleroma/instance_test.exs index d650bdf7c..98eac4459 100644 --- a/test/mix/tasks/pleroma/instance_test.exs +++ b/test/mix/tasks/pleroma/instance_test.exs @@ -69,7 +69,7 @@ defmodule Mix.Tasks.Pleroma.InstanceTest do "test/uploads", "--static-dir", "./test/../test/instance/static/", - "--strip-uploads-location", + "--strip-uploads-metadata", "y", "--read-uploads-description", "y", @@ -95,7 +95,7 @@ defmodule Mix.Tasks.Pleroma.InstanceTest do assert generated_config =~ "http: [ip: {127, 0, 0, 1}, port: 4000]" assert generated_config =~ - "filters: [Pleroma.Upload.Filter.Exiftool.StripLocation, Pleroma.Upload.Filter.Exiftool.ReadDescription]" + "filters: [Pleroma.Upload.Filter.Exiftool.StripMetadata, Pleroma.Upload.Filter.Exiftool.ReadDescription]" assert generated_config =~ "base_url: \"https://media.pleroma.social/media\"" assert File.read!(tmp_path() <> "setup.psql") == generated_setup_psql() diff --git a/test/pleroma/config/deprecation_warnings_test.exs b/test/pleroma/config/deprecation_warnings_test.exs index d1d80db8f..fd8a558ee 100644 --- a/test/pleroma/config/deprecation_warnings_test.exs +++ b/test/pleroma/config/deprecation_warnings_test.exs @@ -21,7 +21,7 @@ defmodule Pleroma.Config.DeprecationWarningsTest do assert capture_log(fn -> DeprecationWarnings.check_exiftool_filter() end) =~ """ !!!DEPRECATION WARNING!!! - Your config is using Exiftool as a filter instead of Exiftool.StripLocation. This should work for now, but you are advised to change to the new configuration to prevent possible issues later: + Your config is using Exiftool as a filter instead of Exiftool.StripMetadata. This should work for now, but you are advised to change to the new configuration to prevent possible issues later: ``` config :pleroma, Pleroma.Upload, @@ -33,19 +33,19 @@ defmodule Pleroma.Config.DeprecationWarningsTest do ``` config :pleroma, Pleroma.Upload, - filters: [Pleroma.Upload.Filter.Exiftool.StripLocation] + filters: [Pleroma.Upload.Filter.Exiftool.StripMetadata] ``` """ end - test "changes setting to exiftool strip location" do + test "changes setting to exiftool strip metadata" do clear_config( [Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Exiftool, Pleroma.Upload.Filter.Exiftool.ReadDescription] ) expected_config = [ - Pleroma.Upload.Filter.Exiftool.StripLocation, + Pleroma.Upload.Filter.Exiftool.StripMetadata, Pleroma.Upload.Filter.Exiftool.ReadDescription ] @@ -58,7 +58,7 @@ defmodule Pleroma.Config.DeprecationWarningsTest do clear_config( [Pleroma.Upload, :filters], [ - Pleroma.Upload.Filter.Exiftool.StripLocation, + Pleroma.Upload.Filter.Exiftool.StripMetadata, Pleroma.Upload.Filter.Exiftool.ReadDescription ] ) diff --git a/test/pleroma/upload/filter/exiftool/strip_location_test.exs b/test/pleroma/upload/filter/exiftool/strip_location_test.exs index 1c9667e77..6f8178115 100644 --- a/test/pleroma/upload/filter/exiftool/strip_location_test.exs +++ b/test/pleroma/upload/filter/exiftool/strip_location_test.exs @@ -2,7 +2,7 @@ # Copyright © 2017-2021 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only -defmodule Pleroma.Upload.Filter.Exiftool.StripLocationTest do +defmodule Pleroma.Upload.Filter.Exiftool.StripMetadataTest do use Pleroma.DataCase alias Pleroma.Upload.Filter @@ -21,7 +21,7 @@ defmodule Pleroma.Upload.Filter.Exiftool.StripLocationTest do tempfile: Path.absname("test/fixtures/DSCN0010_tmp.jpg") } - assert Filter.Exiftool.StripLocation.filter(upload) == {:ok, :filtered} + assert Filter.Exiftool.StripMetadata.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"]) @@ -37,6 +37,6 @@ defmodule Pleroma.Upload.Filter.Exiftool.StripLocationTest do content_type: "image/webp" } - assert Filter.Exiftool.StripLocation.filter(upload) == {:ok, :noop} + assert Filter.Exiftool.StripMetadata.filter(upload) == {:ok, :noop} end end From 9da0fe930e1fb9b4e2e810dc400b3e925a46505a Mon Sep 17 00:00:00 2001 From: timorl Date: Fri, 19 Apr 2024 18:07:50 +0200 Subject: [PATCH 6/8] Format, but this time with a non-ancient version of elixir --- lib/pleroma/upload/filter/exiftool/read_description.ex | 5 ++++- lib/pleroma/web/plugs/o_auth_scopes_plug.ex | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/upload/filter/exiftool/read_description.ex b/lib/pleroma/upload/filter/exiftool/read_description.ex index 03d698a81..5dfe60cd8 100644 --- a/lib/pleroma/upload/filter/exiftool/read_description.ex +++ b/lib/pleroma/upload/filter/exiftool/read_description.ex @@ -33,7 +33,10 @@ defmodule Pleroma.Upload.Filter.Exiftool.ReadDescription do defp read_when_empty(_, file, tag) do try do {tag_content, 0} = - System.cmd("exiftool", ["-b", "-s3", tag, file], stderr_to_stdout: true, parallelism: true) + System.cmd("exiftool", ["-b", "-s3", tag, file], + stderr_to_stdout: true, + parallelism: true + ) tag_content = String.trim(tag_content) diff --git a/lib/pleroma/web/plugs/o_auth_scopes_plug.ex b/lib/pleroma/web/plugs/o_auth_scopes_plug.ex index f017c8bc7..e4d098a7d 100644 --- a/lib/pleroma/web/plugs/o_auth_scopes_plug.ex +++ b/lib/pleroma/web/plugs/o_auth_scopes_plug.ex @@ -34,7 +34,9 @@ defmodule Pleroma.Web.Plugs.OAuthScopesPlug do permissions = Enum.join(missing_scopes, " #{op} ") error_message = - dgettext("errors", "Insufficient permissions: %{permissions}.", permissions: permissions) + dgettext("errors", "Insufficient permissions: %{permissions}.", + permissions: permissions + ) conn |> put_resp_content_type("application/json") From 09d3ccf770573f22e2cd1ad6b085ad4544178a72 Mon Sep 17 00:00:00 2001 From: timorl Date: Fri, 19 Apr 2024 20:51:54 +0200 Subject: [PATCH 7/8] Read description before stripping metadata --- lib/mix/tasks/pleroma/instance.ex | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/mix/tasks/pleroma/instance.ex b/lib/mix/tasks/pleroma/instance.ex index 45f3812c9..c02efabae 100644 --- a/lib/mix/tasks/pleroma/instance.ex +++ b/lib/mix/tasks/pleroma/instance.ex @@ -324,12 +324,7 @@ defmodule Mix.Tasks.Pleroma.Instance do end defp upload_filters(filters) when is_map(filters) do - enabled_filters = - if filters.strip_metadata do - [Pleroma.Upload.Filter.Exiftool.StripMetadata] - else - [] - end + enabled_filters = [] enabled_filters = if filters.read_description do @@ -338,6 +333,13 @@ defmodule Mix.Tasks.Pleroma.Instance do enabled_filters end + enabled_filters = + if filters.strip_metadata do + enabled_filters ++ [Pleroma.Upload.Filter.Exiftool.StripMetadata] + else + enabled_filters + end + enabled_filters = if filters.anonymize do enabled_filters ++ [Pleroma.Upload.Filter.AnonymizeFilename] From 3f54945033e8a31410723239800b6fd29e9a5449 Mon Sep 17 00:00:00 2001 From: timorl Date: Sun, 21 Apr 2024 19:43:26 +0200 Subject: [PATCH 8/8] Fix the one test that wasn't just being flaky --- test/mix/tasks/pleroma/instance_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mix/tasks/pleroma/instance_test.exs b/test/mix/tasks/pleroma/instance_test.exs index 98eac4459..168343ce5 100644 --- a/test/mix/tasks/pleroma/instance_test.exs +++ b/test/mix/tasks/pleroma/instance_test.exs @@ -95,7 +95,7 @@ defmodule Mix.Tasks.Pleroma.InstanceTest do assert generated_config =~ "http: [ip: {127, 0, 0, 1}, port: 4000]" assert generated_config =~ - "filters: [Pleroma.Upload.Filter.Exiftool.StripMetadata, Pleroma.Upload.Filter.Exiftool.ReadDescription]" + "filters: [Pleroma.Upload.Filter.Exiftool.ReadDescription, Pleroma.Upload.Filter.Exiftool.StripMetadata]" assert generated_config =~ "base_url: \"https://media.pleroma.social/media\"" assert File.read!(tmp_path() <> "setup.psql") == generated_setup_psql()