diff --git a/swift/common/middleware/acl.py b/swift/common/middleware/acl.py index 15bb67395c..2aed09dd2c 100644 --- a/swift/common/middleware/acl.py +++ b/swift/common/middleware/acl.py @@ -207,17 +207,21 @@ def parse_acl_v2(data): Parses a version-2 Swift ACL string and returns a dict of ACL info. :param data: string containing the ACL data in JSON format - :returns: A dict containing ACL info, e.g.: + :returns: A dict (possibly empty) containing ACL info, e.g.: {"groups": [...], "referrers": [...]} - :returns: None if data is None - :returns: empty dictionary if data does not parse as valid JSON + :returns: None if data is None, is not valid JSON or does not parse + as a dict + :returns: empty dictionary if data is an empty string """ if data is None: return None - try: - return json.loads(data) - except ValueError: + if data is '': return {} + try: + result = json.loads(data) + return (result if type(result) is dict else None) + except ValueError: + return None def parse_acl(*args, **kwargs): diff --git a/swift/common/middleware/tempauth.py b/swift/common/middleware/tempauth.py index fea26e1f1d..4f756247b9 100644 --- a/swift/common/middleware/tempauth.py +++ b/swift/common/middleware/tempauth.py @@ -324,7 +324,7 @@ class TempAuth(object): acl_header = 'x-account-access-control' acl_data = req.headers.get(acl_header) result = parse_acl(version=2, data=acl_data) - if (not result and acl_data not in ('', '{}')): + if result is None: return 'Syntax error in input (%r)' % acl_data tempauth_acl_keys = 'admin read-write read-only'.split() diff --git a/test/unit/common/middleware/test_acl.py b/test/unit/common/middleware/test_acl.py index 14ab6b4eb4..e74044d405 100644 --- a/test/unit/common/middleware/test_acl.py +++ b/test/unit/common/middleware/test_acl.py @@ -90,6 +90,17 @@ class TestACL(unittest.TestCase): # No header "hdr" exists -- should return None ({}, None), ({'junk': 'junk'}, None), + + # Empty ACLs should return empty dict + ({'hdr': ''}, {}), + ({'hdr': '{}'}, {}), + ({'hdr': '{ }'}, {}), + + # Bad input -- should return None + ({'hdr': '["array"]'}, None), + ({'hdr': 'null'}, None), + ({'hdr': '"some_string"'}, None), + ({'hdr': '123'}, None), ] for hdrs_in, expected in tests: diff --git a/test/unit/common/middleware/test_tempauth.py b/test/unit/common/middleware/test_tempauth.py index 2f1b718fb2..414f5035d1 100644 --- a/test/unit/common/middleware/test_tempauth.py +++ b/test/unit/common/middleware/test_tempauth.py @@ -1016,13 +1016,16 @@ class TestAccountAcls(unittest.TestCase): def test_acl_syntax_verification(self): test_auth = auth.filter_factory({'user_admin_user': 'testing'})( - FakeApp(iter(NO_CONTENT_RESP * 3))) + FakeApp(iter(NO_CONTENT_RESP * 5))) good_headers = {'X-Auth-Token': 'AUTH_t'} good_acl = '{"read-only":["a","b"]}' 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"}' + not_dict_acl = '["read-only"]' + not_dict_acl2 = 1 + empty_acls = ['{}', '', '{ }'] target = '/v1/AUTH_firstacct' # no acls -- no problem! @@ -1036,6 +1039,14 @@ class TestAccountAcls(unittest.TestCase): resp = req.get_response(test_auth) self.assertEquals(resp.status_int, 204) + # syntactically valid empty acls should go through + for acl in empty_acls: + update = {'x-account-access-control': acl} + req = self._make_request(target, + headers=dict(good_headers, **update)) + resp = req.get_response(test_auth) + self.assertEquals(resp.status_int, 204) + errmsg = 'X-Account-Access-Control invalid: %s' # syntactically invalid acls get a 400 update = {'x-account-access-control': bad_acl} @@ -1058,6 +1069,20 @@ class TestAccountAcls(unittest.TestCase): self.assertEquals(resp.status_int, 400) self.assertEquals(errmsg % "Value", resp.body[:39]) + # acls with wrong json structure also get a 400 + update = {'x-account-access-control': not_dict_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 % "Syntax error", resp.body[:46]) + + # acls with wrong json structure also get a 400 + update = {'x-account-access-control': not_dict_acl2} + 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 % "Syntax error", resp.body[:46]) + def test_acls_propagate_to_sysmeta(self): test_auth = auth.filter_factory({'user_admin_user': 'testing'})( FakeApp(iter(NO_CONTENT_RESP * 3)))