Commit 6b289998 by Vincent Pelletier

all: Remove references to im_self.

It is superseded by __self__, which (where applicable) prevents
acquisition and getattr-based traversal, improving performance.
Patch AccessControl.users.BasicUser._check_context to extend this change
to zope code (and simplify it in the process).
Also, make __ac_local_roles__ accesses consistent with other places in
our own code as well as in PAS & AccessControl.
1 parent 240a8d26
......@@ -16,7 +16,9 @@
"""
from copy import deepcopy
from collections import defaultdict
from Acquisition import aq_inner, aq_parent
from AccessControl.Permissions import manage_users as ManageUsers
from Products.PluggableAuthService.PluggableAuthService import registerMultiPlugin
from Products.PluggableAuthService.permissions import ManageGroups
......@@ -28,28 +30,27 @@ def mergedLocalRoles(object):
"""Returns a merging of object and its ancestors'
__ac_local_roles__."""
# Modified to take into account _getAcquireLocalRoles
merged = {}
object = getattr(object, 'aq_inner', object)
merged = defaultdict(list)
object = aq_inner(object)
while 1:
if getattr(object, '__ac_local_roles__', None) is not None:
roles = object.__ac_local_roles__ or {}
if callable(roles): roles = roles()
for k, v in roles.iteritems():
merged.setdefault(k, []).extend(v)
local_role_dict = getattr(object, '__ac_local_roles__', None)
if local_role_dict:
if callable(local_role_dict):
local_role_dict = local_role_dict() or {}
for k, v in local_role_dict.iteritems():
merged[k] += v
# block acquisition
if getattr(object, '_getAcquireLocalRoles', None) is not None:
if not object._getAcquireLocalRoles():
break
if getattr(object, 'aq_parent', None) is not None:
object = object.aq_parent
object = getattr(object, 'aq_inner', object)
if not getattr(object, '_getAcquireLocalRoles', lambda: True)():
break
parent = aq_parent(object)
if parent is not None:
object = aq_inner(parent)
continue
if getattr(object, 'im_self', None) is not None:
object = object.im_self
object = getattr(object, 'aq_inner', object)
self = getattr(object, '__self__', None)
if self is not None:
object = aq_inner(self)
continue
break
return deepcopy(merged)
def initialize(context):
......
......@@ -106,23 +106,23 @@ class Setter(Method):
class __roles__:
@staticmethod
def rolesForPermissionOn(ob):
im_self = ob.im_self
self = ob.__self__
name = '%s__roles__' % ob.__name__
# Lookup on the class, as getRoles gives priority to ob.__roles__
# over class.ob__roles__, this way we have an opportunity to define
# security on the class for generated methods.
# We explictly call _aq_dynamic to prevent acquiering the attribute
# from container
roles = getattr(im_self.__class__, name, im_self)
if roles is im_self:
roles = im_self._aq_dynamic(name)
roles = getattr(self.__class__, name, self)
if roles is self:
roles = self._aq_dynamic(name)
if roles is None:
return rolesForPermissionOn(None, im_self, ('Manager',),
return rolesForPermissionOn(None, self, ('Manager',),
'_Modify_portal_content_Permission')
# if roles has an __of__ method, call it explicitly, as the Method
# already has an __of__ method that has been already called at this
# point.
return getattr(roles, '__of__', lambda aq_parent: roles)(im_self)
return getattr(roles, '__of__', lambda aq_parent: roles)(self)
from Products.CMFCore.Expression import Expression
......@@ -189,17 +189,17 @@ class Getter(Method):
class __roles__:
@staticmethod
def rolesForPermissionOn(ob):
im_self = ob.im_self
self = ob.__self__
name = '%s__roles__' % ob.__name__
# we explictly call _aq_dynamic to prevent acquiering the attribute
# from container
roles = getattr(im_self.__class__, name, im_self)
if roles is im_self:
roles = im_self._aq_dynamic(name)
roles = getattr(self.__class__, name, self)
if roles is self:
roles = self._aq_dynamic(name)
if roles is None:
return rolesForPermissionOn(None, im_self, ('Manager',),
return rolesForPermissionOn(None, self, ('Manager',),
'_Access_contents_information_Permission')
return getattr(roles, '__of__', lambda aq_parent: roles)(im_self)
return getattr(roles, '__of__', lambda aq_parent: roles)(self)
class Tester(Method):
......
......@@ -12,11 +12,40 @@
#
##############################################################################
from __future__ import absolute_import
import AccessControl.users
import AccessControl.owner
from AccessControl import SpecialUsers as SU
from Acquisition import aq_inContextOf, aq_base
from Products.ERP5Type.TransactionalVariable import getTransactionalVariable
# Patch description:
# Original _check_context checks whether given "object" is a method by
# accessing im_self on it. If there is none, it will suddenly be trying to
# acquire one (and failing to, which is good). That acquisition costs a lot
# of time compared to this method simplicity.
# Instead, backport part of
# https://github.com/zopefoundation/AccessControl/commit/14db9b87483471b15e442c10bab1400c88b079a5
# which switched to __self__ attribute (and apply further simplifications).
# As this attribute starts with an underscore, acquisition will ignore it,
# avoiding this waste of time.
def _check_context(self, object):
# Check that 'object' exists in the acquisition context of
# the parent of the acl_users object containing this user,
# to prevent "stealing" access through acquisition tricks.
# Return true if in context, false if not or if context
# cannot be determined (object is not wrapped).
context = getattr(
getattr(self, '__parent__', None),
'__parent__',
None,
)
if context is None or object is None:
return 1
return aq_inContextOf(getattr(object, '__self__', object), context, 1)
AccessControl.users.BasicUser._check_context = _check_context
# Patch description:
# Original method is called very often: multiple times per restricted python
# script/expression: once per __getattr__ (restricted getattr, actually) call.
# Each call pulls self's owner information, traverses to relevant user database
......
......@@ -25,7 +25,7 @@ def manage_page_footer(self):
# REQUEST['PUBLISHED'] can be the form in the acquisition context of the
# document, or a method bound to the document (after a POST it is a bound method)
published = self.REQUEST.get('PUBLISHED')
document = getattr(published, 'im_self', None) # bound method
document = getattr(published, '__self__', None) # bound method
if document is None:
document = aq_parent(published)
......
......@@ -19,5 +19,5 @@ class PatchClass(tuple):
_, ((cls,),), d = args
for k, v in d.iteritems():
k == "__module__" or setattr(cls, k, v.im_func
if getattr(v, "im_class", None) is cls and v.im_self is None
if getattr(v, "im_class", None) is cls and v.__self__ is None
else v)
......@@ -18,6 +18,7 @@
"""
from copy import deepcopy
from collections import defaultdict
from AccessControl.Permissions import manage_users as ManageUsers
from Products.PluggableAuthService.PluggableAuthService import registerMultiPlugin
......@@ -33,28 +34,27 @@ def mergedLocalRoles(object):
"""Returns a merging of object and its ancestors'
__ac_local_roles__."""
# Modified to take into account _getAcquireLocalRoles
merged = {}
object = getattr(object, 'aq_inner', object)
merged = defaultdict(list)
object = aq_inner(object)
while 1:
if getattr(object, '__ac_local_roles__', None) is not None:
roles = object.__ac_local_roles__ or {}
if callable(roles): roles = roles()
for k, v in roles.iteritems():
merged.setdefault(k, []).extend(v)
local_role_dict = getattr(object, '__ac_local_roles__', None)
if local_role_dict:
if callable(local_role_dict):
local_role_dict = local_role_dict() or {}
for k, v in local_role_dict.iteritems():
merged[k] += v
# block acquisition
if getattr(object, '_getAcquireLocalRoles', None) is not None:
if not object._getAcquireLocalRoles() is not None:
break
if getattr(object, 'aq_parent', None) is not None:
object = object.aq_parent
object = getattr(object, 'aq_inner', object)
if not getattr(object, '_getAcquireLocalRoles', lambda: True)():
break
parent = aq_parent(object)
if parent is not None:
object = aq_inner(parent)
continue
if getattr(object, 'im_self', None) is not None:
object = object.im_self
object = getattr(object, 'aq_inner', object)
self = getattr(object, '__self__', None)
if self is not None:
object = aq_inner(self)
continue
break
return deepcopy(merged)
registerMultiPlugin(EGOVUserManager.EGOVUserManager.meta_type)
......
Styling with Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!