-
Notifications
You must be signed in to change notification settings - Fork 8k
Fix GH-17280: ldap_search() fails when $attributes array has holes #17284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
We relax the constraint that the array must be a list. What really matters is that it only has numeric keys. As shown in the example code, it's really easy to accidentally create a non-list array, so it makes sense to relax the constraint. There are 3 cases left where the array is checked to be a list, in php_ldap_do_search, but I believe this makes sense to keep because the indices of those arrays have a meaning because they should match between different arrays. In that case it will prevent programmer errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be backported to 8.4 (or 8.2 as this is what the bug report claims is also affected?)
Backported to 8.3 probably. Note that 8.2 is already closed for a while for bugfixes.
But it was easier to work on the changes for master first rather than working on 8.3 and then going upwards.
...has holes Backport of phpGH-17284 to fix phpGH-17280 on lower branches.
...has holes Backport of phpGH-17284 to fix phpGH-17280 on lower branches.
dregad
commented
Dec 27, 2024
Many thanks for fixing it so quickly 🚀
I can live without a back port to 8.2, the workaround is simple enough. And I need to be compatible with 7.4 anyway...
...has holes Backport of phpGH-17284 to fix phpGH-17280 on lower branches. Closes phpGH-17287.
We relax the constraint that the array must be a list. What really matters is that it only has numeric keys. As shown in the example code, it's really easy to accidentally create a non-list array, so it makes sense to relax the constraint.
There are 3 cases left where the array is checked to be a list, in php_ldap_do_search, but I believe this makes sense to keep because the indices of those arrays have a meaning because they should match between different arrays. In that case it will prevent programmer errors.