Skip to content

Extend support for routing based on host #1299

@arathunku

Description

@arathunku

Hello, currently due to:

https://github.com/elixir-plug/plug/blob/59cf2b552d2130398e27cd192157faf12d1356f5/lib/plug/router/utils.ex#L58C1-L58C63

     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 😄

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions