Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[3.5] bpo-30458: Disallow control chars in http URLs. (GH-12755)#13207

Merged
larryhastings merged 4 commits into
python:3.5python/cpython:3.5from
hroncok:3.5-issue30458hroncok/cpython:3.5-issue30458Copy head branch name to clipboard
Jul 14, 2019
Merged

[3.5] bpo-30458: Disallow control chars in http URLs. (GH-12755)#13207
larryhastings merged 4 commits into
python:3.5python/cpython:3.5from
hroncok:3.5-issue30458hroncok/cpython:3.5-issue30458Copy head branch name to clipboard

Conversation

@hroncok

@hroncok hroncok commented May 8, 2019

Copy link
Copy Markdown
Contributor

Disallow control chars in http URLs in urllib.urlopen. This addresses a potential security problem for applications that do not sanity check their URLs where http request headers could be injected.

Disable https related urllib tests on a build without ssl (GH-13032)
These tests require an SSL enabled build. Skip these tests when python is built without SSL to fix test failures.

Use http.client.InvalidURL instead of ValueError as the new error case's exception. (GH-13044)

Co-Authored-By: Miro Hrončok miro@hroncok.cz

https://bugs.python.org/issue30458

Disallow control chars in http URLs in urllib.urlopen.  This addresses a potential security problem for applications that do not sanity check their URLs where http request headers could be injected.

Disable https related urllib tests on a build without ssl (pythonGH-13032)
These tests require an SSL enabled build. Skip these tests when python is built without SSL to fix test failures.

Use http.client.InvalidURL instead of ValueError as the new error case's exception. (pythonGH-13044)

Co-Authored-By: Miro Hrončok <miro@hroncok.cz>
@bedevere-bot bedevere-bot added type-bug An unexpected behavior, bug, or error type-security A security issue labels May 8, 2019
@vstinner

vstinner commented May 21, 2019

Copy link
Copy Markdown
Member

Multiple tests failed on AppVeyor:

ERROR: test_networked_good_cert (test.test_httplib.HTTPSTest)
ERROR: test_connect (test.test_ssl.NetworkedTests)
ERROR: test_connect_cadata (test.test_ssl.NetworkedTests)
ERROR: test_handshake (test.test_ssl.NetworkedBIOTests)
...

With TLS certification validation error:

[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:728)

It seems like these failures are related to https://bugs.python.org/issue36816: PR #13200 isn't merged yet.

@vstinner

Copy link
Copy Markdown
Member

It seems like these failures are related to https://bugs.python.org/issue36816: PR #13200 isn't merged yet.

Travis CI basically has the same failures and so it's likely the same issue.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Straightforward backport (cherry-pick). The patch went fine in all other branches.

@hroncok

hroncok commented May 21, 2019

Copy link
Copy Markdown
Contributor Author

Compared to 3.6, I've removed the f-strings.

@larryhastings larryhastings merged commit afe3a49 into python:3.5 Jul 14, 2019
@bedevere-bot

Copy link
Copy Markdown

@larryhastings: Please replace # with GH- in the commit message next time. Thanks!

@larryhastings

Copy link
Copy Markdown
Contributor

Thanks for the 3.5 love!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error type-security A security issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.