php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #42060 [PATCH] LDAP: Add pagedResults support and more
Submitted: 2007-07-21 15:52 UTC Modified: 2012-07-14 12:08 UTC
Votes:153
Avg. Score:4.8 ± 0.5
Reproduced:134 of 136 (98.5%)
Same Version:66 (49.3%)
Same OS:76 (56.7%)
From: iarenuno at eteo dot mondragon dot edu Assigned: pajoye (profile)
Status: Closed Package: *General Issues
PHP Version: PHP 5.4 OS: *
Private report: No CVE-ID: None
 [2007-07-21 15:52 UTC] iarenuno at eteo dot mondragon dot edu
Description:
------------
I've taken Pierangelo Masarati's patches (PAT18 and PAT19) and have updated them to apply to 5.2.3 cleanly (as of today) and fixed a memory leak.

In addition to it, I've added 4 test scripts based on the examples from Pierangelo, with a few modifications by me.

I have tested this patch by building PHP with OpenLDAP 2.2.23 libraries (under Debian Sarge) and run the test scripts against both OpenLDAP 2.2.23 slapd server and MS Active Directory (runnning under W2K3 in W2K3 functional mode). OpenLDAP passes all the tests except passwordPolicy control extension (because it doesn't support it) and MS Active Directory passes only the pagedResult test (because it doesn't support the rest of the implemented controls for which tests exist).

The patch is available at:

http://www.eteo.mondragon.edu/descargas/php-ldap/php-ext-ldap-5.2.3.diff.txt.gz

Could you please add this to the next stable PHP release? I badly need pagedResults control extension support in PHP :-)

Saludos. I?aki.


Patches

paged-ldap-5.3 (last revision 2011-04-28 23:00 UTC by bryant dot david at gmail dot com)
api-rename.patch (last revision 2010-11-04 21:16 UTC by jeanseb at au-fil-du dot net)
php-trunk_ldap-pagination.patch (last revision 2010-11-04 20:47 UTC by jeanseb at au-fil-du dot net)
ext-ldap-review.patch (last revision 2010-08-04 15:54 UTC by jeanseb at au-fil-du dot net)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2007-07-21 16:08 UTC] jani@php.net
New features are not added in bugfix releases so this'll have to wait til we start the 5.3 branch.
 [2007-09-07 18:40 UTC] ando at sys-net dot it
A working patch with test examples lined up to HEAD as of 5 minutes ago is available here

<http://www.sys-net.it/~ando/Download/#PHP>

I'm the original developer, and I'm willing to cooperate in integrating it, if needed.  So you can re-open the bug now.

p.
 [2008-06-05 19:10 UTC] pajoye@php.net
After a little discussions about windows with Howard, he pointed me to this bug report. 

It is now the right time to apply such patch (or any other new features) to ext/ldap as we are getting closer to the PHP 5.3 features freeze.

Ando, do you have the time to work on it for php5.3?


 [2008-07-17 13:00 UTC] ando at sys-net dot it
I didn't get any notification about this message, so I overlooked it; I was pointed here by a user (like many others) interested in the functionalities provided by the patch.  In the meanwhile, I noticed that the code, after more than 2 years of inactivity, is now incompatible with the patch.  Fixing it will require an amount of time that is incompatible with my current schedule.  Feel free to fix it yourself.  Cheers, p.
 [2008-11-16 14:57 UTC] pajoye@php.net
Alexey has ported the patch to 5.3, it will committed in the next days.
 [2009-06-05 15:31 UTC] pajoye@php.net
>> get some help :)
 [2009-06-08 11:30 UTC] dj at krul dot nu
pajoye: I can't find the commit you're referring to (either in HEAD or the 5.3 branch). Has it been committed yet?
 [2009-09-02 11:34 UTC] jochen at keutel dot de
I still can't see this patch in 5.3.x or HEAD; neither in PHP 6.

A lot of people really need this extension; e.g. when talking LDAP to MS Active Directory you *need* the paged results control. Reason: AD returns only 1000 entries without paging.

Another scenario often used is password policies; also her you definitely need parsing of extended LDAP responses.

Please apply this patch !!!

Regards,  Jochen.
 [2009-12-17 14:36 UTC] chris at sogetthis dot com
Who is Alexey and where is the updated patch?
 [2010-01-02 21:39 UTC] danhstevens at gmail dot com
Why is this patch still being overlooked? pajoye: You said there would be a patch committed in a few days - that was well over a year ago and as far as I can see nothing has been committed. Can we get this patch pushed through? Is there any good reason this hasn't been implemented yet? PLEASE tell us when to expect this patch, if ever.
 [2010-01-20 12:56 UTC] asp at iportalmais dot pt
Hello,

I have the same problem.
I think it's very important to add this feature, because a lot of people can not change AD settings for security reasons. 
If it's works with ASP why it not works with PHP?
 [2010-02-23 04:14 UTC] ashley at netspot dot com dot au
This is critical!  Paged results are essential.  I'm having to rewrite some functions of my PHP app in Perl to work around this issue.. :(
 [2010-02-25 07:28 UTC] ashley at netspot dot com dot au
I used I?aki's patch from http://www.eteo.mondragon.edu/descargas/php-ldap/ (tested both the 5.1.6 and 5.2.10 patches) and it works very well without any issues.  It successfully pulled down 200,000+ records in a single search, using a page size of 500.

I used this sample code as a reference for implementing the paging functions: http://wiki.github.com/jcharaoui/mdl19-cegep/php-ldap-paging

Note: you should also check ldap_errno() and break out of the paging loop if any error occur, because, for example, if the LDAP server goes offline in the middle of the paging, that code gets stuck in an infinate loop because the cookie never gets unset (happened to me).

Thanks for your work.. would love to see this included in the core release.
 [2010-05-21 17:23 UTC] jeanseb at au-fil-du dot net
The feature is very important for us too.

I extracted the pagination feature and add some phpt from the patch of Pierangelo and IƱaki and I ported it for the trunk.

Please find attached patch.

NB : this patch adds 2 new calls, see below

A 5.2 version of this patch is available there http://bugs.php.net/bug.php?id=51869

Test script:
---------------
$ds = ldap_connect($ldapHost, $ldapPort);

ldap_set_option($ds, LDAP_OPT_PROTOCOL_VERSION, 3);

ldap_bind($ds, $ldapUser, $ldapPass);

$cookie = '';
do {
    ldap_ctrl_paged_results($ds, $pageSize, true, $cookie);

    $result = ldap_search($ds, $dn, $filter, $justthese);

    $entries = ldap_get_entries($ds, $result);

    ldap_ctrl_paged_results_resp($ds, $result, $cookie);

} while($cookie !== null && $cookie != '');
 [2010-05-21 17:27 UTC] pajoye@php.net
-Package: Feature/Change Request +Package: *General Issues -Assigned To: patrickallaert +Assigned To: pajoye
 [2010-05-21 17:27 UTC] pajoye@php.net
As I said there, 5.2/3 are in bug fixes mode only. Pls provide a patch against trunk.
 [2010-05-27 15:52 UTC] jeanseb at au-fil-du dot net
Any feedback on the patch ?
 [2010-06-15 21:43 UTC] pajoye@php.net
-Status: Assigned +Status: Closed
 [2010-06-15 21:43 UTC] pajoye@php.net
This bug has been fixed in SVN.

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/.
 
Thank you for the report, and for helping us make PHP better.


 [2010-06-16 20:29 UTC] johannes@php.net
Quick review of the patch shows this part:

	switch (myargcount) {
+		case 4:
+			convert_to_string_ex(&cookie);
+			lcookie.bv_val = Z_STRVAL_PP(&cookie);
+			lcookie.bv_len = Z_STRLEN_PP(&cookie);
+			/* fallthru */

Why is it done this way? Shouldn't "rl|bz" in zend_parse_parameters be changed to "rl|bs"? I assume the current form will cause trouble with references and stuff which can't be converted to string (object without __toString() method)

Didn't do a deeper review.

As we're rolling 5.3.3 very soon i'd keep it out there for now.
 [2010-06-16 20:31 UTC] pajoye@php.net
-Status: Closed +Status: Feedback
 [2010-08-04 17:59 UTC] jeanseb at au-fil-du dot net
I've attached a patch solving this.

In ldap_ctrl_paged_results_resp the 3rd and 4th args are string & int passed by reference. 

I've defined those args as zval ("zz"), should I change this to string & int "si" too ?
 [2010-08-17 07:48 UTC] qmt9z3 at yahoo dot com
Hello,
Any plans to fold this into the main stable releases?
Appreciate update on when this is going to become
main stream.

Thanks
 [2010-10-06 10:47 UTC] jonas dot ranerfors at gmail dot com
Chance for this to be in next release? 5.3.4?
 [2010-10-13 10:24 UTC] jeanseb at au-fil-du dot net
I'm trying to do some lobbying arround this, without success for the moment.
 [2010-10-13 10:27 UTC] jeanseb at au-fil-du dot net
I have build the ldap extension for PHP 5.2 on WinXP and i can build it for PHP 5.2 or 5.3 on linux if you want.

But it's without any support.
 [2010-10-27 07:41 UTC] luisdv at icon dot co dot za
Can you build it for PHP 5.3? I'm willing to help test it on Windows Server 2003.

How can we help get this into the 5.3.4 release?
 [2010-11-02 21:12 UTC] jeanseb at au-fil-du dot net
Hi,

I can only build it with VC9.

If you're still interested, let me now.

For PHP 5.3.4 i've no idea, i'm trying i haven't any answer from pajoye.
 [2010-11-02 22:28 UTC] jeanseb at au-fil-du dot net
Ok i build it.

Env : Windows XP 32bits
Compiler : Microsoft Visual Studio 2008 (VC9)
Source : tags/php_5_3_3 + patch from trunk@300474 + patch from trunk@303159 + ext-ldap-review.patch
Download : http://www.au-fil-du.net/public/php/php_ldap.PHP5.3.3-withPaginationSupport.Win32-VC9-x86.zip
 [2010-11-02 22:34 UTC] jeanseb at au-fil-du dot net
Some code snippet can be found in one of my previous comments http://bugs.php.net/bug.php?id=42060#1274455432
 [2010-11-03 10:23 UTC] pajoye@php.net
I think it can be applied to trunk already.

I would not apply it right to 5.3 but wait a bit, to see if this patch works well in trunk 1st.

I have however one question, can we use better naming for ldap_ctrl_paged_results_resp? Both "ctrl" and "resp"? It should be control and whatever resp means here.
 [2010-11-03 10:33 UTC] pajoye@php.net
btw, what's the minimum ldap libraries version to be used with this patch? Have you tried ours on Windows? http://pecl2.php.net/downloads/php-windows-builds/php-libs/VC9/x86/
 [2010-11-03 10:45 UTC] jeanseb at au-fil-du dot net
I used :
- openldap-2.3.42-2-vc9-x86.zip
- sasl-2.1.23-vc9-x86.zip
- openssl-0.9.8k-asm-vc9-x86.zip
 [2010-11-03 12:47 UTC] jeanseb at au-fil-du dot net
"resp" means "response"

We can rename the API in ldap_control_paged_result & ldap_control_paged_result_response
 [2010-11-03 13:20 UTC] pajoye@php.net
yes, please do, then I can apply the patch to trunk
 [2010-11-03 23:04 UTC] jeanseb at au-fil-du dot net
I have updated the initial patch (php-trunk_ldap-pagination.patch) to reflect this api change and i have integrated the commit of felipe @303159. 

Tomorrow I'll integrate my review patch (ext-ldap-review.patch) to the initial one and I'll rebuild one, based on trunk.
 [2010-11-04 22:20 UTC] jeanseb at au-fil-du dot net
That's it.

You can apply ext-ldap-review.patch and api-rename.patch on trunk.

For api-rename.patch, I should do some svn add & svn del on the phpt file. I have rename the 3 phpt but the svn diff seems to fail to create a good patch.
 [2010-11-18 22:34 UTC] jeanseb at au-fil-du dot net
Pierre ?


PS : I have build the patch on PHP 5.2 :

http://www.au-fil-du.net/public/php/php_ldap.PHP5.2.14-withPaginationSupport.Win32-VC6-x86.zip
 [2010-11-23 22:37 UTC] sriram dot natarajan at gmail dot com
I don't think this patch will be applied against 5.2 branch. since 5.2 release 
train is in security bug fix only. however, we can ask if this patch can be 
applied against 5.3. 

thanks for the patch.
 [2010-11-24 12:28 UTC] luisdv at icon dot co dot za
@jeanseb, thanks for compiling the dll. Is it for the Thread-Safe version of PHP only?

I've tried running it on my WinXP SP3 box with PHP 5.2.14 VC6 Non-Thread-Safe but when I call PHPInfo() I get a message saying, "PHP Startup: Unable to load dynamic library 'C:\PHP\exe\php_ldap.dll' - The specified module could not be found."

I've checked the C:\PHP\exe directory and the php_ldap.dll file (164kb) that you supplied is in that directory.

When I restore the original php_ldap.dll file PHPInfo() runs fine and includes the LDAP section in the output.
 [2010-11-24 12:37 UTC] luisdv at icon dot co dot za
Just to confirm, I'm referring to the php_ldap.PHP5.2.14-withPaginationSupport.Win32-VC6-x86.zip file that you uploaded on 2010-11-18 21:34 UTC

I'm going to install PHP 5.3.3 on my pc and then try the php_ldap.PHP5.3.3-withPaginationSupport.Win32-VC9-x86.zip file that you provided on 2010-11-02 21:28 UTC.

But is your .dll for PHP TS or NTS?
 [2010-11-24 14:54 UTC] luisdv at icon dot co dot za
I loaded php_ldap.PHP5.3.3-withPaginationSupport.Win32-VC9-x86.zip on a PHP 5.3.3 NTS VC9 instance and I still get the "Unable to load dynamic library... - the specified module could not be found" message and no ldap section is displayed by PHPInfo().

I'll relook at my whole setup again after a beer! Maybe I'm doing something stupid...
 [2010-11-24 19:35 UTC] jeanseb at au-fil-du dot net
Good question.

I tested it with TS version only.

Can you test with a ts version ?
 [2010-12-10 14:37 UTC] jeanseb at au-fil-du dot net
Pierre ?
 [2010-12-10 14:43 UTC] pajoye@php.net
It will be done when it is done. There is no real hurry. I was busy with other branches than trunk, in case you missed the recent releases.
 [2011-03-21 21:43 UTC] liveoutloud2day at gmail dot com
Can this be committed?  Please?  Pretty Please?

[2007-07-21 13:52 UTC] is a long time ago.  Can this get committed to code that can become the next version of PHP?  

This just adds optional arguments to a call that you won't use unless you know what you are doing.  It won't break anything else.  Can it please get committed?

Is there anything I can do to help get it committed to the trunk?

Thanks!
 [2011-04-29 01:20 UTC] bryant dot david at gmail dot com
Hey guys,

I was not able to get the patches on this page to work, and paged results was a must-have for our installation. I took 
jeanseb's patch (great work by the way - thank you!) and modified it. I got it to work with the 5.3 build from 04/28/2011 
(over at http://snaps.php.net), but I am NOT a C developer by trade, so I'm very open to *constructive* criticism. 

Here's a breakdown of how to get it installed (since I just had to go through all this myself).

  - Obviously, you're going to be compiling PHP, so download the appropriate version for your platform. (again, this patch 
is for 5.3)

  - Once you've got it compiling OK, install the (attached) patch, paged-ldap-5.3, by doing the following (I'm on a Mac - 
you're on own if your in Windows, sorry): cd into ext/ldap in your PHP source directory and run 

    patch < /path/to/paged-ldap-5.3

  - Recompile

Assuming all went well, here's a modified version of jeanseb's script to test it:


<?php

    $ds = ldap_connect('ad.example.com');

    ldap_set_option($ds, LDAP_OPT_PROTOCOL_VERSION, 3);
    ldap_bind($ds, 'CN=myaccount,OU=users,DC=ad,DC=example,DC=com', 'password');


    $pageSize    = 100;

    $cookie = '';
    do {
        ldap_control_paged_results($ds, $pageSize, true, $cookie);

        $result = ldap_search($ds, 'OU=users,DC=ad,DC=example,DC=com', 'cn=*', array('sAMAccountName'));
        $entries = ldap_get_entries($ds, $result);
        
        foreach($entries as $e)
            print $e["dn"] . "\n";

        ldap_control_paged_results_response($ds, $result, $cookie);
        
    } while($cookie !== null && $cookie != '');

?>

Good luck! And please, can we get paged LDAP support in some form or another committed?
 [2011-05-18 14:33 UTC] jeanseb at au-fil-du dot net
Can we expect to see ext-ldap-review.patch and api-rename.patch applied on trunk and PHP5.4 branch ?

Thanks.
 [2011-05-19 19:41 UTC] scottmac@php.net
Automatic comment from SVN on behalf of scottmac
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=311264
Log: Tidy up ldap paging code and rename the API as discussed in #42060
 [2011-05-19 19:42 UTC] scottmac@php.net
I applied the rename patch and tidied up the code a little.

Anything else that needs done here?
 [2011-06-02 11:48 UTC] salathe@php.net
The implemented functions are currently called 
"ldap_control_paged_result[_response]" but the tests try to use the plural 
"results" names.  Which are we going to keep, Scott is there any reason you chose 
the singular names?
 [2011-08-23 15:55 UTC] peng1can at gmail dot com
Is this plural/singular typo the only thing holding this patch up?  Can't a choice just be made and move on?
 [2011-08-24 05:54 UTC] jeanseb at au-fil-du dot net
With Pierre we agreed to use singular form.

Could someone with karma fix this ?



https://bugs.php.net/bug.php?id=42060&edit=2#1288788479
 [2011-08-24 06:08 UTC] scottmac@php.net
I fixed the tests, singular made more sense.
 [2011-08-24 06:50 UTC] scottmac@php.net
Automatic comment from SVN on behalf of scottmac
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=315407
Log: Fix typo in tests for ldap bug #42060
 [2012-07-13 15:56 UTC] henrik dot zawischa at sonic-ps dot de
Any news on this? I see the status still is feedback. I'd love to have this in a released version.
Best
Henrik
 [2012-07-14 12:07 UTC] jeanseb@php.net
Thank you for your bug report. This issue has already been fixed
in the latest released version of PHP, which you can download at 
http://www.php.net/downloads.php

It's implemented in PHP 5.4.

See manual : http://fr2.php.net/manual/en/function.ldap-control-paged-result.php
 [2012-07-14 12:07 UTC] jeanseb@php.net
-Status: Feedback +Status: Closed
 [2012-07-14 12:08 UTC] jeanseb@php.net
-PHP Version: 5CVS, 6CVS (2008-11-01) +PHP Version: PHP 5.4
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Mon Dec 30 14:01:28 2024 UTC