remove the second Python2 vs Py3 compat.py #3105
Conversation
| @@ -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') | |||
There was a problem hiding this comment.
it looks like utf8 is some compatibility alias for utf-8 defined in python:
Python 3.13.2 (main, Feb 5 2025, 01:23:35) [GCC 14.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> "foobar".encode('utf8')
b'foobar'
>>> "foobar".encode('foobar')
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
"foobar".encode('foobar')
~~~~~~~~~~~~~~~^^^^^^^^^^
LookupError: unknown encoding: foobar
>>> "foobar".encode('utf-8')
b'foobar'
so this still works, but I think we probably want to use utf-8 instead as that's the correct term (that's also the encoding used in the examples on https://docs.python.org/3/howto/unicode.html)
There was a problem hiding this 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)
No description provided.