pyopenssl icon indicating copy to clipboard operation
pyopenssl copied to clipboard

Retry on `WANT_WRITE` & `WANT_READ` in `sendall()`

Open webknjaz opened this issue 3 years ago • 3 comments

This change introduces retries in OpenSSL.SSL.Connection.sendall() when WANT_WRITE_ERROR or WANT_READ_ERROR happen.

It relies on SSL_MODE_ENABLE_PARTIAL_WRITE being set on the context, that changes the mode of SSL_write() to return errors only if zero bytes has been sent making it safe to retry in these cases.

Ideally, the calling code is supposed to poll()/select() the socket to know when it's okay to attempt the next retry (hence it is readable or writable) but it's not available in the sendall() method and just retrying the operation is good enough.

Fixes #176

Refs:

  • http://openssl.6102.n7.nabble.com/SSL-MODE-ACCEPT-MOVING-WRITE-BUFFER-td6421.html
  • https://stackoverflow.com/a/28992313/595220
  • https://www.openssl.org/docs/manmaster/man3/SSL_write.html
  • https://stackoverflow.com/a/20817394/595220

webknjaz avatar Nov 09 '20 00:11 webknjaz

Is there a way we can cover this with a test? And is SSL_MODE_ENABLE_PARTIAL_WRITE being set anywhere?

reaperhulk avatar Nov 09 '20 04:11 reaperhulk

And is SSL_MODE_ENABLE_PARTIAL_WRITE being set anywhere?

It's set in the context initializer: https://github.com/pyca/pyopenssl/blob/124a013/src/OpenSSL/SSL.py#L660. Also, the docstring of send() seems to imply the same: https://github.com/pyca/pyopenssl/blob/124a013/src/OpenSSL/SSL.py#L1623-L1625 — I did not understand why is that at first but now it makes sense.

Is there a way we can cover this with a test?

I'm not sure how to properly trigger this in a stable manner at the moment. Although, the suspicion is that maybe if we could limit the buffer size of the test socket and feed a sufficiently big chunk of data into it, it should work. https://github.com/pyca/pyopenssl/blob/124a0134fdb7decb0136b4b6f7892b87b919e74e/tests/test_ssl.py#L2659-L2690 seems to be doing this exactly.

webknjaz avatar Nov 09 '20 09:11 webknjaz

I'm trying to come up with with a reproducer @ https://github.com/pyca/pyopenssl/pull/955 but it still needs polishing.

webknjaz avatar Nov 09 '20 18:11 webknjaz