Session ID is not unique anymore

[haizz]

All opened tabs in the browser share the same session id because of the change in #505
This breaks all the code which differentiates connections by their ids.
This is extremely critical.

[haizz]

I had to revert to 1.7.13 because 1.7.14 is basically broken.

[mrniko]

Could you suggest a change?

[haizz]

Yes, please restore the old behavior.

[haizz]

This breaks even internal netty-socketio code, like joinRoom functionality which is supposed to be isolated for each connection:

netty-socketio/src/main/java/com/corundumstudio/socketio/transport/NamespaceClient.java

Line 172 in 5304082

namespace.joinRoom(room, getSessionId());

[AlexMl]

Now I know why our web connections are so messed up.

[moocat]

The rooms functionality is completely broken due to this.

Java 10 also does a " An illegal reflective access operation has occurred" for previous versions, so I would love to be able to use the up-to-date versions without rooms being broken like this.

[Crimscent]

@mrniko, please restore the old behavior, it just breaks everything:

• multiple tabs share the same session and PONG messages go randomly to only one tab resulting in reconnect;
• messages from server are delivered only to one tab randomly;
• etc.

As a result we can't use this server implementation in our projects.

Reference code from latest 1.x nodejs engine.io 1.8.4 (which is used by socket.io 1.7.4):

Server.prototype.handshake = function (transportName, req) {
  var id = this.generateId(req);
  ...
  if (false !== this.cookie) {
    transport.on('headers', function (headers) {
      headers['Set-Cookie'] = cookieMod.serialize(self.cookie, id,
        {
          path: self.cookiePath,
          httpOnly: self.cookiePath ? self.cookieHttpOnly : false
        });
    });
  }
  ...
};

Server.prototype.generateId = function (req) {
  return base64id.generateId();
};

Suggested change:

public class AuthorizeHandler extends ChannelInboundHandlerAdapter implements Disconnectable {
    ...
    private boolean authorize(ChannelHandlerContext ctx, Channel channel, String origin, Map<String, List<String>> params, FullHttpRequest req)
            throws IOException {
        ...
        // remove next line
        // UUID sessionId = this.generateOrGetSessionIdFromRequest(req.headers());
        // replace with next line
        UUID sessionId = UUID.randomUUID();
        ...
    }
    ...
}

[mrniko]

@Crimscent Actually the problem is in this change 057014f. Could you combine both variants - previous code and current into single one and suggest a PR?

[Crimscent]

@mrniko, hmm, I think the old code is not that correct as well, netty-socketio fetches 'io' header on handshake, which is not needed (it's not done by reference 1.x nodejs implementation). Can do PR after clarification.

[mrniko]

@Crimscent OK. I'll do it by myself

[mrniko]

Fixed

[oliverhausler]

@Crimscent This apparently should be fixed, but it's not working for me. Is this fixed for you? If you want, please let's discuss.

The approach of using a single io cookie will not always work. It will work as long as all requests coming from different tabs are meant to be for the same session. If that's not the case, a cookie won't work. [For example, we make requests from frames embedded into different websites. The socket.io request is made from within the frame, so all sessions are bundled when on the same browser. A huge mess.]

[RICH0423]

You can set randomSession to true (from version 1.7.17)

Configuration config = new Configuration();
config.setHostname(socketIOConfig.getHost());
config.setPort(socketIOConfig.getPort());
config.setRandomSession(true);  //default is false