Wez Furlong wrote: > My really really late 2 cents on this. > > Please make sure that your changes don't make the extension depend on > OpenLDAP. Solaris and Windows LDAP implementations are close but not > totally the same as OpenLDAP. > > I haven't looked at your patches and probably won't have time to do > so; I'm merely doing a drive-by peek at the recent traffic on the > lists, so forgive me if you've already taken this into consideration > :)
Considering my changes just call ldap_get_values_len() instead of ldap_get_values(), both of which were implemented on all platforms and arches (i.e. no IFDEF sections) then I assume if ldap_get_values_len() which appears to have existed and been used in PHP code since the start of the extension worked... then using it instead of ldap_get_values() should still work. as per the IETF spec for LDAP C API, the ldap_get_values_len() function basically returns you a char * as well as the length of the data. Instead of just returning you the char * and relying on your code to call strlen(). Which when it comes to binary data is not safe. So it shouldn't affect what LDAP server is being used. > > --Wez. > > On 2/28/07, Doug Goldstein <[EMAIL PROTECTED]> wrote: >> Referencing Bug #38819 & Bug #40671 >> http://bugs.php.net/bug.php?id=38819 >> http://bugs.php.net/bug.php?id=40671 >> >> Essentially I looked through the above mentioned bug, the bugs opened >> with OpenLDAP developers, and then reviewed ext/ldap/ldap.c and it >> appears the API calls made by PHP are not necessarily the safest ways to >> write the PHP wrapper functions. Based on [EMAIL PROTECTED]'s comment >> that the LDAP module is unmaintained I went ahead and made some >> changes. >> >> If you read OpenLDAP's API and comments by OpenLDAP Core Developers, >> available at: >> >> http://www.openldap.org/its/index.cgi/Build?id=4690;selectid=4690 >> http://www.openldap.org/software/man.cgi?query=ldap_get_values&sektion=3 >> &apropos=0&manpath=OpenLDAP+2.1-Release >> >> (Notice I went with OpenLDAP 2.1 docs to quell PHP's urge for backwards >> compatibility) >> >> The functions char **ldap_get_values(ld, entry, attr) and struct berval >> **ldap_get_values_len(ld, entry, attr) are essentially inter-changeable. >> The big difference being that the berval struct provides you with a char >> * and the size_t of the data. Rather then just a char * that you then >> have to strlen() which will result in problems if the returned data is >> not NULL terminated data. PHP's internal functions make the mistake of >> assuming all data will be string data (NULL terminated char *) data, >> which is the cause of the crash in bug #38819. >> >> The patch attached removes all of those assumptions and uses >> ldap_get_values_len() and uses the length provided back by the structure >> to feed add_index_stringl() instead of using add_index_string() which >> will call it's own strlen() on the provided data. >> >> This patch also removes ldap_get_values() as a PHP function and makes it >> an alias of ldap_get_values_len() since there's no difference and the >> same data can be returned, it's just a safer version. >> >> The attached patch fixes the test case provided in bug #38819. >> >> Referencing for my own purposes: >> http://bugs.gentoo.org/show_bug.cgi?id=133467 >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >> >> > -- Doug Goldstein <[EMAIL PROTECTED]> http://dev.gentoo.org/~cardoe/
signature.asc
Description: OpenPGP digital signature
