-
Notifications
You must be signed in to change notification settings - Fork 126
Add bytes_strictness to allow configuring behavior on bytes/text mismatch #171
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,7 +93,8 @@ class SimpleLDAPObject: | |
|
||
def __init__( | ||
self,uri, | ||
trace_level=0,trace_file=None,trace_stack_limit=5,bytes_mode=None | ||
trace_level=0,trace_file=None,trace_stack_limit=5,bytes_mode=None, | ||
bytes_strictness=None, | ||
): | ||
self._trace_level = trace_level | ||
self._trace_file = trace_file or sys.stdout | ||
|
@@ -107,20 +108,26 @@ def __init__( | |
# Bytes mode | ||
# ---------- | ||
|
||
# By default, raise a TypeError when receiving invalid args | ||
self.bytes_mode_hardfail = True | ||
if bytes_mode is None and PY2: | ||
_raise_byteswarning( | ||
"Under Python 2, python-ldap uses bytes by default. " | ||
"This will be removed in Python 3 (no bytes for DN/RDN/field names). " | ||
"Please call initialize(..., bytes_mode=False) explicitly.") | ||
bytes_mode = True | ||
# Disable hard failure when running in backwards compatibility mode. | ||
self.bytes_mode_hardfail = False | ||
elif bytes_mode and not PY2: | ||
raise ValueError("bytes_mode is *not* supported under Python 3.") | ||
# On by default on Py2, off on Py3. | ||
if PY2: | ||
if bytes_mode is None: | ||
bytes_mode = True | ||
if bytes_strictness is None: | ||
_raise_byteswarning( | ||
"Under Python 2, python-ldap uses bytes by default. " | ||
"This will be removed in Python 3 (no bytes for " | ||
"DN/RDN/field names). " | ||
"Please call initialize(..., bytes_mode=False) explicitly.") | ||
bytes_strictness = 'warn' | ||
else: | ||
if bytes_strictness is None: | ||
bytes_strictness = 'error' | ||
else: | ||
if bytes_mode: | ||
raise ValueError("bytes_mode is *not* supported under Python 3.") | ||
bytes_mode = False | ||
bytes_strictness = 'error' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe raise an exception when |
||
self.bytes_mode = bytes_mode | ||
self.bytes_strictness = bytes_strictness | ||
|
||
def _bytesify_input(self, arg_name, value): | ||
"""Adapt a value following bytes_mode in Python 2. | ||
|
@@ -130,38 +137,46 @@ def _bytesify_input(self, arg_name, value): | |
With bytes_mode ON, takes bytes or None and returns bytes or None. | ||
With bytes_mode OFF, takes unicode or None and returns bytes or None. | ||
|
||
This function should be applied on all text inputs (distinguished names | ||
and attribute names in modlists) to convert them to the bytes expected | ||
by the C bindings. | ||
For the wrong argument type (unicode or bytes, respectively), | ||
behavior depends on the bytes_strictness setting. | ||
In all cases, bytes or None are returned (or an exception is raised). | ||
""" | ||
if not PY2: | ||
return value | ||
|
||
if value is None: | ||
return value | ||
|
||
elif self.bytes_mode: | ||
if isinstance(value, bytes): | ||
return value | ||
elif self.bytes_strictness == 'silent': | ||
pass | ||
elif self.bytes_strictness == 'warn': | ||
_raise_byteswarning( | ||
"Received non-bytes value for '{}' in bytes mode; " | ||
"please choose an explicit " | ||
"option for bytes_mode on your LDAP connection".format(arg_name)) | ||
else: | ||
if self.bytes_mode_hardfail: | ||
raise TypeError( | ||
"All provided fields *must* be bytes when bytes mode is on; " | ||
"got type '{}' for '{}'.".format(type(value).__name__, arg_name) | ||
) | ||
else: | ||
_raise_byteswarning( | ||
"Received non-bytes value for '{}' with default (disabled) bytes mode; " | ||
"please choose an explicit " | ||
"option for bytes_mode on your LDAP connection".format(arg_name)) | ||
return value.encode('utf-8') | ||
return value.encode('utf-8') | ||
else: | ||
if not isinstance(value, text_type): | ||
if isinstance(value, unicode): | ||
return value.encode('utf-8') | ||
elif self.bytes_strictness == 'silent': | ||
pass | ||
elif self.bytes_strictness == 'warn': | ||
_raise_byteswarning( | ||
"Received non-text value for '{}' with bytes_mode off and " | ||
"bytes_strictness='warn'".format(arg_name)) | ||
else: | ||
raise TypeError( | ||
"All provided fields *must* be text when bytes mode is off; " | ||
"got type '{}' for '{}'.".format(type(value).__name__, arg_name) | ||
) | ||
assert not isinstance(value, bytes) | ||
return value.encode('utf-8') | ||
return value | ||
|
||
def _bytesify_modlist(self, arg_name, modlist, with_opcode): | ||
"""Adapt a modlist according to bytes_mode. | ||
|
@@ -1064,7 +1079,7 @@ class ReconnectLDAPObject(SimpleLDAPObject): | |
def __init__( | ||
self,uri, | ||
trace_level=0,trace_file=None,trace_stack_limit=5,bytes_mode=None, | ||
retry_max=1,retry_delay=60.0 | ||
bytes_strictness=None, retry_max=1, retry_delay=60.0 | ||
): | ||
""" | ||
Parameters like SimpleLDAPObject.__init__() with these | ||
|
@@ -1078,7 +1093,9 @@ def __init__( | |
self._uri = uri | ||
self._options = [] | ||
self._last_bind = None | ||
SimpleLDAPObject.__init__(self,uri,trace_level,trace_file,trace_stack_limit,bytes_mode) | ||
SimpleLDAPObject.__init__(self, uri, trace_level, trace_file, | ||
trace_stack_limit, bytes_mode, | ||
bytes_strictness=bytes_strictness) | ||
self._reconnect_lock = ldap.LDAPLock(desc='reconnect lock within %s' % (repr(self))) | ||
self._retry_max = retry_max | ||
self._retry_delay = retry_delay | ||
|
@@ -1097,6 +1114,11 @@ def __getstate__(self): | |
|
||
def __setstate__(self,d): | ||
"""set up the object from pickled data""" | ||
hardfail = d.get('bytes_mode_hardfail') | ||
if hardfail: | ||
d.setdefault('bytes_strictness', 'error') | ||
else: | ||
d.setdefault('bytes_strictness', 'warn') | ||
self.__dict__.update(d) | ||
self._last_bind = getattr(SimpleLDAPObject, self._last_bind[0]), self._last_bind[1], self._last_bind[2] | ||
self._ldap_object_lock = self._ldap_lock() | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should spell out the behavior in form of an example and list affected attributes
Bytes mode applies to:
dn
argumentattr
argumentnewrdn
argument forrename
who
andcred
forbind
user
,oldpw
andnewpw
forpasswd
base
,filterstr
andattrlist
members forsearch*
mod_type
(2nd argument) of modlist member foradd*
,modify*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I linked to text mode from those arguments. Does that look good?