ReverseProxy: Follow redirects recursively until redirect_limit

Test post: https://possum.city/notes/ahqdvbhu3wug0at2

changelog.d/reverseproxy-recursive-redirect.fix

@@ -0,0 +1 @@
+ReverseProxy: Recursively follow redirects until redirect_limit is reached

lib/pleroma/reverse_proxy/client/hackney.ex

@@ -4,6 +4,7 @@
 
 defmodule Pleroma.ReverseProxy.Client.Hackney do
   @behaviour Pleroma.ReverseProxy.Client
+  @redirect_limit 5
 
   # In-app redirect handler to avoid Hackney redirect bugs:
   # - https://github.com/benoitc/hackney/issues/527 (relative/protocol-less redirects can crash Hackney)
@@ -31,26 +32,15 @@ defmodule Pleroma.ReverseProxy.Client.Hackney do
 
     if opts[:follow_redirect] != false do
       {_state, req_opts} = Access.get_and_update(opts, :follow_redirect, fn a -> {a, false} end)
+      env = %{method: method, headers: headers, body: body, req_opts: req_opts}
       res = :hackney.request(method, url, headers, body, req_opts)
 
       case res do
         {:ok, code, resp_headers, _client} when code in @redirect_statuses ->
-          :hackney.request(
+          redirect(url, resp_headers, env, @redirect_limit)
-            method,
-            absolute_redirect_url(url, resp_headers),
-            headers,
-            body,
-            req_opts
-          )
 
         {:ok, code, resp_headers} when code in @redirect_statuses ->
-          :hackney.request(
+          redirect(url, resp_headers, env, @redirect_limit)
-            method,
-            absolute_redirect_url(url, resp_headers),
-            headers,
-            body,
-            req_opts
-          )
 
         _ ->
           res
@@ -71,4 +61,26 @@ defmodule Pleroma.ReverseProxy.Client.Hackney do
 
   @impl true
   def close(ref), do: :hackney.close(ref)
+
+  defp redirect(url, resp_headers, env, limit) when limit == 0 do
+    new_url = absolute_redirect_url(url, resp_headers)
+
+    :hackney.request(env.method, new_url, env.headers, env.body, env.req_opts)
+  end
+
+  defp redirect(url, resp_headers, env, limit) do
+    new_url = absolute_redirect_url(url, resp_headers)
+    res = :hackney.request(env.method, new_url, env.headers, env.body, env.req_opts)
+
+    case res do
+      {:ok, code, new_resp_headers, _client} when code in @redirect_statuses ->
+        redirect(new_url, new_resp_headers, env, limit - 1)
+
+      {:ok, code, new_resp_headers} when code in @redirect_statuses ->
+        redirect(new_url, new_resp_headers, env, limit - 1)
+
+      _ ->
+        res
+    end
+  end
 end

test/pleroma/http/hackney_follow_redirect_regression_test.exs

@@ -88,6 +88,49 @@ defmodule Pleroma.HTTP.HackneyFollowRedirectRegressionTest do
     Pleroma.ReverseProxy.Client.Hackney.close(ref)
   end
 
+  test "hackney reverse proxy follows nested redirects via proxy", %{
+    tls_server: tls_server,
+    proxy: proxy
+  } do
+    url = "#{tls_server.base_url}/nested_redirect"
+
+    opts = [
+      pool: :media,
+      proxy: proxy.proxy_url,
+      insecure: true,
+      connect_timeout: 1_000,
+      recv_timeout: 1_000,
+      follow_redirect: true
+    ]
+
+    assert {:ok, 200, _headers, ref} =
+             Pleroma.ReverseProxy.Client.Hackney.request(:get, url, [], "", opts)
+
+    assert collect_body(ref) == "ok"
+    Pleroma.ReverseProxy.Client.Hackney.close(ref)
+  end
+
+  test "hackney reverse proxy stop following redirects after limit", %{
+    tls_server: tls_server,
+    proxy: proxy
+  } do
+    url = "#{tls_server.base_url}/infinite_redirect"
+
+    opts = [
+      pool: :media,
+      proxy: proxy.proxy_url,
+      insecure: true,
+      connect_timeout: 1_000,
+      recv_timeout: 1_000,
+      follow_redirect: true
+    ]
+
+    assert {:ok, 302, _headers, ref} =
+             Pleroma.ReverseProxy.Client.Hackney.request(:get, url, [], "", opts)
+
+    Pleroma.ReverseProxy.Client.Hackney.close(ref)
+  end
+
   defp collect_body(ref, acc \\ "") do
     case Pleroma.ReverseProxy.Client.Hackney.stream_body(ref) do
       :done -> acc
@@ -148,6 +191,12 @@ defmodule Pleroma.HTTP.HackneyFollowRedirectRegressionTest do
          {:ok, data} <- recv_ssl_headers(ssl_socket),
          {:ok, path} <- parse_path(data) do
       case path do
+        "/infinite_redirect" ->
+          send_ssl_response(ssl_socket, 302, "Found", [{"Location", "/infinite_redirect"}], "")
+
+        "/nested_redirect" ->
+          send_ssl_response(ssl_socket, 302, "Found", [{"Location", "/redirect"}], "")
+
         "/redirect" ->
           send_ssl_response(ssl_socket, 302, "Found", [{"Location", "/final"}], "")