Bug 56343 - NPE in org.apache.tomcat.websocket.WsWebSocketContainer.connectToServer
Summary: NPE in org.apache.tomcat.websocket.WsWebSocketContainer.connectToServer
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: WebSocket (show other bugs)
Version: 8.0.5
Hardware: PC All
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-03 07:41 UTC by Przemyslaw Bielicki
Modified: 2014-04-04 21:38 UTC (History)
0 users



Attachments
WsClient.java main class (422 bytes, text/plain)
2014-04-03 13:05 UTC, Przemyslaw Bielicki
Details
EchoClient.java websocket client endpoint (1013 bytes, text/plain)
2014-04-03 13:05 UTC, Przemyslaw Bielicki
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Przemyslaw Bielicki 2014-04-03 07:41:19 UTC
Hi,

I believe there is a bug in  org.apache.tomcat.websocket.WsWebSocketContainer.connectToServer(Object pojo, URI path) method in the following lines:

if (!ClientEndpointConfig.Configurator.class.equals(configuratorClazz)) {
  try {
    configurator = configuratorClazz.newInstance();
  } catch (InstantiationException | IllegalAccessException e) {
    throw new DeploymentException(sm.getString("wsWebSocketContainer.defaultConfiguratorFail"), e);
  }
}

I don't understand why you don't like instantiating ClientEndpointConfig.Configurator class? This is an empty / default configurator for the endpoint which has a default constructor. Because of this bug I had do specify explicitly a dummy configurator e.g.:

import javax.websocket.ClientEndpointConfig;
public class CustomConfigurator extends ClientEndpointConfig.Configurator {
}

@ClientEndpoint(configurator = CustomConfigurator.class)
public class ClientEndpoint {

IMHO it's useless.

Cheers,
Przemyslaw
Comment 1 Remy Maucherat 2014-04-03 09:36:23 UTC
There's no bug in what you describe, it will use a static instance (rather than a new instance).
Comment 2 Przemyslaw Bielicki 2014-04-03 09:56:43 UTC
Hi,

not really:

Exception in thread "main" java.lang.NullPointerException
	at org.apache.tomcat.websocket.WsWebSocketContainer.connectToServer(WsWebSocketContainer.java:259)
	at org.apache.tomcat.websocket.WsWebSocketContainer.connectToServer(WsWebSocketContainer.java:198)
	at org.apache.tomcat.websocket.WsWebSocketContainer.connectToServer(WsWebSocketContainer.java:215)
	at WsClient.main(WsClient.java:21)

Cheers,
Przemyslaw
Comment 3 Mark Thomas 2014-04-03 12:59:32 UTC
Please provide test case (with all source code) for the simplest possible client (I'd strongly suggest the client connects to the Tomcat WebSocket echo example on localhost, port 8080 for easy testing) that demonstrates this issue. That will enable us to figure out exactly what is going on here.
Comment 4 Przemyslaw Bielicki 2014-04-03 13:05:11 UTC
Created attachment 31470 [details]
WsClient.java main class
Comment 5 Przemyslaw Bielicki 2014-04-03 13:05:36 UTC
Created attachment 31471 [details]
EchoClient.java websocket client endpoint
Comment 6 Przemyslaw Bielicki 2014-04-03 13:49:54 UTC
small tip: I'm not using tomcat-websocket-api.jar - I'm using the official javax.websocket-api-1.0.jar instead
Comment 7 Przemyslaw Bielicki 2014-04-03 14:09:53 UTC
and I found that there are some discrepancies in ClientEndpointConfig.java between Tomcat and JSR jars.

IMHO you should align Tomcat WebSocket API to match the one from JSR, to avoid such nasty errors.
Comment 8 Mark Thomas 2014-04-03 22:09:32 UTC
The public API for the Tomcat implementation of the Java WebSocket 1.0 API and any other implementation (inlcuding the RI) should be identical. I was checking this regularly all the way through development so there should not be any differences. If you have spotted a difference in the public API then please open a separate Bugzilla issue for that. Note that the actual implementation beind the API may well differ.

What appears to have happened here is that the Tomcat implementation and the RI are both correctly treating a null client configurator as meaning "use the default NO-OP implementation" but Tomcat and the RI have implemented that differently. The differences stem from Tomcat's desire to have a single default Configurator instance to reduce memory footprint and GC. The difference is problematic because the difference crosses the boundary between the API classes and the implementation classes.

I'm leaning towards calling this a bug in Tomcat primarily because of the Javadoc for ClientEndpointConfig.Builder.configurator(). Tomcat calls this with null which arguably is the wrong thing to do. It would certainly be safer (and should have the same end result) not to call the method at all in this case.
Comment 9 Mark Thomas 2014-04-04 21:38:09 UTC
This has been fixed in 8.0.x for 8.0.6 onwards and in 7.0.x 7.0.54 onwards.