Bug 55314 - Provide option to allow programmatic deployment of server (WebSocket) endpoint at runtime
Summary: Provide option to allow programmatic deployment of server (WebSocket) endpoin...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.0.x-trunk
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-26 16:24 UTC by Rossen Stoyanchev
Modified: 2013-08-16 18:40 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rossen Stoyanchev 2013-07-26 16:24:12 UTC
JSR-356 explicitly states that the addEndpoint methods on ServerContainer "are only operational during the application deployment phase of an application. Specifically, as soon as any of the server endpoints within the application have accepted an opening handshake request, the apis may not longer be used. This restriction may be relaxed in a future version."

There are cases when it is useful to have consolidated handling of HTTP requests including WebSocket handshake requests. For example SockJS [1] defines a URL structure of the form "/endpointPrefix/<server>/<session>/<transport>" where "transport" can be an HTTP-based (e.g. streaming, long polling) or WebSocket (see the "Session URLs" section). The most natural way to implement this is to create a single SockJsServlet that handles all requests under "/endpointPrefix" including requests for "/endpointPrefix/*/*/websocket".

I'd like to propose that in the very least Tomcat make this possible through a flag. I will also create a spec ticket to suggest it for the next iteration. I think it's also worth considering whether this should be the default behavior, with a flag being necessary only to revert back to strict compliance.

For what it's worth, pretty much all WebSocket APIs I'm familiar with (Java and non-Java) do make it possible to create some kind of HTTP request handler that can also initiate a WebSocket handshake. That includes the Tomcat 7 WebSocketServlet for which it's conceivable that some existing users may realize this limitation when upgrading to Tomcat 8.

[1] http://sockjs.github.io/sockjs-protocol/sockjs-protocol-0.3.3.html
Comment 1 Rossen Stoyanchev 2013-07-26 18:13:14 UTC
I've created a spec request also:
https://java.net/jira/browse/WEBSOCKET_SPEC-211
Comment 2 Dave Syer 2013-07-27 11:03:20 UTC
Maybe this is relevant. When I try to add the WsListener to an embedded Tomcat I get this:

2013-07-27 12:01:16.974 ERROR 31782 --- [ost-startStop-1] o.a.c.c.C.[Tomcat].[localhost].[/]       : Exception sending context initialized event to listener instance of class org.apache.tomcat.websocket.server.WsListener

java.lang.UnsupportedOperationException: Section 4.4 of the Servlet 3.0 specification does not permit this method to be called from a ServletContextListener that was not defined in web.xml, a web-fragment.xml file nor annotated with @WebListener
	at org.apache.catalina.core.StandardContext$NoPluggabilityServletContext.addFilter(StandardContext.java:6805)
	at org.apache.tomcat.websocket.server.WsServerContainer.setServletContext(WsServerContainer.java:132)
	at org.apache.tomcat.websocket.server.WsSci.init(WsSci.java:131)
	at org.apache.tomcat.websocket.server.WsListener.contextInitialized(WsListener.java:33)
	at org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:4838)
Comment 3 Mark Thomas 2013-07-31 13:28:19 UTC
(In reply to Dave Syer from comment #2)
> Maybe this is relevant. When I try to add the WsListener to an embedded
> Tomcat I get this:

That looks to be a different issue. There are examples of how to do this without hitting the restriction imposed by Servlet 3.0 / Section 4.4 in the Tomcat unit tests. Essentially, you need to do this:

ctx.addApplicationListener(new ApplicationListener(WsListener.class.getName(), false));


The restriction on programmtic deployment is enforced in WsServerContainer.addEndpoint(ServerEndpointConfig)
Comment 4 Mark Thomas 2013-07-31 14:18:51 UTC
An option to do this has been added and it has been enabled by default.

Should you wish to change this you can either set the org.apache.tomcat.websocket.STRICT_SPEC_COMPLIANCE system property to true that will change the default vaue of this option to not allow additions or you can set the org.apache.tomcat.websocket.noAddAfterHandshake servlet context attribute which explicitly sets the option for a single context.
Comment 5 Rossen Stoyanchev 2013-08-05 21:09:41 UTC
Mark, many thanks for the change although it looks like we had different ideas about the end result. From what I can see the solution makes it possible to call ServerContainer.addEndpoint at runtime after which WsFilter handles matching WebSocket handshake request.

What I had in mind is the ability to create a Servlet that can do centralized HTTP request processing including WebSocket handshake requests, i.e. instead of WsFilter. It's what I meant with the comparison to the Tomcat 7 WebSocketServlet.

The obvious issue is that JSR-356 doesn't provide this, at least not yet. The client-side connectToServer is similar in spirit but nothing like it on the server side. I imagined it would be something like:

WsHandshakeRequest wsRequest = ...
WsHandshakeResponse wsResponse = ...
WsServerContainer wsContainer = ...

wsContainer.upgrade(wsRequest, wsResponse, endpoint);

So any intermediate solution would have to be Tomcat specific. Just a thought here on maybe providing a separate interface marked clearly as an intermidiate solution that will remain stable until the spec has an alternative and that will be deprecated afterwards, something like:

NonCompliantWsServerContainer container = ...
container.upgrade(wsRequest, wsResponse, endpoint);
Comment 6 Mark Thomas 2013-08-06 07:51:34 UTC
Re-open to see what can be done with respect to on demand upgrade.

As a (not very pretty) workaround, it should be possible to replicate the code in the WsFilter that triggers the upgrade. The sort of fix I have in mind is to extract the bulk of this out into a helper class (possibly a new class, possibly an existing one) that can then be used by other Servlets / Filters as well.
Comment 7 Mark Thomas 2013-08-07 15:52:38 UTC
I've done some refactoring locally. Would the following method on WsServerContainer meet you requirements?

public void doUpgrade(
        HttpServletRequest request,
        HttpServletResponse response,
        ServerEndpointConfig sec,
        Map<String,String> pathParams)
        throws ServletException, IOException
Comment 8 Mark Thomas 2013-08-08 10:26:01 UTC
Thinking about this some more, wouldn't it be easier from a developer point of view just to forward the request and let WsFilter do all the work?
Comment 9 Mark Thomas 2013-08-15 19:20:31 UTC
I've implemented the method in comment #7

Feel free to re-open if this doesn't meet the requirement.
Comment 10 Rossen Stoyanchev 2013-08-16 18:37:44 UTC
I've tried this and it works as expected. Thanks!
Comment 11 Mark Thomas 2013-08-16 18:40:47 UTC
Excellent. Thanks for the feedback.