Skip to content
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.

Sign up for GitHub

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

Jump to bottom

bpo-30458: Use InvalidURL instead of ValueError. #13044

Merged
merged 2 commits into from
May 1, 2019
Merged

bpo-30458: Use InvalidURL instead of ValueError. #13044

merged 2 commits into from
May 1, 2019

Conversation

gpshead
Copy link
Member

@gpshead gpshead commented May 1, 2019 •
edited by bedevere-bot
Loading

Change the fix to raise InvalidURL for consistency with other http.client APIs instead of ValueError.

https://bugs.python.org/issue30458

@gpshead gpshead self-assigned this May 1, 2019
@gpshead gpshead changed the title bpo-30458: Use InvaludURL instead of ValueError. bpo-30458: Use InvalidURL instead of ValueError. May 1, 2019
@gpshead gpshead merged commit b7378d7 into python:master May 1, 2019
@gpshead gpshead deleted the use-invalidurl-not-valueerror branch May 1, 2019 20:39
@gpshead gpshead mentioned this pull request May 1, 2019
Merged
hroncok added a commit to hroncok/cpython that referenced this pull request May 7, 2019
@gpshead @hroncok
bpo-30458: Disallow control chars in http URLs. (pythonGH-12755)
233e321 
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>
This was referenced May 7, 2019
Merged
Merged
hroncok added a commit to hroncok/cpython that referenced this pull request May 7, 2019
@gpshead @hroncok
bpo-30458: Disallow control chars in http URLs. (pythonGH-12755)
50fe2a1 
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>
gpshead pushed a commit that referenced this pull request May 7, 2019
@hroncok @gpshead
bpo-30458: Disallow control chars in http URLs. (GH-12755) (GH-13154)
7e200e0 
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)

Backport Co-Authored-By: Miro Hrončok <miro@hroncok.cz>
ned-deily pushed a commit that referenced this pull request May 8, 2019
@hroncok @ned-deily
bpo-30458: Disallow control chars in http URLs. (GH-12755) (GH-13155)
c50d437 
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>
hroncok added a commit to hroncok/cpython that referenced this pull request May 8, 2019
@gpshead @hroncok
bpo-30458: Disallow control chars in http URLs. (pythonGH-12755)
d46584b 
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>
@hroncok hroncok mentioned this pull request May 8, 2019
Merged
@vstinner vstinner mentioned this pull request May 14, 2019
Merged
vstinner added a commit that referenced this pull request May 21, 2019
@vstinner
bpo-30458: Disallow control chars in http URLs (GH-12755) (GH-13154) (G…
bb8071a 
…H-13315)

Disallow control chars in http URLs in urllib2.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 httplib.InvalidURL instead of ValueError as the new error case's
exception. (GH-13044)

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

(cherry picked from commit 7e200e0)

Notes on backport to Python 2.7:

* test_urllib tests urllib.urlopen() which quotes the URL and so is
  not vulerable to HTTP Header Injection.
* Add tests to test_urllib2 on urllib2.urlopen().
* Reject non-ASCII characters: range 0x80-0xff.
@mcepl mcepl mentioned this pull request May 31, 2019
Closed
3 tasks
@tapakund tapakund mentioned this pull request Jun 3, 2019
Closed
larryhastings pushed a commit that referenced this pull request Jul 14, 2019
@hroncok @larryhastings
bpo-30458: Disallow control chars in http URLs. (GH-12755) (#13207)
afe3a49 
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>
@hemberger
Copy link
Contributor

hemberger commented Jul 17, 2019 •
edited
Loading

After this change, the documentation for the InvalidURL exception is no longer accurate (it only mentions the port case): https://docs.python.org/3/library/http.client.html#http.client.InvalidURL

exception http.client.InvalidURL
A subclass of HTTPException, raised if a port is given and is either non-numeric or empty.

@vstinner
Copy link
Member

Please open an issue at bugs.python.org to suggest updating InvalidURL documentation.

mingwandroid pushed a commit to mingwandroid/cpython that referenced this pull request Aug 9, 2019
@hroncok @mingwandroid
bpo-30458: Disallow control chars in http URLs. (pythonGH-12755) (pyt…
b528811 
…honGH-13154)

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)

Backport Co-Authored-By: Miro Hrončok <miro@hroncok.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@gpshead gpshead

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gpshead @hemberger @vstinner @the-knights-who-say-ni @bedevere-bot