Skip to content

feat(ldap.dn): Add support for different formats in ldap.dn2str() via flags #466

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat(ldap.dn): Add support for different formats in ldap.dn2str() via flags #466

wants to merge 4 commits into from

Conversation

spaceone
Copy link
Contributor

@spaceone spaceone commented Apr 1, 2022 •
edited
Loading

In C dn2str() supports flags which works by providing LDAP_DN_FORMAT_UFN, LDAP_DN_FORMAT_AD_CANONICAL, ….
These symbols do exist in Python, but could not be used ultimately because the Python counterpart was pure Python and did not pass to dn2str(3).

Fix #257

@spaceone spaceone force-pushed the dn2str-flags branch 2 times, most recently from 6948a13 to f25fa60 Compare April 1, 2022 14:01
@droideck
Copy link
Contributor

@spaceone hi!
The PR says 'WIP', - is it still true?

@spaceone
Copy link
Contributor Author

@spaceone hi! The PR says 'WIP', - is it still true?

@droideck Well, I would say it needs code clenaup but the functionality is done. But before I try to cleanup I would like to get feedback.

@droideck
Copy link
Contributor

Overall, I think it'll be nice to have the feature.
@mistotebe what do you think?

@mistotebe
Copy link
Contributor

I agree, getting better DN handling would be nice.

spaceone
spaceone commented Sep 28, 2022
View reviewed changes
@spaceone spaceone changed the title WIP: Add support for flags in ldap.dn2str() feat(ldap.dn): Add support for different formats in ldap.dn2str() via flags Jun 3, 2025
@spaceone spaceone requested a review from mistotebe June 3, 2025 21:51
@spaceone spaceone force-pushed the dn2str-flags branch 3 times, most recently from ebdee71 to 7c80d5c Compare June 4, 2025 06:25
@spaceone
Copy link
Contributor Author

spaceone commented Jun 4, 2025

Performance comparison:

C:

python3 -m timeit -s 'import ldap.dn' "ldap.dn.dn2str([[('uid', 'test, 42', 1)],[('ou', 'Testing', 1)],[('dc', 'example', 1)],[('dc', 'com', 1)]], ldap.DN_FORMAT_LDAPV3)"
200000 loops, best of 5: 1.41 usec per loop

Python:

python3 -m timeit -s 'import ldap.dn' "ldap.dn.dn2str([[('uid', 'test, 42', 1)],[('ou', 'Testing', 1)],[('dc', 'example', 1)],[('dc', 'com', 1)]], 0)"
50000 loops, best of 5: 4.1 usec per loop

mistotebe
mistotebe previously approved these changes Jun 10, 2025
View reviewed changes
Copy link
Contributor

@mistotebe mistotebe left a comment

Choose a reason for hiding this comment

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

LGTM, maybe be consistent about using raw strings (r'cn=\C3...' vs. 'cn=\\C3...')

spaceone added 3 commits June 11, 2025 21:43
@spaceone
test(ldap.dn): Add test cases for ldap.dn.dn2str() with different for…
5f69cb5 
…mat flags
@spaceone
feat(ldap.dn): Add support for different formats in `ldap.dn.dn2str()…
b070418 
…` via flags

In C `dn2str()` supports `flags` which works by providing one of `LDAP_DN_FORMAT_UFN`, `LDAP_DN_FORMAT_AD_CANONICAL`, `LDAP_DN_FORMAT_DCE`, `LDAP_DN_FORMAT_LDAPV3`.
These symbols do exist in Python, but could not be used ultimately because the Python counterpart was pure Python and did not pass to `dn2str(3)`.

Fix python-ldap#257
@spaceone
feat(ldap.dn): add ldap.dn.normalize()
3411d7c
@spaceone
Copy link
Contributor Author

LGTM, maybe be consistent about using raw strings (r'cn=\C3...' vs. 'cn=\\C3...')

Thanks for approval. I added a commit which unifies the whole file, to use raw-strings.

@spaceone spaceone requested a review from mistotebe June 13, 2025 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mistotebe mistotebe Awaiting requested review from mistotebe

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ldap.dn.dn2str() does not support flags
3 participants
@spaceone @droideck @mistotebe