-
Notifications
You must be signed in to change notification settings - Fork 602
Description
Hello, currently due to:
String.last(host) == "." -> quote do: unquote(host) <> _
It's possible to match only by full subdomain. host matching is already limited due to compile time optimization but by forcing full subdomain matching, it's not possible to use it easily for review apps like heroku(appname-<random>.example.com) or other review-style dynamic hosts.
There's currently a workaround for Plug, by creating a plug, we can easily modify host in conn and ensure that given route will match configured host: in the router. This is not enough to work with sockets or LiveView, these are skipping plug processing. Example: https://github.com/phoenixframework/phoenix_live_view/blob/main/lib/phoenix_live_view/route.ex#L68. LiveView keeps spinning in this case, even though original route matched correctly.
Another workaround is to create a Plug, that will based on host, use another Router and DO NOT use "host" matching at all. This is poor UX when using ~p"" to generate & validate links in Phoenix. Multiple routers cannot be easily used with VerifiedRoutes in a single module.
Over 11 years(!), host: a9eba75 matcher was added but without clear reason why only "." at the end would be supported, and not:
cond do
is_nil(host) -> quote do: _
is_binary(host) -> quote do: unquote(host) <> _
end
My assumption is that it's due to performance optimization, and not security, as for security we can use other mechanisms to validate the domain, and fully, not only subdomain.
I ran Benchee with the following example in LiveBook (so Evaluated instead of compiled function) :
defmodule RouterA do
def match("example.com"), do: true
def match(_), do: false
end
defmodule RouterB do
def match("example.com" <> _), do: true
def match(_), do: false
end
Benchee.run(
%{
"full match" => fn -> RouterA.match("example.com") end,
"match-and-destruct" => fn -> RouterB.match("example.com") end,
},
time: 60,
memory_time: 60
)And the result is following:
Name ips average deviation median 99th %
full match 1.81 M 551.72 ns ±2763.04% 430 ns 940 ns
match-and-destruct 1.51 M 664.35 ns ±3275.19% 500 ns 1170 ns
Comparison:
full match 1.81 M
match-and-destruct 1.51 M - 1.20x slower +112.63 ns
So there's meaningful difference, and I'd expect even bigger difference when compiled, so simply dropping line in cond is most likely not the way to go...
Non-breaking alternative in cond, could be:
String.last(host) == "*" -> quote do: unquote(String.slice(host, 0..-2)) <> _
I'd be more than happy to find some alternative workaround that doesn't require changes in Plug 😄