return False in connect handler won't disconnect connection

[StoneMoe]

Version: 4.1 (current latest)

I found this when working on this issue(miguelgrinberg/python-engineio#128 (comment))

In Socket.IO server options documentation, it says:

if the connect handler returns False a disconnect is issued

The server accept the connection actually, but won't send any data to the client.
Is this an expected behavior?

[miguelgrinberg]

I believe it is the client who is supposed to close the physical connection when a disconnect is received (and in fact there is an open issue about that as well).

[StoneMoe]

Thanks for your explaination,

After looking into the implement details, I found a False result from connect handler will send an ERROR packet when always_connect is False(which is default), or DISCONNECT packet when always_connect is True.

This behavior is missing in documentation and should be more consistent IMO.

And I think "Believe clients should do something" by default is not a good idea, maybe a force connection closing in the issue you mentioned is a better consideration.

I believe it is the client who is supposed to close the physical connection when a disconnect is received (and in fact there is an open issue about that as well).

[lazercorn]

@StoneMoe Kind of a late response but I want to clarify that there seems to be two main reasons why the connection isn't fully closed on return False by default.

1. Multiple connections still being multiplexed onto the same socketio connection.
2. When the server wants to close a connection, the best practice as already stated by Miguel is to tell the client to initiate the close. This is due to the server not needing to enter the TIME_WAIT state(only the one who initiates the close does) and can clean up the memory allocated to that connection instantly. Only if after a specific amount of time past and client still hasn't closed the connection do you then initiate the close from the server side. You should read TIME_WAIT and its design implications for protocols and scalable client server systems, as it directly addresses this issue.

[StoneMoe]

@lazercorn Thanks for the reply, discussion and opinions are never late :)

1. Multiple connections still being multiplexed onto the same socketio connection.

Yes you're right.
DISCONNECT is designed as namespace-level event, not for connection.
(https://github.com/socketio/socket.io-protocol#1---disconnect)

2. When the server wants to close a connection, the best practice as already stated by Miguel is to tell the client to initiate the close. This is due to the server not needing to enter the TIME_WAIT state(only the one who initiates the close does) and can clean up the memory allocated to that connection instantly. Only if after a specific amount of time past and client still hasn't closed the connection do you then initiate the close from the server side. You should read TIME_WAIT and its design implications for protocols and scalable client server systems, as it directly addresses this issue.

I would prefer to put a doubt on this, since Flask-SocketIO isn't only for large scale system, and clients aren't always able to be trusted. When you say "best practice" it should be limited in some senario.
Also Flask-SocketIO seems won't actively close invalid connection after a reasonable time period.(correct me if I wrong)

[lazercorn]

@StoneMoe Yes you're correct that Flask-SocketIO does not seem to close invalid connections after some time. You would have to create a custom solution. My solution is to return True, not False. Then send a custom disconnect event to client telling them to initiate the close. Reason I returned True is if you returned False and tried to disconnect client from server after, you'd find client doesn't disconnect.

[StoneMoe]

@lazercorn Your custom solution will also leave dangling connections when client doesn't close connection as you expect. this may lead to DoS vulnerability by exhausting file descriptor resource, that's why I opened this issue.

But I will leave this issue closed.

[lazercorn]

@StoneMoe That's why you must initiate close from server if client doesn't initiate close after some time, otherwise as you mentioned, asking the client to initiate close is pointless if the client isn't trusted. I figured because I had already mentioned it, that was implied. By doing this, you prevent resource exhaustion by malicious clients abusing TIME_WAIT and also avoid unnecessary resource exhaustion by nonmalicious clients.

[StoneMoe]

@lazercorn Exactly, that's why we need to take scenario into account :D