| msg110723 - (view) |
Author: Dirkjan Ochtman (djc) *  |
Date: 2010-07-19 09:09 |
| Rejecting invalid input seems better in this case. This was changed in . Can we preface the normal fast path with something like: if s is None: raise TypeError('can only quote strings') It used to raise: Traceback (most recent call last): File "/usr/lib64/python2.6/urllib.py", line 1222, in quote res = map(safe_map.__getitem__, s) TypeError: argument 2 to map() must support iteration |
|
|
| msg110725 - (view) |
Author: Senthil Kumaran (orsenthil) * |
Date: 2010-07-19 10:03 |
| The fast path was intended to return the empty string. When s is None, it should return a TypeError. |
|
|
| msg110793 - (view) |
Author: Senthil Kumaran (orsenthil) * |
Date: 2010-07-19 18:19 |
| Fixed and committed in revision 82977. And similar changes for py3k in revision 82983. |
|
|
| msg111736 - (view) |
Author: Florent Xicluna (flox) * |
Date: 2010-07-27 22:16 |
| For 3.2, there's some other corner cases which are not covered by the patch. Example: >>> unquote(()) >>> unquote({}) See attached patch. |
|
|
| msg111769 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2010-07-28 04:06 |
| There are a couple of things that I don't like in the patch: 1) raising a TypeError with a proper message seems better than raising an AttributeError (maybe check if not isinstance(string, (str, bytes)): raise TypeError(...)?); 2) from the snippet I see in the patch, ISTM that unquote_to_bytes accepts both bytes and strings and returns always bytes but unquote might return both bytes and strings (I'm not aware of the design decisions behind these API, but this looks at least inconsistent and maybe wrong (shouldn't it only accept one type?)). |
|
|
| msg111771 - (view) |
Author: Florent Xicluna (flox) * |
Date: 2010-07-28 06:50 |
| 1/ The AttributeError on unquote() is backward compatible with 2.6, 2.7 and 3.1. (issue 9301 is about backward compatibility) 2/ All the quote*/unquote* functions accept both str and bytes (except quote_from_bytes). I don't find a strong reason to change this. |
|
|
| msg112098 - (view) |
Author: Senthil Kumaran (orsenthil) * |
Date: 2010-07-30 19:22 |
| Flox, agree to your patch on checking for unquote({}) and unquote(()). AttributeError on unquote(None) was a mistake in previous releases and it was not intentional. We stand at a chance of correcting that when we are explicitly raising an Exception. Perhaps in another bug report to explicitly track it. Ezio: It was conscious design decision to support both bytes and string for these functions. I think, discussed in |
|
|
| msg112101 - (view) |
Author: Senthil Kumaran (orsenthil) * |
Date: 2010-07-30 19:35 |
| Fixed in revision 83294. Opened Issue9432 just to track that change in Exception behavior. |
|
|
| msg112114 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2010-07-31 05:18 |
| > All the quote*/unquote* functions accept both str and bytes (except quote_from_bytes). According to the documentation [0] unquote() only accepts str (not bytes), so checking for b'' in "if string in (b'', '')" seems wrong to me, especially if then it returns b''. [0]: http://docs.python.org/dev/py3k/library/urllib.parse.html#urllib.parse.unquote |
|
|
| msg112123 - (view) |
Author: Florent Xicluna (flox) * |
Date: 2010-07-31 09:04 |
| Fixed in r83319. Thanks. |
|
|