[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Changes by Robert Collins robe...@robertcollins.net: -- resolution: - fixed stage: commit review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Roundup Robot added the comment: New changeset 7c78279afc30 by Robert Collins in branch 'default': Issue #20059: urllib.parse raises ValueError on all invalid ports. https://hg.python.org/cpython/rev/7c78279afc30 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Robert Collins added the comment: So, I think this is worth applying. The discussion around :ipp etc is irrelevant here: this patch changes large or negative ints to be a valueerror, as non-ints are. The only question is where. I think this is in the category of 'will only break buggy applications' - applications that already handle ValueError to deal with bad inputs, will not be broken. Applications that depend on ports outside the valid range for ports will be broken, but thats the definition of the bug. So I propose to apply to 2.7/3.4/3.5/3.6, but I'm going to seek a second opinion. -- nosy: +rbcollins ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
R. David Murray added the comment: Because it raises an error where none was raised before, I'd only apply this to 3.6. This is especially true since this issue is not a *bug* report, but a shouldn't this be more consistent report. That is, there's no great weight (the OP wasn't even sure it should be changed) in favor of the backport, so even a small possibility of breaking working code argues against the backport. (To be clear: by working code I'm envisioning code that is getting that None value and using it as an error signal or treating it the same as a missing port. Such code is technically broken but could be working if the default port happens to work, or the only out of range values it encounters are integers, or it is happy with the exception bubbling up in the non-integer cases.) -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Robert Collins added the comment: ok, 3.6 only. -- versions: +Python 3.6 -Python 2.7, Python 3.4, Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Martin Panter added the comment: If we take the 3.6-only path, does that warrant adding “Version changed” notices, and/or a What’s New entry? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Martin Panter added the comment: Added versioning notices in port-ValueError.v3.patch. -- Added file: http://bugs.python.org/file40061/port-ValueError.v3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
R. David Murray added the comment: Hmm. Good question. I think it probably does, because it means getting an exception where one did not previously happen. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Martin Panter added the comment: Patch v2 just changes a test to use “with self.assertRaises()”. The behaviour of urlparse() succeeding and then result.port failing is indeed odd and surprising. Hopefully documenting this behaviour will help with the “surprising” aspect. But changing it would surely break compatibility. For example, the following is still potentially useful: urlsplit(http://localhost:ipp/;).netloc 'localhost:ipp' On Unix, some programs look up port names in /etc/services, even though this is not valid for URLs according to the RFCs, and urlsplit().port does not support it. In this case “ipp” would be whatever port the Internet Printing Protocol server is configured to run on. -- Added file: http://bugs.python.org/file38642/port-ValueError.v2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Demian Brecht added the comment: It is surprising that urlsplit() does not raise any exception I have a bit of a TL;DR in #20271, trying to capture what the responsibilities of split and parse methods in urllib are and what they should be if consistency is something that we're after. Around the patch though: It seems quite odd to me to be raising exceptions on accessors rather than on instantiation or when the parsing occurs (in the body of urlparse). Wouldn't it better to raise the exception there? -- nosy: +demian.brecht ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Berker Peksag added the comment: I think it is worth to be applied to maintained releases. I'd commit this only to the default branch. Changing the return value from None to an exception after three 3.4 bugfix releases(3.4.1, 3.4.2 and 3.4.3 -- also since 3.4.3 was released in Feb 2015, 3.4.4 will probably be the last bugfix release of 3.4) would not be the best action. I understand that the current situation is inconsistent, but I'm not sure it's worth it to commit to the 2.7 and 3.4 branches. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Serhiy Storchaka added the comment: LGTM and I think it is worth to be applied to maintained releases. -- nosy: +orsenthil stage: - commit review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Martin Panter added the comment: See also Issue 20271, which has a proposed patch with more strict urlsplit() etc behaviour before even returning a SplitResult object. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Changes by Berker Peksag berker.pek...@gmail.com: -- nosy: +berker.peksag ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Martin Panter added the comment: Mapping out-of-range ports to None was added in Issue 14036, though I don’t understand why that approach was taken instead of raising ValueError. Here is a patch to raise ValueError for out-of-range integer values instead. -- keywords: +patch Added file: http://bugs.python.org/file38073/port-ValueError.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Martin Panter added the comment: I would go for raising ValueError for port numbers out of range. The value of None was already defined to mean that no port is included in the URL. Also, the ValueError exception should be documented. It is surprising that urlsplit() does not raise any exception, but then simply getting the “port” attribute does raise the exception. BTW your third example is wrong: urlparse(http://www.example.com:;).port is None True It’s probably safest to keep this one as it is, but maybe it also needs documenting :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Changes by Serhiy Storchaka storch...@gmail.com: -- nosy: +serhiy.storchaka versions: +Python 2.7, Python 3.4, Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Changes by Chris Rebert pyb...@rebertia.com: -- nosy: +cvrebert ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
Changes by Martin Panter vadmium...@gmail.com: -- nosy: +vadmium ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20059] Inconsistent urlparse/urllib.parse handling of invalid port values?
New submission from Chad Birch: I'm not sure if this is something that needs adjustment, but it seems somewhat inconsistent to me. After using urlparse() on various urls with invalid port values, trying to access .port on the result will raise a ValueError. This case includes urls such as: http://www.example.com:asdf; http://www.example.com:1.5; http://www.example.com:; However, as of May 24 2012 (http://hg.python.org/cpython/diff/d769e64aed79/Lib/urllib/parse.py), if the invalid port value is an integer, accessing .port will result in None. So this includes urls such as: http://www.example.com:66000; http://www.example.com:-1; Should these two cases be made consistent, so that either .port is always None or always results in a ValueError if the port section of the url is invalid? I'd be happy to write a patch for it if it's wanted, but I thought I'd check first (and see which of the two options would be correct, if so). -- components: Library (Lib) messages: 206881 nosy: chad.birch priority: normal severity: normal status: open title: Inconsistent urlparse/urllib.parse handling of invalid port values? type: behavior ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20059 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
