◐ Shell
reader mode source ↗
Skip to content

[3.4] bpo-30500: urllib: Simplify splithost by calling into urlparse. (#1849)#2291

Merged
larryhastings merged 1 commit into
python:3.4from
vstinner:splithost34
Jul 12, 2017
Merged

[3.4] bpo-30500: urllib: Simplify splithost by calling into urlparse. (#1849)#2291
larryhastings merged 1 commit into
python:3.4from
vstinner:splithost34

Conversation

@vstinner

@vstinner vstinner commented Jun 20, 2017

Copy link
Copy Markdown
Member

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)

@vstinner vstinner added the type-security A security issue label Jun 20, 2017
@vstinner

vstinner commented Jun 20, 2017

Copy link
Copy Markdown
Member Author

@larryhastings: Larry, would you mind to review and merge this fix please?

It fixes an important security vulnerability:
http://python-security.readthedocs.io/vuln/bpo-30500_urllib_connects_to_a_wrong_host.html

cc @ned-deily.

@vstinner

vstinner commented Jun 20, 2017

Copy link
Copy Markdown
Member Author

I tested manually that "./python -m test test_urlparse" pass. Sadly, 3.4 has no pre-commit CI yet.

@larryhastings

larryhastings commented Jul 11, 2017

Copy link
Copy Markdown
Contributor

I accepted a PR from Serhiy and now there's a conflict from Misc/NEWS. Do you mind changing it to NEWS.d?

@larryhastings

larryhastings commented Jul 11, 2017

Copy link
Copy Markdown
Contributor

I'll accept this PR once you fix the conflicts.

@postmasters @vstinner
bpo-30500: urllib: Simplify splithost by calling into urlparse. (#1849) …
3e4a539 
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)
@vstinner

vstinner commented Jul 11, 2017

Copy link
Copy Markdown
Member Author

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.

@larryhastings

larryhastings commented Jul 12, 2017

Copy link
Copy Markdown
Contributor

I'm willing to consider PR 2475 for 3.4, but we can discuss it over there.

@larryhastings larryhastings merged commit cc54c1c into python:3.4 Jul 12, 2017
@vstinner vstinner deleted the splithost34 branch July 12, 2017 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

type-security A security issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner @larryhastings @the-knights-who-say-ni @postmasters