LDAP controls in response View as Markdown

 The "control-patch" looks okay. But the "warning-fix-patch" really

doesn't.
You're actually changing some functions behaviour with some of
those
changes. Maybe you didn't notice but there's ldap_get_values() and
ldap_get_values_len() separately. And there's ldap_bind() and
ldap_sasl_bind(),

What I'm doing is driven by the fact that the ldap_value() family, and
the ldap_bind*() are being deprecated (that is: they're in the library,
but they're not declared in ldap.h any more, and they are implemented as
wrappers to the replacements I'm using, at the cost of extra execution
time and resources, plus decent compiler warnings).

 Perhaps with OpenLDAP, but what about the other implementations with
 which this extension works just fine? You didn't wrap all those

changes
inside the #ifdef's.

Right, sorry. And note that this is true only for OpenLDAP 2.3 (which is
considered stable by the OpenLDAP project but maybe not by distributors
yet...). I'm pretty confident the reworking I put in place is conformant
with most APIs, but, as I said, I likely have no chance to test it
shortly, so I wouldn't stress too much on it.

ldap_valuelen() is the safe implementation of ldapvalue(), because
it can also handle non-null terminated values; this doesn't inhibit the
handling null-terminated ones. Moreover, PHP needs the length of the
string anyway, so... Note that this would allow PHP to use only
ldap_get_values() for both, making the API more simple and robust (I
vaguely recall the issue that brought to implementing
ldap_get_values_len() in PHP, I think it was somewhere in PHP 3 and the
patch took a while to get there).

 I agree it's better this way, but (even as unlikely it is) changing

this
MIGHT break someone's script.

Well, GIGO :) If one uses ldap_get_values_len() with strings (i.e.
null-terminated arrays) the result is fine and consistent; the BER
contains the info about the value length so there's not even the
performance penalty of a strlen(). If one was relying on using
ldap_get_values() for non-null terminated strings ...

 Anyway, it might be the time to cleanup the API of this extension
 and make it simpler. It's pretty confusing as it is.

 The returned arrays are also weird with their "count" fields and

such.

I added them for consistency with the rest of the data returned by PHP's
ldap_get_{entries,attributes,values}(); I'm fine if they get removed, we
also save the counter...

Should I do the cleanup? Should I simply remove the "warning" stuff and
the "count" stuff in the arrays?

Anyway, none of these is an issue to me, so I'm fine with the controls
patch. A final comment: apart from some fuzziness complaints, both
patches apply also to PHP 4 without changes. This means we can get into
production immediately (well, after some testing... :)

 This is a new feature and new features are no longer added in PHP 4.
 We only add new stuff in PHP >= 5.1.

I was speaking for our own packages; usually I don't expect anything to be
backported so far, it's fair enough to see contributed features in the
mainstream at some point.

Thanks for your time. p.

--
Pierangelo Masarati
mailto:pierangelo.masarati@sys-net.it

Ing. Pierangelo Masarati
Responsabile Open Solution

SysNet s.n.c.
Via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it

Office: +39.02.23998309
Mobile: +39.333.4963172
Email: pierangelo.masarati@sys-net.it

 Anyway, it might be the time to cleanup the API of this extension
 and make it simpler. It's pretty confusing as it is.
 The returned arrays are also weird with their "count" fields and such.

OK, Jani.

I've removed those "count" stuff; also, I noted that I forgot things
like declaring the new arg to ldap_parse_result() to be passed by
reference, and few more details which I fixed. I also removed all the
occurrences of the ldap_value_len substitutions.
This is in <php-ldap-result-controls.2.txt>.

I've also added a tentative <php-ldap-known-controls.0.txt> that
partially implements encoding and decoding the pagedResults control (I
use it as a benchmark because it goes on the request and on the
response). If you find this interface useful, I'd extend it to most of
the standard track controls we support in OpenLDAP. As a C programmer,
I prefer to provide a nice and clean high level PHP interface to known
controls in addition to access to the raw encoding for skilled
developers. In my view, the API should be, for each control:

a) a ldap_ctrl_<name>() call to encode it;

b) a(n optional) ldap_ctrl_<name>_resp() call to decode the related
response, if the control requires any.

Furthermore, the (a) call:

a.1) when passed a valid LDAP handler, should set the control using the
LDAP-level ldap_set_option() API;

a.2) otherwise, it could return a PHP array containing the OID, the
value and the criticality flag, ready to be passed to the PHP-level
ldap_set_option(); this would allow to incrementally build controls
together.

The (b) call should return all the requested fields contained in the
control response.

The pagedResults calls contained in the second patch should provide a
clear example, although the second functionality of the
ldap_ctrl_paged_results() call is not implemented yet. Its usage is
exemplified in the attached <pagedResults.php> (to test it, you only
need to run OpenLDAP 2.3 test003 with "./run -k test003" from the tests/
directory; the "-k" will keep the server alive after populating it with
the right stuff).

This second patch is clearly a work in progress, I don't expect it to be
considered for inclusion in its current state; I'd rather appreciate
some feedback while I keep developing it.

p.

Ing. Pierangelo Masarati
Responsabile Open Solution

SysNet s.n.c.
Via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it

Office: +39.02.23998309
Mobile: +39.333.4963172
Email: pierangelo.masarati@sys-net.it