From d1e7923f8ae95513bd4a71df73b357b9433416f1 Mon Sep 17 00:00:00 2001 From: Benjamin Milde Date: Wed, 16 Aug 2023 10:54:46 +0200 Subject: [PATCH 1/3] Resolve port 0 to dynamic port when server is started --- lib/phoenix/endpoint/cowboy2_adapter.ex | 14 +++++++++- lib/phoenix/endpoint/supervisor.ex | 36 +++++++++++++++++++++++-- test/phoenix/endpoint/endpoint_test.exs | 22 +++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/lib/phoenix/endpoint/cowboy2_adapter.ex b/lib/phoenix/endpoint/cowboy2_adapter.ex index 84117a9c7a..13abcdcca4 100644 --- a/lib/phoenix/endpoint/cowboy2_adapter.ex +++ b/lib/phoenix/endpoint/cowboy2_adapter.ex @@ -80,7 +80,7 @@ defmodule Phoenix.Endpoint.Cowboy2Adapter do Application.ensure_all_started(:ssl) end - ref = Module.concat(endpoint, scheme |> Atom.to_string() |> String.upcase()) + ref = build_ref(endpoint, scheme) plug = if code_reloader? do @@ -133,6 +133,18 @@ defmodule Phoenix.Endpoint.Cowboy2Adapter do _ -> scheme end + @doc false + def dynamic_port(endpoint, scheme) do + case :ranch.get_addr(build_ref(endpoint, scheme)) do + {:local, _unix_path} -> raise "Trying to determine dynamic port, but the adapter is configured to use unix sockets. Remove the configuration for `[port: 0]` on the #{scheme} scheme." + {_addr, port} -> port + end + end + + defp build_ref(endpoint, scheme) do + Module.concat(endpoint, scheme |> Atom.to_string() |> String.upcase()) + end + # TODO: Remove this once {:system, env_var} deprecation is removed defp port_to_integer({:system, env_var}), do: port_to_integer(System.get_env(env_var)) defp port_to_integer(port) when is_binary(port), do: String.to_integer(port) diff --git a/lib/phoenix/endpoint/supervisor.ex b/lib/phoenix/endpoint/supervisor.ex index d1e66a113f..7fe9b4a94d 100644 --- a/lib/phoenix/endpoint/supervisor.ex +++ b/lib/phoenix/endpoint/supervisor.ex @@ -351,8 +351,8 @@ defmodule Phoenix.Endpoint.Supervisor do {scheme, port} = cond do - https -> {"https", https[:port] || 443} - http -> {"http", http[:port] || 80} + https -> {"https", transport_port(endpoint, https, :https)} + http -> {"http", transport_port(endpoint, http, :http)} true -> {"http", 80} end @@ -369,6 +369,38 @@ defmodule Phoenix.Endpoint.Supervisor do %URI{scheme: scheme, port: port, host: host} end + defp transport_port(endpoint, config, scheme) do + cond do + value = config[:port] -> + value + |> port_to_integer() + |> handle_dynamic_port(endpoint, scheme) + + scheme == :https -> + 443 + + scheme == :http -> + 80 + end + end + + def handle_dynamic_port(0, endpoint, scheme) do + config = + cond do + server = endpoint.config(:server) -> [server: server] + true -> [] + end + + if server?(config) do + adapter = endpoint.config(:adapter) || Phoenix.Endpoint.Cowboy2Adapter + adapter.dynamic_port(endpoint, scheme) + else + 0 + end + end + + def handle_dynamic_port(port, _, _), do: port + defp warmup_static(endpoint, %{"latest" => latest, "digests" => digests}) do Phoenix.Config.put_new(endpoint, :cache_static_manifest_latest, latest) with_vsn? = !endpoint.config(:cache_manifest_skip_vsn) diff --git a/test/phoenix/endpoint/endpoint_test.exs b/test/phoenix/endpoint/endpoint_test.exs index beb946a994..25d9a4a863 100644 --- a/test/phoenix/endpoint/endpoint_test.exs +++ b/test/phoenix/endpoint/endpoint_test.exs @@ -205,6 +205,28 @@ defmodule Phoenix.Endpoint.EndpointTest do assert_receive {^pid, :sample} end + @tag :capture_log + test "uses bound dynamic port if server is started" do + Application.put_env(:phoenix, __MODULE__.DynamicPortEndpoint, http: [port: 0], server: true) + defmodule DynamicPortEndpoint do + use Phoenix.Endpoint, otp_app: :phoenix + end + DynamicPortEndpoint.start_link() + assert %{port: port} = DynamicPortEndpoint.url() |> URI.parse() + assert port > 1023 + end + + @tag :capture_log + test "uses dynamic port 0 if server is not started" do + Application.put_env(:phoenix, __MODULE__.DynamicPortEndpoint, http: [port: 0], server: false) + defmodule DynamicPortEndpoint do + use Phoenix.Endpoint, otp_app: :phoenix + end + DynamicPortEndpoint.start_link() + assert %{port: port} = DynamicPortEndpoint.url() |> URI.parse() + assert port == 0 + end + @tag :capture_log test "uses url configuration for static path" do Application.put_env(:phoenix, __MODULE__.UrlEndpoint, url: [path: "/api"]) From 3c4ded32ec25e3ccc348ffd23de3b79b83626c54 Mon Sep 17 00:00:00 2001 From: Benjamin Milde Date: Wed, 16 Aug 2023 10:58:12 +0200 Subject: [PATCH 2/3] Add check for callback --- lib/phoenix/endpoint/supervisor.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/phoenix/endpoint/supervisor.ex b/lib/phoenix/endpoint/supervisor.ex index 7fe9b4a94d..65666f32cf 100644 --- a/lib/phoenix/endpoint/supervisor.ex +++ b/lib/phoenix/endpoint/supervisor.ex @@ -391,8 +391,9 @@ defmodule Phoenix.Endpoint.Supervisor do true -> [] end - if server?(config) do - adapter = endpoint.config(:adapter) || Phoenix.Endpoint.Cowboy2Adapter + adapter = endpoint.config(:adapter) || Phoenix.Endpoint.Cowboy2Adapter + + if function_exported?(adapter, :dynamic_port, 2) and server?(config) do adapter.dynamic_port(endpoint, scheme) else 0 From cf5911fe84371cb229c600372c2c67c0aa30d3c7 Mon Sep 17 00:00:00 2001 From: Benjamin Milde Date: Wed, 16 Aug 2023 11:22:18 +0200 Subject: [PATCH 3/3] Deduplicate endpoint module --- test/phoenix/endpoint/endpoint_test.exs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/phoenix/endpoint/endpoint_test.exs b/test/phoenix/endpoint/endpoint_test.exs index 25d9a4a863..a72ddd375a 100644 --- a/test/phoenix/endpoint/endpoint_test.exs +++ b/test/phoenix/endpoint/endpoint_test.exs @@ -207,12 +207,12 @@ defmodule Phoenix.Endpoint.EndpointTest do @tag :capture_log test "uses bound dynamic port if server is started" do - Application.put_env(:phoenix, __MODULE__.DynamicPortEndpoint, http: [port: 0], server: true) - defmodule DynamicPortEndpoint do + Application.put_env(:phoenix, __MODULE__.DynamicPortStartedEndpoint, http: [port: 0], server: true) + defmodule DynamicPortStartedEndpoint do use Phoenix.Endpoint, otp_app: :phoenix end - DynamicPortEndpoint.start_link() - assert %{port: port} = DynamicPortEndpoint.url() |> URI.parse() + DynamicPortStartedEndpoint.start_link() + assert %{port: port} = DynamicPortStartedEndpoint.url() |> URI.parse() assert port > 1023 end