search

solita / clamav-java

Simple ClamAV client for streaming data to clamd server
GNU Lesser General Public License v2.1
106 stars 46 forks source link

Added support for connecting to LocalSocket (Unix Socket) files #21

Closed hayes-roach closed 1 week ago

hayes-roach commented 3 years ago

@lokori @drogin @anttivi-solita Can I get someone to review this, please?

drogin commented 3 years ago

@hayes-roach I have added a PR for index out of bounds exception as well, which has been open for quite a while. I'm not sure if the authors/solita is actively maintaining this repository anymore. I have no authority here, although I have previously contributed a bit.

As for my feedback on the PR: Two things makes me reluctant:

  1. The import of a code we don't fully understand/control. Maybe not much of an issue, but so far the project has been very minimal, with few dependencies and potential security flaws.
  2. To use UNIX Sockets, I presume the library will use JNI, which at least historically is considered slow.

And point 2 makes me ask, why do you need local sockets? ClamAV can be configured to use regular TCP/IP sockets and loopback/localhost interface. Is it because you want to avoid the IP-layer to gain performance? If so, I would benchmark if it actually is faster. My theory is that your code would gain speed due to UNIX Socket, but lose speed due to JNI. I'm unsure of the scale between the two, so I would benchmark

hayes-roach commented 3 years ago

@drogin Thanks for the feedback, I understand the dependency situation as it could possibly bring in some security problems. My reasoning for adding UNIX Sockets is because of improved performance as well as security issues that can arise if the TCP/IP sockets are not configured correctly. We are currently using this repository with UNIX Sockets in some of our services.