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
This commit is contained in:
Tim Burke 2019-03-21 10:33:41 -07:00
parent f55167a735
commit 93b49c5e48
2 changed files with 60 additions and 7 deletions

View File

@ -35,7 +35,8 @@ from six import StringIO
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 @@ class SwiftHttpProtocol(wsgi.HttpProtocol):
'''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):
"""

View File

@ -27,6 +27,7 @@ import types
import eventlet.wsgi
import six
from six import BytesIO
from six.moves.urllib.parse import quote
@ -984,11 +985,6 @@ class TestWSGI(unittest.TestCase):
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 @@ class TestSwiftHttpProtocol(unittest.TestCase):
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):