Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:88320
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain kippdata.de designates 195.227.30.149 as permitted sender)
To: Andreas Heigl <andreas@heigl.org>, PHP internals <internals@lists.php.net>
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>
 <1D4FAD35-B39D-48B0-AC68-546F945AD067@heigl.org> <55CCAC0E.50008@kippdata.de>
 <55CCB888.1050605@heigl.org>
Cc: =?UTF-8?Q?C=c3=b4me_BERNIGAUD?= <come.bernigaud@opensides.be>
Message-ID: <55FC0A3F.2020009@kippdata.de>
Date: Fri, 18 Sep 2015 14:57:35 +0200
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.2.0
MIME-Version: 1.0
In-Reply-To: <55CCB888.1050605@heigl.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] LDAP module requirements?
From: rainer.jung@kippdata.de (Rainer Jung)

I checked the latest 5.6 release and it looks fine to me. Thanks for 
incorporating the patch!

Rainer

Am 13.08.2015 um 17:32 schrieb Andreas Heigl:
> Hi Rainer.
>
> Am 13.08.15 um 16:39 schrieb Rainer Jung:
>> Am 12.08.2015 um 13:17 schrieb Andreas Heigl:
>>> Hi Rainer.
>>>
>>>> Am 12.08.2015 um 13:00 schrieb Rainer Jung <rainer.jung@kippdata.de>:
>>>>
>>>> Hi Côme,
>>>>
>>>>> Am 11.08.2015 um 16:58 schrieb Côme 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.
>>>>>
>>>>> Can you test including the attached file in ext/ldap/ldap.c, and not
>>>>> defining HAVE_3ARG_SETREBINDPROC and LDAP_CONTROL_PAGEDRESULTS ?
>>>>
>>>> thanks for the patch proposal. There's a few problems with it and I
>>>> will come back to you with an alternative proposal soon. For the moment:
>>>>
>>>>
>>>> - the definition of ldap_control_find in terms of at the top of
>>>> ldap.c currently is protected by
>>>>
>>>> #if !defined(HAVE_LDAP_CONTROL_FIND)
>>>>
>>>> It should be
>>>>
>>>> #if defined(LDAP_CONTROL_PAGEDRESULTS) &&
>>>> !defined(HAVE_LDAP_CONTROL_FIND)
>>>>
>>>> That's not related to your patch.
>>>>
>>>>
>>>> - in
>>>>
>>>> void ldap_memvfree( void **value )
>>>> {
>>>>    ldap_value_free(value);
>>>> }
>>>>
>>>>
>>>> it should probably be
>>>>
>>>> ldap_value_free((char **)value);
>>>>
>>>> otherwise we get compiler warnings.
>>>>
>>>>
>>>> - instead of ldap_open() we can use ldap_init() (recommended).
>>>>
>>>>
>>>> - using ldap_open() or ldap_init() with a url does not work, we have
>>>> to use host and port.
>>>>
>>>>
>>>> - the use of ldap_sasl_bind_s() in PHP_FUNCTION(ldap_bind) does not
>>>> work, without setting "LDAP_OPT_PROTOCOL_VERSION" to LDAP_VERSION3
>>>> using ldap_set_option(). Maybe it works for OpenLDAP, but Netscape
>>>> and Solaris document it must be set and indeed return
>>>> LDAP_NOT_SUPPORTED immediately otherwise. But since 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.
>>>>
>>>>
>>>> As I said, I'll come up with a patch proposal soon.
>>>>
>>>> The patch should also help for other LDAP implementations than
>>>> OpenLDAP or Solaris.
>>>>
>>> The main question here is whether we want PHP to support other
>>> implementations. The documentation clearly states that OpenLDAP shall
>>> be used and even Oracle states on an official site (I don't actually
>>> have the URL at hand but it has been in an email to this thread) that
>>> PHPs LDAP extension has to be built against OpenLDAP.
>>
>> That's you (project) call. You are doing the work, you decide. I
>> wouldn't rely on Oracle for a judgement ;)
>
> We should all decide (somehow) ;) And thanks for the warning about Oracle ;)
>>
>> Also note that the code and m4 macros have quite a few places where they
>> react on other LDAP SDKs, like Windows, Oracle (not sun) and I think
>> also Netware. But those are not really strong arguments for multi-SDK
>> support. You can purge those places and the argument is gone. I really
>> think it depends on your goals.
>>
>> But you should consider 5.6 a stable release. Maybe I'm the only guy on
>> the planet haven't build the ldap extension against non-OpenLDAP, but I
>> doubt it. 7.0 is in front of the doors and compatibility is much less an
>> issue there, maybe combined with a NEWS entry strengthening, that you
>> now really demand OpenLDAP and throw an error, if you can't find it
>> during configure.
>
> You are actually right on that one. We should keep BC in the 5.6
> release. That was the reason for the call to what other libs are used
> where no one stepped up for others and therefore there wasn't any BC
> expected.... :(
>
> @Côme, can we fix that in the 5.6 branch somehow?
>
>>
>>> Come has done a great job to clean up the LDAP code to start
>>> implementing long awaited features and I'm not really sure whether we
>>> should give that up for maintaining compatibility with some edge-case
>>> usages. So I'm very interested in your patch to see how we can rech
>>> both goals!
>>
>> Agreed, and much of the cleanup wouldn't have to be undone to make the
>> result still compatible with more SDKs. For instance all of the new use
>> of the "_ext" variants of functions is nice and OK also for other LDAP
>> implementations.
>
> That's great! I'm not that deep into the C-business (yet) therefore I'll
> need a day or two to digest the patch. I'd just love to see the
> LDAP-extension move towards better implementation of such things as
> paginated and/or serversided sorted results.
>
> Thanks for your input and contribution!
>
> Cheers
>
> Andreas