-
Notifications
You must be signed in to change notification settings - Fork 204
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
Fix to manual version of ldap_escape() #90
base: master
Are you sure you want to change the base?
Conversation
…nections/Ldap.php, and added tests to tests/ConnectionTests.php to correct adLDAP's manual version of PPHP 5.6 ldap_escape() which improperly escapes in some contexts.
Are you planning to merge this ? On PHP < 5.6 it makes it practically impossible to query for anything reasonably |
@rnowosielski If you're saying that my edits break the system, I'll have to ask for more information, because I have a lot of tests showing that how LDAP.php currently double escapes characters in certain circumstances, and in other circumstances, LDAP.php tries to escape some slash characters but just deletes them instead. Can you provide any detail as to how my edits make reasonably querying anything impossible? Also, I'm running php 5.5.9, and the original code escapes characters incorrectly, but with my fixes, escaping matches ldap_escape() from php 5.6. |
Sorry. Maybe I was unclear. I was referring to the version currently available via composer. This pull request fixes the problem in question, that's why I am intrinsically interested in this pull request being merged :) |
OH! :) Yes, I had created this pull request to merge with the main adldap code a while ago, but no activity from anyone with write access to this repo as far as merging pull requests. I can't merge this because I have no write access to this repo. I posted another temporary fix in the issue I linked in the pull request description because I saw the low activity on this repo. Basically, if you use this code to escape charaters instead of the adldap build in method, your special characters will be escaped properly. Also, a commenter on issue #83 suggests that this problem was fixed in a different adldap repo located here |
@Darthpbal would you consider closing or archiving this PR ? I guess at this point it is unlikely to be merged and it clutters my "open PRs where you were mentioned" view. Unless there is a way to remove oneself somehow but I couldn't find a way so far. |
Added a fix to the
manualEscape()
andmanualEscapeWithFlags()
in src/Connections/Ldap.php that causes the errors in the open issue #83.