What's the point of calling this function when $result === null? Is that legal for its API?

Hum. Not sure. It will probably return an empty string. I have to admit that I never paginated LDAP result so IDK. At least I should send 0 to keep compatibility.

shouldn't we skip calling this method instead here (and keep$this->serverctrls = [];?)
see my other comment

I think so. I just wanted to not break anything since I can't test it. It was actually used here in the same context before the refactoring of#38392

See:https://github.com/symfony/symfony/pull/38392/files#diff-24b79f3ac1a99938f5acb158a450e38d30c1984a5d333b5b20f2c38a73d10e31L183

to really be compatible and seamless, this should be anelseif - otherwise, we're affecting the behavior on php <=7.3.

a test case would really help if you can have a look

There's a return in the previous if content, how can this impact the previous statements? 🤔

I was not able to run the test entire test suite on my computer :( . I'll give another try, but if you have any insights about running slapd that'd be great ^^' .

Looking at the original PR :https://github.com/symfony/symfony/pull/38392/files#diff-24b79f3ac1a99938f5acb158a450e38d30c1984a5d333b5b20f2c38a73d10e31L183-R177

I think the fix, should be to change this line to:

and keep the rest of the code.

/cc@lucasaba

Not sure. The original error is on#38874 isWarning: ldap_parse_result() expects parameter 2 to be resource, int given.

The only place whereldap_parse_result receive an in is when it is called from
$this->controlPagedResultResponse($con, 0, '');

I think the correct solutions is to call it with$this->controlPagedResultResponse($con, null, '');

But your proposal should work as well... I need to test it. I'll check with a working LDAP env.

@lucasaba I think error on#38874 is a result of mistake on#38392
Whyldap_control_paged_result was replaced bycontrolPagedResultResponse, notcontrolPagedResult?

my point is, when. you replacedldap_parse_result() bycontrolPagedResult in#38392, in that line you replaced it bycontrolPagedResultResponse.

This explain why the second argument is0 and not a ressources.

I've created a new repository with a test openldap server with the configuration needed by symony's tests.
You can find it here:https://github.com/lucasaba/symfony-ldap-test-server

Justdocker-compose up it and launch test suite.

@jderusse suggestion fixes the problem. But I need some more time to make other checks. I'm unable to delete entries and I'm not sure if the problem is the LDAP server or the code.

By the way: I had to modify AdapterTest and remove

constPAGINATION_REQUIRED_CONFIG = ['options' => ['protocol_version' =>3,        ],    ];

And I had to add, in LdapTestCase, afterldap_connect, the following:

ldap_set_option($h,LDAP_OPT_PROTOCOL_VERSION,3);

@jderusse You're completly right! Its all my bad.
@Nek- I could launch locally the tests and they all pass. I should've done it earlier. I'm not able to add the ldap server on github actions because I still don't have enough knowledge.

To launch the tests, just clonehttps://github.com/lucasaba/symfony-ldap-test-server, build it and launch it. Then you can run phpunit against the LDAP tests.

Just some more notes (since you're working on it):

• remove definition of thePAGINATION_REQUIRED_CONFIG constant fromsrc/Symfony/Component/Ldap/Tests/Adapter/ExtLdap.php and all its reference from the same file (it doesn't work)
• addldap_set_option($h, LDAP_OPT_PROTOCOL_VERSION, 3); onsrc/Symfony/Component/Ldap/Tests/LdapTestCase.php just after the connection
• at row 85, insrc/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/LdapManagerTest.php, modify the test. The exception is aLdapException.AlreadyExistsException is used for an already existing connection.
• after thetestLdapAddDouble there is a new entry ('cn=Elsa Amrouche,dc=symfony,dc=com'). This will break all successive tests.... you should find a way to remove the entry just after the test....
• at row 213, insrc/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/LdapManagerTest.php substitute the line with:

// $newEntry->getAttribute('cn') returns an array not a string$this->assertContains($originalCN,$newEntry->getAttribute('cn'));$this->assertContains('Kevin',$newEntry->getAttribute('cn'));

Idk if it's related to be being under WSL ^^' but even with the docker it's not working (even with rebinding ports).

Since I can't run the whole test suite I think cleaning the test suite is not a super good idea since it's out of the scope of this PR.