Commit 7774d783 authored by Jason R. Coombs's avatar Jason R. Coombs Committed by GitHub

bpo-38216, bpo-36274: Allow subclasses to separately override validation and...

bpo-38216, bpo-36274: Allow subclasses to separately override validation and encoding behavior (GH-16448)

* bpo-38216: Allow bypassing input validation

* bpo-36274: Also allow the URL encoding to be overridden.

* bpo-38216, bpo-36274: Add tests demonstrating a hook for overriding validation, test demonstrating override encoding, and a test to capture expectation of the interface for the URL.

* Call with skip_host to avoid tripping on the host checking in the URL.

* Remove obsolete comment.

* Make _prepare_path_encoding its own attr.

This makes overriding just that simpler.

Also, don't use the := operator to make backporting easier.

* Add a news entry.

* _prepare_path_encoding -> _encode_prepared_path()

* Once again separate the path validation and request encoding, drastically simplifying the behavior. Drop the guarantee that all processing happens in _prepare_path.
parent 441b10cf
...@@ -1085,18 +1085,15 @@ class HTTPConnection: ...@@ -1085,18 +1085,15 @@ class HTTPConnection:
else: else:
raise CannotSendRequest(self.__state) raise CannotSendRequest(self.__state)
# Save the method we use, we need it later in the response phase # Save the method for use later in the response phase
self._method = method self._method = method
if not url:
url = '/' url = url or '/'
# Prevent CVE-2019-9740. self._validate_path(url)
if match := _contains_disallowed_url_pchar_re.search(url):
raise InvalidURL(f"URL can't contain control characters. {url!r} "
f"(found at least {match.group()!r})")
request = '%s %s %s' % (method, url, self._http_vsn_str) request = '%s %s %s' % (method, url, self._http_vsn_str)
# Non-ASCII characters should have been eliminated earlier self._output(self._encode_request(request))
self._output(request.encode('ascii'))
if self._http_vsn == 11: if self._http_vsn == 11:
# Issue some standard headers for better HTTP/1.1 compliance # Issue some standard headers for better HTTP/1.1 compliance
...@@ -1174,6 +1171,18 @@ class HTTPConnection: ...@@ -1174,6 +1171,18 @@ class HTTPConnection:
# For HTTP/1.0, the server will assume "not chunked" # For HTTP/1.0, the server will assume "not chunked"
pass pass
def _encode_request(self, request):
# ASCII also helps prevent CVE-2019-9740.
return request.encode('ascii')
def _validate_path(self, url):
"""Validate a url for putrequest."""
# Prevent CVE-2019-9740.
match = _contains_disallowed_url_pchar_re.search(url)
if match:
raise InvalidURL(f"URL can't contain control characters. {url!r} "
f"(found at least {match.group()!r})")
def putheader(self, header, *values): def putheader(self, header, *values):
"""Send a request header line to the server. """Send a request header line to the server.
......
...@@ -1155,6 +1155,34 @@ class BasicTest(TestCase): ...@@ -1155,6 +1155,34 @@ class BasicTest(TestCase):
thread.join() thread.join()
self.assertEqual(result, b"proxied data\n") self.assertEqual(result, b"proxied data\n")
def test_putrequest_override_validation(self):
"""
It should be possible to override the default validation
behavior in putrequest (bpo-38216).
"""
class UnsafeHTTPConnection(client.HTTPConnection):
def _validate_path(self, url):
pass
conn = UnsafeHTTPConnection('example.com')
conn.sock = FakeSocket('')
conn.putrequest('GET', '/\x00')
def test_putrequest_override_encoding(self):
"""
It should be possible to override the default encoding
to transmit bytes in another encoding even if invalid
(bpo-36274).
"""
class UnsafeHTTPConnection(client.HTTPConnection):
def _encode_request(self, str_url):
return str_url.encode('utf-8')
conn = UnsafeHTTPConnection('example.com')
conn.sock = FakeSocket('')
conn.putrequest('GET', '/☃')
class ExtendedReadTest(TestCase): class ExtendedReadTest(TestCase):
""" """
Test peek(), read1(), readline() Test peek(), read1(), readline()
...@@ -1279,6 +1307,7 @@ class ExtendedReadTest(TestCase): ...@@ -1279,6 +1307,7 @@ class ExtendedReadTest(TestCase):
p = self.resp.peek(0) p = self.resp.peek(0)
self.assertLessEqual(0, len(p)) self.assertLessEqual(0, len(p))
class ExtendedReadTestChunked(ExtendedReadTest): class ExtendedReadTestChunked(ExtendedReadTest):
""" """
Test peek(), read1(), readline() in chunked mode Test peek(), read1(), readline() in chunked mode
......
Allow the rare code that wants to send invalid http requests from the
`http.client` library a way to do so. The fixes for bpo-30458 led to
breakage for some projects that were relying on this ability to test their
own behavior in the face of bad requests.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment