-
Notifications
You must be signed in to change notification settings - Fork 127
Closed
Milestone
Description
The way how _bytesify_input
handles errors and warnings is too inflexible:
python-ldap/Lib/ldap/ldapobject.py
Lines 125 to 158 in ad46c11
def _bytesify_input(self, arg_name, value): | |
"""Adapt a value following bytes_mode in Python 2. | |
In Python 3, returns the original value unmodified. | |
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. | |
""" | |
if not PY2: | |
return value | |
if value is None: | |
return value | |
elif self.bytes_mode: | |
if isinstance(value, bytes): | |
return value | |
else: | |
if self.bytes_mode_hardfail: | |
raise TypeError("All provided fields *must* be bytes when bytes mode is on; got %r" % (value,)) | |
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') | |
else: | |
if not isinstance(value, text_type): | |
raise TypeError("All provided fields *must* be text when bytes mode is off; got %r" % (value,)) | |
assert not isinstance(value, bytes) | |
return value.encode('utf-8') |
- For
bytes_mode=None
(default), the function emits a bytes warning when the input is text, then encodes it as UTF-8 bytes. Warnings are slow and can cause performance regressions (see Possible performance regression with LDAPBytesWarning #133). The warnings are always emitted, even when warnings are disabled and not shown to the user. The warnings module does not allow early opt-out. - For
bytes_mode=False
(strictly future-compatible) there is no warnings mode at all. When the function is called with bytes as input, it always fails withTypeError
. TypeErrors make it really, really hard to port an existing application to future compatible API. There should be a mode that emits just warnings.
Proposal
I suggest that we introduce a bytes_warning
flag:
bytes_warning=BYTES_WARNINGS_SILENT
emits no warnings and doesn't raise TypeError forbytes_mode
None, True, and False.bytes_warning=BYTES_WARNINGS_WARN
emits a warning when input is text forbytes_mode=None
/bytes_mode=True
or input is bytes forbytes_mode=False
.bytes_warning=BYTES_WARNINGS_ERROR
raises a TypeError when input is text forbytes_mode=None
/bytes_mode=True
or input is bytes forbytes_mode=False
.bytes_warning=None
(default) depends onbytes_mode
:bytes_mode=None
->BYTES_WARNINGS_WARN
bytes_mode=True
->BYTES_WARNINGS_ERROR
bytes_mode=False
->BYTES_WARNINGS_ERROR
The flags are defined as:
BYTES_WARNINGS_SILENT = 0
BYTES_WARNINGS_WARN = 1
BYTES_WARNINGS_ERROR = 2
Metadata
Metadata
Assignees
Labels
No labels