On Thu, Nov 5, 2015 at 3:41 AM, Bishop Bettini <bis...@php.net> wrote:

> On Tue, Nov 3, 2015 at 11:13 PM, Côme Chilliet <c...@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

Reply via email to