Skip to content

Retry on WANT_WRITE & WANT_READ in sendall() #954

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Retry on WANT_WRITE & WANT_READ in sendall() #954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ Deprecations:
Changes:
^^^^^^^^

- Fixed the inability of ``OpenSSL.SSL.Connection.sendall()`` to
keep with sending data over the wire after ``SSL_ERROR_WANT_READ``
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
keep with sending data over the wire after ``SSL_ERROR_WANT_READ``
keep sending data over the wire after ``SSL_ERROR_WANT_READ``

or ``SSL_ERROR_WANT_WRITE`` is returned by ``SSL_write()``.
`#176 <https://github.com/pyca/pyopenssl/pull/176>`_
- Added a new optional ``chain`` parameter to ``OpenSSL.crypto.X509StoreContext()``
where additional untrusted certificates can be specified to help chain building.
`#948 <https://github.com/pyca/pyopenssl/pull/948>`_
Expand Down
9 changes: 8 additions & 1 deletion src/OpenSSL/SSL.py
View file
Open in desktop
Copy link
Contributor

@julianz- julianz- Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here seems a bit confusing. If result or bytes_written is > 0, then this was a successful partial or full write. No need to call _raise_ssl_error(). Just advance the buffers and continue. If bytes_written <= 0, then this was a failure and we need to see what kind of failure occurred by calling _raise_ssl_error(). The comment that the flag SSL_MODE_ENABLE_PARTIAL_WRITE guarantees no bytes have been written when there is a failure I think is misleading. Even without that flag after a WantReadError or WantWriteError, we can assume no bytes made it and we need to retry the same buffer again. All SSL_MODE_ENABLE_PARTIAL_WRITE does I think is allow for the possibility of partial successful writes.

So I would suggest something like:

bytes_written = _lib.SSL_write(
    self._ssl, data + total_sent, min(left_to_send, 2147483647)
)
if bytes_written > 0:
    # we had a successful partial or full write
    total_sent += bytes_written
    left_to_send -= bytes_written
else:
  # bytes_written <= 0 means there was a failure
  try:
      # Check for retryable errors (WantRead/WantWrite) or fatal errors.
      self._raise_ssl_error(self._ssl, bytes_written)
  except (WantReadError, WantWriteError):
      # Retryable error: Do nothing (total_sent remains unchanged).
      continue
  # Other errors propagate up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reaperhulk @alex do you have opinions on this suggestion?

Original file line number Diff line number Diff line change
Expand Up @@ -2053,7 +2053,14 @@ def sendall(self, buf, flags=0):
result = _lib.SSL_write(
self._ssl, data + total_sent, min(left_to_send, 2147483647)
)
self._raise_ssl_error(self._ssl, result)
try:
self._raise_ssl_error(self._ssl, result)
except (WantReadError, WantWriteError):
# NOTE: The use of SSL_MODE_ENABLE_PARTIAL_WRITE
# NOTE: above guarantees that in case of failure
# NOTE: no bytes have been written so we don't need
# NOTE: to update the counters, just need to retry.
continue
total_sent += result
left_to_send -= result

Expand Down