# LDAP controls in response - Thread: https://externals.io/message/19927 - Original archive: https://news-web.php.net/php.internals/19927 - Started by Pierangelo Masarati on 2005-11-08T21:28:35+00:00 - Messages: 10 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #19927 — Pierangelo Masarati — 2005-11-08T21:28:35+00:00 URL: https://externals.io/message/19927#19927 Stig, I need to use LDAP controls in PHP, including control response from server to client, so I patched the 5.0.5/HEAD code to add an extra arg to ldap_parse_result() and ldap_parse_reference(). I'd need this patch in production at some point, that's why it would be great to see it merged into the mainstream; however, I'm little familiar with the internals of PHP, so please excuse me if I missed anything in coding and in the submission procedure. The very same patch applies to HEAD and to 5.0.5. If you don't mind, I'm posting it to you right now, pending further work. It also includes some extra work I did to eliminate some warnings from OpenLDAP 2.3, but it works fine with 2.2 as well. All changes specific to OpenLDAP are protected behind the LDAP_API_FEATURE_X_OPENLDAP macro. Unfortunately I have no chances to check it with other APIs right now. I'm also including a trivial script I tested with ./sapi/cli/php against the server resulting from test003 of OpenLDAP 2.3; to reproduce, just cd openldap/tests/ ../run -k test003 cd php ../sapi/cli/php pagedResults.php If the code looks fine, I plan to document the new API, which is completely backwards compatible, and add some facilities to encode/decode the control values; hopefully, I won't have to get to writing a complete wrapper around liblber! My idea is to provide dumb helpers that encode well-known controls through a trivial API; e.g., for pagedResults: ldap_control_paged_results($handler, $size, $iscritical[, $cookie]) ldap_control_paged_results_resp($result, &$cookie[, &$iscritical[, & $estimate]]) for passwdPolicy: ldap_control_passwd_policy($handler[, $iscritical]) ldap_control_passwd_policy_resp($result, &$warning, &$error) Please let me know if you need me to do anything else. Sincerely, 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 --- ## #19928 — Jani Taskinen — 2005-11-08T22:37:40+00:00 URL: https://externals.io/message/19927#19928 · In reply to: #19927 I quickly glanced through your patch and if you don't mind, can you please separate the warning-fixes and functionality patch? (one patch for fixes, one for adding new stuff :) It's a bit too much to try and see what was added and what is supposed to silence some warning. --Jani > Stig, > > I need to use LDAP controls in PHP, including control response from > server to client, so I patched the 5.0.5/HEAD code to add an extra arg > to ldap_parse_result() and ldap_parse_reference(). I'd need this patch > in production at some point, that's why it would be great to see it > merged into the mainstream; however, I'm little familiar with the > internals of PHP, so please excuse me if I missed anything in coding and > in the submission procedure. > > The very same patch applies to HEAD and to 5.0.5. If you don't mind, > I'm posting it to you right now, pending further work. It also includes > some extra work I did to eliminate some warnings from OpenLDAP 2.3, but > it works fine with 2.2 as well. All changes specific to OpenLDAP are > protected behind the LDAP_API_FEATURE_X_OPENLDAP macro. Unfortunately I > have no chances to check it with other APIs right now. I'm also > including a trivial script I tested with ./sapi/cli/php against the > server resulting from test003 of OpenLDAP 2.3; to reproduce, just > > cd openldap/tests/ > ../run -k test003 > cd php > ../sapi/cli/php pagedResults.php > > If the code looks fine, I plan to document the new API, which is > completely backwards compatible, and add some facilities to > encode/decode the control values; hopefully, I won't have to get to > writing a complete wrapper around liblber! > > My idea is to provide dumb helpers that encode well-known controls > through a trivial API; e.g., for pagedResults: > > ldap_control_paged_results($handler, $size, $iscritical[, $cookie]) > ldap_control_paged_results_resp($result, &$cookie[, &$iscritical[, & > $estimate]]) > > for passwdPolicy: > > ldap_control_passwd_policy($handler[, $iscritical]) > ldap_control_passwd_policy_resp($result, &$warning, &$error) > > Please let me know if you need me to do anything else. > > Sincerely, p. --- ## #19929 — Pierangelo Masarati — 2005-11-08T23:22:57+00:00 URL: https://externals.io/message/19927#19929 · In reply to: #19928 > I quickly glanced through your patch and if you don't mind, > can you please separate the warning-fixes and functionality patch? > (one patch for fixes, one for adding new stuff :) > > It's a bit too much to try and see what was added and what is supposed > to silence some warning. Sure. I quickly separated them by editing the patch, so you'll likely get some fuzz message because of wrong line count. They should apply in any order, though, because there was no code overlapping. I successfully did warnings, controls. Sincerely, 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 --- ## #19930 — Jani Taskinen — 2005-11-09T05:05:23+00:00 URL: https://externals.io/message/19927#19930 · In reply to: #19929 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(), --Jani > > > […] > > Sure. > > I quickly separated them by editing the patch, so you'll likely get some > fuzz message because of wrong line count. They should apply in any > order, though, because there was no code overlapping. I successfully > did warnings, controls. > > Sincerely, 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 --- ## #19931 — Pierangelo Masarati — 2005-11-09T07:24:27+00:00 URL: https://externals.io/message/19927#19931 · In reply to: #19930 On Wed, 2005-11-09 at 07:05 +0200, Jani Taskinen wrote: > 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). ldap_*value*_len() is the safe implementation of ldap_*value*(), 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). ldap_sasl_bind(), despite the misleading name, doesn't imply the use of SASL: it's a neutral interface to authentication which, when passed the LDAP_SASL_SIMPLE argument, exactly does LDAP's simple bind. I understand its use is really misleading, given the presence of the very same name in PHP for the function that actually wraps ldap_sasl_interactive_bind_s(); however, this is an implementation detail that wouldn't get exposed to end-users. 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... :) Thanks, 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 --- ## #19932 — Jani Taskinen — 2005-11-09T08:03:54+00:00 URL: https://externals.io/message/19927#19932 · In reply to: #19931 > > > > […] > > 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. > ldap_*value*_len() is the safe implementation of ldap_*value*(), 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. 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. > 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. --Jani --- ## #19933 — Pierangelo Masarati — 2005-11-09T08:36:15+00:00 URL: https://externals.io/message/19927#19933 · In reply to: #19932 > > > > […] > > 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. > > > […] > > 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? > > > […] > > 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 --- ## #19949 — Pierangelo Masarati — 2005-11-10T17:58:34+00:00 URL: https://externals.io/message/19927#19949 · In reply to: #19932 > 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 . I've also added a tentative 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_() call to encode it; b) a(n optional) ldap_ctrl__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 (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 --- ## #19980 — Pierangelo Masarati — 2005-11-14T14:29:52+00:00 URL: https://externals.io/message/19927#19980 · In reply to: #19949 > 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. Jani, sorry to bother you again. Do you think there are chances to see controls in LDAP results in PHP 5/HEAD any soon? Another thing we'd need is LDAP extended operations (mainly for RFC 3062 password modify exop), so I'd like to add an API for extended operations as well. However, as these enhancements grow bigger, I think they should be better harmonized with the existing LDAP extension API, or even better with any foreseen API development. If anybody is (interested in) discussing LDAP API extension development, please let me know. 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 --- ## #19983 — Jani Taskinen — 2005-11-14T20:14:42+00:00 URL: https://externals.io/message/19927#19983 · In reply to: #19980 > sorry to bother you again. Do you think there are chances to see controls > in LDAP results in PHP 5/HEAD any soon? Another thing we'd need is LDAP I'll look into it but this won't get into 5.1.0. We're gonna release 5.1.1 quite soon after and it will contain your patch. I just need to free some time to sit down and look it over again. :) --Jani