ring
ring copied to clipboard
ring-clojure
Ring Jetty Adapter attempts to handle requests that have already been handled
The proxy-handler function checks the dispatcher type to avoid processing certain requests but it does not check .isHandled so you cannot put Ring's handler into a HandlerCollection and have it behave properly if any earlier handlers may have already handled the request. It does, however, (.setHandled true) so you can put it at the start of a HandlerCollection -- except that it will try to handle all incoming requests.
At work, we're adding WebSocket support to one of our Ring/Jetty processes and we ran into this problem because we use a :configurator to update the server's handlers to have a WebSocket handler and then the regular Ring handler (so non-WS requests should "fall through" to the main Ring app). Unfortunately, because proxy-handler doesn't check the status of the incoming request, it went ahead and processed the (already-handled) WS request and then overwrote the response object with the result of the Ring handler 😞
Changing the when-not in that function to add (or .. (.isHandled request)) would solve this problem.
We worked around it at work by wrapping the proxied handler in another proxied handler that performed that check first.
This seems like a reasonable change. An equivalent alteration to the asynchonous proxy handler function would also be required, along with a short test to prevent regressions (probably the trickiest part).
On a related note, it would be nice if the Ring Jetty adapter exposed the ability to set a context path but I haven't fully figured out how/where that would need to be exposed. Once I do, I'll create an issue around that with details on changes.
If I get time in the upcoming week or so, I can look at putting together a PR (but since we have a workaround, it may be low priority, at least in terms of spending work time on it).
I haven't used the async Ring stuff so I don't feel comfortable putting together a PR for that whole change (and haven't had time to even look at this in the past month or so).
Just FYI: We've switched to https://github.com/sunng87/ring-jetty9-adapter/ at work so we could upgrade to Jetty 11 and because that has WebSocket support built-in we no longer needed multiple Jetty handlers -- so this issue is an extremely low priority for us now.
This should be fixed in the latest alpha due to the change in how the handler is passed to Jetty.