-
Notifications
You must be signed in to change notification settings - Fork 37.7k
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
SubProtocolWebSocketHandler does not update lastSessionCheckTime [SPR-13745] #18318
Comments
Kevin McLaughlin commented One other note, though it doesn't show up in this stack trace, we are also using spring-session backed by redis. |
Rossen Stoyanchev commented The "No messages received after 60722 ms" message indicates that a WebSocket connection was established but no further WebSocket messages were received, or at least not one with a STOMP CONNECT frame. The WebSocket connection is proactively closed after 60 seconds by the You're right about the lastSessionCheckTime flag, will fix that. There is a scheduled task in TransportHandlingSockJsService that cleans up closed sessions every 5 seconds. There is a scheduled task for WebSocketMessageBrokerStats every 15 min by default. I can't think of what the 90 min timer would be. A session shouldn't be closed more than once. |
Kevin McLaughlin commented Been trying to get a small test case/project to reproduce this, but, not easy. Will turn up debug logs in an environment where this is happening tomorrow and see if that sheds any light. I had thought that maybe the current security context was not being properly applied since it is closing sessions in response to a message from a different session. That still feels like what it is doing, but, when I step through it I can see SecurityContextChannelInterceptor setting the correct user. |
Rossen Stoyanchev commented I'm curious to see though if you're applying any authorization in your security config. |
Kevin McLaughlin commented Basically this: @Configuration
public class WebSocketSecurityConfig extends AbstractSecurityWebSocketMessageBrokerConfigurer {
protected void configureInbound(MessageSecurityMetadataSourceRegistry messages) {
messages.simpTypeMatchers(SimpMessageType.values()).hasRole(WebsocketExceptionApplication.ROLE_USER)
.anyMessage().denyAll();
}
} At debug level:
|
Rossen Stoyanchev commented So ROLE_USER is required but |
Kevin McLaughlin commented I've gotten a bit further. What I think is happening is we have the web socket URL open in spring (http) security. For example: /hello/178/1ya215g4/eventsource. Something (probably our web-tests) is requesting this URL in an unauthenticated state and then sending nothing else (or possibly being denied on sending a message). Then checkSessions comes along and tries to close it and hits the auth check. |
Rossen Stoyanchev commented That sounds pretty plausible. The only thing I see that needs fixing from our side is making sure lastSessionCheckTime is updated. If so I will update the title accordingly. |
Kevin McLaughlin commented I fixed up our http web security config (protect "/hello") and the logs have been clean for awhile. Will check again in the morning, but, so far so good. I also turned on the the boot/tomcat access logs and I can see all the 'bad' web socket initiation requests being redirected to our login page. Thanks for bearing with me. |
Juergen Hoeller commented Kevin McLaughlin, have you had a chance to try a recent Juergen |
Kevin McLaughlin commented After I fixed the security config, haven't had any problems. I also switched our app over to boot 1.3.1.BUILD-SNAPSHOT (which pulls 4.2.4.BUILD-SNAPSHOT) and everything seems fine so far. |
Kevin McLaughlin opened SPR-13745 and commented
We're getting a lot of the exceptions below in our logs. I believe they are caused by our selenium tests not properly establishing the full connection. That said, there looks to be an issue w/ spring security integration when trying to close the "half-open" connections so the exceptions keep repeating.
Couple other things I noticed in SubProtocolWebSocketHandler.java that look questionable to me:
Here is the full exception:
Affects: 4.1.8, 4.2.3
Referenced from: commits ff8bbc9, f5e681e
Backported to: 4.1.9
The text was updated successfully, but these errors were encountered: