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

Can't restore sessionId from Cookie. #505

Closed
wuxudong opened this issue Dec 9, 2017 · 6 comments · Fixed by #506
Closed

Can't restore sessionId from Cookie. #505

wuxudong opened this issue Dec 9, 2017 · 6 comments · Fixed by #506
Milestone

Comments

@wuxudong
Copy link
Contributor

wuxudong commented Dec 9, 2017

When the server reboot, the client will reconnect, and the sessionId should remains the same.

But I found it changed every time.

After dig a little further, I find that AuthorizeHandler tries to fetch sessionId by headers.getAll("io") , but always get empty.

In the comment, it says will retrieve sessionId from io cookie. but the code tries to get sessionId from header directly.

Is it a bug? or am I missing anything?

/**
        This method will either generate a new random sessionId or will retrieve the value stored
        in the "io" cookie.  Failures to parse will cause a logging warning to be generated and a
        random uuid to be generated instead (same as not passing a cookie in the first place).
    */
    private UUID generateOrGetSessionIdFromRequest(HttpHeaders headers) {
        List<String> values = headers.getAll("io");
        if (values.size() == 1) {
            try {
                return UUID.fromString(values.get(0));
            } catch ( IllegalArgumentException iaex ) {
                log.warn("Malformed UUID received for session! io=" + values.get(0));
            }
        }
        return UUID.randomUUID();
    }
@wuxudong
Copy link
Contributor Author

wuxudong commented Dec 9, 2017

everything works well after I change code below. SessionId is restored when server reboot.

    /**
        This method will either generate a new random sessionId or will retrieve the value stored
        in the "io" cookie.  Failures to parse will cause a logging warning to be generated and a
        random uuid to be generated instead (same as not passing a cookie in the first place).
    */
private UUID generateOrGetSessionIdFromRequest(HttpHeaders headers) {
        for (String cookieHeader: headers.getAll(HttpHeaderNames.COOKIE)) {
            Set<Cookie> cookies = CookieDecoder.decode(cookieHeader);

            for (Cookie cookie : cookies) {
                if (cookie.name().equals("io")) {
                    try {
                        return UUID.fromString(cookie.getValue());
                    } catch ( IllegalArgumentException iaex ) {
                        log.warn("Malformed UUID received for cookie! io=" + cookie.getValue());
                    }
                }
            }
        }

        List<String> values = headers.getAll("io");
        if (values.size() == 1) {
            try {
                return UUID.fromString(values.get(0));
            } catch ( IllegalArgumentException iaex ) {
                log.warn("Malformed UUID received for session! io=" + values.get(0));
            }
        }
        return UUID.randomUUID();
    }

@wuxudong
Copy link
Contributor Author

@mrniko any update?

@pablojr
Copy link

pablojr commented Dec 13, 2017

@wuxudong any chance you can provide a pull request so @mrniko can review it and eventually commit your changes into master branch? Don't forget this is an open source projects and any help is welcomed
By the way, you should update the method's comments since it's the same as original one, and it isn't reflecting the fact that session will come from cookie or header

@wuxudong
Copy link
Contributor Author

@pablojr Thanks for your advice. I will create a PR. And is this really a bug? or I miss some necessary config?

wuxudong added a commit to wuxudong/netty-socketio that referenced this issue Dec 13, 2017
@mrniko mrniko added this to the 1.7.14 milestone Dec 13, 2017
mrniko added a commit that referenced this issue Dec 13, 2017
restore sessionId from io Cookie. #505
@haizz
Copy link

haizz commented Mar 13, 2018

I'm sorry, but this not only breaks everything because all opened browser tabs begin to share the same socket id #526 , but also it is not compatible with reference engine.io implementation: socketio/engine.io#422

@mrniko mrniko reopened this Mar 13, 2018
@mrniko mrniko modified the milestones: 1.7.14, 1.7.15 Mar 13, 2018
@mrniko
Copy link
Owner

mrniko commented May 15, 2018

Fixed

@mrniko mrniko closed this as completed May 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants