php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #61853 Use of non-deprecated functions
Submitted: 2012-04-25 20:37 UTC Modified: 2015-07-15 12:02 UTC
From: etienne at lamaisondebarbie dot ch Assigned: mcmic (profile)
Status: Closed Package: LDAP related
PHP Version: master-Git-2012-04-25 (Git) OS: Debian testing
Private report: No CVE-ID: None
 [2012-04-25 20:37 UTC] etienne at lamaisondebarbie dot ch
Description:
------------
This patch aims to remove usage of deprecated ldap function. I did change the php_ldap_do_search to use ldap_search_ext and ldap_search_ext_s.


Patches

patch_61853_and_61921.patch (last revision 2013-02-24 17:05 UTC by etienne at lamaisondebarbie dot ch)
php_ldap_do-ifdef.patch (last revision 2012-04-26 10:14 UTC by etienne at lamaisondebarbie dot ch)
php_ldap_do_modify.patch (last revision 2012-04-26 08:06 UTC by etienne at lamaisondebarbie dot ch)
php_ldap_do_search.patch (last revision 2012-04-25 20:52 UTC by etienne at lamaisondebarbie dot ch)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-04-26 09:29 UTC] pajoye@php.net
Hi!

The patch looks good.

However I would prefer to add some #ifdef and to use the old versions when an old 
ldap client is used.

We may as well drop these old versions in php-next tho'.
 [2012-04-26 10:13 UTC] etienne at lamaisondebarbie dot ch
I did a new patch that bundle both patch and use #ifdef. I'm not sure about LDAP_API_VERSION, but > 2000 seems to be ok.
Also, in the original version used ldap_modify_ext_s, I changed to ldap_modify_s.
 [2013-02-22 08:32 UTC] ab@php.net
-Assigned To: +Assigned To: ab
 [2013-02-22 15:19 UTC] ab@php.net
I've tested your last patch with ifdefs, the code compiles at least :) But the version check isn't smart enough, thus I get errors when compiling with openldap 2.3.48, but it pass though with 2.4.33 . That's because both have api version 3001. So that means for one that some more #ifdef are needed there, and second - that at least for what I see we should check exact by lib version, no by api version. And the point lays somewhere betwee 2.3.48 and 2.4.33 :)
 [2013-02-22 15:19 UTC] ab@php.net
-Status: Assigned +Status: Feedback
 [2013-02-22 15:21 UTC] ab@php.net
I'm still testing the functionality in whole. Please add also some tests for what you did so far.
 [2013-02-22 16:37 UTC] etienne at lamaisondebarbie dot ch
Does php-ldap compile under 2.3.48 without my patch ? (tests are already available in tests directory).
 [2013-02-22 16:37 UTC] etienne at lamaisondebarbie dot ch
-Status: Feedback +Status: Assigned
 [2013-02-22 16:57 UTC] etienne at lamaisondebarbie dot ch
And anyway, *_ext family was designed in 1998 and first implemented in libldap in 1999. ldap_search was tagged as deprecated in 2006. There is no reason to put those ifdef in the first place, I don't think people like to have 6 years old unmaintained code running on their webserver. On windows those functions are supported since Windows 2k ...
 [2013-02-22 17:39 UTC] ab@php.net
Yes, the code from both patches (#61921 and this one) compiles fine, but there is a link error with 2.3.42 (not 2.3.48, my bad), it can't find ldap_control_create. The latest, 2.4.33, compiles good and tests pass. Only one test fails for me ext\ldap\tests\ldap_sasl_bind_basic.phpt , but that's disregarding of with or without patch. That's may be because of the server configuration. If you mean no new test need to be added, so be.

About that #ifdef - it's not about server but client libs. IMHO, there are still enough "stable" distro versions in use, like centos5 and so on, which have older openldap by default. At the end of the day, it's not a big deal to put just a bit more appropriate checks and be compatible with older client libs. As it's almost done now anyway :) It's just a bit confusing both 2.3 and 2.4 series having the api version of 3001, but from the other side - this way the devs mean it stands not very far from each other.
 [2013-02-22 21:19 UTC] etienne at lamaisondebarbie dot ch
Yes you are right, ldap_control_create is not available everywhere (not in older libldap and not in winldap). I used it naturally and forget to check for this one.
It has to be re-implemented into php-ldap, I'll do as soon as possible.
 [2013-02-24 16:57 UTC] etienne at lamaisondebarbie dot ch
Here is the patch including patch from #61853 and #61921 with some helper functions in file ldap_dev_utils.c. This goes against current php-src head.
 [2013-02-25 10:06 UTC] ab@php.net
-Status: Assigned +Status: Feedback
 [2013-02-25 10:06 UTC] ab@php.net
Hi,

i've tested your patch on windows and linux, it looks good. However there are 
some warnings now (after LDAP_DEPRECATED removal in the config.* files):

ldap.c: In function ‘_close_ldap_link’:
ldap.c:98: warning: implicit declaration of function ‘ldap_unbind_s’
ldap.c: In function ‘_php_ber_to_array_recursive’:
ldap.c:137: warning: unused variable ‘root’
ldap.c: In function ‘_php_ber_from_array_recursive’:
ldap.c:366: warning: unused variable ‘i’
ldap.c: In function ‘zif_ldap_connect’:
ldap.c:763: warning: implicit declaration of function ‘ldap_init’
ldap.c:763: warning: assignment makes pointer from integer without a cast
ldap.c: In function ‘zif_ldap_bind’:
ldap.c:826: warning: implicit declaration of function ‘ldap_bind_s’
ldap.c: In function ‘php_ldap_do_search’:
ldap.c:1034: warning: unused variable ‘old_ldap_sizelimit’
ldap.c: In function ‘zif_ldap_explode_dn’:
ldap.c:1748: warning: implicit declaration of function ‘ldap_value_free’
ldap.c: In function ‘zif_ldap_delete’:
ldap.c:1981: warning: implicit declaration of function ‘ldap_delete_s’
ldap.c: In function ‘zif_ldap_compare’:
ldap.c:2057: warning: implicit declaration of function ‘ldap_compare_s’
ldap.c: In function ‘zif_ldap_sort’:
ldap.c:2095: warning: implicit declaration of function ‘ldap_sort_entries’

That "implicit declaration" stuff is for sure because of the removed 
LDAP_DEPRECATED definition. Fixing that were now the finishing straight i'd say. 
Thanks for you work :)
 [2013-02-25 11:48 UTC] etienne at lamaisondebarbie dot ch
-Status: Feedback +Status: Assigned
 [2013-02-25 11:48 UTC] etienne at lamaisondebarbie dot ch
Ok, I'm working on it, but how did you get those warnings, cause I didn't get any ?
 [2013-02-25 13:51 UTC] ab@php.net
Only one thing is crossing my mind is that you could haven't run './buildconf --force' after changing the config.* files. If so, that macros was still defined.
 [2013-04-19 11:30 UTC] ab@php.net
@etienne any updates on this front?
 [2015-07-01 15:39 UTC] ab@php.net
-Assigned To: ab +Assigned To: mcmic
 [2015-07-01 15:39 UTC] ab@php.net
Hi Come,

this one is about the exact topic you're working on right now. Should be checked and closed as well :)

Thanks.
 [2015-07-02 05:42 UTC] mcmic@php.net
-Status: Assigned +Status: Closed
 [2015-07-02 05:42 UTC] mcmic@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2015-07-14 09:44 UTC] etienne at lamaisondebarbie dot ch
I see no problem in closing this bug report but the patch goes a little bit further ... it contains everything to have support for ldap extended operation.
 [2015-07-15 12:02 UTC] mcmic@php.net
Would you be able to convert it to a patch/PR on the current tree? (PHP-5.6 branch would be good)
Adding support for EXOP is one of the planned improvements.
 [2015-08-24 09:27 UTC] etienne at lamaisondebarbie dot ch
I'll time in september, I'll work on this patch again.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 15:01:30 2024 UTC