Allow "socket" option to pass existing socket (e.g. for SOCKS5 proxying)

[laszloKajan]

Thank you very much for this excellent module.

Please consider my pull request: I would need this in order to be able to use ldapjs with an pre-existing SocksClient socket (that comes from 'socks') with 'ldap://' and STARTTLS.

('ldaps://' already works with a pre-existing socket which can be passed via 'tlsOptions'.)

You could mention the ability to use a proxy on the documentation website http://ldapjs.org/client.html . If need be, I can contribute my example code.

Best regards, Laszlo

[jsumners]

Thank you for the contribution. I think this would need to be supplied as an option at construction time and then used within the connect method. Requiring a pre-existing connection to be passed to the connect method is not an ideal API. Also, we will need unit tests before this can be accepted.

[laszloKajan]

Dear James!

Thank you for the contribution. I think this would need to be supplied as an option at construction time and then used within the connect method. Requiring a pre-existing connection to be passed to the connect method is not an ideal API. Also, we will need unit tests before this can be accepted.

Thank you for writing back to me.
In my pull request, I went for an implementation with the smallest number of lines changed.
I'll implement what you suggest when I have time.

Best regards,
Laszlo

[jsumners]

👋

On February 22, 2023, we released version 3 of this library. As a result, we are closing this issue/pull request.

Please see issue #839 for more information, including how to proceed if you feel this closure is in error.