On Fri, Jan 27, 2023 at 8:54 AM Larry Garfield <la...@garfieldtech.com> wrote: > > On Fri, Jan 27, 2023, at 3:00 AM, Andreas Heigl wrote: > > Hey Folks. > > > > I think it would be a good idea to deprecate calling ldap_connect with 2 > > parameters host and port. > > > > Wait: What? > > > > Currently there are three ways one can call ldap_connect. > > > > 1. With a string $ldap_uri > > 2. With a string $host and an int $port, > > 3. With even more parameters for those that did compile PHP with OracleLDAP. > > > > The 3rd way of calling it is not even documented in the docs as it is a > > very niche edge-case that would only confuse most people. > > > > The 2nd way of calling the function is based on the since some years > > deprecated underlying ldap_open function. Internally we already moved to > > the underlying ldap_initialize-function that requires passing an > > LDAP-URI. For that we are already converting the passed host and port > > into an LDAP-URI of the form 'ldap://$host:$port'. > > > > This already illustrates one of the issues that this way of calling the > > function implies: It is not possible to use ldaps as a schema using that > > way of calling ldap_connect as it will always use ldap as schema. No > > matter which port is passed. > > > > A second reason why I think we should deprecate calling ldap_connect > > with two parameters is, that it does not allow one to pass multiple > > ldap-servers as it is possible using the LDAP-URI. > > > > This is for sure a BC-break but in my opinion a rather small one as > > there are not many users actually using it and there is a clear and easy > > migration path for those that use it: Instead of calling > > > > ldap_connect($host, $port) > > > > one calls > > > > ldap_connect("ldap://$host:$port??369";) > > > > Also most of the users should not be affected at all as they are using > > 3rd party libraries that are already only using an LDAP-URI when calling > > ldap_connect like Laminas\Ldap or Symfony\Ldap > > > > The documentation at https://www.php.net/ldap_connect also explicitly > > states (for some time by now) that using host and port is considered > > deprecated. > > > > Named parameters btw also only support ldap_connect(uri: > > 'ldap://example.com') and ldap_connect(host:'example.com', port:369) > > will throw an error. > > > > There already is a PR open[1] that implements the deprecation so that > > for the upcoming PHP8 releases each call to ldap_connect with 2 > > parameters would emit a deprecation message so that people have enough > > time to adapt their code before we can actually remove using two > > parameters in the next major release. > > > > Thanks for your comments. > > > > Cheers > > > > Andreas > > > > [1] https://github.com/php/php-src/pull/5177 > > This would require an RFC, obviously, but it seems reasonable to me. > "Variable meaning parameters" was always a bad idea, and cleaning them up is > a good idea. > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php >
I disagree that this needs an RFC. The docs have long-said it's deprecated; adding a deprecation message _in code_ to match shouldn't require an RFC. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
