[Ldap] Bypass the use of ldap_control_paged_result on PHP >= 7.3
#38392
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Side note: to test the component I've created an OpenLDAP instance on docker using latest osixia/openldap. TO make test working, I had to add Even with this configuration, some test fails with message It would be a good idea to have a docker environment prepared to test those methods....at least in order to have a common environment. |
derrabus
reviewed
Oct 4, 2020
derrabus
reviewed
Oct 4, 2020
nicolas-grekas
changed the title
ldap_control_paged_result if PHP version is 8.0 o…ldap_control_paged_result on PHP >= 7.3
lucasaba
commented
Oct 5, 2020
stof
requested changes
Oct 5, 2020
I agree. We run such tests in |
nicolas-grekas
approved these changes
Oct 7, 2020
nicolas-grekas
force-pushed
the
38352-ldap-deprecation
branch
from
2458d2a
to
d332b30
Compare
|
Thank you @lucasaba. |
|
Thanks to you all! Has been a pleasure and a precious lesson! |
Merged
Nek-
added a commit
to Nek-/symfony
that referenced
this pull request
Oct 28, 2020
After PR symfony#38392 there is a little issue in the Symfony code base that occurs only for PHP 7.4 and PHP 8.0. This is related to issue symfony#38874
Nek-
added a commit
to Nek-/symfony
that referenced
this pull request
Oct 29, 2020
After PR symfony#38392 there is a little issue in the Symfony code base that occurs only for PHP 7.4 and PHP 8.0. This is related to issue symfony#38874
Nek-
added a commit
to Nek-/symfony
that referenced
this pull request
Oct 29, 2020
After PR symfony#38392 there is a little issue in the Symfony code base that occurs only for PHP 7.4 and PHP 8.0. This is related to issue symfony#38874
Nek-
added a commit
to Nek-/symfony
that referenced
this pull request
Oct 29, 2020
After PR symfony#38392 there is a little issue in the Symfony code base that occurs only for PHP 7.4 and PHP 8.0. This is related to issue symfony#38874
Closed
Merged
fabpot
added a commit
that referenced
this pull request
Nov 9, 2020
This PR was merged into the 4.4 branch. Discussion ---------- [Ldap] Fix pagination | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #38874 | License | MIT | Doc PR | N/A This replaces #38875 to fix a bug introduced by #38392 Commits ------- 4fe0a6f Fix LDAP pagination
chalasr
added a commit
that referenced
this pull request
Nov 16, 2020
This PR was merged into the 4.4 branch. Discussion ---------- Fix critical extension when reseting paged control | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - The issue has been introduced here in #38392 and prevent performing an operation after fetched a paginated result => ldap throws an `Could not XXX: Critical extension is unavailable` At this line: https://github.com/symfony/symfony/pull/38392/files#diff-24b79f3ac1a99938f5acb158a450e38d30c1984a5d333b5b20f2c38a73d10e31L183, the previous code called `ldap_control_paged_result($con, 0);` using the default value (`false`) for the `$critical` argument. The replaced version always use `true`. This PR restore the previous behavior by using `false` when reseting the pagination. Commits ------- a2b7476 Fix critical extension when reseting paged control
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As stated on #38352 ldap_control_paged_result and ldap_control_paged_result_response have been deprecated since PHP 7.4 and will be removed on PHP 8.0.
With this fix, Query uses serverctrls to handle LDAP results pagination.
Since
serverctrlswhere introduced in PHP 7.3 and they are the only way to circumvent the usage ofldap_control_paged_result, I've added a new Query class implementation which usesserverctrlsto control pagination.To do so I've also had to update the LDAP Adapter in order to use the new class if PHP version 7.3 or greater are found