fishy commented on code in PR #3105:
URL: https://github.com/apache/thrift/pull/3105#discussion_r1962376833
##########
lib/py/src/protocol/TBinaryProtocol.py:
##########
@@ -146,7 +146,7 @@ def readMessageBegin(self):
if self.strictRead:
raise TProtocolException(type=TProtocolException.BAD_VERSION,
message='No protocol version header')
- name = binary_to_str(self.trans.readAll(sz))
+ name = self.trans.readAll(sz).decode('utf8')
Review Comment:
(this is an issue already existing in today's code, not introduced in this
PR) also this logic kind of assumes that the name in message begin need to be
utf-8 encoded. if it's not, this python code will throw some errors, for
example:
```
>>> b'\xe7\x8e'.decode('utf-8')
Traceback (most recent call last):
File "<python-input-0>", line 1, in <module>
b'\xe7\x8e'.decode('utf-8')
~~~~~~~~~~~~~~~~~~^^^^^^^^^
UnicodeDecodeError: 'utf-8' codec can't decode bytes in position 0-1:
unexpected end of data
```
and I don't believe "the name of a message needs to be utf-8 encoded" is
part of thrift spec so this is potentially a violation to the spec? cc @Jens-G
(again, this is not a new issue introduced by this PR, so I don't think it's
a blocking issue of merging this PR)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
