[Py3] WSGI part 2 #427
Closed
[Py3] WSGI part 2 #427
Conversation
This file contains 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
tiran
reviewed
Jan 31, 2017
ipaserver/plugins/baseldap.py
Outdated
| delval = unicode(base64.b64encode(delval)) | ||
| raise errors.AttrValueNotFound(attr=attr, value=delval) | ||
| delval = base64.b64encode(delval).decode('ascii') | ||
| raise errors.AttrValueNotFound(attr=attr, value=delval) |
ipaserver/plugins/baseldap.py
Outdated
| @@ -255,7 +255,8 @@ def entry_to_dict(entry, **options): | |||
| value = list(entry.raw[attr]) | |||
| for (i, v) in enumerate(value): | |||
| try: | |||
| value[i] = v.decode('utf-8') | |||
| if isinstance(v, bytes): | |||
| value[i] = v.decode('utf-8') | |||
There was a problem hiding this comment.
entry.raw[] should always return a list of bytes. Why the if?
There was a problem hiding this comment.
will be fixed in ipaldap
ipalib/rpc.py
Outdated
| @@ -308,7 +308,7 @@ def json_encode_binary(val, version): | |||
| encoded = encoded.decode('ascii') | |||
| return {'__base64__': encoded} | |||
| elif isinstance(val, Decimal): | |||
| return {'__base64__': base64.b64encode(str(val))} | |||
| return {'__base64__': base64.b64encode(str(val).encode('ascii'))} | |||
There was a problem hiding this comment.
The value must be text, but base64.b64encode() returns binary data, so you should also decode its result.
ipalib/rpc.py
Outdated
| @@ -308,7 +308,8 @@ def json_encode_binary(val, version): | |||
| encoded = encoded.decode('ascii') | |||
| return {'__base64__': encoded} | |||
| elif isinstance(val, Decimal): | |||
| return {'__base64__': base64.b64encode(str(val))} | |||
| return {'__base64__': base64.b64encode( | |||
| str(val).encode('ascii')).decode('ascii')} | |||
There was a problem hiding this comment.
Could we please get rid of this non-sense and return unicode(val)? (It is a backward compatible change.)
Using unicode(bytes) call causes undesired side effect that is inserting `b` character to result. This obviously causes issues with binary base64 data https://fedorahosted.org/freeipa/ticket/4985
Do not encode attribute names https://fedorahosted.org/freeipa/ticket/4985
DNS labels are bytes so bytes must be used for comparison https://fedorahosted.org/freeipa/ticket/4985
In py3 keys() doesn't return list but iterator so it must be transformed to tuple otherwise iterator will be broken. https://fedorahosted.org/freeipa/ticket/4985
ToASCII() returns bytes, it must be decoded to unicode https://fedorahosted.org/freeipa/ticket/4985
The encode method of LDAPClient didn't return DNSName as bytes but string in py3. In py2 it returns non-unicode string so it can be encoded safely by ascii as to_text() method returns only ascii characters. https://fedorahosted.org/freeipa/ticket/4985
for Decimal only from client to server direction uses __base64__
notation. Server replies with pure string for Decimal data, and also
server is able to parse string and create decimal values where needed.
without this we need ugly py3 code:
- return {'__base64__': base64.b64encode(str(val))}
+ return {'__base64__': base64.b64encode(
+ str(val).encode('ascii')).decode('ascii')}
https://fedorahosted.org/freeipa/ticket/4985
HonzaCholasta
added
ack
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
with this PR: