Commit b2670748 authored by Vincent Pelletier's avatar Vincent Pelletier Committed by Your Name

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.
parent 3b3c65b2
...@@ -16,7 +16,9 @@ ...@@ -16,7 +16,9 @@
""" """
from copy import deepcopy from copy import deepcopy
from collections import defaultdict
from Acquisition import aq_inner, aq_parent
from AccessControl.Permissions import manage_users as ManageUsers from AccessControl.Permissions import manage_users as ManageUsers
from Products.PluggableAuthService.PluggableAuthService import registerMultiPlugin from Products.PluggableAuthService.PluggableAuthService import registerMultiPlugin
from Products.PluggableAuthService.permissions import ManageGroups from Products.PluggableAuthService.permissions import ManageGroups
...@@ -28,28 +30,27 @@ def mergedLocalRoles(object): ...@@ -28,28 +30,27 @@ def mergedLocalRoles(object):
"""Returns a merging of object and its ancestors' """Returns a merging of object and its ancestors'
__ac_local_roles__.""" __ac_local_roles__."""
# Modified to take into account _getAcquireLocalRoles # Modified to take into account _getAcquireLocalRoles
merged = {} merged = defaultdict(list)
object = getattr(object, 'aq_inner', object) object = aq_inner(object)
while 1: while 1:
if getattr(object, '__ac_local_roles__', None) is not None: local_role_dict = getattr(object, '__ac_local_roles__', None)
roles = object.__ac_local_roles__ or {} if local_role_dict:
if callable(roles): roles = roles() if callable(local_role_dict):
for k, v in roles.iteritems(): local_role_dict = local_role_dict() or {}
merged.setdefault(k, []).extend(v) for k, v in local_role_dict.iteritems():
merged[k] += v
# block acquisition # block acquisition
if getattr(object, '_getAcquireLocalRoles', None) is not None: if not getattr(object, '_getAcquireLocalRoles', lambda: True)():
if not object._getAcquireLocalRoles():
break break
if getattr(object, 'aq_parent', None) is not None: parent = aq_parent(object)
object = object.aq_parent if parent is not None:
object = getattr(object, 'aq_inner', object) object = aq_inner(parent)
continue continue
if getattr(object, 'im_self', None) is not None: self = getattr(object, '__self__', None)
object = object.im_self if self is not None:
object = getattr(object, 'aq_inner', object) object = aq_inner(self)
continue continue
break break
return deepcopy(merged) return deepcopy(merged)
def initialize(context): def initialize(context):
......
...@@ -106,23 +106,23 @@ class Setter(Method): ...@@ -106,23 +106,23 @@ class Setter(Method):
class __roles__: class __roles__:
@staticmethod @staticmethod
def rolesForPermissionOn(ob): def rolesForPermissionOn(ob):
im_self = ob.im_self self = ob.__self__
name = '%s__roles__' % ob.__name__ name = '%s__roles__' % ob.__name__
# Lookup on the class, as getRoles gives priority to ob.__roles__ # Lookup on the class, as getRoles gives priority to ob.__roles__
# over class.ob__roles__, this way we have an opportunity to define # over class.ob__roles__, this way we have an opportunity to define
# security on the class for generated methods. # security on the class for generated methods.
# We explictly call _aq_dynamic to prevent acquiering the attribute # We explictly call _aq_dynamic to prevent acquiering the attribute
# from container # from container
roles = getattr(im_self.__class__, name, im_self) roles = getattr(self.__class__, name, self)
if roles is im_self: if roles is self:
roles = im_self._aq_dynamic(name) roles = self._aq_dynamic(name)
if roles is None: if roles is None:
return rolesForPermissionOn(None, im_self, ('Manager',), return rolesForPermissionOn(None, self, ('Manager',),
'_Modify_portal_content_Permission') '_Modify_portal_content_Permission')
# if roles has an __of__ method, call it explicitly, as the Method # if roles has an __of__ method, call it explicitly, as the Method
# already has an __of__ method that has been already called at this # already has an __of__ method that has been already called at this
# point. # 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 from Products.CMFCore.Expression import Expression
...@@ -189,17 +189,17 @@ class Getter(Method): ...@@ -189,17 +189,17 @@ class Getter(Method):
class __roles__: class __roles__:
@staticmethod @staticmethod
def rolesForPermissionOn(ob): def rolesForPermissionOn(ob):
im_self = ob.im_self self = ob.__self__
name = '%s__roles__' % ob.__name__ name = '%s__roles__' % ob.__name__
# we explictly call _aq_dynamic to prevent acquiering the attribute # we explictly call _aq_dynamic to prevent acquiering the attribute
# from container # from container
roles = getattr(im_self.__class__, name, im_self) roles = getattr(self.__class__, name, self)
if roles is im_self: if roles is self:
roles = im_self._aq_dynamic(name) roles = self._aq_dynamic(name)
if roles is None: if roles is None:
return rolesForPermissionOn(None, im_self, ('Manager',), return rolesForPermissionOn(None, self, ('Manager',),
'_Access_contents_information_Permission') '_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): class Tester(Method):
......
...@@ -12,10 +12,39 @@ ...@@ -12,10 +12,39 @@
# #
############################################################################## ##############################################################################
from __future__ import absolute_import from __future__ import absolute_import
import AccessControl.users
import AccessControl.owner import AccessControl.owner
from AccessControl import SpecialUsers as SU from AccessControl import SpecialUsers as SU
from Acquisition import aq_inContextOf, aq_base
from Products.ERP5Type.TransactionalVariable import getTransactionalVariable 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: # Patch description:
# Original method is called very often: multiple times per restricted python # Original method is called very often: multiple times per restricted python
# script/expression: once per __getattr__ (restricted getattr, actually) call. # script/expression: once per __getattr__ (restricted getattr, actually) call.
......
...@@ -25,7 +25,7 @@ def manage_page_footer(self): ...@@ -25,7 +25,7 @@ def manage_page_footer(self):
# REQUEST['PUBLISHED'] can be the form in the acquisition context of the # 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) # document, or a method bound to the document (after a POST it is a bound method)
published = self.REQUEST.get('PUBLISHED') published = self.REQUEST.get('PUBLISHED')
document = getattr(published, 'im_self', None) # bound method document = getattr(published, '__self__', None) # bound method
if document is None: if document is None:
document = aq_parent(published) document = aq_parent(published)
......
...@@ -19,5 +19,5 @@ class PatchClass(tuple): ...@@ -19,5 +19,5 @@ class PatchClass(tuple):
_, ((cls,),), d = args _, ((cls,),), d = args
for k, v in d.iteritems(): for k, v in d.iteritems():
k == "__module__" or setattr(cls, k, v.im_func 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) else v)
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
""" """
from copy import deepcopy from copy import deepcopy
from collections import defaultdict
from AccessControl.Permissions import manage_users as ManageUsers from AccessControl.Permissions import manage_users as ManageUsers
from Products.PluggableAuthService.PluggableAuthService import registerMultiPlugin from Products.PluggableAuthService.PluggableAuthService import registerMultiPlugin
...@@ -33,28 +34,27 @@ def mergedLocalRoles(object): ...@@ -33,28 +34,27 @@ def mergedLocalRoles(object):
"""Returns a merging of object and its ancestors' """Returns a merging of object and its ancestors'
__ac_local_roles__.""" __ac_local_roles__."""
# Modified to take into account _getAcquireLocalRoles # Modified to take into account _getAcquireLocalRoles
merged = {} merged = defaultdict(list)
object = getattr(object, 'aq_inner', object) object = aq_inner(object)
while 1: while 1:
if getattr(object, '__ac_local_roles__', None) is not None: local_role_dict = getattr(object, '__ac_local_roles__', None)
roles = object.__ac_local_roles__ or {} if local_role_dict:
if callable(roles): roles = roles() if callable(local_role_dict):
for k, v in roles.iteritems(): local_role_dict = local_role_dict() or {}
merged.setdefault(k, []).extend(v) for k, v in local_role_dict.iteritems():
merged[k] += v
# block acquisition # block acquisition
if getattr(object, '_getAcquireLocalRoles', None) is not None: if not getattr(object, '_getAcquireLocalRoles', lambda: True)():
if not object._getAcquireLocalRoles() is not None:
break break
if getattr(object, 'aq_parent', None) is not None: parent = aq_parent(object)
object = object.aq_parent if parent is not None:
object = getattr(object, 'aq_inner', object) object = aq_inner(parent)
continue continue
if getattr(object, 'im_self', None) is not None: self = getattr(object, '__self__', None)
object = object.im_self if self is not None:
object = getattr(object, 'aq_inner', object) object = aq_inner(self)
continue continue
break break
return deepcopy(merged) return deepcopy(merged)
registerMultiPlugin(EGOVUserManager.EGOVUserManager.meta_type) registerMultiPlugin(EGOVUserManager.EGOVUserManager.meta_type)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment