Skip to content

Commit f5b1abb

Browse files
jaracobenjaminp
authored andcommitted
[2.7] bpo-38216, bpo-36274: Allow subclasses to separately override validation and encoding behavior (GH-16476)
Backporting this change, I observe a couple of things: 1. The _encode_request call is no longer meaningful because the request construction will implicitly encode the request using the default encoding when the format string is used (request = '%s %s %s'...). In order to keep the code as consistent as possible, I decided to include the call as a pass-through. I'd be just as happy to remove it entirely, but I'll leave that up to the reviewer to decide. It's okay that this functionality is disabled on Python 2 because this functionality was mainly around bpo-36274, which was mainly a concern with the transition to Python 3. 2. Because _encode_request is no longer meaningful, neither is the test for it, so I've removed that test. Therefore, the meaningful part of this test is that for bpo-38216, adding a (underscore-protected) hook to customize/disable validation. (cherry picked from commit 7774d78) Co-authored-by: Jason R. Coombs <[email protected]>
1 parent e7e58fe commit f5b1abb

File tree

3 files changed

+40
-11
lines changed

3 files changed

+40
-11
lines changed

Lib/httplib.py

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -933,19 +933,15 @@ def putrequest(self, method, url, skip_host=0, skip_accept_encoding=0):
933933
else:
934934
raise CannotSendRequest()
935935

936-
# Save the method we use, we need it later in the response phase
936+
# Save the method for use later in the response phase
937937
self._method = method
938-
if not url:
939-
url = '/'
940-
# Prevent CVE-2019-9740.
941-
match = _contains_disallowed_url_pchar_re.search(url)
942-
if match:
943-
raise InvalidURL("URL can't contain control characters. %r "
944-
"(found at least %r)"
945-
% (url, match.group()))
946-
hdr = '%s %s %s' % (method, url, self._http_vsn_str)
947938

948-
self._output(hdr)
939+
url = url or '/'
940+
self._validate_path(url)
941+
942+
request = '%s %s %s' % (method, url, self._http_vsn_str)
943+
944+
self._output(self._encode_request(request))
949945

950946
if self._http_vsn == 11:
951947
# Issue some standard headers for better HTTP/1.1 compliance
@@ -1018,6 +1014,21 @@ def putrequest(self, method, url, skip_host=0, skip_accept_encoding=0):
10181014
# For HTTP/1.0, the server will assume "not chunked"
10191015
pass
10201016

1017+
def _encode_request(self, request):
1018+
# On Python 2, request is already encoded (default)
1019+
return request
1020+
1021+
def _validate_path(self, url):
1022+
"""Validate a url for putrequest."""
1023+
# Prevent CVE-2019-9740.
1024+
match = _contains_disallowed_url_pchar_re.search(url)
1025+
if match:
1026+
msg = (
1027+
"URL can't contain control characters. {url!r} "
1028+
"(found at least {matched!r})"
1029+
).format(matched=match.group(), url=url)
1030+
raise InvalidURL(msg)
1031+
10211032
def putheader(self, header, *values):
10221033
"""Send a request header line to the server.
10231034

Lib/test/test_httplib.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,20 @@ def test_proxy_tunnel_without_status_line(self):
702702
with self.assertRaisesRegexp(socket.error, "Invalid response"):
703703
conn._tunnel()
704704

705+
def test_putrequest_override_validation(self):
706+
"""
707+
It should be possible to override the default validation
708+
behavior in putrequest (bpo-38216).
709+
"""
710+
class UnsafeHTTPConnection(httplib.HTTPConnection):
711+
def _validate_path(self, url):
712+
pass
713+
714+
conn = UnsafeHTTPConnection('example.com')
715+
conn.sock = FakeSocket('')
716+
conn.putrequest('GET', '/\x00')
717+
718+
705719
class OfflineTest(TestCase):
706720
def test_responses(self):
707721
self.assertEqual(httplib.responses[httplib.NOT_FOUND], "Not Found")
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Allow the rare code that wants to send invalid http requests from the
2+
`http.client` library a way to do so. The fixes for bpo-30458 led to
3+
breakage for some projects that were relying on this ability to test their
4+
own behavior in the face of bad requests.

0 commit comments

Comments
 (0)