[WICKET-6493] WebSocket SessionIds are wrong (HttpSession one used instead of Websocket one) + NPE if no HttpSession is found during Handshake Request

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 7.7.0
• Fix Version/s: 7.10.0, 8.0.0-M9
• Component/s: wicket-native-websocket
• Environment: Linux, JDK 8, Jetty 9
• Assignee: Martin Tzvetanov Grigorov
• Reporter: Torsten Krah
• Created: 10/Nov/17 10:45
• Updated: 06/Feb/18 12:53
• Resolved: 30/Dec/17 22:05

Description

Reading the HandshakeRequest javadoc this is written:

    /**
     * Return a reference to the HttpSession that the web socket handshake that 
     * started this conversation was part of, if the implementation
     * is part of a Java EE web container.
     *
     * @return the http session or {@code null} if either the websocket
     * implementation is not part of a Java EE web container, or there is
     * no HttpSession associated with the opening handshake request.
     */
    Object getHttpSession();

So its valid to have NO HttpSession assigned to a JSR356 request.

But Wicket seems to "expect" this because in JavaxUpgradeHttpRequest the HttpSession is extracted from the userProperties which is not there, if it was "null" in the HandshakeRequest.
In the processing the chain will reach the AbstractWebSocketProcessor constructor which calls

this.sessionId = request.getSession(true).getId();

which is going to fail here because there is no HttpSession on the crafted request used here.

I wonder why the sessionId is taken from the HttpSession at all - shouldn't it be the JSR356 session id which you can get with session.getId().
If you look e.g. at the IWebSocketConnectionRegistry javadocs most methods have their sessionId argument documented with:

	 * @param sessionId
	 *      the web socket client session id

but actually the HttpSession id is used instead of the WebSocket sessionId - which is a totally different one.

So i am proposing to use the "real" websocket session id here taken from the constructor call:

public JavaxWebSocketProcessor(JavaxUpgradeHttpRequest request, final Session session, final WebApplication application, EndpointConfig endpointConfig)

where there is the session argument where session.getId() actually returns the websocket session id which can last longer and is 100% there - the HttpSession must not be there at all as written.

Unfortunately the javadoc on IWebSocketConnectionRegistry for

	/**
	 * @param application
	 *            the web application to look in
	 * @param sessionId
	 * @return collection of web socket connection used by a client with the given session id
	 */
	Collection<IWebSocketConnection> getConnections(Application application, String sessionId);

does not state which "sessionId" is meant here - i guess its the HttpSession - in that case please clarify the JavaDocs here.
The code maybe need to "track"the HttpSession id to handle that usecase - but in an optional way because it may not be there.

Issue Links

• is duplicated by: WICKET-6483 JavaxUpgradeHttpRequest can cause NullPointerException