Re: [potential?] bug in Tomcat 7's websocket UpgradeUtil when handling concurrent connections that use a custom ServerEndpointConfig.Configurator

[Mark Thomas <ma...@apache.org>]

On 27/10/2013 21:14, Bob DeRemer wrote:
> First off, this is the 2nd [potential] bug I've reposted from the
> user list, so my apologies.  After reading the mailing list
> descriptions, it seems potential bugs should be posted to the DEV
> list.  If this is wrong, just let me know.

Generally, if you are unsure post to the users list first. If it needs
to move to the dev list, someone will say so. There is often merit in
discussing it on the users list anyway as it givers other folks a chance
to say "me too" and add their information to the discussion.

When a users list discussion reaches the point where folks are fairly
sure there is a bug then a Bugzilla entry can be created. Often you'll
see a committer ask someone to create one as a result of a discussion.

Posts like this to the dev list are fine but there is a risk (unlikely
in this case) that the report gets forgotten about if a Bugzilla entry
isn't created.

I haven't looked into the detail of this problem - that is next on my
TODO list - but I wanted to address the "Is it OK to do this?" aspect first.

Mark

> Potential Bug?
> 
> ============
> 
> 
> 
> I [think] there may be a problem in Tomcat's static
> UpgradeUtil.doUpgrade logic when handling concurrent
> connection/upgrade requests that rely on a custom
> ServerEndpointConfig.Configurator.modifyHandshake to grab
> [per-upgrade-request] client header values and inject them into the
> wsSession that is being created.
> 
> 
> 
> Specifically, the static doUpgrade does not appear to make a copy of
> the ServerEndpointConfig before calling modifyHandshake.   As a
> result, any per-connection headers the Configurator may grab and put
> in the ServerEndpointConfig.UserProperties map will be overwritten by
> the last upgrade request that occurs before the upgrade logic creates
> the new wsSession in the WsHttpUpgradeHandler.init call.
> 
> 
> 
> I am able to replicate this very easily by using the following server
> configurator code.  By making concurrent websocket connect requests
> that place a unique "client-id" in the upgrade request headers, then
> grabbing that "client-id" property using the code below, ALL
> websocket sessions that get created will have the last "client-id"
> header value that came in concurrently.
> 
> 
> 
> public class Jsr356ServerConfigurator extends
> ServerEndpointConfig.Configurator {
> 
> @Override
> 
> public void modifyHandshake(ServerEndpointConfig config,
> HandshakeRequest request, HandshakeResponse response)
> 
> {
> 
> // get the request headers - looking for security claims and endpoint
> ID
> 
> // claims will be checked in the OnOpen call and the connection
> closed if AUTH check fails
> 
> Map<String, List<String>> headers = request.getHeaders();
> 
> 
> 
> String id = headers.get("client-id").get(0);
> 
> 
> 
> config.getUserProperties().put("client-id", id);
> 
> }
> 
> }
> 
> 
> 
> @ServerEndpoint(...)
> 
> Public class MyServer
> 
> {
> 
> @OnOpen
> 
> public void onOpen(Session session)
> 
> {
> 
> String id = (String) session.getUserProperties().get("client-id");
> 
> _logger.warn("client ID: {}", id);
> 
> }
> 
> }
> 
> 
> 
> 
> 
> Based on chapter 3 of the JSR-356 API document, the actual websocket
> handshake process defined in the websocket spec,  and the online
> description of the process in this stackoverflow link
> (http://stackoverflow.com/questions/17936440/accessing-httpsession-from-httpservletrequest-in-a-web-socket-socketendpoint/17994303#17994303),
> it appears that we should be able to pass per-client information in
> the upgrade headers and we should be able to get them into the
> endpoint INSTANCE's Session user properties.
> 
> 
> 
> If this is a bug, please confirm and I will create a bugzilla entry,
> as it is very important that that we be able to do what I've
> described above.
> 
> 
> 
> If this is not a bug, can someone please describe how to obtain
> per-client instance data during the websocket handshake/onOpen
> process?
> 
> 
> 
> 
> 
> Thanks,
> 
> Bob
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org