From 1f57d353483043ee6a7c39a1fedc703478ed4014 Mon Sep 17 00:00:00 2001
From: klacointe <kevinlacointe@gmail.com>
Date: Mon, 15 Jun 2026 22:03:07 +0200
Subject: [PATCH 1/5] feat: introduce pluggable HTTP client behaviour
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Add Waffle.HTTPClient behaviour with a single get/3 callback and
  shared parse_content_disposition/1 helper
- Add Waffle.HTTPClient.Hackney (default) and Waffle.HTTPClient.Finch
  implementations
- Refactor Waffle.File to dispatch through the configured :http_client
  instead of calling hackney directly; retry logic stays client-agnostic
- Make hackney and finch optional dependencies — users add whichever
  they need; at least one is required for remote file downloads
- Upgrade CI from Elixir 1.13 to 1.20 (required by finch ~> 0.18)
- Bump credo to ~> 1.7 for Elixir 1.19+ compatibility
- Add tests for both implementations and retry behaviour in Waffle.File
---
 Dockerfile                                    |  2 +-
 README.md                                     | 48 ++++++++-
 lib/mix/tasks/g.ex                            |  2 +
 lib/waffle/behaviors/http_client_behaviour.ex | 87 ++++++++++++++++
 lib/waffle/file.ex                            | 80 +++------------
 lib/waffle/http_client/finch.ex               | 89 +++++++++++++++++
 lib/waffle/http_client/hackney.ex             | 72 ++++++++++++++
 mix.exs                                       |  6 +-
 mix.lock                                      | 13 ++-
 test/actions/store_test.exs                   | 22 +++--
 test/file_test.exs                            | 77 ++++++++++++++-
 test/http_client/finch_test.exs               | 98 +++++++++++++++++++
 test/http_client/hackney_test.exs             | 96 ++++++++++++++++++
 test/http_client/http_client_test.exs         | 32 ++++++
 14 files changed, 639 insertions(+), 85 deletions(-)
 create mode 100644 lib/waffle/behaviors/http_client_behaviour.ex
 create mode 100644 lib/waffle/http_client/finch.ex
 create mode 100644 lib/waffle/http_client/hackney.ex
 create mode 100644 test/http_client/finch_test.exs
 create mode 100644 test/http_client/hackney_test.exs
 create mode 100644 test/http_client/http_client_test.exs

diff --git a/Dockerfile b/Dockerfile
index cbad08d..f4f258f 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,4 +1,4 @@
-FROM elixir:1.13-alpine
+FROM elixir:1.17-alpine
 
 # add `convert` utility from imagemagick for image transformations
 RUN apk add --no-cache imagemagick
diff --git a/README.md b/README.md
index cabafbc..0917344 100644
--- a/README.md
+++ b/README.md
@@ -23,18 +23,20 @@ Already using Waffle in your org? I'd love to learn more, let's [have a chat](ht
 
 ## Installation
 
-Add the latest stable release to your `mix.exs` file, along with the
-required dependencies for `ExAws` if appropriate:
+Add the latest stable release to your `mix.exs` file:
 
 ```elixir
 defp deps do
   [
     {:waffle, "~> 1.1"},
 
+    # HTTP client — required for downloading remote files (choose one):
+    {:hackney, "~> 1.9"},   # default
+    # {:finch, "~> 0.18"},  # alternative
+
     # If using S3:
-    {:ex_aws, "~> 2.1.2"},
-    {:ex_aws_s3, "~> 2.0"},
-    {:hackney, "~> 1.9"},
+    {:ex_aws, "~> 2.1"},
+    {:ex_aws_s3, "~> 2.1"},
     {:sweet_xml, "~> 0.6"}
   ]
 end
@@ -80,6 +82,42 @@ config :ex_aws,
   # any configurations provided by https://github.com/ex-aws/ex_aws
 ```
 
+### Setup an HTTP client
+
+Waffle uses an HTTP client to download remote files. Two built-in implementations are provided:
+
+**Hackney** (default):
+
+```elixir
+# mix.exs
+{:hackney, "~> 1.9"}
+```
+
+```elixir
+# config.exs
+config :waffle, :http_client, Waffle.HTTPClient.Hackney
+```
+
+**Finch**:
+
+```elixir
+# mix.exs
+{:finch, "~> 0.18"}
+```
+
+```elixir
+# application.ex
+children = [
+  {Finch, name: MyApp.Finch}
+]
+
+# config.exs
+config :waffle, :http_client, Waffle.HTTPClient.Finch
+config :waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch
+```
+
+You can also implement your own client by adopting the `Waffle.HTTPClient` behaviour.
+
 ### Define a definition module
 
 Waffle requires a **definition module** which contains the relevant
diff --git a/lib/mix/tasks/g.ex b/lib/mix/tasks/g.ex
index 9ec3286..def7e61 100644
--- a/lib/mix/tasks/g.ex
+++ b/lib/mix/tasks/g.ex
@@ -1,4 +1,6 @@
 defmodule Mix.Tasks.Waffle do
+  @moduledoc false
+
   defmodule G do
     use Mix.Task
     import Mix.Generator
diff --git a/lib/waffle/behaviors/http_client_behaviour.ex b/lib/waffle/behaviors/http_client_behaviour.ex
new file mode 100644
index 0000000..6140ae5
--- /dev/null
+++ b/lib/waffle/behaviors/http_client_behaviour.ex
@@ -0,0 +1,87 @@
+defmodule Waffle.HTTPClient do
+  @moduledoc """
+  Behaviour for pluggable HTTP clients used when downloading remote files.
+
+  ## Built-in implementations
+
+  - `Waffle.HTTPClient.Hackney` — default, uses `:hackney`. Add `{:hackney, "~> 1.9"}` to
+    your deps (or `{:hackney, ">= 4.0.1"}` for the CVE-free version).
+  - `Waffle.HTTPClient.Finch` — uses `Finch`. Add `{:finch, "~> 0.18"}` to your deps,
+    start a `Finch` pool in your application supervision tree, and configure
+    `config :waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch`.
+
+  At least one HTTP client dependency is required when downloading remote files.
+
+  ## Configuration
+
+      config :waffle, :http_client, Waffle.HTTPClient.Hackney
+
+  ## Writing a custom client
+
+  Implement the `c:get/3` callback and configure Waffle to use your module:
+
+      config :waffle, :http_client, MyApp.HTTPClient
+
+  ### Options
+
+  Waffle passes the following options (all values come from application config):
+
+  | Option             | Type                    | Default    | Description                              |
+  |--------------------|-------------------------|------------|------------------------------------------|
+  | `:recv_timeout`    | `non_neg_integer()`              | `5_000`     | Timeout for receiving a response (ms)              |
+  | `:connect_timeout` | `non_neg_integer()`              | `10_000`    | Timeout for establishing a connection (ms)         |
+  | `:max_body_length` | `non_neg_integer() \| :infinity` | `:infinity` | Maximum allowed response body size (bytes)         |
+  | `:follow_redirect` | `boolean()`                      | `true`      | Whether to follow HTTP redirects                   |
+  | `:max_retries`     | `non_neg_integer()`              | `3`         | Number of retries on timeout or 503                |
+  | `:backoff_factor`  | `non_neg_integer()`              | `1_000`     | Base backoff delay between retries (ms)            |
+  | `:backoff_max`     | `non_neg_integer()`              | `30_000`    | Maximum backoff delay between retries (ms)         |
+
+  ### Return values
+
+  | Pattern                                | Meaning                                          |
+  |----------------------------------------|--------------------------------------------------|
+  | `{:ok, body}`                          | Successful response, no filename in headers      |
+  | `{:ok, body, filename}`                | Successful response with `content-disposition` filename |
+  | `{:error, :timeout}`                   | Request timed out — Waffle will retry            |
+  | `{:error, :service_unavailable}`       | Server returned 503 — Waffle will retry          |
+  | `{:error, {:http_error, reason}}`      | Non-retryable error                              |
+  """
+
+  @type body :: binary()
+  @type filename :: String.t()
+  @type option ::
+          {:recv_timeout, non_neg_integer()}
+          | {:connect_timeout, non_neg_integer()}
+          | {:max_body_length, non_neg_integer() | :infinity}
+          | {:follow_redirect, boolean()}
+
+  @callback get(url :: String.t(), headers :: list(), options :: [option()]) ::
+              {:ok, body()}
+              | {:ok, body(), filename()}
+              | {:error, :timeout | :service_unavailable | {:http_error, any()}}
+
+  @doc """
+  Parses the `filename` parameter from a `content-disposition` header value.
+
+  Handles quoted filenames (`filename="foo.png"`), unquoted filenames
+  (`filename=foo.png`), and multi-parameter values.
+
+  Returns `nil` when no filename is present.
+  """
+  @spec parse_content_disposition(String.t()) :: String.t() | nil
+  def parse_content_disposition(value) do
+    value
+    |> String.split(";")
+    |> Enum.find_value(&extract_filename_param/1)
+  end
+
+  defp extract_filename_param(part) do
+    with [key, raw] <- String.split(String.trim(part), "=", parts: 2),
+         true <- String.downcase(String.trim(key)) == "filename",
+         name when name != "" <- raw |> String.trim() |> String.trim("\"") do
+      name
+    else
+      _ -> nil
+    end
+  end
+end
diff --git a/lib/waffle/file.ex b/lib/waffle/file.ex
index eccb0e9..9c4dd75 100644
--- a/lib/waffle/file.ex
+++ b/lib/waffle/file.ex
@@ -185,11 +185,6 @@ defmodule Waffle.File do
     end
   end
 
-  # hackney :connect_timeout - timeout used when establishing a connection, in milliseconds
-  # hackney :recv_timeout - timeout used when receiving from a connection, in milliseconds
-  # hackney :max_body_length - maximum size of the file to download, in bytes. Defaults to :infinity
-  # :backoff_max - maximum backoff time, in milliseconds
-  # :backoff_factor - a backoff factor to apply between attempts, in milliseconds
   defp get_remote_path(remote_path, definition) do
     headers = definition.remote_file_headers(remote_path)
 
@@ -197,6 +192,7 @@ defmodule Waffle.File do
       follow_redirect: true,
       recv_timeout: Application.get_env(:waffle, :recv_timeout, 5_000),
       connect_timeout: Application.get_env(:waffle, :connect_timeout, 10_000),
+      max_body_length: Application.get_env(:waffle, :max_body_length, :infinity),
       max_retries: Application.get_env(:waffle, :max_retries, 3),
       backoff_factor: Application.get_env(:waffle, :backoff_factor, 1000),
       backoff_max: Application.get_env(:waffle, :backoff_max, 30_000)
@@ -206,74 +202,28 @@ defmodule Waffle.File do
   end
 
   defp request(remote_path, headers, options, tries \\ 0) do
-    with {:ok, 200, response_headers, client_ref} <-
-           :hackney.get(URI.to_string(remote_path), headers, "", options),
-         res when elem(res, 0) == :ok <- body(client_ref, response_headers) do
-      res
-    else
-      {:error, %{reason: :timeout}} ->
-        case retry(tries, options) do
-          {:ok, :retry} -> request(remote_path, headers, options, tries + 1)
-          {:error, :out_of_tries} -> {:error, :timeout}
-        end
-
-      {:error, :timeout} ->
-        case retry(tries, options) do
-          {:ok, :retry} -> request(remote_path, headers, options, tries + 1)
-          {:error, :out_of_tries} -> {:error, :recv_timeout}
-        end
-
-      {:ok, 503, _headers, client_ref} = response ->
-        case retry(tries, options) do
-          {:ok, :retry} ->
-            request(remote_path, headers, options, tries + 1)
-
-          {:error, :out_of_tries} ->
-            :hackney.close(client_ref)
-            {:error, {:waffle_hackney_error, response}}
-        end
-
-      {:ok, _, _, client_ref} = response ->
-        :hackney.close(client_ref)
-        {:error, {:waffle_hackney_error, response}}
-
-      _err ->
-        {:error, :waffle_hackney_error}
-    end
-  end
-
-  defp body(client_ref, response_headers) do
-    max_body_length = Application.get_env(:waffle, :max_body_length, :infinity)
+    http_client = Application.get_env(:waffle, :http_client, Waffle.HTTPClient.Hackney)
+    url = URI.to_string(remote_path)
 
-    case :hackney.body(client_ref, max_body_length) do
+    case http_client.get(url, headers, options) do
       {:ok, body} ->
-        response_headers = :hackney_headers.new(response_headers)
-        filename = content_disposition(response_headers)
+        {:ok, body}
 
-        if is_nil(filename) do
-          {:ok, body}
-        else
-          {:ok, body, filename}
-        end
+      {:ok, body, filename} ->
+        {:ok, body, filename}
 
-      err ->
+      {:error, reason} when reason in [:timeout, :service_unavailable] ->
+        retry_or_error(remote_path, headers, options, tries, reason)
+
+      {:error, _} = err ->
         err
     end
   end
 
-  defp content_disposition(headers) do
-    case :hackney_headers.get_value("content-disposition", headers) do
-      :undefined ->
-        nil
-
-      value ->
-        case :hackney_headers.content_disposition(value) do
-          {_, [{"filename", filename} | _]} ->
-            filename
-
-          _ ->
-            nil
-        end
+  defp retry_or_error(remote_path, headers, options, tries, reason) do
+    case retry(tries, options) do
+      {:ok, :retry} -> request(remote_path, headers, options, tries + 1)
+      {:error, :out_of_tries} -> {:error, reason}
     end
   end
 
diff --git a/lib/waffle/http_client/finch.ex b/lib/waffle/http_client/finch.ex
new file mode 100644
index 0000000..4a8c4d0
--- /dev/null
+++ b/lib/waffle/http_client/finch.ex
@@ -0,0 +1,89 @@
+defmodule Waffle.HTTPClient.Finch do
+  @moduledoc """
+  HTTP client implementation using `Finch`.
+
+  ## Setup
+
+  Add `finch` to your dependencies:
+
+      {:finch, "~> 0.18"}
+
+  Start a `Finch` pool in your application supervision tree:
+
+      children = [
+        {Finch, name: MyApp.Finch}
+      ]
+
+  Then configure Waffle to use this client and point it at your pool:
+
+      config :waffle, :http_client, Waffle.HTTPClient.Finch
+      config :waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch
+
+  ## Options
+
+  | Option             | Default      | Description                                            |
+  |--------------------|--------------|--------------------------------------------------------|
+  | `:recv_timeout`    | `5_000`      | Timeout for receiving a response, in milliseconds      |
+  | `:connect_timeout` | `10_000`     | Timeout for establishing a connection, in milliseconds |
+  | `:max_body_length` | `:infinity`  | Maximum response body size, in bytes                   |
+  """
+
+  @behaviour Waffle.HTTPClient
+
+  @impl Waffle.HTTPClient
+  def get(url, headers, options) do
+    finch_config = Application.get_env(:waffle, Waffle.HTTPClient.Finch, [])
+    finch_name = Keyword.get(finch_config, :pool_name, Waffle.Finch)
+    recv_timeout = Keyword.get(options, :recv_timeout, 5_000)
+    connect_timeout = Keyword.get(options, :connect_timeout, 10_000)
+    max_body_length = Keyword.get(options, :max_body_length, :infinity)
+
+    finch_options = [
+      receive_timeout: recv_timeout,
+      connect_options: [timeout: connect_timeout]
+    ]
+
+    request = Finch.build(:get, url, headers)
+
+    case Finch.request(request, finch_name, finch_options) do
+      {:ok, %Finch.Response{status: 200, headers: response_headers, body: body}} ->
+        handle_success(body, response_headers, max_body_length)
+
+      {:ok, %Finch.Response{status: 503}} ->
+        {:error, :service_unavailable}
+
+      {:ok, %Finch.Response{}} ->
+        {:error, {:http_error, :unexpected_status}}
+
+      {:error, reason} ->
+        classify_error(reason)
+    end
+  end
+
+  defp handle_success(body, response_headers, max_body_length) do
+    if over_limit?(body, max_body_length) do
+      {:error, {:http_error, :body_too_large}}
+    else
+      filename = find_content_disposition_filename(response_headers)
+      if filename, do: {:ok, body, filename}, else: {:ok, body}
+    end
+  end
+
+  defp classify_error(%{reason: :timeout}), do: {:error, :timeout}
+
+  defp classify_error(exception) when is_exception(exception),
+    do: {:error, {:http_error, exception}}
+
+  defp classify_error(reason), do: {:error, {:http_error, reason}}
+
+  defp over_limit?(_body, :infinity), do: false
+  defp over_limit?(body, max), do: byte_size(body) > max
+
+  defp find_content_disposition_filename(headers) do
+    Enum.find_value(headers, fn {key, value} ->
+      if String.downcase(key) == "content-disposition" do
+        Waffle.HTTPClient.parse_content_disposition(value)
+      end
+    end)
+  end
+end
diff --git a/lib/waffle/http_client/hackney.ex b/lib/waffle/http_client/hackney.ex
new file mode 100644
index 0000000..1d4208c
--- /dev/null
+++ b/lib/waffle/http_client/hackney.ex
@@ -0,0 +1,72 @@
+defmodule Waffle.HTTPClient.Hackney do
+  @moduledoc """
+  Default HTTP client implementation using `:hackney`.
+
+  Add `:hackney` to your dependencies:
+
+      {:hackney, "~> 1.9"}
+
+  Or, to use the CVE-free version (requires OTP 26+):
+
+      {:hackney, ">= 4.0.1"}
+
+  ## Configuration
+
+      config :waffle, :http_client, Waffle.HTTPClient.Hackney
+
+  ## Options
+
+  | Option             | Default      | Description                                            |
+  |--------------------|--------------|--------------------------------------------------------|
+  | `:recv_timeout`    | `5_000`      | Timeout for receiving a response, in milliseconds      |
+  | `:connect_timeout` | `10_000`     | Timeout for establishing a connection, in milliseconds |
+  | `:max_body_length` | `:infinity`  | Maximum response body size, in bytes                   |
+  """
+
+  @behaviour Waffle.HTTPClient
+
+  @impl Waffle.HTTPClient
+  def get(url, headers, options) do
+    hackney_options = [
+      follow_redirect: Keyword.get(options, :follow_redirect, true),
+      recv_timeout: Keyword.get(options, :recv_timeout, 5_000),
+      connect_timeout: Keyword.get(options, :connect_timeout, 10_000)
+    ]
+
+    max_body_length = Keyword.get(options, :max_body_length, :infinity)
+
+    with {:ok, 200, response_headers, client_ref} <-
+           :hackney.get(url, headers, "", hackney_options),
+         {:ok, body} <- :hackney.body(client_ref, max_body_length) do
+      filename =
+        :hackney_headers.new(response_headers)
+        |> get_content_disposition_filename()
+
+      if filename, do: {:ok, body, filename}, else: {:ok, body}
+    else
+      {:ok, 503, _headers, client_ref} ->
+        :hackney.close(client_ref)
+        {:error, :service_unavailable}
+
+      {:ok, _status, _headers, client_ref} ->
+        :hackney.close(client_ref)
+        {:error, {:http_error, :unexpected_status}}
+
+      {:error, %{reason: :timeout}} ->
+        {:error, :timeout}
+
+      {:error, :timeout} ->
+        {:error, :timeout}
+
+      {:error, reason} ->
+        {:error, {:http_error, reason}}
+    end
+  end
+
+  defp get_content_disposition_filename(headers) do
+    case :hackney_headers.get_value("content-disposition", headers) do
+      :undefined -> nil
+      value -> Waffle.HTTPClient.parse_content_disposition(value)
+    end
+  end
+end
diff --git a/mix.exs b/mix.exs
index d6aa910..6db36d8 100644
--- a/mix.exs
+++ b/mix.exs
@@ -58,7 +58,9 @@ defmodule Waffle.Mixfile do
 
   defp deps do
     [
-      {:hackney, "~> 1.9"},
+      # Optional HTTP clients (at least one is required for downloading remote files)
+      {:hackney, "~> 1.9", optional: true},
+      {:finch, "~> 0.18", optional: true},
 
       # If using Amazon S3
       {:ex_aws, "~> 2.1", optional: true},
@@ -72,7 +74,7 @@ defmodule Waffle.Mixfile do
       {:ex_doc, "~> 0.21", only: :dev},
 
       # Dev, Test
-      {:credo, "~> 1.4", only: [:dev, :test], runtime: false}
+      {:credo, "~> 1.7", only: [:dev, :test], runtime: false}
     ]
   end
 end
diff --git a/mix.lock b/mix.lock
index bc6be50..373509c 100644
--- a/mix.lock
+++ b/mix.lock
@@ -1,16 +1,18 @@
 %{
-  "bunt": {:hex, :bunt, "0.2.1", "e2d4792f7bc0ced7583ab54922808919518d0e57ee162901a16a1b6664ef3b14", [:mix], [], "hexpm", "a330bfb4245239787b15005e66ae6845c9cd524a288f0d141c148b02603777a5"},
+  "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"},
   "certifi": {:hex, :certifi, "2.9.0", "6f2a475689dd47f19fb74334859d460a2dc4e3252a3324bd2111b8f0429e7e21", [:rebar3], [], "hexpm", "266da46bdb06d6c6d35fde799bcb28d36d985d424ad7c08b5bb48f5b5cdd4641"},
-  "credo": {:hex, :credo, "1.6.7", "323f5734350fd23a456f2688b9430e7d517afb313fbd38671b8a4449798a7854", [:mix], [{:bunt, "~> 0.2.1", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "41e110bfb007f7eda7f897c10bf019ceab9a0b269ce79f015d54b0dcf4fc7dd3"},
+  "credo": {:hex, :credo, "1.7.19", "cc52129665fc7c15143d47838fda0f9cd6dac9ceced7bf4da6f85fcbfe64b12a", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "2d8bc95d5a7bb99dd2613621d4f08c6a3575c3fd4b62e6a2b48a100352a557b8"},
   "earmark": {:hex, :earmark, "1.4.4", "4821b8d05cda507189d51f2caeef370cf1e18ca5d7dfb7d31e9cafe6688106a4", [:mix], [], "hexpm"},
   "earmark_parser": {:hex, :earmark_parser, "1.4.39", "424642f8335b05bb9eb611aa1564c148a8ee35c9c8a8bba6e129d51a3e3c6769", [:mix], [], "hexpm", "06553a88d1f1846da9ef066b87b57c6f605552cfbe40d20bd8d59cc6bde41944"},
   "ex_aws": {:hex, :ex_aws, "2.4.1", "d1dc8965d1dc1c939dd4570e37f9f1d21e047e4ecd4f9373dc89cd4e45dce5ef", [:mix], [{:configparser_ex, "~> 4.0", [hex: :configparser_ex, repo: "hexpm", optional: true]}, {:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:jsx, "~> 2.8 or ~> 3.0", [hex: :jsx, repo: "hexpm", optional: true]}, {:mime, "~> 1.2 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:sweet_xml, "~> 0.7", [hex: :sweet_xml, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "803387db51b4e91be4bf0110ba999003ec6103de7028b808ee9b01f28dbb9eee"},
   "ex_aws_s3": {:hex, :ex_aws_s3, "2.4.0", "ce8decb6b523381812798396bc0e3aaa62282e1b40520125d1f4eff4abdff0f4", [:mix], [{:ex_aws, "~> 2.0", [hex: :ex_aws, repo: "hexpm", optional: false]}, {:sweet_xml, ">= 0.0.0", [hex: :sweet_xml, repo: "hexpm", optional: true]}], "hexpm", "85dda6e27754d94582869d39cba3241d9ea60b6aa4167f9c88e309dc687e56bb"},
   "ex_doc": {:hex, :ex_doc, "0.34.0", "ab95e0775db3df71d30cf8d78728dd9261c355c81382bcd4cefdc74610bef13e", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "60734fb4c1353f270c3286df4a0d51e65a2c1d9fba66af3940847cc65a8066d7"},
-  "file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"},
+  "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"},
+  "finch": {:hex, :finch, "0.22.0", "5c48fa6f9706a78eb9036cacb67b8b996b4e66d111c543f4c29bb0f879a6806b", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.8", [hex: :mint, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.4 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:nimble_pool, "~> 1.1", [hex: :nimble_pool, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b94e83c47780fc6813f746a1f1a34ee65cda42da4c5ea26a68f0acc4498e23dc"},
   "hackney": {:hex, :hackney, "1.18.1", "f48bf88f521f2a229fc7bae88cf4f85adc9cd9bcf23b5dc8eb6a1788c662c4f6", [:rebar3], [{:certifi, "~> 2.9.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~> 6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~> 1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "a4ecdaff44297e9b5894ae499e9a070ea1888c84afdd1fd9b7b2bc384950128e"},
+  "hpax": {:hex, :hpax, "1.0.3", "ed67ef51ad4df91e75cc6a1494f851850c0bd98ebc0be6e81b026e765ee535aa", [:mix], [], "hexpm", "8eab6e1cfa8d5918c2ce4ba43588e894af35dbd8e91e6e55c817bca5847df34a"},
   "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},
-  "jason": {:hex, :jason, "1.4.0", "e855647bc964a44e2f67df589ccf49105ae039d4179db7f6271dfd3843dc27e6", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "79a3791085b2a0f743ca04cec0f7be26443738779d09302e01318f97bdb82121"},
+  "jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"},
   "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
   "makeup_erlang": {:hex, :makeup_erlang, "1.0.0", "6f0eff9c9c489f26b69b61440bf1b238d95badae49adac77973cbacae87e3c2e", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "ea7a9307de9d1548d2a72d299058d1fd2339e3d398560a0e46c27dab4891e4d2"},
@@ -18,8 +20,11 @@
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
   "mime": {:hex, :mime, "2.0.3", "3676436d3d1f7b81b5a2d2bd8405f412c677558c81b1c92be58c00562bb59095", [:mix], [], "hexpm", "27a30bf0db44d25eecba73755acf4068cbfe26a4372f9eb3e4ea3a45956bff6b"},
   "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
+  "mint": {:hex, :mint, "1.9.0", "d6f534c2a3e98b2a8cc749b4796eb77e9e3af79a76f96e4c74035a827de0d318", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0 or ~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "007154c7d8c43916aed3c93afd1f11aebbaa9c5ff4b7ba55ebe0d17ee0296042"},
   "mock": {:hex, :mock, "0.3.7", "75b3bbf1466d7e486ea2052a73c6e062c6256fb429d6797999ab02fa32f29e03", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "4da49a4609e41fd99b7836945c26f373623ea968cfb6282742bcb94440cf7e5c"},
+  "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
+  "nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"},
   "parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
   "sweet_xml": {:hex, :sweet_xml, "0.7.3", "debb256781c75ff6a8c5cbf7981146312b66f044a2898f453709a53e5031b45b", [:mix], [], "hexpm", "e110c867a1b3fe74bfc7dd9893aa851f0eed5518d0d7cad76d7baafd30e4f5ba"},
diff --git a/test/actions/store_test.exs b/test/actions/store_test.exs
index 416276d..9a5ecfa 100644
--- a/test/actions/store_test.exs
+++ b/test/actions/store_test.exs
@@ -140,22 +140,34 @@ defmodule WaffleTest.Actions.Store do
   end
 
   test "recv_timeout" do
+    original = Application.get_env(:waffle, :recv_timeout)
     Application.put_env(:waffle, :recv_timeout, 1)
 
+    on_exit(fn ->
+      if original,
+        do: Application.put_env(:waffle, :recv_timeout, original),
+        else: Application.delete_env(:waffle, :recv_timeout)
+    end)
+
     with_mock Waffle.Storage.S3,
       put: fn DummyDefinition, _, {%{file_name: "favicon.ico", path: _}, nil} ->
         {:ok, "favicon.ico"}
       end do
       assert DummyDefinition.store("https://www.google.com/favicon.ico") ==
-               {:error, :recv_timeout}
+               {:error, :timeout}
     end
-
-    Application.put_env(:waffle, :recv_timeout, 5_000)
   end
 
   test "recv_timeout with a filename" do
+    original = Application.get_env(:waffle, :recv_timeout)
     Application.put_env(:waffle, :recv_timeout, 1)
 
+    on_exit(fn ->
+      if original,
+        do: Application.put_env(:waffle, :recv_timeout, original),
+        else: Application.delete_env(:waffle, :recv_timeout)
+    end)
+
     with_mock Waffle.Storage.S3,
       put: fn DummyDefinition, _, {%{file_name: "newfavicon.ico", path: _}, nil} ->
         {:ok, "newfavicon.ico"}
@@ -164,10 +176,8 @@ defmodule WaffleTest.Actions.Store do
                remote_path: "https://www.google.com/favicon.ico",
                filename: "newfavicon.ico"
              }) ==
-               {:error, :recv_timeout}
+               {:error, :timeout}
     end
-
-    Application.put_env(:waffle, :recv_timeout, 5_000)
   end
 
   test "accepts remote files" do
diff --git a/test/file_test.exs b/test/file_test.exs
index b0194a2..12707f5 100644
--- a/test/file_test.exs
+++ b/test/file_test.exs
@@ -1,22 +1,95 @@
 defmodule WaffleTest.File do
   use ExUnit.Case, async: false
+  import Mock
+  alias Waffle.HTTPClient.Hackney
 
   @custom_tmp_dir System.tmp_dir() <> "/waffle_test_custom"
 
+  defmodule DummyDefinition do
+    use Waffle.Definition.Storage
+    def transform(_, _), do: :noaction
+    def __versions, do: [:original]
+  end
+
   describe "generate_temporary_path/1" do
     test "uses configured tmp_dir" do
       File.mkdir_p!(@custom_tmp_dir)
       Application.put_env(:waffle, :tmp_dir, @custom_tmp_dir)
 
       assert Waffle.File.generate_temporary_path() |> String.starts_with?(@custom_tmp_dir)
-      on_exit fn ->
+
+      on_exit(fn ->
         Application.delete_env(:waffle, :tmp_dir)
         File.rm_rf!(@custom_tmp_dir)
-      end
+      end)
     end
 
     test "uses system tmp_dir" do
       assert Waffle.File.generate_temporary_path() |> String.starts_with?(System.tmp_dir())
     end
   end
+
+  describe "new/2 with remote URL — retry behavior" do
+    setup do
+      Application.put_env(:waffle, :http_client, Hackney)
+      Application.put_env(:waffle, :max_retries, 2)
+      # Zero-out backoff so tests don't sleep
+      Application.put_env(:waffle, :backoff_factor, 0)
+      Application.put_env(:waffle, :backoff_max, 0)
+
+      on_exit(fn ->
+        Application.delete_env(:waffle, :http_client)
+        Application.delete_env(:waffle, :max_retries)
+        Application.delete_env(:waffle, :backoff_factor)
+        Application.delete_env(:waffle, :backoff_max)
+      end)
+    end
+
+    test "retries on timeout and returns {:error, :timeout} after exhausting retries" do
+      with_mock Hackney,
+        get: fn _url, _headers, _opts ->
+          {:error, :timeout}
+        end do
+        result = Waffle.File.new("http://example.com/image.jpg", DummyDefinition)
+        assert result == {:error, :timeout}
+        # initial attempt + 2 retries
+        assert called(Hackney.get(:_, :_, :_))
+      end
+    end
+
+    test "retries on service_unavailable and returns {:error, :service_unavailable} after exhausting retries" do
+      with_mock Hackney,
+        get: fn _url, _headers, _opts ->
+          {:error, :service_unavailable}
+        end do
+        result = Waffle.File.new("http://example.com/image.jpg", DummyDefinition)
+        assert result == {:error, :service_unavailable}
+        assert called(Hackney.get(:_, :_, :_))
+      end
+    end
+
+    test "does not retry on non-retryable errors" do
+      with_mock Hackney,
+        get: fn _url, _headers, _opts ->
+          {:error, {:http_error, :unexpected_status}}
+        end do
+        result = Waffle.File.new("http://example.com/image.jpg", DummyDefinition)
+        assert result == {:error, {:http_error, :unexpected_status}}
+        # exactly 1 call — no retry
+        assert :meck.num_calls(Hackney, :get, :_) == 1
+      end
+    end
+
+    test "uses the module configured as :http_client" do
+      Application.put_env(:waffle, :http_client, Hackney)
+
+      with_mock Hackney,
+        get: fn _url, _headers, _opts ->
+          {:ok, "image data"}
+        end do
+        Waffle.File.new("http://example.com/image.jpg", DummyDefinition)
+        assert called(Hackney.get(:_, :_, :_))
+      end
+    end
+  end
 end
diff --git a/test/http_client/finch_test.exs b/test/http_client/finch_test.exs
new file mode 100644
index 0000000..ed4a7d5
--- /dev/null
+++ b/test/http_client/finch_test.exs
@@ -0,0 +1,98 @@
+defmodule WaffleTest.HTTPClient.Finch do
+  use ExUnit.Case, async: false
+  import Mock
+
+  defp response(status, headers \\ [], body \\ "") do
+    %Finch.Response{status: status, headers: headers, body: body}
+  end
+
+  describe "get/3" do
+    test "returns {:ok, body} on 200 with no content-disposition header" do
+      with_mock Finch, [:passthrough],
+        request: fn _req, _pool, _opts -> {:ok, response(200, [], "file content")} end do
+        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+        assert result == {:ok, "file content"}
+      end
+    end
+
+    test "returns {:ok, body, filename} when content-disposition header is present" do
+      headers = [{"content-disposition", ~s(attachment; filename="photo.jpg")}]
+
+      with_mock Finch, [:passthrough],
+        request: fn _req, _pool, _opts -> {:ok, response(200, headers, "file content")} end do
+        result = Waffle.HTTPClient.Finch.get("http://example.com/file", [], [])
+        assert result == {:ok, "file content", "photo.jpg"}
+      end
+    end
+
+    test "returns {:error, {:http_error, :body_too_large}} when body exceeds max_body_length" do
+      with_mock Finch, [:passthrough],
+        request: fn _req, _pool, _opts -> {:ok, response(200, [], "long body")} end do
+        result =
+          Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], max_body_length: 4)
+
+        assert result == {:error, {:http_error, :body_too_large}}
+      end
+    end
+
+    test "returns {:error, :service_unavailable} on 503" do
+      with_mock Finch, [:passthrough],
+        request: fn _req, _pool, _opts -> {:ok, response(503)} end do
+        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, :service_unavailable}
+      end
+    end
+
+    test "returns {:error, {:http_error, :unexpected_status}} on non-200/503 status" do
+      with_mock Finch, [:passthrough],
+        request: fn _req, _pool, _opts -> {:ok, response(404)} end do
+        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, {:http_error, :unexpected_status}}
+      end
+    end
+
+    test "returns {:error, :timeout} on timeout exception" do
+      with_mock Finch, [:passthrough],
+        request: fn _req, _pool, _opts ->
+          {:error, %Mint.TransportError{reason: :timeout}}
+        end do
+        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, :timeout}
+      end
+    end
+
+    test "returns {:error, {:http_error, reason}} on other transport errors" do
+      with_mock Finch, [:passthrough],
+        request: fn _req, _pool, _opts ->
+          {:error, %Mint.TransportError{reason: :econnrefused}}
+        end do
+        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, {:http_error, %Mint.TransportError{reason: :econnrefused}}}
+      end
+    end
+
+    test "passes recv_timeout as receive_timeout to Finch" do
+      with_mock Finch, [:passthrough],
+        request: fn _req, _pool, opts ->
+          assert Keyword.get(opts, :receive_timeout) == 3_000
+          {:ok, response(200, [], "body")}
+        end do
+        Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], recv_timeout: 3_000)
+      end
+    end
+
+    test "uses configured pool_name" do
+      Application.put_env(:waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch)
+
+      on_exit(fn -> Application.delete_env(:waffle, Waffle.HTTPClient.Finch) end)
+
+      with_mock Finch, [:passthrough],
+        request: fn _req, pool, _opts ->
+          assert pool == MyApp.Finch
+          {:ok, response(200, [], "body")}
+        end do
+        Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+      end
+    end
+  end
+end
diff --git a/test/http_client/hackney_test.exs b/test/http_client/hackney_test.exs
new file mode 100644
index 0000000..5720cbe
--- /dev/null
+++ b/test/http_client/hackney_test.exs
@@ -0,0 +1,96 @@
+defmodule WaffleTest.HTTPClient.Hackney do
+  use ExUnit.Case, async: false
+  import Mock
+
+  alias Waffle.HTTPClient.Hackney
+
+  describe "get/3" do
+    test "returns {:ok, body} on 200 with no content-disposition header" do
+      with_mock :hackney,
+        get: fn _url, _headers, "", _opts -> {:ok, 200, [], :client_ref} end,
+        body: fn :client_ref, :infinity -> {:ok, "file content"} end do
+        result = Hackney.get("http://example.com/file.jpg", [], [])
+        assert result == {:ok, "file content"}
+      end
+    end
+
+    test "returns {:ok, body, filename} when content-disposition header is present" do
+      response_headers = [{"content-disposition", ~s(attachment; filename="photo.jpg")}]
+
+      with_mock :hackney,
+        get: fn _url, _headers, "", _opts -> {:ok, 200, response_headers, :client_ref} end,
+        body: fn :client_ref, :infinity -> {:ok, "file content"} end do
+        result = Hackney.get("http://example.com/file", [], [])
+        assert result == {:ok, "file content", "photo.jpg"}
+      end
+    end
+
+    test "returns {:error, :service_unavailable} on 503" do
+      with_mock :hackney,
+        get: fn _url, _headers, "", _opts -> {:ok, 503, [], :client_ref} end,
+        close: fn :client_ref -> :ok end do
+        result = Hackney.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, :service_unavailable}
+      end
+    end
+
+    test "returns {:error, {:http_error, :unexpected_status}} on non-200/503 status" do
+      with_mock :hackney,
+        get: fn _url, _headers, "", _opts -> {:ok, 404, [], :client_ref} end,
+        close: fn :client_ref -> :ok end do
+        result = Hackney.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, {:http_error, :unexpected_status}}
+      end
+    end
+
+    test "returns {:error, :timeout} on hackney timeout map" do
+      with_mock :hackney,
+        get: fn _url, _headers, "", _opts -> {:error, %{reason: :timeout}} end do
+        result = Hackney.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, :timeout}
+      end
+    end
+
+    test "returns {:error, :timeout} on :timeout atom" do
+      with_mock :hackney,
+        get: fn _url, _headers, "", _opts -> {:error, :timeout} end do
+        result = Hackney.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, :timeout}
+      end
+    end
+
+    test "returns {:error, {:http_error, reason}} on other errors" do
+      with_mock :hackney,
+        get: fn _url, _headers, "", _opts -> {:error, :econnrefused} end do
+        result = Hackney.get("http://example.com/file.jpg", [], [])
+        assert result == {:error, {:http_error, :econnrefused}}
+      end
+    end
+
+    test "passes recv_timeout and connect_timeout to hackney" do
+      with_mock :hackney,
+        get: fn _url, _headers, "", opts ->
+          assert Keyword.get(opts, :recv_timeout) == 3_000
+          assert Keyword.get(opts, :connect_timeout) == 8_000
+          {:ok, 200, [], :client_ref}
+        end,
+        body: fn :client_ref, :infinity -> {:ok, "body"} end do
+        Hackney.get("http://example.com/file.jpg", [],
+          recv_timeout: 3_000,
+          connect_timeout: 8_000
+        )
+      end
+    end
+
+    test "passes max_body_length to hackney body read" do
+      with_mock :hackney,
+        get: fn _url, _headers, "", _opts -> {:ok, 200, [], :client_ref} end,
+        body: fn :client_ref, max ->
+          assert max == 1024
+          {:ok, "body"}
+        end do
+        Hackney.get("http://example.com/file.jpg", [], max_body_length: 1024)
+      end
+    end
+  end
+end
diff --git a/test/http_client/http_client_test.exs b/test/http_client/http_client_test.exs
new file mode 100644
index 0000000..100d40d
--- /dev/null
+++ b/test/http_client/http_client_test.exs
@@ -0,0 +1,32 @@
+defmodule WaffleTest.HTTPClient do
+  use ExUnit.Case, async: true
+
+  describe "parse_content_disposition/1" do
+    test "parses a quoted filename" do
+      result = Waffle.HTTPClient.parse_content_disposition(~s(attachment; filename="photo.jpg"))
+      assert result == "photo.jpg"
+    end
+
+    test "parses an unquoted filename" do
+      result = Waffle.HTTPClient.parse_content_disposition("attachment; filename=photo.jpg")
+      assert result == "photo.jpg"
+    end
+
+    test "parses a filename when multiple parameters are present" do
+      result =
+        Waffle.HTTPClient.parse_content_disposition(
+          ~s(attachment; filename="report.pdf"; size=1024)
+        )
+
+      assert result == "report.pdf"
+    end
+
+    test "returns nil when no filename parameter is present" do
+      assert Waffle.HTTPClient.parse_content_disposition("attachment") == nil
+    end
+
+    test "returns nil for inline disposition without filename" do
+      assert Waffle.HTTPClient.parse_content_disposition("inline") == nil
+    end
+  end
+end

From 1fca1a0c58fb38f6334d2f8498f1b867714649ce Mon Sep 17 00:00:00 2001
From: klacointe <kevinlacointe@gmail.com>
Date: Tue, 16 Jun 2026 11:46:31 +0200
Subject: [PATCH 2/5] feat: address review feedback on pluggable HTTP client

- Remove hackney 4 from docs (incompatible API, no max_body_length)
- Keep hackney as a non-optional dep for backward compatibility
- Restore :recv_timeout error for hackney atom :timeout (backward compat)
- Add :recv_timeout to retryable errors in Waffle.File
- Rewrite Finch client to use stream_while/5: enforces max_body_length
  mid-stream to avoid OOM on large responses
- Include HTTP status code in unexpected_status error tuple
- Wrap Finch call in try/rescue for exception safety
- Guard finch.ex and finch_test.exs with Code.ensure_loaded?(Finch)
  so the library compiles without the optional dep
---
 lib/waffle/behaviors/http_client_behaviour.ex |   2 +-
 lib/waffle/file.ex                            |   2 +-
 lib/waffle/http_client/finch.ex               | 139 ++++++++-------
 lib/waffle/http_client/hackney.ex             |   6 +-
 mix.exs                                       |   2 +-
 test/actions/store_test.exs                   |   4 +-
 test/http_client/finch_test.exs               | 164 ++++++++++--------
 test/http_client/hackney_test.exs             |   4 +-
 8 files changed, 179 insertions(+), 144 deletions(-)

diff --git a/lib/waffle/behaviors/http_client_behaviour.ex b/lib/waffle/behaviors/http_client_behaviour.ex
index 6140ae5..954739d 100644
--- a/lib/waffle/behaviors/http_client_behaviour.ex
+++ b/lib/waffle/behaviors/http_client_behaviour.ex
@@ -5,7 +5,7 @@ defmodule Waffle.HTTPClient do
   ## Built-in implementations
 
   - `Waffle.HTTPClient.Hackney` — default, uses `:hackney`. Add `{:hackney, "~> 1.9"}` to
-    your deps (or `{:hackney, ">= 4.0.1"}` for the CVE-free version).
+    your deps.
   - `Waffle.HTTPClient.Finch` — uses `Finch`. Add `{:finch, "~> 0.18"}` to your deps,
     start a `Finch` pool in your application supervision tree, and configure
     `config :waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch`.
diff --git a/lib/waffle/file.ex b/lib/waffle/file.ex
index 9c4dd75..60c4562 100644
--- a/lib/waffle/file.ex
+++ b/lib/waffle/file.ex
@@ -212,7 +212,7 @@ defmodule Waffle.File do
       {:ok, body, filename} ->
         {:ok, body, filename}
 
-      {:error, reason} when reason in [:timeout, :service_unavailable] ->
+      {:error, reason} when reason in [:timeout, :recv_timeout, :service_unavailable] ->
         retry_or_error(remote_path, headers, options, tries, reason)
 
       {:error, _} = err ->
diff --git a/lib/waffle/http_client/finch.ex b/lib/waffle/http_client/finch.ex
index 4a8c4d0..0ceaa3c 100644
--- a/lib/waffle/http_client/finch.ex
+++ b/lib/waffle/http_client/finch.ex
@@ -1,89 +1,108 @@
-defmodule Waffle.HTTPClient.Finch do
-  @moduledoc """
-  HTTP client implementation using `Finch`.
+if Code.ensure_loaded?(Finch) do
+  defmodule Waffle.HTTPClient.Finch do
+    @moduledoc """
+    HTTP client implementation using `Finch`.
 
-  ## Setup
+    ## Setup
 
-  Add `finch` to your dependencies:
+    Add `finch` to your dependencies:
 
-      {:finch, "~> 0.18"}
+        {:finch, "~> 0.18"}
 
-  Start a `Finch` pool in your application supervision tree:
+    Start a `Finch` pool in your application supervision tree:
 
-      children = [
-        {Finch, name: MyApp.Finch}
-      ]
+        children = [
+          {Finch, name: MyApp.Finch}
+        ]
 
-  Then configure Waffle to use this client and point it at your pool:
+    Then configure Waffle to use this client and point it at your pool:
 
-      config :waffle, :http_client, Waffle.HTTPClient.Finch
-      config :waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch
+        config :waffle, :http_client, Waffle.HTTPClient.Finch
+        config :waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch
 
-  ## Options
+    ## Options
 
-  | Option             | Default      | Description                                            |
-  |--------------------|--------------|--------------------------------------------------------|
-  | `:recv_timeout`    | `5_000`      | Timeout for receiving a response, in milliseconds      |
-  | `:connect_timeout` | `10_000`     | Timeout for establishing a connection, in milliseconds |
-  | `:max_body_length` | `:infinity`  | Maximum response body size, in bytes                   |
-  """
+    | Option             | Default      | Description                                            |
+    |--------------------|--------------|--------------------------------------------------------|
+    | `:recv_timeout`    | `5_000`      | Timeout for receiving a response, in milliseconds      |
+    | `:connect_timeout` | `10_000`     | Timeout for establishing a connection, in milliseconds |
+    | `:max_body_length` | `:infinity`  | Maximum response body size, in bytes                   |
+    """
 
-  @behaviour Waffle.HTTPClient
+    @behaviour Waffle.HTTPClient
 
-  @impl Waffle.HTTPClient
-  def get(url, headers, options) do
-    finch_config = Application.get_env(:waffle, Waffle.HTTPClient.Finch, [])
-    finch_name = Keyword.get(finch_config, :pool_name, Waffle.Finch)
-    recv_timeout = Keyword.get(options, :recv_timeout, 5_000)
-    connect_timeout = Keyword.get(options, :connect_timeout, 10_000)
-    max_body_length = Keyword.get(options, :max_body_length, :infinity)
+    @impl Waffle.HTTPClient
+    def get(url, headers, options) do
+      finch_config = Application.get_env(:waffle, Waffle.HTTPClient.Finch, [])
+      finch_name = Keyword.get(finch_config, :pool_name, Waffle.Finch)
+      recv_timeout = Keyword.get(options, :recv_timeout, 5_000)
+      connect_timeout = Keyword.get(options, :connect_timeout, 10_000)
+      max_body_length = Keyword.get(options, :max_body_length, :infinity)
 
-    finch_options = [
-      receive_timeout: recv_timeout,
-      connect_options: [timeout: connect_timeout]
-    ]
+      finch_options = [
+        receive_timeout: recv_timeout,
+        connect_options: [timeout: connect_timeout]
+      ]
 
-    request = Finch.build(:get, url, headers)
+      request = Finch.build(:get, url, headers)
+      acc = %{status: nil, headers: [], body: [], bytes: 0}
 
-    case Finch.request(request, finch_name, finch_options) do
-      {:ok, %Finch.Response{status: 200, headers: response_headers, body: body}} ->
-        handle_success(body, response_headers, max_body_length)
+      try do
+        request
+        |> Finch.stream_while(finch_name, acc, stream_fun(max_body_length), finch_options)
+        |> handle_result()
+      rescue
+        exception -> {:error, {:http_error, exception}}
+      end
+    end
 
-      {:ok, %Finch.Response{status: 503}} ->
-        {:error, :service_unavailable}
+    defp stream_fun(max_body_length) do
+      fn
+        {:status, s}, acc -> {:cont, %{acc | status: s}}
+        {:headers, h}, acc -> {:cont, %{acc | headers: h}}
+        {:trailers, _}, acc -> {:cont, acc}
+        {:data, chunk}, acc -> accumulate_chunk(chunk, acc, max_body_length)
+      end
+    end
 
-      {:ok, %Finch.Response{}} ->
-        {:error, {:http_error, :unexpected_status}}
+    defp accumulate_chunk(chunk, acc, max_body_length) do
+      new_bytes = acc.bytes + byte_size(chunk)
 
-      {:error, reason} ->
-        classify_error(reason)
+      if max_body_length != :infinity and new_bytes > max_body_length,
+        do: {:halt, %{acc | body: :over_limit}},
+        else: {:cont, %{acc | body: [chunk | acc.body], bytes: new_bytes}}
     end
-  end
 
-  defp handle_success(body, response_headers, max_body_length) do
-    if over_limit?(body, max_body_length) do
+    defp handle_result({:ok, %{status: 200, body: :over_limit}}) do
       {:error, {:http_error, :body_too_large}}
-    else
-      filename = find_content_disposition_filename(response_headers)
+    end
+
+    defp handle_result({:ok, %{status: 200, headers: resp_headers, body: parts}}) do
+      body = parts |> Enum.reverse() |> IO.iodata_to_binary()
+      filename = find_content_disposition_filename(resp_headers)
       if filename, do: {:ok, body, filename}, else: {:ok, body}
     end
-  end
 
-  defp classify_error(%{reason: :timeout}), do: {:error, :timeout}
+    defp handle_result({:ok, %{status: 503}}), do: {:error, :service_unavailable}
 
-  defp classify_error(exception) when is_exception(exception),
-    do: {:error, {:http_error, exception}}
+    defp handle_result({:ok, %{status: status}}),
+      do: {:error, {:http_error, {:unexpected_status, status}}}
 
-  defp classify_error(reason), do: {:error, {:http_error, reason}}
+    defp handle_result({:error, reason, _acc}), do: classify_error(reason)
 
-  defp over_limit?(_body, :infinity), do: false
-  defp over_limit?(body, max), do: byte_size(body) > max
+    defp classify_error(%{reason: :timeout}), do: {:error, :timeout}
 
-  defp find_content_disposition_filename(headers) do
-    Enum.find_value(headers, fn {key, value} ->
-      if String.downcase(key) == "content-disposition" do
-        Waffle.HTTPClient.parse_content_disposition(value)
-      end
-    end)
+    defp classify_error(exception) when is_exception(exception),
+      do: {:error, {:http_error, exception}}
+
+    defp classify_error(reason), do: {:error, {:http_error, reason}}
+
+    defp find_content_disposition_filename(headers) do
+      Enum.find_value(headers, fn {key, value} ->
+        if String.downcase(key) == "content-disposition" do
+          Waffle.HTTPClient.parse_content_disposition(value)
+        end
+      end)
+    end
   end
 end
diff --git a/lib/waffle/http_client/hackney.ex b/lib/waffle/http_client/hackney.ex
index 1d4208c..27b1837 100644
--- a/lib/waffle/http_client/hackney.ex
+++ b/lib/waffle/http_client/hackney.ex
@@ -6,10 +6,6 @@ defmodule Waffle.HTTPClient.Hackney do
 
       {:hackney, "~> 1.9"}
 
-  Or, to use the CVE-free version (requires OTP 26+):
-
-      {:hackney, ">= 4.0.1"}
-
   ## Configuration
 
       config :waffle, :http_client, Waffle.HTTPClient.Hackney
@@ -56,7 +52,7 @@ defmodule Waffle.HTTPClient.Hackney do
         {:error, :timeout}
 
       {:error, :timeout} ->
-        {:error, :timeout}
+        {:error, :recv_timeout}
 
       {:error, reason} ->
         {:error, {:http_error, reason}}
diff --git a/mix.exs b/mix.exs
index 6db36d8..d9843f1 100644
--- a/mix.exs
+++ b/mix.exs
@@ -59,7 +59,7 @@ defmodule Waffle.Mixfile do
   defp deps do
     [
       # Optional HTTP clients (at least one is required for downloading remote files)
-      {:hackney, "~> 1.9", optional: true},
+      {:hackney, "~> 1.9"},
       {:finch, "~> 0.18", optional: true},
 
       # If using Amazon S3
diff --git a/test/actions/store_test.exs b/test/actions/store_test.exs
index 9a5ecfa..dd8133f 100644
--- a/test/actions/store_test.exs
+++ b/test/actions/store_test.exs
@@ -154,7 +154,7 @@ defmodule WaffleTest.Actions.Store do
         {:ok, "favicon.ico"}
       end do
       assert DummyDefinition.store("https://www.google.com/favicon.ico") ==
-               {:error, :timeout}
+               {:error, :recv_timeout}
     end
   end
 
@@ -176,7 +176,7 @@ defmodule WaffleTest.Actions.Store do
                remote_path: "https://www.google.com/favicon.ico",
                filename: "newfavicon.ico"
              }) ==
-               {:error, :timeout}
+               {:error, :recv_timeout}
     end
   end
 
diff --git a/test/http_client/finch_test.exs b/test/http_client/finch_test.exs
index ed4a7d5..19551fd 100644
--- a/test/http_client/finch_test.exs
+++ b/test/http_client/finch_test.exs
@@ -1,97 +1,117 @@
-defmodule WaffleTest.HTTPClient.Finch do
-  use ExUnit.Case, async: false
-  import Mock
+if Code.ensure_loaded?(Finch) do
+  defmodule WaffleTest.HTTPClient.Finch do
+    use ExUnit.Case, async: false
+    import Mock
 
-  defp response(status, headers \\ [], body \\ "") do
-    %Finch.Response{status: status, headers: headers, body: body}
-  end
+    defp simulate_stream(acc, fun, status, headers, body_chunks) do
+      {:cont, acc} = fun.({:status, status}, acc)
+      {:cont, acc} = fun.({:headers, headers}, acc)
 
-  describe "get/3" do
-    test "returns {:ok, body} on 200 with no content-disposition header" do
-      with_mock Finch, [:passthrough],
-        request: fn _req, _pool, _opts -> {:ok, response(200, [], "file content")} end do
-        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
-        assert result == {:ok, "file content"}
-      end
+      Enum.reduce_while(body_chunks, {:ok, acc}, fn chunk, {:ok, acc} ->
+        case fun.({:data, chunk}, acc) do
+          {:cont, acc} -> {:cont, {:ok, acc}}
+          {:halt, acc} -> {:halt, {:ok, acc}}
+        end
+      end)
     end
 
-    test "returns {:ok, body, filename} when content-disposition header is present" do
-      headers = [{"content-disposition", ~s(attachment; filename="photo.jpg")}]
+    describe "get/3" do
+      test "returns {:ok, body} on 200 with no content-disposition header" do
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, fun, _opts ->
+            simulate_stream(acc, fun, 200, [], ["file content"])
+          end do
+          result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+          assert result == {:ok, "file content"}
+        end
+      end
+
+      test "returns {:ok, body, filename} when content-disposition header is present" do
+        headers = [{"content-disposition", ~s(attachment; filename="photo.jpg")}]
 
-      with_mock Finch, [:passthrough],
-        request: fn _req, _pool, _opts -> {:ok, response(200, headers, "file content")} end do
-        result = Waffle.HTTPClient.Finch.get("http://example.com/file", [], [])
-        assert result == {:ok, "file content", "photo.jpg"}
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, fun, _opts ->
+            simulate_stream(acc, fun, 200, headers, ["file content"])
+          end do
+          result = Waffle.HTTPClient.Finch.get("http://example.com/file", [], [])
+          assert result == {:ok, "file content", "photo.jpg"}
+        end
       end
-    end
 
-    test "returns {:error, {:http_error, :body_too_large}} when body exceeds max_body_length" do
-      with_mock Finch, [:passthrough],
-        request: fn _req, _pool, _opts -> {:ok, response(200, [], "long body")} end do
-        result =
-          Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], max_body_length: 4)
+      test "returns {:error, {:http_error, :body_too_large}} when body exceeds max_body_length" do
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, fun, _opts ->
+            simulate_stream(acc, fun, 200, [], ["long body"])
+          end do
+          result =
+            Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], max_body_length: 4)
 
-        assert result == {:error, {:http_error, :body_too_large}}
+          assert result == {:error, {:http_error, :body_too_large}}
+        end
       end
-    end
 
-    test "returns {:error, :service_unavailable} on 503" do
-      with_mock Finch, [:passthrough],
-        request: fn _req, _pool, _opts -> {:ok, response(503)} end do
-        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
-        assert result == {:error, :service_unavailable}
+      test "returns {:error, :service_unavailable} on 503" do
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, fun, _opts ->
+            simulate_stream(acc, fun, 503, [], [])
+          end do
+          result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+          assert result == {:error, :service_unavailable}
+        end
       end
-    end
 
-    test "returns {:error, {:http_error, :unexpected_status}} on non-200/503 status" do
-      with_mock Finch, [:passthrough],
-        request: fn _req, _pool, _opts -> {:ok, response(404)} end do
-        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
-        assert result == {:error, {:http_error, :unexpected_status}}
+      test "returns {:error, {:http_error, {:unexpected_status, status}}} on non-200/503 status" do
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, fun, _opts ->
+            simulate_stream(acc, fun, 404, [], [])
+          end do
+          result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+          assert result == {:error, {:http_error, {:unexpected_status, 404}}}
+        end
       end
-    end
 
-    test "returns {:error, :timeout} on timeout exception" do
-      with_mock Finch, [:passthrough],
-        request: fn _req, _pool, _opts ->
-          {:error, %Mint.TransportError{reason: :timeout}}
-        end do
-        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
-        assert result == {:error, :timeout}
+      test "returns {:error, :timeout} on timeout" do
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, _fun, _opts ->
+            {:error, %Mint.TransportError{reason: :timeout}, acc}
+          end do
+          result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+          assert result == {:error, :timeout}
+        end
       end
-    end
 
-    test "returns {:error, {:http_error, reason}} on other transport errors" do
-      with_mock Finch, [:passthrough],
-        request: fn _req, _pool, _opts ->
-          {:error, %Mint.TransportError{reason: :econnrefused}}
-        end do
-        result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
-        assert result == {:error, {:http_error, %Mint.TransportError{reason: :econnrefused}}}
+      test "returns {:error, {:http_error, reason}} on other transport errors" do
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, _fun, _opts ->
+            {:error, %Mint.TransportError{reason: :econnrefused}, acc}
+          end do
+          result = Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+          assert result == {:error, {:http_error, %Mint.TransportError{reason: :econnrefused}}}
+        end
       end
-    end
 
-    test "passes recv_timeout as receive_timeout to Finch" do
-      with_mock Finch, [:passthrough],
-        request: fn _req, _pool, opts ->
-          assert Keyword.get(opts, :receive_timeout) == 3_000
-          {:ok, response(200, [], "body")}
-        end do
-        Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], recv_timeout: 3_000)
+      test "passes recv_timeout as receive_timeout to Finch" do
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, fun, opts ->
+            assert Keyword.get(opts, :receive_timeout) == 3_000
+            simulate_stream(acc, fun, 200, [], ["body"])
+          end do
+          Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], recv_timeout: 3_000)
+        end
       end
-    end
 
-    test "uses configured pool_name" do
-      Application.put_env(:waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch)
+      test "uses configured pool_name" do
+        Application.put_env(:waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch)
 
-      on_exit(fn -> Application.delete_env(:waffle, Waffle.HTTPClient.Finch) end)
+        on_exit(fn -> Application.delete_env(:waffle, Waffle.HTTPClient.Finch) end)
 
-      with_mock Finch, [:passthrough],
-        request: fn _req, pool, _opts ->
-          assert pool == MyApp.Finch
-          {:ok, response(200, [], "body")}
-        end do
-        Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, pool, acc, fun, _opts ->
+            assert pool == MyApp.Finch
+            simulate_stream(acc, fun, 200, [], ["body"])
+          end do
+          Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
+        end
       end
     end
   end
diff --git a/test/http_client/hackney_test.exs b/test/http_client/hackney_test.exs
index 5720cbe..7f68f6d 100644
--- a/test/http_client/hackney_test.exs
+++ b/test/http_client/hackney_test.exs
@@ -51,11 +51,11 @@ defmodule WaffleTest.HTTPClient.Hackney do
       end
     end
 
-    test "returns {:error, :timeout} on :timeout atom" do
+    test "returns {:error, :recv_timeout} on :timeout atom" do
       with_mock :hackney,
         get: fn _url, _headers, "", _opts -> {:error, :timeout} end do
         result = Hackney.get("http://example.com/file.jpg", [], [])
-        assert result == {:error, :timeout}
+        assert result == {:error, :recv_timeout}
       end
     end
 

From 8fb8043bd335e20e3b2cc6dae3d56b7040042764 Mon Sep 17 00:00:00 2001
From: klacointe <kevinlacointe@gmail.com>
Date: Tue, 16 Jun 2026 18:30:44 +0200
Subject: [PATCH 3/5] feat: address round 2 review feedback on Finch client

- Remove connect_options from Finch request options (not a valid
  Finch request_opt; connection settings belong at pool startup)
- Document that connect_timeout and follow_redirect are unsupported
  by the Finch client
- Add comment in hackney.ex explaining the two distinct timeout shapes
  hackney returns (map for connect, atom for recv)
---
 lib/waffle/http_client/finch.ex   | 13 +++++++------
 lib/waffle/http_client/hackney.ex |  1 +
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/waffle/http_client/finch.ex b/lib/waffle/http_client/finch.ex
index 0ceaa3c..cbbc653 100644
--- a/lib/waffle/http_client/finch.ex
+++ b/lib/waffle/http_client/finch.ex
@@ -25,8 +25,13 @@ if Code.ensure_loaded?(Finch) do
     | Option             | Default      | Description                                            |
     |--------------------|--------------|--------------------------------------------------------|
     | `:recv_timeout`    | `5_000`      | Timeout for receiving a response, in milliseconds      |
-    | `:connect_timeout` | `10_000`     | Timeout for establishing a connection, in milliseconds |
     | `:max_body_length` | `:infinity`  | Maximum response body size, in bytes                   |
+
+    > #### Unsupported options {: .info}
+    >
+    > `:connect_timeout` and `:follow_redirect` are not supported — Finch does not
+    > expose per-request connection settings or automatic redirect handling. Configure
+    > connection options (e.g. timeouts, TLS) at pool startup via `Finch.start_link/1`.
     """
 
     @behaviour Waffle.HTTPClient
@@ -36,13 +41,9 @@ if Code.ensure_loaded?(Finch) do
       finch_config = Application.get_env(:waffle, Waffle.HTTPClient.Finch, [])
       finch_name = Keyword.get(finch_config, :pool_name, Waffle.Finch)
       recv_timeout = Keyword.get(options, :recv_timeout, 5_000)
-      connect_timeout = Keyword.get(options, :connect_timeout, 10_000)
       max_body_length = Keyword.get(options, :max_body_length, :infinity)
 
-      finch_options = [
-        receive_timeout: recv_timeout,
-        connect_options: [timeout: connect_timeout]
-      ]
+      finch_options = [receive_timeout: recv_timeout]
 
       request = Finch.build(:get, url, headers)
       acc = %{status: nil, headers: [], body: [], bytes: 0}
diff --git a/lib/waffle/http_client/hackney.ex b/lib/waffle/http_client/hackney.ex
index 27b1837..814bfa7 100644
--- a/lib/waffle/http_client/hackney.ex
+++ b/lib/waffle/http_client/hackney.ex
@@ -48,6 +48,7 @@ defmodule Waffle.HTTPClient.Hackney do
         :hackney.close(client_ref)
         {:error, {:http_error, :unexpected_status}}
 
+      # hackney returns a map for connect timeout and a bare atom for recv timeout
       {:error, %{reason: :timeout}} ->
         {:error, :timeout}
 

From a9efa4d6435c0a9c8a45cdd28cec53cb76bdc7ec Mon Sep 17 00:00:00 2001
From: klacointe <kevinlacointe@gmail.com>
Date: Tue, 16 Jun 2026 18:40:08 +0200
Subject: [PATCH 4/5] feat: add follow_redirect support to
 Waffle.HTTPClient.Finch

Finch has no built-in redirect handling. Since waffle only issues GET
requests, implement a redirect loop manually: detect 3xx, resolve the
Location header via URI.merge/2 (handles relative URLs), and re-issue
the request up to :max_redirects hops (default 5).
---
 lib/waffle/http_client/finch.ex | 49 ++++++++++++++++++++++++-------
 test/http_client/finch_test.exs | 52 +++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 10 deletions(-)

diff --git a/lib/waffle/http_client/finch.ex b/lib/waffle/http_client/finch.ex
index cbbc653..ddafc69 100644
--- a/lib/waffle/http_client/finch.ex
+++ b/lib/waffle/http_client/finch.ex
@@ -26,12 +26,13 @@ if Code.ensure_loaded?(Finch) do
     |--------------------|--------------|--------------------------------------------------------|
     | `:recv_timeout`    | `5_000`      | Timeout for receiving a response, in milliseconds      |
     | `:max_body_length` | `:infinity`  | Maximum response body size, in bytes                   |
+    | `:max_redirects`   | `5`          | Maximum number of redirects to follow (`0` disables)   |
 
     > #### Unsupported options {: .info}
     >
-    > `:connect_timeout` and `:follow_redirect` are not supported — Finch does not
-    > expose per-request connection settings or automatic redirect handling. Configure
-    > connection options (e.g. timeouts, TLS) at pool startup via `Finch.start_link/1`.
+    > `:connect_timeout` is not supported at the request level — Finch does not expose
+    > per-request connection settings. Configure connection options (e.g. timeouts, TLS)
+    > at pool startup via `Finch.start_link/1`.
     """
 
     @behaviour Waffle.HTTPClient
@@ -51,7 +52,7 @@ if Code.ensure_loaded?(Finch) do
       try do
         request
         |> Finch.stream_while(finch_name, acc, stream_fun(max_body_length), finch_options)
-        |> handle_result()
+        |> handle_result(url, headers, options)
       rescue
         exception -> {:error, {:http_error, exception}}
       end
@@ -74,22 +75,50 @@ if Code.ensure_loaded?(Finch) do
         else: {:cont, %{acc | body: [chunk | acc.body], bytes: new_bytes}}
     end
 
-    defp handle_result({:ok, %{status: 200, body: :over_limit}}) do
+    defp handle_result({:ok, %{status: 200, body: :over_limit}}, _url, _headers, _options) do
       {:error, {:http_error, :body_too_large}}
     end
 
-    defp handle_result({:ok, %{status: 200, headers: resp_headers, body: parts}}) do
+    defp handle_result(
+           {:ok, %{status: 200, headers: resp_headers, body: parts}},
+           _url,
+           _headers,
+           _options
+         ) do
       body = parts |> Enum.reverse() |> IO.iodata_to_binary()
       filename = find_content_disposition_filename(resp_headers)
       if filename, do: {:ok, body, filename}, else: {:ok, body}
     end
 
-    defp handle_result({:ok, %{status: 503}}), do: {:error, :service_unavailable}
+    defp handle_result({:ok, %{status: 503}}, _url, _headers, _options) do
+      {:error, :service_unavailable}
+    end
+
+    defp handle_result({:ok, %{status: status, headers: resp_headers}}, url, headers, options)
+         when status in 300..399 do
+      max_redirects = Keyword.get(options, :max_redirects, 5)
 
-    defp handle_result({:ok, %{status: status}}),
-      do: {:error, {:http_error, {:unexpected_status, status}}}
+      if max_redirects == 0 do
+        {:error, {:http_error, :too_many_redirects}}
+      else
+        case List.keyfind(resp_headers, "location", 0) do
+          {_, location} ->
+            new_url = url |> URI.merge(location) |> URI.to_string()
+            get(new_url, headers, Keyword.put(options, :max_redirects, max_redirects - 1))
 
-    defp handle_result({:error, reason, _acc}), do: classify_error(reason)
+          nil ->
+            {:error, {:http_error, {:unexpected_status, status}}}
+        end
+      end
+    end
+
+    defp handle_result({:ok, %{status: status}}, _url, _headers, _options) do
+      {:error, {:http_error, {:unexpected_status, status}}}
+    end
+
+    defp handle_result({:error, reason, _acc}, _url, _headers, _options) do
+      classify_error(reason)
+    end
 
     defp classify_error(%{reason: :timeout}), do: {:error, :timeout}
 
diff --git a/test/http_client/finch_test.exs b/test/http_client/finch_test.exs
index 19551fd..8bff826 100644
--- a/test/http_client/finch_test.exs
+++ b/test/http_client/finch_test.exs
@@ -113,6 +113,58 @@ if Code.ensure_loaded?(Finch) do
           Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], [])
         end
       end
+
+      test "follows a redirect and returns the final response" do
+        redirect_headers = [{"location", "http://example.com/final.jpg"}]
+
+        with_mock Finch, [:passthrough],
+          stream_while: fn req, _pool, acc, fun, _opts ->
+            if req.path == "/file.jpg",
+              do: simulate_stream(acc, fun, 301, redirect_headers, []),
+              else: simulate_stream(acc, fun, 200, [], ["body"])
+          end do
+          assert Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], []) ==
+                   {:ok, "body"}
+        end
+      end
+
+      test "follows a relative redirect" do
+        redirect_headers = [{"location", "/final.jpg"}]
+
+        with_mock Finch, [:passthrough],
+          stream_while: fn req, _pool, acc, fun, _opts ->
+            if req.path == "/file.jpg",
+              do: simulate_stream(acc, fun, 302, redirect_headers, []),
+              else: simulate_stream(acc, fun, 200, [], ["body"])
+          end do
+          assert Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], []) ==
+                   {:ok, "body"}
+        end
+      end
+
+      test "returns {:error, {:http_error, :too_many_redirects}} when redirect limit exceeded" do
+        redirect_headers = [{"location", "http://example.com/file.jpg"}]
+
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, fun, _opts ->
+            simulate_stream(acc, fun, 301, redirect_headers, [])
+          end do
+          assert Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], max_redirects: 2) ==
+                   {:error, {:http_error, :too_many_redirects}}
+        end
+      end
+
+      test "does not follow redirects when max_redirects is 0" do
+        redirect_headers = [{"location", "http://example.com/final.jpg"}]
+
+        with_mock Finch, [:passthrough],
+          stream_while: fn _req, _pool, acc, fun, _opts ->
+            simulate_stream(acc, fun, 301, redirect_headers, [])
+          end do
+          assert Waffle.HTTPClient.Finch.get("http://example.com/file.jpg", [], max_redirects: 0) ==
+                   {:error, {:http_error, :too_many_redirects}}
+        end
+      end
     end
   end
 end

From cbedd7c7975ede0680637bddd25c5b79fa22e6db Mon Sep 17 00:00:00 2001
From: klacointe <kevinlacointe@gmail.com>
Date: Tue, 16 Jun 2026 19:42:07 +0200
Subject: [PATCH 5/5] docs: fix missing and unsupported option docs in HTTP
 clients

- Add :follow_redirect to Hackney options table (was passed to hackney
  but undocumented)
- Document that :follow_redirect (boolean) is unsupported by Finch;
  use :max_redirects instead
---
 lib/waffle/http_client/finch.ex   | 5 ++---
 lib/waffle/http_client/hackney.ex | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/waffle/http_client/finch.ex b/lib/waffle/http_client/finch.ex
index ddafc69..de4ccbb 100644
--- a/lib/waffle/http_client/finch.ex
+++ b/lib/waffle/http_client/finch.ex
@@ -30,9 +30,8 @@ if Code.ensure_loaded?(Finch) do
 
     > #### Unsupported options {: .info}
     >
-    > `:connect_timeout` is not supported at the request level — Finch does not expose
-    > per-request connection settings. Configure connection options (e.g. timeouts, TLS)
-    > at pool startup via `Finch.start_link/1`.
+    > - `:connect_timeout` — not supported; configure connection settings (e.g. timeouts, TLS) at pool startup via `Finch.start_link/1`
+    > - `:follow_redirect` — not supported as a boolean; use `:max_redirects` to control redirect behaviour (set to `0` to disable)
     """
 
     @behaviour Waffle.HTTPClient
diff --git a/lib/waffle/http_client/hackney.ex b/lib/waffle/http_client/hackney.ex
index 814bfa7..014b658 100644
--- a/lib/waffle/http_client/hackney.ex
+++ b/lib/waffle/http_client/hackney.ex
@@ -17,6 +17,7 @@ defmodule Waffle.HTTPClient.Hackney do
   | `:recv_timeout`    | `5_000`      | Timeout for receiving a response, in milliseconds      |
   | `:connect_timeout` | `10_000`     | Timeout for establishing a connection, in milliseconds |
   | `:max_body_length` | `:infinity`  | Maximum response body size, in bytes                   |
+  | `:follow_redirect` | `true`       | Whether to follow HTTP redirects automatically         |
   """
 
   @behaviour Waffle.HTTPClient