py3: Be able to parse non-RFC-compliant request lines

There's a bug in CPython [1] that causes servers to mis-parse request
lines that include the bytes \x85 or \xa0.  Naturally, we have
functional tests that (with high probability) will send such request
lines. There's a fix proposed, but the earliest it's likely to land
would be for 3.8, and we need to be able to target an earlier Python.

So, intercept the request line immediately before parsing and re-write
it to be RFC-compliant. Note that this is done for Python 2 as well,
though there should be no change in the request environment that
eventlet eventually hands to us.

[1] https://bugs.python.org/issue33973

Change-Id: Ie648f5c04d4415f3b620fb196fa567ce7575d522

Author: tipabu

swift/common/wsgi.py

@@ -35,7 +35,8 @@
 
 from swift.common import utils, constraints
 from swift.common.storage_policy import BindPortsCache
-from swift.common.swob import Request, wsgi_unquote
+from swift.common.swob import Request, wsgi_quote, wsgi_unquote, \
+    wsgi_quote_plus, wsgi_unquote_plus, wsgi_to_bytes, bytes_to_wsgi
 from swift.common.utils import capture_stdio, disable_fallocate, \
     drop_privileges, get_logger, NullLogger, config_true_value, \
     validate_configuration, get_hub, config_auto_int_value, \
@@ -433,6 +434,36 @@ def get_default_type(self):
             '''If the client didn't provide a content type, leave it blank.'''
             return ''
 
+    def parse_request(self):
+        if not six.PY2:
+            # request lines *should* be ascii per the RFC, but historically
+            # we've allowed (and even have func tests that use) arbitrary
+            # bytes. This breaks on py3 (see https://bugs.python.org/issue33973
+            # ) but the work-around is simple: munge the request line to be
+            # properly quoted. py2 will do the right thing without this, but it
+            # doesn't hurt to re-write the request line like this and it
+            # simplifies testing.
+            if self.raw_requestline.count(b' ') >= 2:
+                parts = self.raw_requestline.split(b' ', 2)
+                path, q, query = parts[1].partition(b'?')
+                # unquote first, so we don't over-quote something
+                # that was *correctly* quoted
+                path = wsgi_to_bytes(wsgi_quote(wsgi_unquote(
+                    bytes_to_wsgi(path))))
+                query = b'&'.join(
+                    sep.join([
+                        wsgi_to_bytes(wsgi_quote_plus(wsgi_unquote_plus(
+                            bytes_to_wsgi(key)))),
+                        wsgi_to_bytes(wsgi_quote_plus(wsgi_unquote_plus(
+                            bytes_to_wsgi(val))))
+                    ])
+                    for part in query.split(b'&')
+                    for key, sep, val in (part.partition(b'='), ))
+                parts[1] = path + q + query
+                self.raw_requestline = b' '.join(parts)
+            # else, mangled protocol, most likely; let base class deal with it
+        return wsgi.HttpProtocol.parse_request(self)
+
 
 class SwiftHttpProxiedProtocol(SwiftHttpProtocol):
     """

test/unit/common/test_wsgi.py

@@ -27,6 +27,7 @@
 
 import eventlet.wsgi
 
+import six
 from six import BytesIO
 from six.moves.urllib.parse import quote
 
@@ -984,11 +985,6 @@ def test_make_env_keeps_infocache(self):
 
 
 class TestSwiftHttpProtocol(unittest.TestCase):
-    def setUp(self):
-        patcher = mock.patch('swift.common.wsgi.wsgi.HttpProtocol')
-        self.mock_super = patcher.start()
-        self.addCleanup(patcher.stop)
-
     def _proto_obj(self):
         # Make an object we can exercise... note the base class's __init__()
         # does a bunch of work, so we just new up an object like eventlet.wsgi
@@ -1041,12 +1037,38 @@ def test_swift_http_protocol_parse_request_no_proxy(self):
 
         self.assertEqual(False, proto_obj.parse_request())
 
-        self.assertEqual([], self.mock_super.mock_calls)
         self.assertEqual([
             mock.call(400, "Bad HTTP/0.9 request type ('jimmy')"),
         ], proto_obj.send_error.mock_calls)
         self.assertEqual(('a', '123'), proto_obj.client_address)
 
+    def test_request_line_cleanup(self):
+        def do_test(line_from_socket, expected_line=None):
+            if expected_line is None:
+                expected_line = line_from_socket
+
+            proto_obj = self._proto_obj()
+            proto_obj.raw_requestline = line_from_socket
+            with mock.patch('swift.common.wsgi.wsgi.HttpProtocol') \
+                    as mock_super:
+                proto_obj.parse_request()
+
+            self.assertEqual([mock.call.parse_request(proto_obj)],
+                             mock_super.mock_calls)
+            self.assertEqual(proto_obj.raw_requestline, expected_line)
+
+        do_test(b'GET / HTTP/1.1')
+        do_test(b'GET /%FF HTTP/1.1')
+
+        if not six.PY2:
+            do_test(b'GET /\xff HTTP/1.1', b'GET /%FF HTTP/1.1')
+            do_test(b'PUT /Here%20Is%20A%20SnowMan:\xe2\x98\x83 HTTP/1.0',
+                    b'PUT /Here%20Is%20A%20SnowMan%3A%E2%98%83 HTTP/1.0')
+            do_test(
+                b'POST /?and%20it=fixes+params&'
+                b'PALMTREE=\xf0%9f\x8c%b4 HTTP/1.1',
+                b'POST /?and+it=fixes+params&PALMTREE=%F0%9F%8C%B4 HTTP/1.1')
+
 
 class TestProxyProtocol(unittest.TestCase):
     def _run_bytes_through_protocol(self, bytes_from_client, protocol_class):