Skip to content
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

Add ldap_escape() #458

Merged
merged 1 commit into from
Oct 18, 2013
Merged

Add ldap_escape() #458

merged 1 commit into from
Oct 18, 2013

Conversation

DaveRandom
Copy link
Contributor

This patch defines a new function and two new constants in ext/ldap:

  • ldap_escape()
  • LDAP_ESCAPE_DN
  • LDAP_ESCAPE_FILTER

Escaping is done according to the relevant RFCs:
http://tools.ietf.org/html/rfc1960 (FIlter)
http://tools.ietf.org/html/rfc2253 (DN)

The function also allows the user to specify a list of characters that will not be escaped.

Resolves https://bugs.php.net/bug.php?id=47607

@m6w6
Copy link
Contributor

m6w6 commented Oct 3, 2013

I'd rather see static tables than generating the escape maps dynamically.

@DaveRandom
Copy link
Contributor Author

@m6w6 The reason behind this particular design choice is to enable the ignore list to exist.

This patch is a variation on something I've used in userland for a while, and I can tell you with absolute certainty that the ignore list has real-world value, it's not just a "seems like people might need this" feature. In order to implement this element it needs to be a dynamically generated map.

I can remove this portion of the functionality so it's just ldap_escape($str, $mode) but for me (and therefore I'm sure for others) it would make it less useful.

@m6w6
Copy link
Contributor

m6w6 commented Oct 4, 2013

Oh well, I should have had a closer look. I didn't notice yoiu were changing the map after generating.

@php-pulls php-pulls merged commit 0d534d7 into php:master Oct 18, 2013
@DaveRandom DaveRandom deleted the ldap_escape-dev branch March 3, 2014 16:35
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.

3 participants