feat: Allow connection to clamd through a TLS proxy #106
Conversation
inspect permissions in later node
babel-eslint was replaced by @babel/eslint-parser
peer deps once again no need for react packages
|
Thanks for your contribution @carboneater! Looks like the CI suite tests all got cancelled after about 1 minute. Are you able to see the results or not? If not, I'll see what I can do to get you access to seeing them. |
Yes, I can see tests failed with
However, that's the first time I hit this case in github actions... According to the github post, (Side note, even on my branch, the pipeline is flaky on the following test, but that feels like a race condition to me.) |
|
Looks like when I re-ran the tests, everything worked. GitHub Actions don't seem super reliable. But maybe pinning Jaunty was the key. |
There was a problem hiding this comment.
I really appreciate you taking this issue on @carboneater. Looks like dependencies needed some major updates too, so, thanks for taking care of that. I added you as a contributor in the package file :)
There's just a couple really minor things I'd like you to address and I'll get this rebased and merged in.
| @@ -7,10 +7,10 @@ on: | |||
| - master | |||
| jobs: | |||
| test: | |||
| runs-on: ubuntu-18.04 | |||
| runs-on: ubuntu-22.04 | |||
There was a problem hiding this comment.
This might have been the key to getting things to work. 🤞🏼
index.js
Outdated
| @@ -104,6 +106,7 @@ class NodeClam { | |||
| * @param {boolean} [options.clamdscan.reloadDb=false] - If true, will re-load the DB on ever call (slow) | |||
| * @param {boolean} [options.clamdscan.active=true] - If true, this module will consider using the `clamdscan` binary | |||
| * @param {boolean} [options.clamdscan.bypassTest=false] - If true, check to see if socket is avaliable | |||
| * @param {boolean} [options.clamdscan.tls=false] - If true, connect to a TLS-Termination proxy in from of ClamAV | |||
There was a problem hiding this comment.
You have a typo in this JSDoc comment. Did you mean "...in form of ClamAV"?
There was a problem hiding this comment.
Yikes!
I was going for in front of
There was a problem hiding this comment.
That makes a lot more sense, haha.
Co-authored-by: Kyle Farris <kylefarris@gmail.com>
|
I still need to re-run the tests multiple times to get them to complete. Not sure what's up with that. But, the tests have passed so I'll get this merged in. |
Quite a small code change, which required quite a lot of CI patches to get back in working order.