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

ConnectionRefusedError doest not reject connection #1279

Closed
johaven opened this issue May 17, 2020 · 6 comments
Closed

ConnectionRefusedError doest not reject connection #1279

johaven opened this issue May 17, 2020 · 6 comments

Comments

@johaven
Copy link

johaven commented May 17, 2020

Expected behavior
From docs, we can use ConnectionRefusedError to reject connection.
In fact connection is still established, even when returning False

To Reproduce

@socketio.on('connect')
def connect():
        raise ConnectionRefusedError('unauthorized!')

Logs

78c764452b074e1f93a7f8b643410614: Sending packet OPEN data {'sid': '78c764452b074e1f93a7f8b643410614', 'upgrades': ['websocket'], 'pingTimeout': 60000, 'pingInterval': 25000}
78c764452b074e1f93a7f8b643410614: Sending packet MESSAGE data 4"unauthorized!"
127.0.0.1:5000 [pid: 1385|app: 0|req: 10/10] 127.0.0.1 () {44 vars in 1956 bytes} [Sun May 17 16:13:49 2020] GET /socket.io/?EIO=3&transport=polling&t=N8Y-zXe => generated 135 bytes in 1 msecs (HTTP/1.1 200) 3 headers in 123 bytes (3 switches on core 249)
127.0.0.1:5000 [pid: 1385|app: 0|req: 11/11] 127.0.0.1 () {44 vars in 1924 bytes} [Sun May 17
78c764452b074e1f93a7f8b643410614: Received request to upgrade to websocket
127.0.0.1:5000 [pid: 1385|app: 0|req: 13/12] 127.0.0.1 () {44 vars in 2030 bytes} [Sun May 17 16:13:49 2020] GET /socket.io/?EIO=3&transport=polling&t=N8Y-zYR&sid=78c764452b074e1f93a7f8b643410614 => generated 4 bytes in 3 msecs (HTTP/1.1 200) 3 headers in 123 bytes (3 switches on core 249)
127.0.0.1:5000 [pid: 1385|app: 0|req: 14/13] 127.0.0.1 () {42 vars in 1878 bytes} [Sun May 17 (HTTP/1.1 200) 8 headers in 299 bytes (1 switches on core 249)
78c764452b074e1f93a7f8b643410614: Upgrade to websocket successful

With lambda exception

@socketio.on('connect')
def connect():
        raise Exception('NO')

Logs

cc128087b313464b96280a473ef60e71: Sending packet OPEN data {'sid': 'cc128087b313464b96280a473ef60e71', 'upgrades': ['websocket'], 'pingTimeout': 60000, 'pingInterval': 25000}
connect handler error
Exception: NO
Application rejected connection
127.0.0.1:5000 [pid: 1394|app: 0|req: 1/1] 127.0.0.1 () {44 vars in 1954 bytes} [Sun May 17 16:15:48 2020] GET /socket.io/?EIO=3&transport=polling&t=N8Y_QKh => generated 12 bytes in 4 msecs (HTTP/1.1 401) 3 headers in 119 bytes (3 switches on core 249)
@miguelgrinberg
Copy link
Owner

miguelgrinberg commented May 17, 2020

This has been reported already: miguelgrinberg/python-socketio#391.

I'm going to close this one as a duplicate since this issue is already on my radar and there is a lot of information in the referenced issue.

@1111mp
Copy link

1111mp commented Jan 28, 2021

@miguelgrinberg
Regarding identity verification, I have checked a lot of your documentation and related issues. My final understanding is: when the verification fails, the server can send a connect_error to the client through raise ConnectionRefusedError('authentication failed') or return False. As for disconnection, you need to manually disconnect() in the connect_error event callback in the client to realize the disconnection.
Because when the client not calls disconnect() during the error_connect time, the server log shows that the subsequent heartbeat monitoring mechanism will not terminate.

9RqgqWlYWmvDvPUcAAAG: Received request to upgrade to websocket
9RqgqWlYWmvDvPUcAAAG: Received packet MESSAGE data 0
9RqgqWlYWmvDvPUcAAAG: Sending packet MESSAGE data 4{"message":"authentication failed"}
9RqgqWlYWmvDvPUcAAAG: Upgrade to websocket successful
9RqgqWlYWmvDvPUcAAAG: Sending packet PING data None
9RqgqWlYWmvDvPUcAAAG: Received packet PONG data 
9RqgqWlYWmvDvPUcAAAG: Sending packet PING data None
9RqgqWlYWmvDvPUcAAAG: Received packet PONG data 
9RqgqWlYWmvDvPUcAAAG: Sending packet PING data None
9RqgqWlYWmvDvPUcAAAG: Received packet PONG data 
9RqgqWlYWmvDvPUcAAAG: Sending packet PING data None
9RqgqWlYWmvDvPUcAAAG: Received packet PONG data 
9RqgqWlYWmvDvPUcAAAG: Sending packet PING data None
9RqgqWlYWmvDvPUcAAAG: Received packet PONG data 
9RqgqWlYWmvDvPUcAAAG: Sending packet PING data None
9RqgqWlYWmvDvPUcAAAG: Received packet PONG data 

Is that right?
When I use nodejs to develop socketio services, the implementation of authentication will be very simple. You can refer to: https://stackoverflow.com/questions/36788831/authenticating-socket-io-connections-using-jwt
No need to pass the event again and finally have the client to actually close the connection.
Does it currently support this similar way to achieve identity verification?
If not, will you consider supporting it?
I am a javascript developer and I am currently learning python. If what I said is wrong, please correct me.
Finally, thank you very much

@miguelgrinberg
Copy link
Owner

@1111mp the client should be exited on its own, unless maybe if you are connecting multiple namespaces and there is at least one that connected successfully. If you have a simple client and server example that you can give me to test this I'd appreciate it.

@1111mp
Copy link

1111mp commented Jan 29, 2021

@miguelgrinberg I will post one of the simplest demo code:
server(This is all the code):

from flask import Flask
from flask_socketio import SocketIO, ConnectionRefusedError


app = Flask(__name__)
app.config['SECRET_KEY'] = 'secret!'
socketio = SocketIO(app, cors_allowed_origins='*',
                    logger=True, engineio_logger=True)


@socketio.on('connect')
def connect():
    raise ConnectionRefusedError('authentication failed')


if __name__ == '__main__':
    socketio.run(app)

I used pipenv,This is Pipfile file:

[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
flask = "*"
flask-socketio = "*"
eventlet = "*"

[dev-packages]
autopep8 = "*"

[requires]
python_version = "3.9"

client(html file):

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Document</title>
  </head>

  <body></body>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/socket.io/3.1.0/socket.io.js"></script>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/uuid/8.1.0/uuidv4.min.js"></script>
  <script>
    // const url = "ws://192.168.80.208:3000"
    const url = "ws://127.0.0.1:5000";

    let socket = io(url, {
      transports: ["polling", "websocket"],
      // extraHeaders: { token: "7307e906-c307-4918-b38f-0b00583f23e4" },
      transportOptions: {
        polling: {
          extraHeaders: {
            token: "1c74024f-1a9c-437b-bf1c-2f22adaa0642",
            userId: 100252,
          },
        },
      },
      reconnectionAttempts: 20,
    });

    socket.on("connect", (socket) => {
      console.log("connect successed");
    });

    socket.on("connect_error", (err) => {
      console.log("connect_error");
      console.log(err);
      console.log(err.message);
      if (err.message === "authentication failed") {
        // socket.disconnect()
      }
    });

    socket.on("disconnect", (res) => {
      console.log(res);
    });

    socket.on("connect_failed", (res) => {
      console.log(res);
    });

    socket.on("error", (err) => {
      console.log(err);
    });
  </script>
</html>

python 3.9.1. windows10.
If code socket.disconnect() is not called, the heartbeat detection mechanism will always run.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Feb 15, 2021

@1111mp When you asked me about the behavior of the client I assumed your were using a Python based client. You are using a JavaScript client, so I cannot really answer for the behavior of that client. Maybe with that client you do have to call disconnect when you get a connection error. I have no control in that case.

@siktec-lab
Copy link

console.log(res);

Old but worth mentioning - @1111mp your observation seems to be right. When you get a connect_error on the js client side you should disconnect() manually. It seems that if you don't the server will establish the connection (yet its not usable) and will PING / PONG it....

I suspect it has to be with some sort of reconnection mechanism or maybe expecting another connect / reconnect but I'm not sure nor can't find any reference of that in the client source code.

For me manually handling and executing diconnect() did the trick and kept the connection pool clean...

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

No branches or pull requests

4 participants