Re: One last ldap_connect headache

On Thu, Nov 5, 2015 at 3:41 AM, Bishop Bettini <bishop@php.net> wrote: > On Tue, Nov 3, 2015 at 11:13 PM, Côme Chilliet <come@opensides.be> wrote: > > > I got mail from someone saying that in previous version, calling > > ldap_connect($host, NULL) would use default port. > > While now it is considered as trying to use port 0 and trigger an error.. > > > > I believe the current behavior (interpret as zero and trigger error) is > correct. > we have precedence for both behavior. > > > > > I’m a bit troubled about it because the documentation says: > > resource ldap_connect ([ string $hostname = NULL [, int $port = 389 ]] ) > > So $port defaults to 389 and not to NULL, and it says nothing about > > accepting NULL as second parameter. > > > > Let's compare to another PHP function that takes a numeric optional > parameter with a non-zero default: > > array array_unique ( array $array [, int $sort_flags = SORT_STRING ] ) > > Note that SORT_STRING === 2, SORT_REGULAR === 0. > > If you pass null for the second argument, you get the SORT_REGULAR > behavior, not the default SORT_STRING behavior: > > print_r(array_unique([ 1, '1', '1a' ], null)); > on the other hand functions like ftp_connect assumes the default port when passed NULL: [tyrael@Ferencs-MBP]$ php -r '$c=ftp_connect("ftp.de.debian.org", NULL);ftp_login($c, "anonymous", "");print_r(ftp_nlist($c, "-la ."));' Array ( [0] => debian-security [1] => backports.org [2] => debian-archive [3] => . [4] => HEADER.html [5] => .welcome.msg [6] => debian-backports [7] => debian-ports [8] => debian-cd [9] => debian [10] => .. [11] => pub ) > > > That said, it does seem handful to be able to pass NULL to ask for default > > port without remembering which one it is. > > The context is something like: > > $port = NULL; > > if (isset($options['port'])) { > > $port = $options['port']; > > } > > $resource = ldap_connect($host, $port); > > > > A few reasons I'd offer as arguments against this. $port is deprecated, so > why add features to deprecated arguments? Other PHP internal functions > don't behave this way (array_unique, socket_create_listen, ssh2_connect, > etc.), so why go against the grain? Why not document (in a comment) the > preferred way of doing this, which might be: > > if (isset($options['port'])) > $resource = ldap_connect($host, $options['port']); > else > $resource = ldap_connect($host); > I'm fine with changing this for future versions but my understanding is that this BC break (along with some others) was introduced in a micro release (5.6.11 afair), so at least for 5.6 I would prefer restoring the old behavior and for 7.0 I'm fine with the change but please document it in NEWS and UPGRADING so we can incorporate it in the docs. -- Ferenc Kovács @Tyr43l - http://tyrael.hu