go-ldap / ldap

Basic LDAP v3 functionality for the GO programming language.
Other
2.19k stars 352 forks source link

Connection.Start is deprecated without alternative #507

Closed dornimaug closed 2 months ago

dornimaug commented 2 months ago

As discussed in https://github.com/go-ldap/ldap/issues/356 and implemented in https://github.com/go-ldap/ldap/pull/499 the Connection.Start function has been deprecated, but there is now no way to use net.DialContext instead of one of the ldap.Dial-functions. We currently initialize connections this way:

    dialer := tls.Dialer{
        ...
    }
    netConn, err := dialer.DialContext(ctx, "tcp", server)
    if err != nil {
        ...
    }
    c := ldap.NewConn(netConn, true)
    c.SetTimeout(readTimeout)
    c.Start()

How can the same (using DialContext and setting a timeout) be achieved through non-deprecated APIs of the ldap package?

Should ldap.NewConn not have been deprecated alongside Connection.Start or is there any legitimate way left to use it?

cpuschma commented 2 months ago

NewConn will be marked deprecated as well in the next release. You can use DialWithDialer to specify a custom dialer to use: https://github.com/go-ldap/ldap/blob/22600126bef6ad8aa565a61df56efbdc445157e5/conn.go#L130-L135

You can use the connection option in Dial, DialTLS or DialURL: https://github.com/go-ldap/ldap/blob/22600126bef6ad8aa565a61df56efbdc445157e5/v3/conn.go#L225-L229

dornimaug commented 2 months ago

There is no DialOpt to provide a context.Context to be used together with a net.Dialer's DialContext function. go-ldap currently uses Dial functions and never DialContext. So there is still no way to implement the same functionality as in my code snippet using non-deprecated APIs. The use of DialContext/context.Context is necessary to make cancellation of the dial operation possible.

cpuschma commented 2 months ago

Agreed. Context implementation is still open and mostly unfinished (see #406). I'll consider revertig the deprecation, as we didn't thought of this use case. Sorry for any inconveniences. The function should work just fine in the meantime.

gustavoluvizotto commented 2 months ago

Hi, Please see https://github.com/go-ldap/ldap/issues/356#issuecomment-2051221652

cpuschma commented 2 months ago

I just released a new version with the reverted commit: https://github.com/go-ldap/ldap/releases/tag/v3.4.8

Again, thank you for pointing this out, I'm sorry for any inconveniences.