ssl.SSLError during handshake can teardown worker process

[normal-cock]

prevent socket which is listening been close when TcpConnectionUninitializedException is raised

[normal-cock]

The abnormal case is that, when TcpConnectionUninitializedException is raised the worker process close the listening socket making the worker not work any more.

[abhinavsingh]

I am wondering why is TcpConnectionUninitializedException being thrown here, we are just passing fileno=conn.fileno() to ProtocolHandler class, so connection is pretty much initialized.

TcpConnectionUninitializedException can still be thrown, for example, if client connection is closed, _conn object has been deleted but code is still trying to access the connection property (either to send or recv data). In which case, I think it's a programming bug.

Do you have an example stack trace for further investigation? Are you using proxy.py via a custom plugin? Thank you.

[normal-cock]

In my case, TcpConnectionUninitializedException have been thrown as result of the failure of ssl handshake of when https is enabled. In this case, result of self.optionally_wrap_socket is None, so TcpConnectionUninitializedException is thrown.

class ProtocolHandler(threading.Thread):
   def __init__
        ...
        conn = self.optionally_wrap_socket(self.fromfd(fileno))
        if conn is None:
            raise TcpConnectionUninitializedException()

I'm not using proxy.py via a custom plugin. I use it with a self-signed certificate.

[abhinavsingh]

Makes sense, indeed ssl wrap can fail. However, I think we can improve here a little so that we can log more helpful context. Logging logger.error('TcpConnectionUninitializedException while handling new connection') isn't very clear of what actually happened. I probably just raised TcpConnectionUninitializedException without thinking twice about it.

How about we remove the except Exception from here https://github.com/abhinavsingh/proxy.py/blob/develop/proxy.py#L1543-L1561 so that SSL handshake exception is actually propagated back to worker.

Then in the worker we can:

1. catch the exception
2. log helpful message
3. also shutdown/close client connection

Wdyt?

[normal-cock]

Sounds better and I've changed the code.

[abhinavsingh]

Awesome, thanks.