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

Bugfixes and Improvements #23

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

mrname
Copy link

@mrname mrname commented Mar 25, 2021

A few bugfixes and improvements:

  • Allow specifying max chunk size. In the case of smaller files it can make sense to send them in a single chunk
  • Switch socket calls from send to sendall. This ensures that the entire message is pushed in the socket call. In our use case, NOT using sendall caused an issue when using clamd in association with gevent, since each socket message may or may not have pushed the full chunk, and they are working cooperatively. Aside from this use case, it also proved to be a more efficient way to send the socket messages.
  • Set timeout prior to connection. This ensures that if the server is not available for some reason, timeout will be respected during initial connection.

@nilstoedtmann
Copy link

We are observing a similar issue, where python-clamd sends an incomplete INSTREAM chunk at some point (in one case it was chunk no #1012). Once that happens, the chunks get misaligned, the clamd service parses the next chunk's length from the wrong bytes and all sorts of entertaining nonsense ensues.
We are also using gevent, so this is a hot candidate for causing our issue.

@nilstoedtmann
Copy link

Regarding max_chunk_size:

If I understand the clamd protocol correctly, then StreamMaxLength is the maximum total length of all chunks together, not of the individual chunks. So what is the disadvantage of sending the whole buffer in a single chunk anyway, regardless of size?

@stringfellow
Copy link

@graingert - do you have any space to look at this PR? The change-set fixes what is (for us) a major and intermittent bug, and it would be great to be using master release version, rather than a branch :)

stringfellow added a commit to DemandLogic/clammy that referenced this pull request May 6, 2021
- use sendall() instead of send()
- allow custom max_buffer_size
- set socket timeout before connect

Thanks to @mrname
ranguli pushed a commit to ranguli/clammy that referenced this pull request Jan 18, 2022
- use sendall() instead of send()
- allow custom max_buffer_size
- set socket timeout before connect

Thanks to @mrname
ranguli added a commit to ranguli/clammy that referenced this pull request Jan 18, 2022
* apply changes from graingert#23 (#8)

- use sendall() instead of send()
- allow custom max_buffer_size
- set socket timeout before connect

Thanks to @mrname

* Add CI, code quality improvements, fix tests

* Don't run pytest in CI since there's no clamd

Co-authored-by: Steve Pike <[email protected]>
@Viicos
Copy link

Viicos commented Oct 23, 2023

Although a few forks have been created (https://pypi.org/project/pyClamd/, https://github.com/ranguli/clammy), they now seem unmaintained (last one was archived recently).

I've made yet another fork here: https://github.com/Viicos/clamdpy. Improvements from this PR have been included, and type hints are included. Credit goes to https://github.com/graingert/python-clamd/

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.

4 participants