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

HttpSessionHandshakeInterceptor's getSession(request) [SPR-12840] #17438

Closed
spring-issuemaster opened this issue Mar 20, 2015 · 4 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Mar 20, 2015

Keesun Baik opened SPR-12840 and commented

On the websocket document, it describes about HttpSessionHandshakeInterceptor and I've used it but I didn't get the session's id for the request. So, I've debugged it for a while and I got the reason why I didn't get it.

private HttpSession getSession(ServerHttpRequest request) {
	if (request instanceof ServletServerHttpRequest) {
		ServletServerHttpRequest serverRequest = (ServletServerHttpRequest) request;
		return serverRequest.getServletRequest().getSession(false);
	}
	return null;
}

As you can see, the getSession(false) is used to get the HttpSession from ServletServerHttpRequest, and (as you may know) it returns session object or null if there's no current session. That's why I can't see the session id when I tried to connect websocket without using getSession() before.

like below:

@RequestMapping("/")
public String lounge() {
    return "/lounge/index.html";
}

I didn't use getSession() explicitly or implicitly. The controller is just serving a static HTML file like this:

<!DOCTYPE html>
<html>
<head lang="en">
    <meta charset="UTF-8">
    <title>lounge</title>
</head>
<body>
<p>Hi there,</p>
<p>
    <a href="javascript:entrance()">Entrance</a>
</p>

<script src="https://d1fxtkz8shb9d2.cloudfront.net/sockjs-0.3.js"></script>
<script type="application/javascript">
function entrance() {
    var sock = new SockJS('http://localhost:8080/echo');
    sock.onopen = function() {
        console.log('open');
        sock.send('message');
    };
    sock.onmessage = function(e) {
        console.log('message', e);
    };
    sock.onclose = function() {
        console.log('close');
    };
}
</script>
</body>
</html>

In this case, HttpSessionHandshakeInterceptor can't provide the session id, even though I config like this.

@Override
public void registerWebSocketHandlers(WebSocketHandlerRegistry registry) {
    registry.addHandler(echoHandler(), "/echo")
            .addInterceptors(new HttpSessionHandshakeInterceptor())
            .withSockJS();
}

It was a little bit long to describe, I just create this issue to know why you made decision to use getSession(false) and I want to suggest how about using getSession() or providing an option that the developers can use it for a flag to decide to use getSession(true) or getSession(false).

And, If you agree with me and If you don't mind, I want to contribute with this issue using pull-request on the Github.

Thanks for reading.


Affects: 4.1.5

Referenced from: commits 73d6d30

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 23, 2015

Rossen Stoyanchev commented

I think the assumption is that if you're interested in the HTTP session id the application is likely using the HTTP session but we err on the side of caution and don't cause its creation by default. That said having such a flag makes sense so feel free to submit a PR.

Also I presume that you're using the HTTP session in some way since you're interested in the HTTP session id?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 23, 2015

Keesun Baik commented

Thanks for your comment and I can understand what's your intention for the default.

Right, I will use the HTTP session at some time, and I want it to be created at the time when I need it. That said, In some cases, I can use the HttpSessionHandshakeInterceptor before I create the HTTP session, and the other cases, I can use it after creating new HTTP session for signing in , adding some attributes or something. it would be good if I can use the HTTP session id in both cases.

I will submit a PR that adding a flag. Thanks.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 26, 2015

Rossen Stoyanchev commented

I went ahead and added it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 26, 2015

Keesun Baik commented

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.