Skip to content
Permalink
Browse files

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
  • Loading branch information...
tipabu committed Mar 21, 2019
1 parent f55167a commit 93b49c5e4818774febb7aa59fc21cbbcf8db2344
Showing with 60 additions and 7 deletions.
  1. +32 −1 swift/common/wsgi.py
  2. +28 −6 test/unit/common/test_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):
"""
@@ -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):

0 comments on commit 93b49c5

Please sign in to comment.
You can’t perform that action at this time.