openstack/swift
Code Issues Proposed changes

Fix tempauth acl checks when simplejson has no speedups

Browse Source 
As documented in linked bug report, tempauth unit tests
were seen to fail on a system where simplejson was
installed but without the speedups extension. This
is because the tempauth account acl validation checks
that values are type str, but without the speedups
extension the json parser is returning unicode objects.

Fix is to have the acl validator tolerate those objects
being unicode or str.

Also change common/bufferedhttp.py to coerce ring device
to type str when constructing a path, in order to avoid
a UnicodeDecodeError when httplib sends a message that
has non-ascii header values.

Change-Id: I01524282cbaa25dc4b6dfa09f3f4723516cdba99
Closes-Bug: 1425776
This commit is contained in:
Alistair Coles 2015-02-26 15:16:22 +00:00
parent ad66801915
commit 2080f7dbd8
4 changed files with 73 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
swift/common/bufferedhttp.py
View File

@ -155,6 +155,11 @@ def http_connect(ipaddr, port, device, partition, method, path,
path = path.encode("utf-8")
except UnicodeError as e:
logging.exception(_('Error encoding to UTF-8: %s'), str(e))
if isinstance(device, unicode):
try:
device = device.encode("utf-8")
except UnicodeError as e:
logging.exception(_('Error encoding to UTF-8: %s'), str(e))
path = quote('/' + device + '/' + str(partition) + path)
return http_connect_raw(
ipaddr, port, method, path, headers, query_string, ssl)

8
swift/common/middleware/tempauth.py
View File

@ -447,16 +447,16 @@ class TempAuth(object):
# on ACLs, TempAuth is not such an auth system. At this point,
# it thinks it is authoritative.
if key not in tempauth_acl_keys:
return 'Key %r not recognized' % key
return "Key '%s' not recognized" % key
for key in tempauth_acl_keys:
if key not in result:
continue
if not isinstance(result[key], list):
return 'Value for key %r must be a list' % key
return "Value for key '%s' must be a list" % key
for grantee in result[key]:
if not isinstance(grantee, str):
return 'Elements of %r list must be strings' % key
if not isinstance(grantee, basestring):
return "Elements of '%s' list must be strings" % key
# Everything looks fine, no errors found
internal_hdr = get_sys_meta_prefix('account') + 'core-access-control'

30
test/unit/common/middleware/test_tempauth.py
View File

@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2011-2015 OpenStack Foundation
#
# Licensed under the Apache License, Version 2.0 (the "License");
@ -13,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import json
import unittest
from contextlib import contextmanager, nested
from base64 import b64encode
@ -22,7 +24,7 @@ import mock
from swift.common.middleware import tempauth as auth
from swift.common.middleware.acl import format_acl
from swift.common.swob import Request, Response
from swift.common.utils import split_path, get_swift_info
from swift.common.utils import split_path
NO_CONTENT_RESP = (('204 No Content', {}, ''),) # mock server response
@ -111,10 +113,6 @@ class TestAuth(unittest.TestCase):
def setUp(self):
self.test_auth = auth.filter_factory({})(FakeApp())
def test_swift_info(self):
info = get_swift_info()
self.assertTrue(info['tempauth']['account_acls'])
def _make_request(self, path, **kwargs):
req = Request.blank(path, **kwargs)
req.environ['swift.cache'] = FakeMemcache()
@ -1200,7 +1198,8 @@ class TestAccountAcls(unittest.TestCase):
user_groups = test_auth._get_user_groups('admin', 'admin:user',
'AUTH_admin')
good_headers = {'X-Auth-Token': 'AUTH_t'}
good_acl = '{"read-only":["a","b"]}'
good_acl = json.dumps({"read-only": [u"á", "b"]})
bad_list_types = '{"read-only": ["a", 99]}'
bad_acl = 'syntactically invalid acl -- this does not parse as JSON'
wrong_acl = '{"other-auth-system":["valid","json","but","wrong"]}'
bad_value_acl = '{"read-write":["fine"],"admin":"should be a list"}'
@ -1220,7 +1219,9 @@ class TestAccountAcls(unittest.TestCase):
req = self._make_request(target, user_groups=user_groups,
headers=dict(good_headers, **update))
resp = req.get_response(test_auth)
self.assertEquals(resp.status_int, 204)
self.assertEquals(resp.status_int, 204,
'Expected 204, got %s, response body: %s'
% (resp.status_int, resp.body))
# syntactically valid empty acls should go through
for acl in empty_acls:
@ -1243,14 +1244,25 @@ class TestAccountAcls(unittest.TestCase):
req = self._make_request(target, headers=dict(good_headers, **update))
resp = req.get_response(test_auth)
self.assertEquals(resp.status_int, 400)
self.assertEquals(errmsg % "Key '", resp.body[:39])
self.assertTrue(resp.body.startswith(
errmsg % "Key 'other-auth-system' not recognized"), resp.body)
# acls with good keys but bad values also get a 400
update = {'x-account-access-control': bad_value_acl}
req = self._make_request(target, headers=dict(good_headers, **update))
resp = req.get_response(test_auth)
self.assertEquals(resp.status_int, 400)
self.assertEquals(errmsg % "Value", resp.body[:39])
self.assertTrue(resp.body.startswith(
errmsg % "Value for key 'admin' must be a list"), resp.body)
# acls with non-string-types in list also get a 400
update = {'x-account-access-control': bad_list_types}
req = self._make_request(target, headers=dict(good_headers, **update))
resp = req.get_response(test_auth)
self.assertEquals(resp.status_int, 400)
self.assertTrue(resp.body.startswith(
errmsg % "Elements of 'read-only' list must be strings"),
resp.body)
# acls with wrong json structure also get a 400
update = {'x-account-access-control': not_dict_acl}

60
test/unit/common/test_bufferedhttp.py
View File

@ -1,4 +1,5 @@
# Copyright (c) 2010-2012 OpenStack Foundation
# -*- coding: utf-8 -*-
# Copyright (c) 2010-2012 OpenStack Foundation
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -12,6 +13,7 @@
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import mock
import unittest
@ -22,6 +24,24 @@ from eventlet import spawn, Timeout, listen
from swift.common import bufferedhttp
class MockHTTPSConnection(object):
def __init__(self, hostport):
pass
def putrequest(self, method, path, skip_host=0):
self.path = path
pass
def putheader(self, header, *values):
# Verify that path and values can be safely joined
# Essentially what Python 2.7 does that caused us problems.
'\r\n\t'.join((self.path,) + values)
def endheaders(self):
pass
class TestBufferedHTTP(unittest.TestCase):
def test_http_connect(self):
@ -76,22 +96,6 @@ class TestBufferedHTTP(unittest.TestCase):
raise Exception(err)
def test_nonstr_header_values(self):
class MockHTTPSConnection(object):
def __init__(self, hostport):
pass
def putrequest(self, method, path, skip_host=0):
pass
def putheader(self, header, *values):
# Essentially what Python 2.7 does that caused us problems.
'\r\n\t'.join(values)
def endheaders(self):
pass
origHTTPSConnection = bufferedhttp.HTTPSConnection
bufferedhttp.HTTPSConnection = MockHTTPSConnection
try:
@ -106,6 +110,28 @@ class TestBufferedHTTP(unittest.TestCase):
finally:
bufferedhttp.HTTPSConnection = origHTTPSConnection
def test_unicode_values(self):
# simplejson may decode the ring devices as str or unicode
# depending on whether speedups is installed and/or the values are
# non-ascii. Verify all types are tolerated in combination with
# whatever type path might be and possible encoded non-ascii in
# a header value.
with mock.patch('swift.common.bufferedhttp.HTTPSConnection',
MockHTTPSConnection):
for dev in ('sda', u'sda', u'sdá', u'sdá'.encode('utf-8')):
for path in (
'/v1/a', u'/v1/a', u'/v1/á', u'/v1/á'.encode('utf-8')):
for header in ('abc', u'abc', u'ábc'.encode('utf-8')):
try:
bufferedhttp.http_connect(
'127.0.0.1', 8080, dev, 1, 'GET', path,
headers={'X-Container-Meta-Whatever': header},
ssl=True)
except Exception as e:
self.fail(
'Exception %r for device=%r path=%r header=%r'
% (e, dev, path, header))
if __name__ == '__main__':
unittest.main()