Skip to content

bpo-38216, bpo-36274: Allow subclasses to override validation and encoding behavior #16321

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.

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

Closed
wants to merge 9 commits into from
33 changes: 23 additions & 10 deletions Lib/http/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1085,18 +1085,16 @@ def putrequest(self, method, url, skip_host=False,
else:
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
if not url:
url = '/'
# Prevent CVE-2019-9740.
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)

# Non-ASCII characters should have been eliminated earlier
self._output(request.encode('ascii'))
request = b' '.join((
method.encode('ascii'),
self._prepare_path(url or '/'),
self._http_vsn_str.encode('ascii')
))

self._output(request)

if self._http_vsn == 11:
# Issue some standard headers for better HTTP/1.1 compliance
Expand Down Expand Up @@ -1174,6 +1172,21 @@ def putrequest(self, method, url, skip_host=False,
# For HTTP/1.0, the server will assume "not chunked"
pass


def _encode_prepared_path(self, str_url):
# ASCII also helps prevent CVE-2019-9740.
return str_url.encode('ascii')

def _prepare_path(self, url):
"""Validate a url for putrequest and return encoded bytes."""
# 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})")

return self._encode_prepared_path(url)

def putheader(self, header, *values):
"""Send a request header line to the server.

Expand Down
51 changes: 51 additions & 0 deletions Lib/test/test_httplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,56 @@ def run_server():
thread.join()
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 _prepare_path(self, url):
return url.encode('ascii')

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_prepared_path(self, str_url):
return str_url.encode('utf-8')

conn = UnsafeHTTPConnection('example.com')
conn.sock = FakeSocket('')
conn.putrequest('GET', '/☃')

def test_prepare_path_only(self):
"""
Ensure that _prepare_path fully processes the URL
and that no other demands are on the object.
"""
class UnsafeHTTPConnection(client.HTTPConnection):
def _prepare_path(self, url):
# ignore URL and return some bytes
return b'/'

class InvalidObject:
"""
Stub object that doesn't behave anything like a string
and should cause tests to fail if any demands are put
on the url parameter other than in _prepare_path
(__bool__ allowed).
"""

conn = UnsafeHTTPConnection('example.com')
conn.sock = FakeSocket('')
conn.putrequest('GET', InvalidObject(), skip_host=True)


class ExtendedReadTest(TestCase):
"""
Test peek(), read1(), readline()
Expand Down Expand Up @@ -1279,6 +1329,7 @@ def test_peek_0(self):
p = self.resp.peek(0)
self.assertLessEqual(0, len(p))


class ExtendedReadTestChunked(ExtendedReadTest):
"""
Test peek(), read1(), readline() in chunked mode
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
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.