From 466c950c571a580682b76bdd08f44ccb4f46a50a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5vard=20Lindset?= Date: Sun, 28 Jun 2026 23:40:26 +0200 Subject: [PATCH] Fix strip_metadata/minimize_file_size silent no-op on file-path writes --- lib/image.ex | 11 ++++- lib/image/options/write.ex | 37 +++++++++++++--- test/strip_metadata_test.exs | 83 ++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 test/strip_metadata_test.exs diff --git a/lib/image.ex b/lib/image.ex index 960f7d9..8d0d87b 100644 --- a/lib/image.ex +++ b/lib/image.ex @@ -886,9 +886,18 @@ defmodule Image do end defp loader_options(options) do - "[" <> Enum.map_join(options, ",", fn {k, v} -> "#{k}=#{v}" end) <> "]" + "[" <> Enum.map_join(options, ",", &loader_option/1) <> "]" end + # Flag-valued options are passed to libvips as a list of flag atoms. + # In a filename suffix string flags are written by name joined with `:`, + # e.g. `keep=VIPS_FOREIGN_KEEP_EXIF:VIPS_FOREIGN_KEEP_XMP`. + defp loader_option({key, value}) when is_list(value) do + "#{key}=" <> Enum.map_join(value, ":", &to_string/1) + end + + defp loader_option({key, value}), do: "#{key}=#{value}" + @doc """ Opens an image file for image processing returning an image or raising an exception. diff --git a/lib/image/options/write.ex b/lib/image/options/write.ex index c59fae5..05bf157 100644 --- a/lib/image/options/write.ex +++ b/lib/image/options/write.ex @@ -176,7 +176,7 @@ defmodule Image.Options.Write do options = options |> Keyword.delete(:strip_metadata) - |> Keyword.put(:strip, strip?) + |> put_strip_metadata(strip?) {:cont, options} end @@ -220,7 +220,7 @@ defmodule Image.Options.Write do options = options |> Keyword.delete(:minimize_file_size) - |> Keyword.put(:strip, true) + |> put_strip_metadata(true) |> Keyword.put(:"optimize-coding", true) |> Keyword.put(:interlace, true) |> Keyword.put(:"optimize-scans", true) @@ -235,7 +235,7 @@ defmodule Image.Options.Write do options = options |> Keyword.delete(:minimize_file_size) - |> Keyword.put(:strip, true) + |> put_strip_metadata(true) |> Keyword.put(:palette, true) {:cont, options} @@ -247,7 +247,7 @@ defmodule Image.Options.Write do options |> Keyword.delete(:minimize_file_size) |> Keyword.put(:"min-size", true) - |> Keyword.put(:strip, true) + |> put_strip_metadata(true) |> Keyword.put(:mixed, true) {:cont, options} @@ -259,7 +259,7 @@ defmodule Image.Options.Write do options = options |> Keyword.delete(:minimize_file_size) - |> Keyword.put(:strip, true) + |> put_strip_metadata(true) {:cont, options} end @@ -425,6 +425,33 @@ defmodule Image.Options.Write do # Range 0..6 defp conform_effort(effort, ".webp"), do: round(effort / 10 * 6) + # libvips 8.15 deprecated the boolean `strip` save option in favour of the + # `keep` flag. Vix removes deprecated properties from the operation it builds, + # so passing `strip: true` to `Vix.Vips.Image.write_to_file/3` is silently + # ignored on libvips >= 8.15 and no metadata is stripped. The `:memory`, stream + # and `Plug.Conn` targets escaped the bug because they serialise the options + # into a libvips filename suffix string (e.g. `".jpg[strip=true]"`) which is + # parsed by libvips' own option parser, and that parser still honours the + # deprecated `strip` alias. + # + # Emit the modern `keep` flag where it exists, so `strip: true` is converted to + # `keep: [:VIPS_FOREIGN_KEEP_NONE]`, and fall back to `strip` on libvips < 8.15 + # where `keep` does not exist. + defp put_strip_metadata(options, false), do: options + + defp put_strip_metadata(options, true) do + if keep_option_supported?() do + Keyword.put(options, :keep, [:VIPS_FOREIGN_KEEP_NONE]) + else + Keyword.put(options, :strip, true) + end + end + + defp keep_option_supported? do + {:ok, version} = Image.vips_version() + Version.compare(version, "8.15.0") != :lt + end + defp image_type_from("", "") do {:error, %Image.Error{message: "Cannot determine image type", reason: "Cannot determine image type"}} diff --git a/test/strip_metadata_test.exs b/test/strip_metadata_test.exs new file mode 100644 index 0000000..afaa2f7 --- /dev/null +++ b/test/strip_metadata_test.exs @@ -0,0 +1,83 @@ +defmodule Image.StripMetadataTest do + use ExUnit.Case, async: true + import Image.TestSupport + + alias Vix.Vips.Image, as: Vimage + + # Counts the number of EXIF/XMP metadata fields present on the image + # at `path` after reloading it from disk. + defp metadata_field_count(path) do + {:ok, image} = Image.open(path, access: :random) + {:ok, names} = Vimage.header_field_names(image) + Enum.count(names, &(&1 =~ ~r/exif|xmp/i)) + end + + defp out(name) do + unique = System.unique_integer([:positive]) + path = Path.join(System.tmp_dir!(), "image_strip_test_#{unique}_#{name}") + on_exit(fn -> File.rm(path) end) + path + end + + describe "strip_metadata: true when writing to a file path" do + test "removes EXIF/XMP metadata from a JPEG" do + {:ok, image} = Image.open(image_path("Kip_small.jpg"), access: :random) + + kept = out("keep.jpg") + stripped = out("strip.jpg") + + assert {:ok, _} = Image.write(image, kept, strip_metadata: false) + assert {:ok, _} = Image.write(image, stripped, strip_metadata: true) + + # The source image genuinely has metadata, and not stripping retains it. + assert metadata_field_count(kept) > 0 + # Stripping on a file path must actually remove it. + assert metadata_field_count(stripped) == 0 + assert File.stat!(stripped).size < File.stat!(kept).size + end + + test "removes EXIF/XMP metadata from a PNG" do + {:ok, image} = Image.open(image_path("jose.png"), access: :random) + + kept = out("keep.png") + stripped = out("strip.png") + + assert {:ok, _} = Image.write(image, kept, strip_metadata: false) + assert {:ok, _} = Image.write(image, stripped, strip_metadata: true) + + assert metadata_field_count(kept) > 0 + assert metadata_field_count(stripped) == 0 + end + end + + describe "minimize_file_size: true when writing to a file path" do + test "removes EXIF/XMP metadata from a JPEG" do + {:ok, image} = Image.open(image_path("Kip_small.jpg"), access: :random) + + stripped = out("min.jpg") + assert {:ok, _} = Image.write(image, stripped, minimize_file_size: true) + assert metadata_field_count(stripped) == 0 + end + + test "removes EXIF/XMP metadata from a PNG" do + {:ok, image} = Image.open(image_path("jose.png"), access: :random) + + stripped = out("min.png") + assert {:ok, _} = Image.write(image, stripped, minimize_file_size: true) + assert metadata_field_count(stripped) == 0 + end + end + + describe "strip_metadata when writing to :memory" do + # already worked, guards against regression + + test "removes EXIF/XMP metadata from a JPEG buffer" do + {:ok, image} = Image.open(image_path("Kip_small.jpg"), access: :random) + + {:ok, kept} = Image.write(image, :memory, suffix: ".jpg", strip_metadata: false) + {:ok, stripped} = Image.write(image, :memory, suffix: ".jpg", strip_metadata: true) + + assert byte_size(stripped) < byte_size(kept) + end + end +end