[Python-checkins] [2.7] bpo-38216, bpo-36274: Allow subclasses to separately override validation and encoding behavior (GH-16476)

[Benjamin Peterson]

https://github.com/python/cpython/commit/f5b1abbb3b0083381925dcd5898ae6d019224826
commit: f5b1abbb3b0083381925dcd5898ae6d019224826
branch: 2.7
author: Jason R. Coombs <jaraco at jaraco.com>
committer: Benjamin Peterson <benjamin at python.org>
date: 2019-10-07T19:00:01-07:00
summary:

[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 7774d7831e8809795c64ce27f7df52674581d298)

Co-authored-by: Jason R. Coombs <jaraco at jaraco.com>

files:
A Misc/NEWS.d/next/Library/2019-09-27-15-24-45.bpo-38216.-7yvZR.rst
M Lib/httplib.py
M Lib/test/test_httplib.py

diff --git a/Lib/httplib.py b/Lib/httplib.py
index 1b41c346e090a..79532b91149b1 100644
--- a/Lib/httplib.py
+++ b/Lib/httplib.py
@@ -933,19 +933,15 @@ def putrequest(self, method, url, skip_host=0, skip_accept_encoding=0):
         else:
             raise CannotSendRequest()
 
-        # 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.
-        match = _contains_disallowed_url_pchar_re.search(url)
-        if match:
-            raise InvalidURL("URL can't contain control characters. %r "
-                             "(found at least %r)"
-                             % (url, match.group()))
-        hdr = '%s %s %s' % (method, url, self._http_vsn_str)
 
-        self._output(hdr)
+        url = url or '/'
+        self._validate_path(url)
+
+        request = '%s %s %s' % (method, url, self._http_vsn_str)
+
+        self._output(self._encode_request(request))
 
         if self._http_vsn == 11:
             # 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):
             # For HTTP/1.0, the server will assume "not chunked"
             pass
 
+    def _encode_request(self, request):
+        # On Python 2, request is already encoded (default)
+        return request
+
+    def _validate_path(self, url):
+        """Validate a url for putrequest."""
+        # Prevent CVE-2019-9740.
+        match = _contains_disallowed_url_pchar_re.search(url)
+        if match:
+            msg = (
+                "URL can't contain control characters. {url!r} "
+                "(found at least {matched!r})"
+            ).format(matched=match.group(), url=url)
+            raise InvalidURL(msg)
+
     def putheader(self, header, *values):
         """Send a request header line to the server.
 
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index 44ffac7036886..5462fdd503c83 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -702,6 +702,20 @@ def test_proxy_tunnel_without_status_line(self):
         with self.assertRaisesRegexp(socket.error, "Invalid response"):
             conn._tunnel()
 
+    def test_putrequest_override_validation(self):
+        """
+        It should be possible to override the default validation
+        behavior in putrequest (bpo-38216).
+        """
+        class UnsafeHTTPConnection(httplib.HTTPConnection):
+            def _validate_path(self, url):
+                pass
+
+        conn = UnsafeHTTPConnection('example.com')
+        conn.sock = FakeSocket('')
+        conn.putrequest('GET', '/\x00')
+
+
 class OfflineTest(TestCase):
     def test_responses(self):
         self.assertEqual(httplib.responses[httplib.NOT_FOUND], "Not Found")
diff --git a/Misc/NEWS.d/next/Library/2019-09-27-15-24-45.bpo-38216.-7yvZR.rst b/Misc/NEWS.d/next/Library/2019-09-27-15-24-45.bpo-38216.-7yvZR.rst
new file mode 100644
index 0000000000000..ac8e2b042d929
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-09-27-15-24-45.bpo-38216.-7yvZR.rst
@@ -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.