Tests WIP

Remove the :send_user_agent setting. We should always send the user agent.

Remove duplicate setting of the user agent from ReverseProxy module

config/benchmark.exs

@@ -70,8 +70,6 @@
 
 config :pleroma, :http_security, report_uri: "https://endpoint.com"
 
-config :pleroma, :http, send_user_agent: false
-
 rum_enabled = System.get_env("RUM_ENABLED") == "true"
 config :pleroma, :database, rum_enabled: rum_enabled
 IO.puts("RUM enabled: #{rum_enabled}")

config/config.exs

@@ -180,7 +180,6 @@
 # Configures http settings, upstream proxy etc.
 config :pleroma, :http,
   proxy_url: nil,
-  send_user_agent: true,
   user_agent: :default,
   adapter: []
 

config/description.exs

@@ -2618,10 +2618,6 @@
         description: "Proxy URL",
         suggestions: ["localhost:9020", {:socks5, :localhost, 3090}]
       },
-      %{
-        key: :send_user_agent,
-        type: :boolean
-      },
       %{
         key: :user_agent,
         type: [:string, :atom],

config/test.exs

@@ -96,8 +96,6 @@
 
 config :pleroma, :http_security, report_uri: "https://endpoint.com"
 
-config :pleroma, :http, send_user_agent: false
-
 rum_enabled = System.get_env("RUM_ENABLED") == "true"
 config :pleroma, :database, rum_enabled: rum_enabled
 IO.puts("RUM enabled: #{rum_enabled}")

lib/pleroma/http/request_builder.ex

@@ -34,8 +34,7 @@ def url(request, u), do: %{request | url: u}
   @spec headers(Request.t(), Request.headers()) :: Request.t()
   def headers(request, headers) do
     headers_list =
-      with true <- Pleroma.Config.get([:http, :send_user_agent]),
+      with nil <- Enum.find(headers, fn {key, _val} -> String.downcase(key) == "user-agent" end) do
-           nil <- Enum.find(headers, fn {key, _val} -> String.downcase(key) == "user-agent" end) do
         [{"user-agent", Pleroma.Application.user_agent()} | headers]
       else
         _ ->

lib/pleroma/reverse_proxy.ex

@@ -231,7 +231,6 @@ defp build_req_headers(headers, opts) do
     |> downcase_headers()
     |> Enum.filter(fn {k, _} -> k in @keep_req_headers end)
     |> build_req_range_or_encoding_header(opts)
-    |> build_req_user_agent_header(opts)
     |> Keyword.merge(Keyword.get(opts, :req_headers, []))
   end
 
@@ -246,15 +245,6 @@ defp build_req_range_or_encoding_header(headers, _opts) do
     end
   end
 
-  defp build_req_user_agent_header(headers, _opts) do
-    List.keystore(
-      headers,
-      "user-agent",
-      0,
-      {"user-agent", Pleroma.Application.user_agent()}
-    )
-  end
-
   defp build_resp_headers(headers, opts) do
     headers
     |> Enum.filter(fn {k, _} -> k in @keep_resp_headers end)

test/pleroma/reverse_proxy_test.exs

@@ -14,61 +14,45 @@ defmodule Pleroma.ReverseProxyTest do
     test "do not track successful request", %{conn: conn} do
       url = "/success"
 
+      Tesla.Mock.mock(fn %{url: ^url} ->
+        %Tesla.Env{
+          status: 200,
+          body: ""
+        }
+      end)
+
       conn = ReverseProxy.call(conn, url)
 
       assert conn.status == 200
       assert Cachex.get(:failed_proxy_url_cache, url) == {:ok, nil}
     end
-  end
 
-  test "use Pleroma's user agent in the request; don't pass the client's", %{conn: conn} do
+    test "use Pleroma's user agent in the request; don't pass the client's", %{conn: conn} do
-    conn =
+      wanted_headers = [{"user-agent", Pleroma.Application.user_agent()}]
-      conn
-      |> Plug.Conn.put_req_header("user-agent", "fake/1.0")
-      |> ReverseProxy.call("/user-agent")
 
-    assert json_response(conn, 200) == %{"user-agent" => Pleroma.Application.user_agent()}
+      Tesla.Mock.mock(fn %{url: "/user-agent", headers: ^wanted_headers} ->
-  end
+        %Tesla.Env{
+          status: 200,
+          body: ""
+        }
+      end)
 
-  test "closed connection", %{conn: conn} do
+      conn =
-    ClientMock
+        conn
-    |> expect(:request, fn :get, "/closed", _, _, _ -> {:ok, 200, [], %{}} end)
+        |> Plug.Conn.put_req_header("user-agent", "fake/1.0")
-    |> expect(:stream_body, fn _ -> {:error, :closed} end)
+        |> ReverseProxy.call("/user-agent")
-    |> expect(:close, fn _ -> :ok end)
 
-    conn = ReverseProxy.call(conn, "/closed")
+      assert response(conn, 200)
-    assert conn.halted
+    end
-  end
 
-  defp stream_mock(invokes, with_close? \\ false) do
+    test "closed connection", %{conn: conn} do
-    ClientMock
+      ClientMock
-    |> expect(:request, fn :get, "/stream-bytes/" <> length, _, _, _ ->
+      |> expect(:request, fn :get, "/closed", _, _, _ -> {:ok, 200, [], %{}} end)
-      Registry.register(ClientMock, "/stream-bytes/" <> length, 0)
+      |> expect(:stream_body, fn _ -> {:error, :closed} end)
+      |> expect(:close, fn _ -> :ok end)
 
-      {:ok, 200, [{"content-type", "application/octet-stream"}],
+      conn = ReverseProxy.call(conn, "/closed")
-       %{url: "/stream-bytes/" <> length}}
+      assert conn.halted
-    end)
-    |> expect(:stream_body, invokes, fn %{url: "/stream-bytes/" <> length} = client ->
-      max = String.to_integer(length)
-
-      case Registry.lookup(ClientMock, "/stream-bytes/" <> length) do
-        [{_, current}] when current < max ->
-          Registry.update_value(
-            ClientMock,
-            "/stream-bytes/" <> length,
-            &(&1 + 10)
-          )
-
-          {:ok, "0123456789", client}
-
-        [{_, ^max}] ->
-          Registry.unregister(ClientMock, "/stream-bytes/" <> length)
-          :done
-      end
-    end)
-
-    if with_close? do
-      expect(ClientMock, :close, fn _ -> :ok end)
     end
   end
 
@@ -85,15 +69,6 @@ test "length returns error if content-length more than option", %{conn: conn} do
                ReverseProxy.call(conn, "/huge-file", max_body_length: 4)
              end) == ""
     end
-
-    test "max_body_length returns error if streaming body more than that option", %{conn: conn} do
-      stream_mock(3, true)
-
-      assert capture_log(fn ->
-               ReverseProxy.call(conn, "/stream-bytes/50", max_body_length: 30)
-             end) =~
-               "Elixir.Pleroma.ReverseProxy request to /stream-bytes/50 failed while reading/chunking: :body_too_large"
-    end
   end
 
   describe "HEAD requests" do
@@ -108,16 +83,8 @@ test "common", %{conn: conn} do
     end
   end
 
-  defp error_mock(status) when is_integer(status) do
-    ClientMock
-    |> expect(:request, fn :get, "/status/" <> _, _, _, _ ->
-      {:error, status}
-    end)
-  end
-
   describe "returns error on" do
     test "500", %{conn: conn} do
-      error_mock(500)
       url = "/status/500"
 
       capture_log(fn -> ReverseProxy.call(conn, url) end) =~
@@ -130,7 +97,6 @@ test "500", %{conn: conn} do
     end
 
     test "400", %{conn: conn} do
-      error_mock(400)
       url = "/status/400"
 
       capture_log(fn -> ReverseProxy.call(conn, url) end) =~
@@ -141,7 +107,6 @@ test "400", %{conn: conn} do
     end
 
     test "403", %{conn: conn} do
-      error_mock(403)
       url = "/status/403"
 
       capture_log(fn ->
@@ -152,55 +117,10 @@ test "403", %{conn: conn} do
       {:ok, ttl} = Cachex.ttl(:failed_proxy_url_cache, url)
       assert ttl > 100_000
     end
-
-    test "204", %{conn: conn} do
-      url = "/status/204"
-      expect(ClientMock, :request, fn :get, _url, _, _, _ -> {:ok, 204, [], %{}} end)
-
-      capture_log(fn ->
-        conn = ReverseProxy.call(conn, url)
-        assert conn.resp_body == "Request failed: No Content"
-        assert conn.halted
-      end) =~
-        "[error] Elixir.Pleroma.ReverseProxy: request to \"/status/204\" failed with HTTP status 204"
-
-      assert Cachex.get(:failed_proxy_url_cache, url) == {:ok, true}
-      assert Cachex.ttl(:failed_proxy_url_cache, url) == {:ok, nil}
-    end
-  end
-
-  test "streaming", %{conn: conn} do
-    stream_mock(21)
-    conn = ReverseProxy.call(conn, "/stream-bytes/200")
-    assert conn.state == :chunked
-    assert byte_size(conn.resp_body) == 200
-    assert Conn.get_resp_header(conn, "content-type") == ["application/octet-stream"]
-  end
-
-  defp headers_mock(_) do
-    ClientMock
-    |> expect(:request, fn :get, "/headers", headers, _, _ ->
-      Registry.register(ClientMock, "/headers", 0)
-      {:ok, 200, [{"content-type", "application/json"}], %{url: "/headers", headers: headers}}
-    end)
-    |> expect(:stream_body, 2, fn %{url: url, headers: headers} = client ->
-      case Registry.lookup(ClientMock, url) do
-        [{_, 0}] ->
-          Registry.update_value(ClientMock, url, &(&1 + 1))
-          headers = for {k, v} <- headers, into: %{}, do: {String.capitalize(k), v}
-          {:ok, Jason.encode!(%{headers: headers}), client}
-
-        [{_, 1}] ->
-          Registry.unregister(ClientMock, url)
-          :done
-      end
-    end)
-
-    :ok
   end
 
   describe "keep request headers" do
-    setup [:headers_mock]
+    # setup [:headers_mock]
 
     test "header passes", %{conn: conn} do
       conn =
@@ -247,32 +167,12 @@ test "add cache-control", %{conn: conn} do
     end
   end
 
-  defp disposition_headers_mock(headers) do
-    ClientMock
-    |> expect(:request, fn :get, "/disposition", _, _, _ ->
-      Registry.register(ClientMock, "/disposition", 0)
-
-      {:ok, 200, headers, %{url: "/disposition"}}
-    end)
-    |> expect(:stream_body, 2, fn %{url: "/disposition"} = client ->
-      case Registry.lookup(ClientMock, "/disposition") do
-        [{_, 0}] ->
-          Registry.update_value(ClientMock, "/disposition", &(&1 + 1))
-          {:ok, "", client}
-
-        [{_, 1}] ->
-          Registry.unregister(ClientMock, "/disposition")
-          :done
-      end
-    end)
-  end
-
   describe "response content disposition header" do
     test "not atachment", %{conn: conn} do
-      disposition_headers_mock([
+      # disposition_headers_mock([
-        {"content-type", "image/gif"},
+      #  {"content-type", "image/gif"},
-        {"content-length", "0"}
+      #  {"content-length", "0"}
-      ])
+      # ])
 
       conn = ReverseProxy.call(conn, "/disposition")
 
@@ -280,10 +180,10 @@ test "not atachment", %{conn: conn} do
     end
 
     test "with content-disposition header", %{conn: conn} do
-      disposition_headers_mock([
+      # disposition_headers_mock([
-        {"content-disposition", "attachment; filename=\"filename.jpg\""},
+      #  {"content-disposition", "attachment; filename=\"filename.jpg\""},
-        {"content-length", "0"}
+      #  {"content-length", "0"}
-      ])
+      # ])
 
       conn = ReverseProxy.call(conn, "/disposition")