Skip to content

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

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

Merged
merged 10 commits into from
Sep 28, 2019
Merged
27 changes: 18 additions & 9 deletions Lib/http/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1085,18 +1085,15 @@ 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})")

url = url or '/'
self._validate_path(url)

request = '%s %s %s' % (method, url, self._http_vsn_str)

# Non-ASCII characters should have been eliminated earlier
self._output(request.encode('ascii'))
self._output(self._encode_request(request))

if self._http_vsn == 11:
# Issue some standard headers for better HTTP/1.1 compliance
Expand Down Expand Up @@ -1174,6 +1171,18 @@ def putrequest(self, method, url, skip_host=False,
# For HTTP/1.0, the server will assume "not chunked"
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):
"""Send a request header line to the server.

Expand Down
29 changes: 29 additions & 0 deletions Lib/test/test_httplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,34 @@ 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 _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):
"""
Test peek(), read1(), readline()
Expand Down Expand Up @@ -1279,6 +1307,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.