Merge "Fix invalid account acl generating 500 response."

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: None if data is None, is not valid JSON or does not parse
-    :returns: empty dictionary if data does not parse as valid JSON
+        as a dict
+    :returns: empty dictionary if data is an empty string
     """
     if data is None:
         return None
-    try:
+    if data is '':
-        return json.loads(data)
-    except ValueError:
         return {}
+    try:
+        result = json.loads(data)
+        return (result if type(result) is dict else None)
+    except ValueError:
+        return None
 
 
 def parse_acl(*args, **kwargs):

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()

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:

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)))