[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: Wanted to check in on this to see if there was any feedback on this topic? -- ___ Python tracker

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: I was able to get some time together today and created a rough draft for the idea that you had Berker on introducing a new API with more strict parsing rules. This will allow the ValueError to be raised when the URL is parsed rather than

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: Berker and Eric, thank you very much. I really like the idea of introducing a new API with more strict parsing rules for this situation. I would be willing to put some ideas down on a first pass at this. --

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Berker Peksag added the comment: The problem with adding a port_validation argument is that the port attribute is not the only thing that is computed lazily. There is also hostname, username, password attributes etc. I think the best way would be introducing a new

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Berker Peksag added the comment: New changeset 2cb4661707818cfd92556e7fdf9068a993577002 by Berker Peksag (Matt Eaton) in branch 'master': bpo-33034: Improve exception message when cast fails for {Parse,Split}Result.port (GH-6078)

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: Yes, my goal for the patch was to provide a more explicit error message for this situation and to provide a low surface area change to the overall source, knowing that there are future development goals and backward compatibility to keep

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Jonathan added the comment: Thanks for the thoughts. If only the exception message changes, that doesn't really address the issue that caused me to raise this - namely that it seems to be inconsistent with how almost every other Python builtin function I've used

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: This is a very good point, Eric. For backwards compatibility we would have to set the default parameter to false, so we be in the same state we are today by default. Knowing this my vote would be to go with the improvements to the

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Eric V. Smith added the comment: For backward comparability, option C would have to default to not raising an exception. Which would limit it's usefulness, but that's the way it goes. -- ___ Python tracker

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: One more question I would raise based upon a point made earlier by Eric is if option C would be too large of a change for 3.8? Any thoughts on this? -- ___ Python tracker

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: Jonathan, thank you very much for your thoughts I appreciate the pros and cons of each option. In regards to your option C, if we provided a flag to optionally raise the error in urlsplit and urlparse were you thinking the default flag to

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Jonathan added the comment: Interesting conversation As I see it, there are two ways to solve this, both discussed above: A) Python can compute and ValueError at parse-time B) Python can ValueError at property-call time. (Current method) But both have

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Eric V. Smith added the comment: Yes, you're right, it can't be changed. And .port is documented as raising ValueError, so it's all working correctly. I think improving the ValueError message, as the PR does, is as good as we can do. --

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: "Wouldn't we be better off to catch this error at parse time, instead of just improve the error message when .port is called?" I think there could be a case to be made about catching and dealing with this error in urlparse() / urlsplit()

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Eric V. Smith added the comment: Wouldn't we be better off to catch this error at parse time, instead of just improve the error message when .port is called? I'm not sure why port is a property and not just computed. Maybe the least intrusive way of doing this would be to

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Eric V. Smith added the comment: Although having said that, we probably can't make that change for 2.7. And maybe it's even too big a change for 3.8. -- ___ Python tracker

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Matt Eaton added the comment: I agree. I think an explicit exception message would be appropriate when the cast fails to cast from string to int in int(post, 10). Putting in a PR to fix this now. https://github.com/python/cpython/pull/6078 -- nosy:

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Change by Matt Eaton : -- pull_requests: +5838 ___ Python tracker ___ ___

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Change by Matt Eaton : -- keywords: +patch pull_requests: +5837 stage: -> patch review ___ Python tracker ___

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Change by bbayles : -- nosy: +bbayles ___ Python tracker ___ ___ Python-bugs-list mailing

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Jonathan added the comment: Arguably the same logic applies to out-of-range ports: > a = urlsplit('http://example.com:444') > a.port Traceback (most recent call last): File "", line 1, in File "E:\Software\_libs\Python36\lib\urllib\parse.py", line 169, in

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

New submission from Jonathan : (Confirmed in 2.7.14, 3.5.4, and 3.6.3) I have this really bad URL from a crawl: "http://Server=sde; Service=sde:oracle$sde:oracle11g:geopp; User=bodem; Version=SDE.DEFAULT" if I try and parse it with wither urlparse or urlsplit it

[issue33034] urllib.parse.urlparse and urlsplit not raising ValueError for bad port

Change by Jonathan : -- versions: +Python 3.5 ___ Python tracker ___ ___