search

michaelwittig / node-q

Q interfacing with Node.js
MIT License
52 stars 14 forks source link

Add support for TLS socket connections #60

Closed OiNutter closed 2 years ago

OiNutter commented 2 years ago

This both adds support for a useTLS parameter that will then make the library create a TLSSocket as opposed to a normal net.Socket. It also adds an extra one time error handler on the Connection instance before attempting auth to catch an ECONNRESET error that is thrown when attempting to connect to a TLS only kdb server without using a TLS Socket.

I'm happy to try and add tests for this but wasn't sure of the best way to create a test server for it in the test suite. In my tests I used self signed certificates but I had to temporarily override the security settings to get node to allow that.

michaelwittig commented 2 years ago

If you could add an "integration test" that would be great: https://github.com/michaelwittig/node-q#integration-test e.g., we could say that we assume an encrypted node process on port 6000 or so. I'm fine with a self signed cert. Can you document the steps that are needed to spin everything up?

OiNutter commented 2 years ago

Have added a couple of basic tests for handling the error if you connect to tls without setting useTLS and vice versa, and for successfully connecting if you do. Let me know if you think we need more.

michaelwittig commented 2 years ago

@OiNutter Thanks for adding the tests! I added two minor comments. Once they are fixed I'm happy to merge and release this PR.

OiNutter commented 2 years ago

Have made those changes now. Let me know if you want a bit more explanation of the TLS connection, I tried to keep to the same level of detail as the other examples. Most likely if somebody want's that setting they know what for.

michaelwittig commented 2 years ago

Thanks @OiNutter Version 2.6.0 is released.