From d126a7654af03df4ec26ab1be4614279d6c1fec5 Mon Sep 17 00:00:00 2001
From: Seth Michael Larson <sethmichaellarson@gmail.com>
Date: Wed, 12 Feb 2020 20:03:54 -0600
Subject: [PATCH 1/4] Raise ValueError if method contains control characters

---
 CHANGES.rst                                  |    3 +++
 test/with_dummyserver/test_connectionpool.py |   14 +++++++++++---
 test/with_dummyserver/test_poolmanager.py    |    2 +-
 urllib3/connection.py                        |   16 +++++++++++++++-
 4 files changed, 30 insertions(+), 5 deletions(-)

--- a/urllib3/connection.py
+++ b/urllib3/connection.py
@@ -1,4 +1,5 @@
 from __future__ import absolute_import
+import re
 import datetime
 import logging
 import os
@@ -59,7 +60,9 @@ port_by_scheme = {
 # When updating RECENT_DATE, move it to
 # within two years of the current date, and not
 # less than 6 months ago.
-RECENT_DATE = datetime.date(2019, 1, 1)
+RECENT_DATE = datetime.date(2021, 1, 1)
+
+_CONTAINS_CONTROL_CHAR_RE = re.compile(r"[^-!#$%&'*+.^_`|~0-9a-zA-Z]")
 
 
 class DummyConnection(object):
@@ -166,6 +169,17 @@ class HTTPConnection(_HTTPConnection, ob
         conn = self._new_conn()
         self._prepare_conn(conn)
 
+    def putrequest(self, method, url, *args, **kwargs):
+        """Send a request to the server"""
+        match = _CONTAINS_CONTROL_CHAR_RE.search(method)
+        if match:
+            raise ValueError(
+                "Method cannot contain non-token characters %r (found at least %r)"
+                % (method, match.group())
+            )
+
+        return _HTTPConnection.putrequest(self, method, url, *args, **kwargs)
+
     def request_chunked(self, method, url, body=None, headers=None):
         """
         Alternative to the common request method, which sends the
--- a/test/with_dummyserver/test_connectionpool.py
+++ b/test/with_dummyserver/test_connectionpool.py
@@ -1,4 +1,5 @@
 import io
+from http.client import InvalidURL
 import logging
 import socket
 import sys
@@ -7,6 +8,7 @@ import time
 import warnings
 
 import mock
+import pytest
 
 from .. import (
     TARPIT_HOST, VALID_SOURCE_ADDRESSES, INVALID_SOURCE_ADDRESSES,
@@ -22,6 +24,7 @@ from urllib3.exceptions import (
     DecodeError,
     MaxRetryError,
     ReadTimeoutError,
+    ProtocolError,
     NewConnectionError,
     UnrewindableBodyError,
 )
@@ -672,9 +675,8 @@ class TestConnectionPool(HTTPDummyServer
         for addr in INVALID_SOURCE_ADDRESSES:
             pool = HTTPConnectionPool(self.host, self.port, source_address=addr, retries=False)
             # FIXME: This assert flakes sometimes. Not sure why.
-            self.assertRaises(NewConnectionError,
-                              pool.request,
-                              'GET', '/source_address?{0}'.format(addr))
+            with self.assertRaises((InvalidURL, ProtocolError)):
+                pool.request('GET', '/source_address?{0}'.format(addr))
 
     def test_stream_keepalive(self):
         x = 2
@@ -737,6 +739,12 @@ class TestConnectionPool(HTTPDummyServer
         response = pool.request('GET', "http://LoCaLhOsT:%d/" % self.port)
         self.assertEqual(response.status, 200)
 
+    def test_invalid_method_not_allowed(self):
+        for char in [" ", "\r", "\n", "\x00"]:
+            with pytest.raises(ValueError):
+                with HTTPConnectionPool(self.host, self.port) as pool:
+                    pool.request("GET" + char, "/")
+
 
 class TestRetry(HTTPDummyServerTestCase):
     def setUp(self):
--- a/test/with_dummyserver/test_poolmanager.py
+++ b/test/with_dummyserver/test_poolmanager.py
@@ -1,7 +1,7 @@
 import unittest
 import json
 
-from nose.plugins.skip import SkipTest
+from unittest import SkipTest
 from dummyserver.server import HAS_IPV6
 from dummyserver.testcase import (HTTPDummyServerTestCase,
                                   IPv6HTTPDummyServerTestCase)
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,6 +1,9 @@
 Changes
 =======
 
+* Raise ``ValueError`` if control characters are given in
+  the ``method`` parameter of ``HTTPConnection.request()`` (Pull #1800)
+
 * Allow providing a list of headers to strip from requests when redirecting
   to a different host. Defaults to the ``Authorization`` header. Different
   headers can be set via ``Retry.remove_headers_on_redirect``. (Issue #1316)
