Never try to decode bytes as UTF-8 #14
Conversation
There're two issues in the existing code. First, this heuristic often fails on short random strings, making their type unreliable. For example, "t" (transaction ID) in DHT protocol is just two bytes. Second, the code assumed that they keys of dictionaries are always UTF-8 encoded. Although that's true in many BitTorrent-related protocols, bencoding doesn't mandate it, and exceptions do in fact exist: see fuzeman#6. Also, change the code to consistently return bytes/str in all cases, leaving support for passing encoding str/unicode strings, which are transparently encoded to UTF-8.
2 similar comments
|
Thanks for the contribution. Note: I've made some minor changes to pass flake8, but it should functionally still be the same. |
|
I know this has been merged but I just thought I should comment here. I know that this is technically correct, according to the bencode spec. However I believe this should not have been merged in this form. This behaviour should have been put behind a flag so that opting to the old behaviour is still possible. And maybe even allow passing the encoding scheme to try to decode the strings. The old behaviour was clearly working for a lot of people and was an issue for 1 person in 2018. Also, if the content being decode is heavily nested it will be inefficient to parse after and convert both the keys and values from bytes to strings. |
There're two issues in the existing code.
First, this heuristic often fails on short random strings,
making their type unreliable. For example, "t" (transaction ID) in
DHT protocol is just two bytes.
Second, the code assumed that they keys of dictionaries are always
UTF-8 encoded. Although that's true in many BitTorrent-related protocols,
bencoding doesn't mandate it, and exceptions do in fact exist:
see #6.
Also, change the code to consistently return bytes/str in all cases,
leaving support for passing encoding str/unicode strings, which are
transparently encoded to UTF-8.