bpo-32498: urllib.parse.unquote accepts bytes - #7768
Conversation
There was a problem hiding this comment.
test_unquoting_with_bytes_input() requires some work.
- IMO there's no need to duplicate the part of
test_unquoting()that checks all of the ASCII escape sequences. Even if this is kept, it should be refactored into a common helper rather than copy/pasted. - If the above mentioned part is kept, it should actually test that quoted sequences are properly encoded! It currently just checking that unquoted characters are returned unchanged, which is missing the point.
- There should be a few explicit sanity tests where unquoted sequences are returned as-is.
Also (entirely optional), maybe add an example taken from an actual URL? I'd add this to test_unquote() too.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Test non-ascii input as bytes.
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @taleinat: please review the changes made to this pull request. |
taleinat
left a comment
taleinat
left a comment
There was a problem hiding this comment.
The docs must also be updated accordingly, including a "versionchanged" note about this being changed in version 3.9.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Update documentation of changed behaviour in version 3.9.
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @taleinat: please review the changes made to this pull request. |
taleinat
left a comment
taleinat
left a comment
There was a problem hiding this comment.
Apologies for the delay in reviewing this! I have a few additional requests upon a final review, but after those this would be good to go.
| "using unquote(): %r != %r" % (expect, result)) | ||
|
|
||
| def test_unquoting_with_bytes_input(self): | ||
| given = b'bl\xc3\xa5b\xc3\xa6rsyltet\xc3\xb8y' |
There was a problem hiding this comment.
Please add a simpler test case here in addition to the others. Also, please add a (short) comment explaining what each of the more complex test cases is checking.
| @@ -0,0 +1,2 @@ | |||
| Made ``urllib.parse.unquote()`` polymorphic, accepting both string and | |||
There was a problem hiding this comment.
"polymorphic" doesn't help clarity here; IMO just write "Made ... accept both str and bytes."
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be put in the comfy chair! |
Co-Authored-By: Tal Einat <taleinat+github@gmail.com>
…o unquote_handle_bytes
Test ascii input. Simplified news item.
Fixed typo in news item
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @taleinat: please review the changes made to this pull request. |
6 participants
When calling urllib.parse.unquote with a bytes argument, the exception raised says: "a bytes-like object is required, not 'str'". This can be confusing as the user provided a bytes-like object.
The pull request changes urllib.parse.unquote to accept bytes as argument, and forwards the call to urllib.parse.unquote_to_bytes.
https://bugs.python.org/issue32498