Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87730
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain heigl.org from 195.191.240.18 cause and error)
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (1.0)
In-Reply-To: <55CB2749.2020809@kippdata.de>
Date: Wed, 12 Aug 2015 13:17:24 +0200
Cc: PHP internals <internals@lists.php.net>,
 =?utf-8?Q?C=C3=B4me_BERNIGAUD?= <come.bernigaud@opensides.be>
Content-Transfer-Encoding: quoted-printable
Message-ID: <1D4FAD35-B39D-48B0-AC68-546F945AD067@heigl.org>
References: <5582BC3E.30109@opensides.be> <55C8FFF9.40509@kippdata.de> <2E228E18-2AC9-4FA5-AB11-4F7C3365EB0B@heigl.org> <55C92754.8040308@kippdata.de> <55CA0DAD.8010405@opensides.be> <55CB2749.2020809@kippdata.de>
To: Rainer Jung <rainer.jung@kippdata.de>
Subject: Re: [PHP-DEV] LDAP module requirements?
From: andreas@heigl.org (Andreas Heigl)

Hi Rainer.=20

> Am 12.08.2015 um 13:00 schrieb Rainer Jung <rainer.jung@kippdata.de>:
>=20
> Hi C=C3=B4me,
>=20
>> Am 11.08.2015 um 16:58 schrieb C=C3=B4me BERNIGAUD:
>>> On 2015-08-11 00:36, Rainer Jung wrote:
>>> The current problems should be mostly around the above four compiler
>>> warnings. I can test any patches you want me to test.
>>=20
>> Can you test including the attached file in ext/ldap/ldap.c, and not
>> defining HAVE_3ARG_SETREBINDPROC and LDAP_CONTROL_PAGEDRESULTS ?
>=20
> thanks for the patch proposal. There's a few problems with it and I will c=
ome back to you with an alternative proposal soon. For the moment:
>=20
>=20
> - the definition of ldap_control_find in terms of at the top of ldap.c cur=
rently is protected by
>=20
> #if !defined(HAVE_LDAP_CONTROL_FIND)
>=20
> It should be
>=20
> #if defined(LDAP_CONTROL_PAGEDRESULTS) && !defined(HAVE_LDAP_CONTROL_FIND)=

>=20
> That's not related to your patch.
>=20
>=20
> - in
>=20
> void ldap_memvfree( void **value )
> {
>  ldap_value_free(value);
> }
>=20
>=20
> it should probably be
>=20
> ldap_value_free((char **)value);
>=20
> otherwise we get compiler warnings.
>=20
>=20
> - instead of ldap_open() we can use ldap_init() (recommended).
>=20
>=20
> - using ldap_open() or ldap_init() with a url does not work, we have to us=
e host and port.
>=20
>=20
> - the use of ldap_sasl_bind_s() in PHP_FUNCTION(ldap_bind) does not work, w=
ithout setting "LDAP_OPT_PROTOCOL_VERSION" to LDAP_VERSION3 using ldap_set_o=
ption(). Maybe it works for OpenLDAP, but Netscape and Solaris document it m=
ust be set and indeed return LDAP_NOT_SUPPORTED immediately otherwise. But s=
ince in PHP_FUNCTION(ldap_bind) no SASL functionality is actually being used=
, I propose to use ldap_simple_bind_s() instead of ldap_sasl_bind_s(). It is=
 not deprecated and using it actually simplifies the code a bit.
>=20
>=20
> As I said, I'll come up with a patch proposal soon.
>=20
> The patch should also help for other LDAP implementations than OpenLDAP or=
 Solaris.
>=20
The main question here is whether we want PHP to support other implementatio=
ns. The documentation clearly states that OpenLDAP shall be used and even Or=
acle states on an official site (I don't actually have the URL at hand but i=
t has been in an email to this thread) that PHPs LDAP extension has to be bu=
ilt against OpenLDAP.=20

Come has done a great job to clean up the LDAP code to start implementing lo=
ng awaited features and I'm not really sure whether we should give that up f=
or maintaining compatibility with some edge-case usages. So I'm very interes=
ted in your patch to see how we can rech both goals!

In the meantime you can still use the "old" ext/ldap for the newer PHP-branc=
h as it should compile without an issue. The "newer" ext/ldap just removed s=
tuff ;)

Cheers

Andreas
> Regards,
>=20
> Rainer