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

ipaldap: properly escape raw binary values in LDAP filters #408

Closed
wants to merge 1 commit into from
Closed

ipaldap: properly escape raw binary values in LDAP filters #408

wants to merge 1 commit into from

Conversation

HonzaCholasta
Copy link
Contributor

@HonzaCholasta HonzaCholasta commented Jan 23, 2017

Manually escape each byte in the value, do not use
ldap.filter.escape_filter_chars() as it does not work with bytes in
Python 3.

https://fedorahosted.org/freeipa/ticket/4985

if six.PY3:
value = value.decode('raw_unicode_escape')
value = u''.join(
u'\\{:02x}'.format(i) for i in six.iterbytes(value))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format() with hex is rather inefficient and slow. An implementation with hexlify() is five to ten times faster.

def enc_hexlify(s):
    hexstring = binascii.hexlify(s).decode('ascii')
    # [-2:0] is empty string for the initial \
    return u'\\'.join(hexstring[i:i+2] for i in six.moves.range(-2, len(s)+2, 2))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not performance critical code, but OK (I like the empty initial string hack 👍).

Manually escape each byte in the value, do not use
ldap.filter.escape_filter_chars() as it does not work with bytes in
Python 3.

https://fedorahosted.org/freeipa/ticket/4985
@MartinBasti MartinBasti self-assigned this Jan 24, 2017
@MartinBasti
Copy link
Contributor

Works for me.

@MartinBasti MartinBasti added the ack Pull Request approved, can be merged label Jan 24, 2017
@MartinBasti
Copy link
Contributor

@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Jan 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants