Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:89094
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.42 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAEYWF=69goSpbPM6P_vwgwQwNddJynKrOmnUaV0AhFHB0ddcSw@mail.gmail.com>
References: <3187501.to7rK1FKUl@mcmic-probook>
	<2680231.U9fny6VzaI@mcmic-probook>
	<CAEYWF=69goSpbPM6P_vwgwQwNddJynKrOmnUaV0AhFHB0ddcSw@mail.gmail.com>
Date: Fri, 6 Nov 2015 11:43:58 +0100
Message-ID: <CAH-PCH58fYRfP2s54ddeZYThdqWRgZYuj_dTvcYWvO1JDuFGAg@mail.gmail.com>
To: bishop@php.net
Cc: =?UTF-8?Q?C=C3=B4me_Chilliet?= <come@opensides.be>, 
	PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11443cb4c4857e0523dceb4c
Subject: Re: [PHP-DEV] One last ldap_connect headache
From: tyra3l@gmail.com (Ferenc Kovacs)

--001a11443cb4c4857e0523dceb4c
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

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=C3=B4me Chilliet <come@opensides.be> w=
rote:
>
> > 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=E2=80=99m a bit troubled about it because the documentation says:
> >  resource ldap_connect ([ string $hostname =3D NULL [, int $port =3D 38=
9 ]] )
> > 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 =3D SORT_STRING ] )
>
> Note that SORT_STRING =3D=3D=3D 2, SORT_REGULAR =3D=3D=3D 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=3Dftp_connect("ftp.de.debian.org",
NULL);ftp_login($c, "anonymous", "");print_r(ftp_nlist($c, "-la ."));'
Array
(
    [0] =3D> debian-security
    [1] =3D> backports.org
    [2] =3D> debian-archive
    [3] =3D> .
    [4] =3D> HEADER.html
    [5] =3D> .welcome.msg
    [6] =3D> debian-backports
    [7] =3D> debian-ports
    [8] =3D> debian-cd
    [9] =3D> debian
    [10] =3D> ..
    [11] =3D> pub
)


>
>
> That said, it does seem handful to be able to pass NULL to ask for defaul=
t
> > port without remembering which one it is.
> > The context is something like:
> > $port =3D NULL;
> > if (isset($options['port'])) {
> >         $port =3D $options['port'];
> > }
> > $resource =3D 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 =3D ldap_connect($host, $options['port']);
> else
>     $resource =3D 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.


--=20
Ferenc Kov=C3=A1cs
@Tyr43l - http://tyrael.hu

--001a11443cb4c4857e0523dceb4c--