From 1e3f8a0e5370ea710b13b9aad4ba4f13613b8f41 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 27 Dec 2018 23:01:52 +0000 Subject: [PATCH] Address some review comments Change-Id: Iacff8a7d7e37557b9a7694ac2df0cfc6b3492024 Related-Change: Ia5815602d05925c5de110accc4dfb1368203bd8d --- swift/common/request_helpers.py | 13 +++------- swift/common/swob.py | 8 +++++-- test/unit/account/test_server.py | 41 +++++++++++++++----------------- test/unit/common/test_swob.py | 18 ++++++++++++++ 4 files changed, 46 insertions(+), 34 deletions(-) diff --git a/swift/common/request_helpers.py b/swift/common/request_helpers.py index a09ee26c15..78fe771f44 100644 --- a/swift/common/request_helpers.py +++ b/swift/common/request_helpers.py @@ -55,7 +55,7 @@ def get_param(req, name, default=None): :param req: request object :param name: parameter name :param default: result to return if the parameter is not found - :returns: HTTP request parameter value + :returns: HTTP request parameter value, as a native string (in py2, as UTF-8 encoded str, not unicode object) :raises HTTPBadRequest: if param not valid UTF-8 byte sequence """ @@ -70,15 +70,8 @@ def get_param(req, name, default=None): body='"%s" parameter not valid UTF-8' % name) else: if value: - try: - if isinstance(value, six.text_type): - value = value.encode('latin1') - except UnicodeEncodeError: - # This happens, remarkably enough, when WSGI string is not - # a valid latin-1, and passed through parse_qsl(). - raise HTTPBadRequest( - request=req, content_type='text/plain', - body='"%s" parameter not valid UTF-8' % name) + # req.params is a dict of WSGI strings, so encoding will succeed + value = value.encode('latin1') try: # Ensure UTF8ness since we're at it value = value.decode('utf8') diff --git a/swift/common/swob.py b/swift/common/swob.py index ffd0cf1fcf..9a4be20b5c 100644 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -751,7 +751,7 @@ class Accept(object): def _req_environ_property(environ_field, is_wsgi_string_field=True): """ Set and retrieve value of the environ_field entry in self.environ. - (Used by both request and response) + (Used by Request) """ def getter(self): return self.environ.get(environ_field, None) @@ -967,7 +967,11 @@ class Request(object): @params.setter def params(self, param_pairs): self._params_cache = None - self.query_string = urllib.parse.urlencode(param_pairs) + if six.PY2: + self.query_string = urllib.parse.urlencode(param_pairs) + else: + self.query_string = urllib.parse.urlencode(param_pairs, + encoding='latin-1') @property def timestamp(self): diff --git a/test/unit/account/test_server.py b/test/unit/account/test_server.py index 4c780d40e2..c34fec74bc 100644 --- a/test/unit/account/test_server.py +++ b/test/unit/account/test_server.py @@ -915,8 +915,8 @@ class TestAccountController(unittest.TestCase): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'}) resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 200) - self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), - ['c1', 'c2']) + self.assertEqual(resp.body.strip().split(b'\n'), + [b'c1', b'c2']) req = Request.blank('/sda1/p/a/c1', environ={'REQUEST_METHOD': 'PUT'}, headers={'X-Put-Timestamp': '1', 'X-Delete-Timestamp': '0', @@ -934,8 +934,8 @@ class TestAccountController(unittest.TestCase): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'}) resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 200) - self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), - ['c1', 'c2']) + self.assertEqual(resp.body.strip().split(b'\n'), + [b'c1', b'c2']) self.assertEqual(resp.content_type, 'text/plain') self.assertEqual(resp.charset, 'utf-8') @@ -944,8 +944,8 @@ class TestAccountController(unittest.TestCase): environ={'REQUEST_METHOD': 'GET'}) resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 200) - self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), - ['c1', 'c2']) + self.assertEqual(resp.body.strip().split(b'\n'), + [b'c1', b'c2']) self.assertEqual(resp.content_type, 'text/plain') self.assertEqual(resp.charset, 'utf-8') @@ -1198,14 +1198,14 @@ class TestAccountController(unittest.TestCase): environ={'REQUEST_METHOD': 'GET'}) resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 200) - self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), - ['c0', 'c1', 'c2']) + self.assertEqual(resp.body.strip().split(b'\n'), + [b'c0', b'c1', b'c2']) req = Request.blank('/sda1/p/a?limit=3&marker=c2', environ={'REQUEST_METHOD': 'GET'}) resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 200) - self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), - ['c3', 'c4']) + self.assertEqual(resp.body.strip().split(b'\n'), + [b'c3', b'c4']) def test_GET_limit_marker_json(self): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', @@ -1474,21 +1474,21 @@ class TestAccountController(unittest.TestCase): environ={'REQUEST_METHOD': 'GET'}) resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 200) - self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), - ['sub.']) + self.assertEqual(resp.body.strip().split(b'\n'), + [b'sub.']) req = Request.blank('/sda1/p/a?prefix=sub.&delimiter=.', environ={'REQUEST_METHOD': 'GET'}) resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 200) self.assertEqual( - resp.body.decode('utf-8').strip().split('\n'), - ['sub.0', 'sub.0.', 'sub.1', 'sub.1.', 'sub.2', 'sub.2.']) + resp.body.strip().split(b'\n'), + [b'sub.0', b'sub.0.', b'sub.1', b'sub.1.', b'sub.2', b'sub.2.']) req = Request.blank('/sda1/p/a?prefix=sub.1.&delimiter=.', environ={'REQUEST_METHOD': 'GET'}) resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 200) - self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), - ['sub.1.0', 'sub.1.1', 'sub.1.2']) + self.assertEqual(resp.body.strip().split(b'\n'), + [b'sub.1.0', b'sub.1.1', b'sub.1.2']) def test_GET_prefix_delimiter_json(self): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', @@ -1836,14 +1836,13 @@ class TestAccountController(unittest.TestCase): # swift.account.server.AccountController.__call__ inbuf = BytesIO() errbuf = StringIO() - outbuf = StringIO() self.controller = AccountController( {'devices': self.testdir, 'mount_check': 'false', 'replication_server': 'false'}) def start_response(*args): - outbuf.write(args[0]) + pass method = 'PUT' env = {'REQUEST_METHOD': method, @@ -1874,13 +1873,12 @@ class TestAccountController(unittest.TestCase): # swift.account.server.AccountController.__call__ inbuf = BytesIO() errbuf = StringIO() - outbuf = StringIO() self.controller = AccountController( {'devices': self.testdir, 'mount_check': 'false', 'replication_server': 'false'}) def start_response(*args): - outbuf.write(args[0]) + pass method = 'PUT' env = {'REQUEST_METHOD': method, @@ -1941,7 +1939,6 @@ class TestAccountController(unittest.TestCase): def test__call__raise_timeout(self): inbuf = WsgiBytesIO() errbuf = StringIO() - outbuf = StringIO() self.logger = debug_logger('test') self.account_controller = AccountController( {'devices': self.testdir, 'mount_check': 'false', @@ -1949,7 +1946,7 @@ class TestAccountController(unittest.TestCase): logger=self.logger) def start_response(*args): - outbuf.write(args[0]) + pass method = 'PUT' diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py index fae53a0704..f97941fa8d 100644 --- a/test/unit/common/test_swob.py +++ b/test/unit/common/test_swob.py @@ -565,6 +565,24 @@ class TestRequest(unittest.TestCase): req.params = new_params self.assertDictEqual(dict(new_params), req.params) + def test_unicode_params(self): + # NB: all of these strings are WSGI strings + req = swift.common.swob.Request.blank( + '/?\xe1\x88\xb4=%E1%88%B4&%FF=\xff') + self.assertEqual(req.params['\xff'], '\xff') + self.assertEqual(req.params['\xe1\x88\xb4'], '\xe1\x88\xb4') + + new_params = {'\xff': '\xe1\x88\xb4', '\xe1\x88\xb4': '\xff'} + req.params = new_params + self.assertDictEqual(new_params, req.params) + self.assertIn('%FF=%E1%88%B4', req.environ['QUERY_STRING']) + self.assertIn('%E1%88%B4=%FF', req.environ['QUERY_STRING']) + + # ...well, until we get to unicode that isn't WSGI-friendly + new_params = ((u'\u1234', u'\u1234'), ) + with self.assertRaises(UnicodeEncodeError): + req.params = new_params + def test_timestamp_missing(self): req = swift.common.swob.Request.blank('/') self.assertRaises(exceptions.InvalidTimestamp,