Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119426
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-85-gd6d859e0cf-fm-20230116.001-gd6d859e0
Mime-Version: 1.0
Message-ID: <ba0844cb-48e0-452a-a390-069fe3a069da@app.fastmail.com>
In-Reply-To: <f90d3a56-a832-7d42-38c0-22ecd4c26901@heigl.org>
References: <f90d3a56-a832-7d42-38c0-22ecd4c26901@heigl.org>
Date: Fri, 27 Jan 2023 09:53:39 -0600
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain
Subject: Re: [PHP-DEV] Deprecate ldap_connect with host and port as separate arguments
From: larry@garfieldtech.com ("Larry Garfield")

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