Skip to content

use sendall vs send which does not guarentee all bytes will be sent#9

Merged
sevein merged 1 commit intoartefactual-labs:mainfrom
RetailCapital:use_sendall_to_ensure_all_bytes_sent
Mar 5, 2026
Merged

use sendall vs send which does not guarentee all bytes will be sent#9
sevein merged 1 commit intoartefactual-labs:mainfrom
RetailCapital:use_sendall_to_ensure_all_bytes_sent

Conversation

@nwray-credibly
Copy link
Copy Markdown

@nwray-credibly nwray-credibly commented Mar 4, 2026

In clamav_client.clamd there are three uses of self.clamd_socket.send. Socket.send is a low-level function which does not (necessarily, by contract) send the entire buffer. Instead it returns the number of bytes actually sent, and the caller is required to manage the unsent bytes by passing them to socket.send until all are sent.

from the python spec for send: https://docs.python.org/3/library/socket.html#socket.socket.send

Send data to the socket. The socket must be connected to a remote socket. The optional flags argument has the same meaning as for recv() above. Returns the number of bytes sent. Applications are responsible for checking that all data has been sent; if only some of the data was transmitted, the application needs to attempt delivery of the remaining data.

compare to sendall: https://docs.python.org/3/library/socket.html#socket.socket.sendall

Unlike send(), this method continues to send data from bytes until either all data has been sent or an error occurs.

In the existing code, the return value from socket.send is not checked. This is something the library generally "gets away with" since it is sending at most 1024 bytes per send. However under gevent this is causing failures since the threads are changed more frequently. So we're seeing the impact of this defect (as was the author of this PR in 2021 against the source repo of clamd: graingert#23).

The PR presented replaces the low-level "send" call with the required "sendall" call which behaves as the code appears to expect.

Copy link
Copy Markdown
Member

@sevein sevein left a comment

Choose a reason for hiding this comment

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

Thank you!

@sevein sevein merged commit 56f3f1d into artefactual-labs:main Mar 5, 2026
0 of 2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants