-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[3.4] bpo-30500: urllib: Simplify splithost by calling into urlparse. (#1849) #2291
Conversation
@@ -865,7 +865,7 @@ def splithost(url): | |||
"""splithost('//host[:port]/path') --> 'host[:port]', '/path'.""" | |||
global _hostprog | |||
if _hostprog is None: | |||
_hostprog = re.compile('^//([^/?]*)(.*)$') | |||
_hostprog = re.compile('//([^/#?]*)(.*)', re.DOTALL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the usage of the DOTALL flag, see:
http://bugs.python.org/issue30500#msg296422
@larryhastings: Larry, would you mind to review and merge this fix please? It fixes an important security vulnerability: cc @ned-deily. |
I tested manually that "./python -m test test_urlparse" pass. Sadly, 3.4 has no pre-commit CI yet. |
I accepted a PR from Serhiy and now there's a conflict from Misc/NEWS. Do you mind changing it to NEWS.d? |
I'll accept this PR once you fix the conflicts. |
Victor: "I tested manually that "./python -m test test_urlparse" pass. Sadly, 3.4 has no pre-commit CI yet." I proposed PR #2475 to add CIs. Larry: "I accepted a PR from Serhiy and now there's a conflict from Misc/NEWS. Do you mind changing it to NEWS.d?" Sure, I created a NEWS.d entry and rebased my change. |
I'm willing to consider PR 2475 for 3.4, but we can discuss it over there. |
The current regex based splitting produces a wrong result. For example::
http://abc#@def
Web browsers parse that URL as
http://abc/#@def
, that is, the hostis
abc
, the path is/
, and the fragment is#@def
.(cherry picked from commit 90e01e5)