Once again separate the path validation and request encoding, drastic…

…ally simplifying the behavior. Drop the guarantee that all processing happens in _prepare_path.
python · jaraco · Sep 28, 2019 · Sep 20, 2019 · Sep 21, 2019 · Sep 21, 2019
commit 21624bcefe5a33bcc5ceed509f7d5f477ea15ff9
diff --git a/Lib/http/client.py b/Lib/http/client.py
@@ -1088,13 +1088,12 @@ def putrequest(self, method, url, skip_host=False,
         # Save the method for use later in the response phase
         self._method = method
 
-        request = b' '.join((
-            method.encode('ascii'),
-            self._prepare_path(url or '/'),
-            self._http_vsn_str.encode('ascii')
-        ))
+        url = url or '/'
+        self._validate_path(url)
 
-        self._output(request)
+        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
@@ -1172,21 +1171,18 @@ 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):
+    def _encode_request(self, request):
         # ASCII also helps prevent CVE-2019-9740.
-        return str_url.encode('ascii')
+        return request.encode('ascii')
 
-    def _prepare_path(self, url):
-        """Validate a url for putrequest and return encoded bytes."""
+    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})")
 
-        return self._encode_prepared_path(url)
-
     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
@@ -1161,8 +1161,8 @@ def test_putrequest_override_validation(self):
         behavior in putrequest (bpo-38216).
         """
         class UnsafeHTTPConnection(client.HTTPConnection):
-            def _prepare_path(self, url):
-                return url.encode('ascii')
+            def _validate_path(self, url):
+                pass
 
         conn = UnsafeHTTPConnection('example.com')
         conn.sock = FakeSocket('')
@@ -1175,35 +1175,13 @@ def test_putrequest_override_encoding(self):
         (bpo-36274).
         """
         class UnsafeHTTPConnection(client.HTTPConnection):
-            def _encode_prepared_path(self, str_url):
+            def _encode_request(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):
     """