Deprecate ldap_connect with host and port as separate arguments

php.internals

Andreas Heigl

3 years ago
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
-- ,,, (o o) +---------------------------------------------------------ooO-(_)-Ooo-+ | Andreas Heigl | | mailto:andreas@heigl.org N 50°22'59.5" E 08°23'58" | | https://andreas.heigl.org | +---------------------------------------------------------------------+ | https://hei.gl/appointmentwithandreas | +---------------------------------------------------------------------+ | GPG-Key: https://hei.gl/keyandreasheiglorg | +---------------------------------------------------------------------+

Larry Garfield

3 years ago
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

Unnamed Person

3 years ago
On Fri, Jan 27, 2023 at 8:54 AM Larry Garfield <larry@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.

Christoph Becker

3 years ago
On 27.01.2023 at 17:06, Levi Morrison via internals wrote:
> On Fri, Jan 27, 2023 at 8:54 AM Larry Garfield <larry@garfieldtech.com> wrote: >> >> On Fri, Jan 27, 2023, at 3:00 AM, Andreas Heigl wrote: >> >>> I think it would be a good idea to deprecate calling ldap_connect with 2 >>> parameters host and port. >> >> 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.> > 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.
In my opinion, a dedicated RFC would be overkill, but it could be added to the deprecations for PHP 8.3 RFC[1] what doesn't require much effort (feel free to add it, Andreas), and this way there would be more visibility, and we have a vote. [1] <https://wiki.php.net/rfc/deprecations_php_8_3>
-- Christoph M. Becker

Larry Garfield

3 years ago
On Fri, Jan 27, 2023, at 10:28 AM, Christoph M. Becker wrote:
> On 27.01.2023 at 17:06, Levi Morrison via internals wrote: > >> On Fri, Jan 27, 2023 at 8:54 AM Larry Garfield <larry@garfieldtech.com> wrote: >>> >>> On Fri, Jan 27, 2023, at 3:00 AM, Andreas Heigl wrote: >>> >>>> I think it would be a good idea to deprecate calling ldap_connect with 2 >>>> parameters host and port. >>> >>> 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.> >> 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. > > In my opinion, a dedicated RFC would be overkill, but it could be added > to the deprecations for PHP 8.3 RFC[1] what doesn't require much effort > (feel free to add it, Andreas), and this way there would be more > visibility, and we have a vote. > > [1] <https://wiki.php.net/rfc/deprecations_php_8_3>
That makes sense to me. --Larry Garfield

Andreas Heigl

3 years ago
On 27.01.23 17:28, Christoph M. Becker wrote:
> On 27.01.2023 at 17:06, Levi Morrison via internals wrote: > >> On Fri, Jan 27, 2023 at 8:54 AM Larry Garfield <larry@garfieldtech.com> wrote: >>> >>> On Fri, Jan 27, 2023, at 3:00 AM, Andreas Heigl wrote: >>> >>>> I think it would be a good idea to deprecate calling ldap_connect with 2 >>>> parameters host and port. >>> >>> 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.> >> 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. > > In my opinion, a dedicated RFC would be overkill, but it could be added > to the deprecations for PHP 8.3 RFC[1] what doesn't require much effort > (feel free to add it, Andreas),
Done. Cheers Andreas
-- ,,, (o o) +---------------------------------------------------------ooO-(_)-Ooo-+ | Andreas Heigl | | mailto:andreas@heigl.org N 50°22'59.5" E 08°23'58" | | https://andreas.heigl.org | +---------------------------------------------------------------------+ | https://hei.gl/appointmentwithandreas | +---------------------------------------------------------------------+ | GPG-Key: https://hei.gl/keyandreasheiglorg | +---------------------------------------------------------------------+

Côme Chilliet

3 years ago
Le vendredi 27 janvier 2023, 10:00:35 CET Andreas Heigl a écrit :
> Hey Folks. > > I think it would be a good idea to deprecate calling ldap_connect with 2 > parameters host and port.
Hello, My long term plan was to replace it by a constructor for the new \LDAP\Connection class that only accepts the URI syntax. Which would also be better because ldap_connect is a really confusing name as it does not actually connect to anything. But I’m unfamiliar with how to write object methods into PHP modules, and I do not have much time to allocate to php-ldap. If you are interested into working on some OO methods for php-ldap classes, I have a ton of ideas on how to make it awesome. Côme

Girgias

3 years ago
On Tue, 7 Feb 2023 at 12:56, Côme Chilliet <come@chilliet.eu> wrote:
> Le vendredi 27 janvier 2023, 10:00:35 CET Andreas Heigl a écrit : > > Hey Folks. > > > > I think it would be a good idea to deprecate calling ldap_connect with 2 > > parameters host and port. > > Hello, > > My long term plan was to replace it by a constructor for the new > \LDAP\Connection class that only accepts the URI syntax. Which would also > be better because ldap_connect is a really confusing name as it does not > actually connect to anything. > > But I’m unfamiliar with how to write object methods into PHP modules, and > I do not have much time to allocate to php-ldap. > > If you are interested into working on some OO methods for php-ldap > classes, I have a ton of ideas on how to make it awesome. > > Côme >
Please let me know and I can spend some time on it. I also think working with objects was made significantly easier now that we have proper stubs and derive a lot of code generation from it. Best regards, George P. Banyard