search

elixir-plug / plug_cowboy

Plug adapter for the Cowboy web server
Other
243 stars 48 forks source link

Add ability to specify an alternative `:ranch_transport` module? #96

Closed jjcarstens closed 7 months ago

jjcarstens commented 7 months ago

👋🏼 Howdy!

In NervesHub we use websockets for connections with devices and utilize mTLS with them. We wanted to the ability to prevent socket connections as low as possible with some rate limiting and were enabled to do this with Thousand Island in a past PR.

However, due to some recently discovered problems with socket disconnects, we've had to switch back to cowboy until that can be further investigated. While switching, I found that :ranch does a :ranch_transport behavior that can be implemented, but after doing so we realized there is no easy way to use that with :cowboy or Plug.Cowboy via Phoenix.

The end result is a somewhat hack replicating the Phoenix.Endpoint.Cowboy2Adapter and injecting our ranch_transport module into the specific child_spec generated.

Is it desirable to have that functionality either added here or in Phoenix.Endpoint.Cowboy2Adapter? I'd be happy to add, but don't want to do the work if not wanted. We can simply leave this small change in until figuring out how Bandit/Thousand Island can be adjusted for our needs and switch back

josevalim commented 7 months ago

How big would be a patch for this? Is it couple lines of code or a whole new behaviour in here as well?

jjcarstens commented 7 months ago

My really quick review is it can probably be in the transport_options and it is pulled out of there or defaulted. Something like transport_options: [ranch_transport: MyModule] when building the child spec from the scheme. The :ranch_transport behavior already exists so making a new one would be overkill. Maybe a few extra lines if we wanted to validate the provided module actually implements :ranch_transport, but that might be overkill

I don't know how/if this needs to be supported for the http/3 or https/4 functions or what that would entail.

jjcarstens commented 7 months ago

Well, this might all be moot. What I really wanted to do was rate limit a socket at the handshake. However, cowboy has a hard match on {:ok, socket} = ranch:handshake(Ref). It seems that even though there is a behavior for the transport, there might not be much room to actually use it.

I probably just need to give up on that optimization for now until we can get back to Bandit/Thousand Island

josevalim commented 7 months ago

Yeah, it seems to be very specific indeed. I will go ahead and close this, given it is temporary even on your side, but, if it turns out that it only requires passing some options to ranch/cowboy, then feel free to open a PR here.