openstack/swift
Code Issues Proposed changes

Address some review comments

Browse Source 
Change-Id: Iacff8a7d7e37557b9a7694ac2df0cfc6b3492024
Related-Change: Ia5815602d05925c5de110accc4dfb1368203bd8d
This commit is contained in:
Tim Burke
2018-12-27 23:01:52 +00:00
parent 0d29b01d2b
commit 1e3f8a0e53
4 changed files with 46 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
swift/common/request_helpers.py
View File

@@ -55,7 +55,7 @@ def get_param(req, name, default=None):
:param req: request object :param req: request object
:param name: parameter name :param name: parameter name
:param default: result to return if the parameter is not found :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) (in py2, as UTF-8 encoded str, not unicode object)
:raises HTTPBadRequest: if param not valid UTF-8 byte sequence :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) body='"%s" parameter not valid UTF-8' % name)
else: else:
if value: if value:
try: # req.params is a dict of WSGI strings, so encoding will succeed
if isinstance(value, six.text_type): value = value.encode('latin1')
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)
try: try:
# Ensure UTF8ness since we're at it # Ensure UTF8ness since we're at it
value = value.decode('utf8') value = value.decode('utf8')

8
swift/common/swob.py
View File

@@ -751,7 +751,7 @@ class Accept(object):
def _req_environ_property(environ_field, is_wsgi_string_field=True): def _req_environ_property(environ_field, is_wsgi_string_field=True):
""" """
Set and retrieve value of the environ_field entry in self.environ. Set and retrieve value of the environ_field entry in self.environ.
(Used by both request and response) (Used by Request)
""" """
def getter(self): def getter(self):
return self.environ.get(environ_field, None) return self.environ.get(environ_field, None)
@@ -967,7 +967,11 @@ class Request(object):
@params.setter @params.setter
def params(self, param_pairs): def params(self, param_pairs):
self._params_cache = None 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 @property
def timestamp(self): def timestamp(self):

41
test/unit/account/test_server.py
View File

@@ -915,8 +915,8 @@ class TestAccountController(unittest.TestCase):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'}) req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'})
resp = req.get_response(self.controller) resp = req.get_response(self.controller)
self.assertEqual(resp.status_int, 200) self.assertEqual(resp.status_int, 200)
self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), self.assertEqual(resp.body.strip().split(b'\n'),
['c1', 'c2']) [b'c1', b'c2'])
req = Request.blank('/sda1/p/a/c1', environ={'REQUEST_METHOD': 'PUT'}, req = Request.blank('/sda1/p/a/c1', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Put-Timestamp': '1', headers={'X-Put-Timestamp': '1',
'X-Delete-Timestamp': '0', 'X-Delete-Timestamp': '0',
@@ -934,8 +934,8 @@ class TestAccountController(unittest.TestCase):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'}) req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'})
resp = req.get_response(self.controller) resp = req.get_response(self.controller)
self.assertEqual(resp.status_int, 200) self.assertEqual(resp.status_int, 200)
self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), self.assertEqual(resp.body.strip().split(b'\n'),
['c1', 'c2']) [b'c1', b'c2'])
self.assertEqual(resp.content_type, 'text/plain') self.assertEqual(resp.content_type, 'text/plain')
self.assertEqual(resp.charset, 'utf-8') self.assertEqual(resp.charset, 'utf-8')
@@ -944,8 +944,8 @@ class TestAccountController(unittest.TestCase):
environ={'REQUEST_METHOD': 'GET'}) environ={'REQUEST_METHOD': 'GET'})
resp = req.get_response(self.controller) resp = req.get_response(self.controller)
self.assertEqual(resp.status_int, 200) self.assertEqual(resp.status_int, 200)
self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), self.assertEqual(resp.body.strip().split(b'\n'),
['c1', 'c2']) [b'c1', b'c2'])
self.assertEqual(resp.content_type, 'text/plain') self.assertEqual(resp.content_type, 'text/plain')
self.assertEqual(resp.charset, 'utf-8') self.assertEqual(resp.charset, 'utf-8')
@@ -1198,14 +1198,14 @@ class TestAccountController(unittest.TestCase):
environ={'REQUEST_METHOD': 'GET'}) environ={'REQUEST_METHOD': 'GET'})
resp = req.get_response(self.controller) resp = req.get_response(self.controller)
self.assertEqual(resp.status_int, 200) self.assertEqual(resp.status_int, 200)
self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), self.assertEqual(resp.body.strip().split(b'\n'),
['c0', 'c1', 'c2']) [b'c0', b'c1', b'c2'])
req = Request.blank('/sda1/p/a?limit=3&marker=c2', req = Request.blank('/sda1/p/a?limit=3&marker=c2',
environ={'REQUEST_METHOD': 'GET'}) environ={'REQUEST_METHOD': 'GET'})
resp = req.get_response(self.controller) resp = req.get_response(self.controller)
self.assertEqual(resp.status_int, 200) self.assertEqual(resp.status_int, 200)
self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), self.assertEqual(resp.body.strip().split(b'\n'),
['c3', 'c4']) [b'c3', b'c4'])
def test_GET_limit_marker_json(self): def test_GET_limit_marker_json(self):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
@@ -1474,21 +1474,21 @@ class TestAccountController(unittest.TestCase):
environ={'REQUEST_METHOD': 'GET'}) environ={'REQUEST_METHOD': 'GET'})
resp = req.get_response(self.controller) resp = req.get_response(self.controller)
self.assertEqual(resp.status_int, 200) self.assertEqual(resp.status_int, 200)
self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), self.assertEqual(resp.body.strip().split(b'\n'),
['sub.']) [b'sub.'])
req = Request.blank('/sda1/p/a?prefix=sub.&delimiter=.', req = Request.blank('/sda1/p/a?prefix=sub.&delimiter=.',
environ={'REQUEST_METHOD': 'GET'}) environ={'REQUEST_METHOD': 'GET'})
resp = req.get_response(self.controller) resp = req.get_response(self.controller)
self.assertEqual(resp.status_int, 200) self.assertEqual(resp.status_int, 200)
self.assertEqual( self.assertEqual(
resp.body.decode('utf-8').strip().split('\n'), resp.body.strip().split(b'\n'),
['sub.0', 'sub.0.', 'sub.1', 'sub.1.', 'sub.2', 'sub.2.']) [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=.', req = Request.blank('/sda1/p/a?prefix=sub.1.&delimiter=.',
environ={'REQUEST_METHOD': 'GET'}) environ={'REQUEST_METHOD': 'GET'})
resp = req.get_response(self.controller) resp = req.get_response(self.controller)
self.assertEqual(resp.status_int, 200) self.assertEqual(resp.status_int, 200)
self.assertEqual(resp.body.decode('utf-8').strip().split('\n'), self.assertEqual(resp.body.strip().split(b'\n'),
['sub.1.0', 'sub.1.1', 'sub.1.2']) [b'sub.1.0', b'sub.1.1', b'sub.1.2'])
def test_GET_prefix_delimiter_json(self): def test_GET_prefix_delimiter_json(self):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
@@ -1836,14 +1836,13 @@ class TestAccountController(unittest.TestCase):
# swift.account.server.AccountController.__call__ # swift.account.server.AccountController.__call__
inbuf = BytesIO() inbuf = BytesIO()
errbuf = StringIO() errbuf = StringIO()
outbuf = StringIO()
self.controller = AccountController( self.controller = AccountController(
{'devices': self.testdir, {'devices': self.testdir,
'mount_check': 'false', 'mount_check': 'false',
'replication_server': 'false'}) 'replication_server': 'false'})
def start_response(*args): def start_response(*args):
outbuf.write(args[0]) pass
method = 'PUT' method = 'PUT'
env = {'REQUEST_METHOD': method, env = {'REQUEST_METHOD': method,
@@ -1874,13 +1873,12 @@ class TestAccountController(unittest.TestCase):
# swift.account.server.AccountController.__call__ # swift.account.server.AccountController.__call__
inbuf = BytesIO() inbuf = BytesIO()
errbuf = StringIO() errbuf = StringIO()
outbuf = StringIO()
self.controller = AccountController( self.controller = AccountController(
{'devices': self.testdir, 'mount_check': 'false', {'devices': self.testdir, 'mount_check': 'false',
'replication_server': 'false'}) 'replication_server': 'false'})
def start_response(*args): def start_response(*args):
outbuf.write(args[0]) pass
method = 'PUT' method = 'PUT'
env = {'REQUEST_METHOD': method, env = {'REQUEST_METHOD': method,
@@ -1941,7 +1939,6 @@ class TestAccountController(unittest.TestCase):
def test__call__raise_timeout(self): def test__call__raise_timeout(self):
inbuf = WsgiBytesIO() inbuf = WsgiBytesIO()
errbuf = StringIO() errbuf = StringIO()
outbuf = StringIO()
self.logger = debug_logger('test') self.logger = debug_logger('test')
self.account_controller = AccountController( self.account_controller = AccountController(
{'devices': self.testdir, 'mount_check': 'false', {'devices': self.testdir, 'mount_check': 'false',
@@ -1949,7 +1946,7 @@ class TestAccountController(unittest.TestCase):
logger=self.logger) logger=self.logger)
def start_response(*args): def start_response(*args):
outbuf.write(args[0]) pass
method = 'PUT' method = 'PUT'

18
test/unit/common/test_swob.py
View File

@@ -565,6 +565,24 @@ class TestRequest(unittest.TestCase):
req.params = new_params req.params = new_params
self.assertDictEqual(dict(new_params), req.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): def test_timestamp_missing(self):
req = swift.common.swob.Request.blank('/') req = swift.common.swob.Request.blank('/')
self.assertRaises(exceptions.InvalidTimestamp, self.assertRaises(exceptions.InvalidTimestamp,