[3.4] bpo-30500: urllib: Simplify splithost by calling into urlparse. (#1849) #2291
Conversation
Lib/urllib/parse.py
Outdated
There was a problem hiding this comment.
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. |
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 host is ``abc``, the path is ``/``, and the fragment is ``#@def``. (cherry picked from commit 90e01e5)
|
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. |
4 participants
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)