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

LDAP encoding of slash character #507

Closed
bdthomsen opened this issue Jul 31, 2019 · 4 comments
Closed

LDAP encoding of slash character #507

bdthomsen opened this issue Jul 31, 2019 · 4 comments

Comments

@bdthomsen
Copy link

I have run across references indicating that the slash ('/') character should be encoded for LDAP queries, such as https://docs.microsoft.com/en-us/windows/win32/adsi/search-filter-syntax#special-characters . Given that there is no harm in doing so, I recommend updating the encodeForLDAP() function to encode the slash character with a "\2f", in case there are situations where a slash character could be problematic.

@kwwall
Copy link
Contributor

kwwall commented Aug 7, 2019

@bdthomsen I searched RFC 2254 which is the reference that your ADSI search filter link referenced and found no evidence of '/' should be treated as a special character. (The only mention it in the RFC is in the EBNF grammar where it is used as a separator.) Also, that RFC only mentions the 5 characters that we currently quote. I've also searched the referenced RFCs 4710, 4715, and 3377 and '/' seems to hold no special meaning there either.

So, my first question to you is "Are you sure that this is not Microsoft Active Directory Services Interfaces (ADSI) specific search filter criteria rather than general LDAP search filter criteria as defined by the appropriate RFCs?"

And my second question is "If we quote '/' as '\2f', is there really 'no harm in doing so'?" The reason I ask this if there were "no harm" (which I would interpret as "does not change the semantic meaning") of doing this, then why wouldn't we just bother to quote ALL the characters of search criteria? If the potential exists that (for example) someone's existing search criteria exists, then we may have to add a property that only sets encodes the forward slash character for ADSI.

@xeno6696
Copy link
Collaborator

This one caught me off guard... there's no codec for LDAP. So part of this issue I think will be to create said encoder and the necessary tests that surround it. There's even a TODO comment on line 302 of the default encoder class.

@noloader
Copy link
Contributor

I believe this was cleared at PR #712.

Encoding forward slash is a Microsoft-ism, but it does not harm since the RFC says any character can be encoded. That is, the RFC says you MAY encode a forward slash and Microsoft says you MUST encode a forward slash. The PR encodes the forward slash.

@kwwall
Copy link
Contributor

kwwall commented Jul 18, 2022

Good catch @noloader. I will mark this as closed as per PR #712. I simply didn't look back that far.

@kwwall kwwall closed this as completed Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants