Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

using JettyWebSocketServlet without having a WebSocketUpgradeFilter #5096

Closed
lachlan-roberts opened this issue Jul 29, 2020 · 1 comment · Fixed by #5115
Closed

using JettyWebSocketServlet without having a WebSocketUpgradeFilter #5096

lachlan-roberts opened this issue Jul 29, 2020 · 1 comment · Fixed by #5115
Assignees

Comments

@lachlan-roberts
Copy link
Contributor

Jetty version
10.0.x

Description
The WebSocketUpgradeFilter is added automatically by the JettyWebSocketServletContainerInitializer but the filter is not required if websocket upgrades are being done by a JettyWebSocketServlet.

There should be some option to not automatically create this this in the JettyWebSocketServerContainer.

@joakime
Copy link
Contributor

joakime commented Jul 29, 2020

This hints at a lazy initialized WebSocketUpgradeFilter.

Some things to consider ...

  • The JettyWebSocketServerContainer can be accessed by the application, and it can use .addMapping() at any time.
  • The WebSocketUpgradeFilter should be the first filter in the chain, if we create it.
  • The WebSocketUpgradeFilter can be added by the application (such as in the WEB-INF/web.xml), and we should use that one if present. (cometd is a good example of this usage pattern)

Perhaps we should have a more fundamental component, built into ServletContextHandler, the UpgradeHandler (fitting into the same role as ServletHandler)?

lachlan-roberts added a commit that referenced this issue Aug 3, 2020
…Servlet

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Aug 4, 2020
…lter

Issue #5096 - lazily initialize the WebSocketFilter if only using JettyWebSocketServlet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants