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

Sockjs XHR Fallback on already existing WebSocketServerSockJsSession [SPR-14867] #19433

Closed
spring-projects-issues opened this issue Nov 1, 2016 · 3 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Nov 1, 2016

Michael Nahler opened SPR-14867 and commented

When a web socket client with an existing WebSocketServerSockJsSession falls back to XHR transport then I get the following error:

java.lang.ClassCastException: org.springframework.web.socket.sockjs.transport.session.WebSocketServerSockJsSession cannot be cast to org.springframework.web.socket.sockjs.transport.session.AbstractHttpSockJsSession
	at org.springframework.web.socket.sockjs.transport.TransportHandlingSockJsService.handleTransportRequest(TransportHandlingSockJsService.java:313)
	at org.springframework.web.socket.sockjs.support.AbstractSockJsService.handleRequest(AbstractSockJsService.java:433)
	at org.springframework.web.socket.sockjs.support.SockJsHttpRequestHandler.handleRequest(SockJsHttpRequestHandler.java:132)
	... 96 common frames omitted
Caused by: java.lang.ClassCastException: org.springframework.web.socket.sockjs.transport.session.WebSocketServerSockJsSession cannot be cast to org.springframework.web.socket.sockjs.transport.session.AbstractHttpSockJsSession
	at org.springframework.web.socket.sockjs.transport.handler.AbstractHttpSendingTransportHandler.handleRequest(AbstractHttpSendingTransportHandler.java:57)
	at org.springframework.web.socket.sockjs.transport.TransportHandlingSockJsService.handleTransportRequest(TransportHandlingSockJsService.java:306)
	... 98 common frames omitted

As client I use springs WebSocketStompClient with sockJs and RestTemplateXhrTransport as fallback option:

List<Transport> transports = new ArrayList<>(2);
transports.add(new WebSocketTransport(new StandardWebSocketClient()));
transports.add(new RestTemplateXhrTransport());
SockJsClient sockJsTransport = new SockJsClient(transports);
WebSocketStompClient stompClient = new WebSocketStompClient(sockJsTransport);

Affects: 4.2.6

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 1, 2016

Juergen Hoeller commented

Rossen Stoyanchev, I guess we either need to reject such a scenario with a meaningful exception or somehow relax it to accept the existing session type there...

If this turns out to be easy enough, let's do it for 4.3.4 still.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 1, 2016

Rossen Stoyanchev commented

From a client perspective a fallback occurs if a given transport fails or reaches a connect timeout which is calculated based on the response time of the initial "/info" request. It sounds as though the WebSocket connection succeeds in creating a session on the server side, although perhaps it happens slow enough for the timeout to kick in, causing the fallback to begin. We are actually protected against concurrent processing of success vs failure to connect (this is based on DefaultTransportRequest$ConnectCallback) but we don't proactively do much if success comes too late.

How easy or reliable is it to reproduce this? It would be nice to confirm that's what's going on through logs.

I'm not sure there is much else to be done on the client side about this. On the server side we could anticipate this and detect that the cached session is not of the same transport type as the current request and evict it. That would cause the current request to create a new session.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 3, 2016

Rossen Stoyanchev commented

After some further thought, this is now another check in TransportHandlingSockJsService to validate whether the session type matches the transport type, right next to a similar check whether the user for the current request matches the user that established the SockJS session. If either check fails the request is rejected consistent with these and other more basic validations performed earlier in the base class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants