Uses None instead of mutables for function param defaults

Addressing bug 1307878, changes use of mutable lists and dicts as
default arguments and defaults them within the function. Otherwise,
those defaults can be unexpectedly persisted with the function between
invocations and erupt into mass hysteria on the streets.

To my knowledge there aren't known cases of the current use causing
specific issues, but needs addressing (even stylistically) to avoid
problems in the future -- ones that may crop up as extremely subtle or
intermittent bugs...or worse, security vulnerabilities.

In Glance's case there are ACL-related methods using this, so
although I haven't confirmed one way or the other yet, I've marked it
with SecurityImpact so that a more knowledgeable set of eyes can
review it in this context as well.

Closes-Bug: #1307878
SecurityImpact

Change-Id: Ic17c330eff701ff63701c0029b26db7093a1d73d
This commit is contained in:
Brian Cline 2014-04-15 02:10:28 -05:00
parent 2c0d214a18
commit bebe906ee7
9 changed files with 42 additions and 13 deletions

View File

@ -411,8 +411,13 @@ def add_to_backend(context, scheme, image_id, data, size):
raise exception.StoreAddNotSupported
def set_acls(context, location_uri, public=False, read_tenants=[],
write_tenants=[]):
def set_acls(context, location_uri, public=False, read_tenants=None,
write_tenants=None):
if read_tenants is None:
read_tenants = []
if write_tenants is None:
write_tenants = []
loc = location.get_location_from_uri(location_uri)
scheme = get_store_from_location(location_uri)
store = get_store_from_scheme(context, scheme, loc)

View File

@ -150,8 +150,8 @@ class Store(object):
"""
raise NotImplementedError
def set_acls(self, location, public=False, read_tenants=[],
write_tenants=[]):
def set_acls(self, location, public=False, read_tenants=None,
write_tenants=None):
"""
Sets the read and write access control list for an image in the
backend store.

View File

@ -40,7 +40,9 @@ CONFIG_SECTIONS = [
]
def create_context(policy, roles=[]):
def create_context(policy, roles=None):
if roles is None:
roles = []
return glance.context.RequestContext(roles=roles,
policy_enforcer=policy)

View File

@ -33,7 +33,10 @@ from glance.tests import utils as test_utils
class RequestTest(test_utils.BaseTestCase):
def _set_expected_languages(self, all_locales=[], avail_locales=None):
def _set_expected_languages(self, all_locales=None, avail_locales=None):
if all_locales is None:
all_locales = []
# Override localedata.locale_identifiers to return some locales.
def returns_some_locales(*args, **kwargs):
return all_locales

View File

@ -61,7 +61,10 @@ class V2Token(object):
service = catalog[-1]
service['endpoints'] = [self.base_endpoint]
def add_service(self, s_type, region_list=[]):
def add_service(self, s_type, region_list=None):
if region_list is None:
region_list = []
catalog = self.tok['access']['serviceCatalog']
service_type = {"type": s_type, "name": "glance"}
catalog.append(service_type)

View File

@ -754,8 +754,11 @@ class TestStoreAuthV2(TestStoreAuthV1):
class FakeConnection(object):
def __init__(self, authurl, user, key, retries=5, preauthurl=None,
preauthtoken=None, snet=False, starting_backoff=1,
tenant_name=None, os_options={}, auth_version="1",
tenant_name=None, os_options=None, auth_version="1",
insecure=False, ssl_compression=True):
if os_options is None:
os_options = {}
self.authurl = authurl
self.user = user
self.key = key

View File

@ -107,7 +107,12 @@ class FakeStoreAPI(object):
pass
def set_acls(self, context, uri, public=False,
read_tenants=[], write_tenants=[]):
read_tenants=None, write_tenants=None):
if read_tenants is None:
read_tenants = []
if write_tenants is None:
write_tenants = []
self.acls[uri] = {
'public': public,
'read': read_tenants,

View File

@ -3242,7 +3242,9 @@ class TestAPIProtectedProps(base.IsolatedUnitTest):
db_models.unregister_models(db_api.get_engine())
db_models.register_models(db_api.get_engine())
def _create_admin_image(self, props={}):
def _create_admin_image(self, props=None):
if props is None:
props = {}
request = unit_test_utils.get_fake_request(path='/images')
headers = {'x-image-meta-disk-format': 'ami',
'x-image-meta-container-format': 'ami',
@ -3944,7 +3946,9 @@ class TestAPIPropertyQuotas(base.IsolatedUnitTest):
db_models.unregister_models(db_api.get_engine())
db_models.register_models(db_api.get_engine())
def _create_admin_image(self, props={}):
def _create_admin_image(self, props=None):
if props is None:
props = {}
request = unit_test_utils.get_fake_request(path='/images')
headers = {'x-image-meta-disk-format': 'ami',
'x-image-meta-container-format': 'ami',

View File

@ -445,11 +445,13 @@ class RegistryAPIMixIn(object):
created_at=created_at, updated_at=updated_at,
**kwargs)
def get_api_response_ext(self, http_resp, url='/images', headers={},
def get_api_response_ext(self, http_resp, url='/images', headers=None,
body=None, method=None, api=None,
content_type=None):
if api is None:
api = self.api
if headers is None:
headers = {}
req = webob.Request.blank(url)
for k, v in headers.iteritems():
req.headers[k] = v
@ -563,7 +565,9 @@ class HttplibWsgiAdapter(object):
self.app = app
self.req = None
def request(self, method, url, body=None, headers={}):
def request(self, method, url, body=None, headers=None):
if headers is None:
headers = {}
self.req = webob.Request.blank(url, method=method, headers=headers)
self.req.body = body