fix(http): harden Hackney request lifecycle

test/pleroma/http/adapter_helper/hackney_test.exs

@@ -24,6 +24,13 @@ defmodule Pleroma.HTTP.AdapterHelper.HackneyTest do
       assert opts[:with_body] == true
     end
 
+    test "does not allow connection options to enable Hackney redirects", %{uri: uri} do
+      opts = Hackney.options([follow_redirect: true, force_redirect: true], uri)
+
+      assert opts[:follow_redirect] == false
+      assert opts[:force_redirect] == false
+    end
+
     test "add proxy and opts from config", %{uri: uri} do
       opts = Hackney.options([proxy: "localhost:8123"], uri)
 

test/pleroma/http/hackney_follow_redirect_regression_test.exs

@@ -19,26 +19,6 @@ defmodule Pleroma.HTTP.HackneyFollowRedirectRegressionTest do
     {:ok, tls_server: tls_server, proxy: proxy}
   end
 
-  test "hackney follow_redirect handles relative redirects behind CONNECT proxy", %{
-    tls_server: tls_server,
-    proxy: proxy
-  } do
-    url = "#{tls_server.base_url}/redirect"
-
-    opts = [
-      pool: :media,
-      proxy: proxy.proxy_url,
-      ssl_options: tls_client_options(tls_server),
-      connect_timeout: 1_000,
-      recv_timeout: 1_000,
-      follow_redirect: true,
-      force_redirect: true,
-      with_body: true
-    ]
-
-    assert {:ok, 200, _headers, "ok"} = :hackney.request(:get, url, [], <<>>, opts)
-  end
-
   test "redirects work via proxy when hackney follow_redirect is disabled", %{
     tls_server: tls_server,
     proxy: proxy
@@ -127,6 +107,46 @@ defmodule Pleroma.HTTP.HackneyFollowRedirectRegressionTest do
     Pleroma.ReverseProxy.Client.Hackney.close(ref)
   end
 
+  test "hackney reverse proxy cancels timed out asynchronous requests", %{
+    tls_server: tls_server,
+    proxy: proxy
+  } do
+    url = "#{tls_server.base_url}/delayed"
+
+    opts = [
+      pool: :media,
+      proxy: proxy.proxy_url,
+      ssl_options: tls_client_options(tls_server),
+      connect_timeout: 1_000,
+      recv_timeout: 10,
+      follow_redirect: false
+    ]
+
+    assert {:error, :timeout} =
+             Pleroma.ReverseProxy.Client.Hackney.request(:get, url, [], "", opts)
+
+    refute_receive {:hackney_response, _, _}, 150
+  end
+
+  test "hackney reverse proxy normalizes asynchronous connection errors", %{
+    tls_server: tls_server,
+    proxy: proxy
+  } do
+    url = "#{tls_server.base_url}/closed"
+
+    opts = [
+      pool: :media,
+      proxy: proxy.proxy_url,
+      ssl_options: tls_client_options(tls_server),
+      connect_timeout: 1_000,
+      recv_timeout: 1_000,
+      follow_redirect: false
+    ]
+
+    assert {:error, :closed} =
+             Pleroma.ReverseProxy.Client.Hackney.request(:get, url, [], "", opts)
+  end
+
   defp collect_body(ref, acc \\ "") do
     case Pleroma.ReverseProxy.Client.Hackney.stream_body(ref) do
       :done -> acc
@@ -193,6 +213,13 @@ defmodule Pleroma.HTTP.HackneyFollowRedirectRegressionTest do
          {:ok, data} <- recv_ssl_headers(ssl_socket),
          {:ok, path} <- parse_path(data) do
       case path do
+        "/closed" ->
+          :ok
+
+        "/delayed" ->
+          Process.sleep(100)
+          send_ssl_response(ssl_socket, 200, "OK", [], "ok")
+
         "/infinite_redirect" ->
           send_ssl_response(ssl_socket, 302, "Found", [{"Location", "/infinite_redirect"}], "")
 

test/pleroma/reverse_proxy_test.exs

@@ -53,6 +53,42 @@ defmodule Pleroma.ReverseProxyTest do
       assert conn.status == 200
       assert Cachex.get(:failed_proxy_url_cache, url) == {:ok, nil}
     end
+
+    test "uses max_read_duration as the default HTTP receive timeout", %{conn: conn} do
+      ClientMock
+      |> expect(:request, fn :get, "/timeout", _, _, opts ->
+        assert opts[:recv_timeout] == 123
+        {:error, :timeout}
+      end)
+
+      capture_log(fn -> ReverseProxy.call(conn, "/timeout", max_read_duration: 123) end)
+    end
+
+    test "preserves an explicit HTTP receive timeout", %{conn: conn} do
+      ClientMock
+      |> expect(:request, fn :get, "/timeout", _, _, opts ->
+        assert opts[:recv_timeout] == 456
+        {:error, :timeout}
+      end)
+
+      capture_log(fn ->
+        ReverseProxy.call(conn, "/timeout", max_read_duration: 123, http: [recv_timeout: 456])
+      end)
+    end
+
+    test "keeps unlimited read durations unlimited", %{conn: conn} do
+      for {max_read_duration, url} <- [{0, "/zero-timeout"}, {:infinity, "/infinite-timeout"}] do
+        ClientMock
+        |> expect(:request, fn :get, ^url, _, _, opts ->
+          assert opts[:recv_timeout] == :infinity
+          {:error, :timeout}
+        end)
+
+        capture_log(fn ->
+          ReverseProxy.call(conn, url, max_read_duration: max_read_duration)
+        end)
+      end
+    end
   end
 
   test "use Pleroma's user agent in the request; don't pass the client's", %{conn: conn} do