Skip to content

Fix to manual version of ldap_escape()#90

Open
Darthpbal wants to merge 2 commits into
adldap:masterfrom
Darthpbal:dev
Open

Fix to manual version of ldap_escape()#90
Darthpbal wants to merge 2 commits into
adldap:masterfrom
Darthpbal:dev

Conversation

@Darthpbal

Copy link
Copy Markdown

Added a fix to the manualEscape() and manualEscapeWithFlags() in src/Connections/Ldap.php that causes the errors in the open issue #83.

Darthpbal and others added 2 commits July 30, 2015 12:23
…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.
@rnowosielski

Copy link
Copy Markdown

Are you planning to merge this ? On PHP < 5.6 it makes it practically impossible to query for anything reasonably

@Darthpbal

Copy link
Copy Markdown
Author

@rnowosielski
Yes I wanted to merge this. In my experience, and from a bunch of research I found that without the two lines of non comment code I added to LDAP.php, the file won't escape special characters correctly.

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.

@rnowosielski

Copy link
Copy Markdown

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 :)

@Darthpbal

Copy link
Copy Markdown
Author

@rnowosielski

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

@rnowosielski

rnowosielski commented Jan 31, 2023

Copy link
Copy Markdown

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants