[Zope-Checkins] CVS: Zope2 - DT_In.py:1.52 DT_String.py:1.46 DT_Util.py:1.80 DT_With.py:1.13 DocumentTemplate.py:1.12 cDocumentTemplate.c:1.38 pDocumentTemplate.py:1.29

shane@digicool.com shane@digicool.com
Thu, 21 Jun 2001 13:45:43 -0400 (EDT)


Update of /cvs-repository/Zope2/lib/python/DocumentTemplate
In directory korak.digicool.com:/tmp/cvs-serv24737/lib/python/DocumentTemplate

Modified Files:
	DT_In.py DT_String.py DT_Util.py DT_With.py 
	DocumentTemplate.py cDocumentTemplate.c pDocumentTemplate.py 
Log Message:
Based on some semi-formal performance tests, read guards turned out to be
slower than the old code.  With this change, we're using simple function
calls again to perform security checks.  But the calling sequence is
intended to be easier to comprehend than the old code.  Now instead of
DT_String.String subclasses having a validate() method attached to them, they
subclass AccessControl.DTML.RestrictedDTML, which provides a guarded_getattr()
method and a guarded_getitem() method.

Note that the functionality of guarded_getattr() used to be implemented
both in C and Python (in cDocumentTemplate and DT_Util), but now it's in
one place, ZopeGuards.py.  Thus it's not only reusable but easy to
optimize.

I ran all the tests and ran the new code through the profiler again.  The
change sped up restricted code a little more than expected, which is
definitely a good thing, but that may indicate that nested scopes
have a hidden speed penalty.

Also, RestrictedPython is now restrictive about printing to targets and
two forms of augmented assignment had to be forbidden.



--- Updated File DT_In.py in package Zope2 --
--- DT_In.py	2001/05/16 19:07:02	1.51
+++ DT_In.py	2001/06/21 17:45:12	1.52
@@ -608,7 +608,7 @@
             else:
                 result = []
                 append=result.append
-                read_guard = md.read_guard
+                guarded_getitem = getattr(md, 'guarded_getitem', None)
                 for index in range(first,end):
                     # preset
                     pkw['previous-sequence']= 0
@@ -638,8 +638,8 @@
         
                     if index==last: pkw['sequence-end']=1
 
-                    if read_guard is not None:
-                        try: client = read_guard(sequence)[index]
+                    if guarded_getitem is not None:
+                        try: client = guarded_getitem(sequence, index)
                         except ValidationError, vv:
                             if (params.has_key('skip_unauthorized') and
                                 params['skip_unauthorized']):
@@ -727,11 +727,11 @@
         try:
                 result = []
                 append=result.append
-                read_guard = md.read_guard
+                guarded_getitem = getattr(md, 'guarded_getitem', None)
                 for index in range(l):
                     if index==last: pkw['sequence-end']=1
-                    if read_guard is not None:
-                        try: client = read_guard(sequence)[index]
+                    if guarded_getitem is not None:
+                        try: client = guarded_getitem(sequence, index)
                         except ValidationError, vv:
                             if (self.args.has_key('skip_unauthorized') and
                                 self.args['skip_unauthorized']):

--- Updated File DT_String.py in package Zope2 --
--- DT_String.py	2001/05/30 15:57:30	1.45
+++ DT_String.py	2001/06/21 17:45:12	1.46
@@ -505,7 +505,8 @@
             if globals: push(globals)
             if mapping:
                 push(mapping)
-            md.read_guard=self.read_guard
+            md.guarded_getattr=self.guarded_getattr
+            md.guarded_getitem=self.guarded_getitem
             if client is not None:
                 if type(client)==type(()):
                     md.this=client[-1]
@@ -550,8 +551,8 @@
             if pushed: md._pop(pushed) # Get rid of circular reference!
             md.level=level # Restore previous level
 
-    read_guard__roles__=()
-    read_guard=None
+    guarded_getattr=None
+    guarded_getitem=None
 
     def __str__(self):
         return self.read()

--- Updated File DT_Util.py in package Zope2 --
--- DT_Util.py	2001/06/18 18:44:45	1.79
+++ DT_Util.py	2001/06/21 17:45:12	1.80
@@ -143,19 +143,26 @@
 _marker = []  # Create a new marker object.
 
 def careful_getattr(md, inst, name, default=_marker):
-    read_guard = md.read_guard
-    if read_guard is not None:
-        inst = read_guard(inst)
-    if default is _marker:
-        return getattr(inst, name)
-    else:
-        return getattr(inst, name, default)
+    get = md.guarded_getattr
+    if get is None:
+        get = getattr
+    try:
+        return get(inst, name)
+    except AttributeError:
+        if default is _marker:
+            raise
+        return default
 
 def careful_hasattr(md, inst, name):
-    read_guard = md.read_guard
-    if read_guard is not None:
-        inst = read_guard(inst)
-    return hasattr(inst, name)
+    get = md.guarded_getattr
+    if get is None:
+        get = getattr
+    try:
+        get(inst, name)
+    except (AttributeError, ValidationError):
+        return 0
+    else:
+        return 1
 
 d['getattr']=careful_getattr
 d['hasattr']=careful_hasattr
@@ -195,12 +202,15 @@
 class Eval(RestrictionCapableEval):
 
     def eval(self, md):
-        guard = getattr(md, 'read_guard', None)
-        if guard is not None:
+        gattr = getattr(md, 'guarded_getattr', None)
+        if gattr is not None:
+            gitem = getattr(md, 'guarded_getitem', None)
             self.prepRestrictedCode()
             code = self.rcode
             d = {'_': md, '_vars': md,
-                 '_read_': guard, '__builtins__': None}
+                 '_getattr_': gattr,
+                 '_getitem_': gitem,
+                 '__builtins__': None}
         else:
             self.prepUnrestrictedCode()
             code = self.ucode

--- Updated File DT_With.py in package Zope2 --
--- DT_With.py	2001/04/27 20:27:39	1.12
+++ DT_With.py	2001/06/21 17:45:12	1.13
@@ -139,8 +139,10 @@
         if self.only:
             _md=md
             md=TemplateDict()
-            if hasattr(_md, 'read_guard'):
-                md.read_guard = _md.read_guard
+            if hasattr(_md, 'guarded_getattr'):
+                md.guarded_getattr = _md.guarded_getattr
+            if hasattr(_md, 'guarded_getitem'):
+                md.guarded_getitem = _md.guarded_getitem
 
         md._push(v)
         try: return render_blocks(self.section, md)

--- Updated File DocumentTemplate.py in package Zope2 --
--- DocumentTemplate.py	2001/04/27 20:27:39	1.11
+++ DocumentTemplate.py	2001/06/21 17:45:12	1.12
@@ -146,14 +146,13 @@
     Document templates provide a basic level of access control by
     preventing access to names beginning with an underscore.
     Additional control may be provided by providing document templates
-    with a 'read_guard' method.  This would typically be done by
-    subclassing one or more of the DocumentTemplate classes.
+    with a 'guarded_getattr' and 'guarded_getitem' method.  This would
+    typically be done by subclassing one or more of the DocumentTemplate
+    classes.
 
-    If provided, the the 'read_guard' method will be called when objects
-    are accessed as accessed as instance attributes or when they are
-    accessed through keyed access in an expression..  The 'read_guard'
-    method will be called with the containing object.  It can
-    return a wrapper object from which the attribute will be accessed.
+    If provided, the the 'guarded_getattr' method will be called when
+    objects are accessed as instance attributes or when they are
+    accessed through keyed access in an expression.
 
 Document Templates may be created 4 ways:
 

--- Updated File cDocumentTemplate.c in package Zope2 --
--- cDocumentTemplate.c	2001/05/23 18:20:03	1.37
+++ cDocumentTemplate.c	2001/06/21 17:45:12	1.38
@@ -92,7 +92,7 @@
 static PyObject *py_isDocTemp=0, *py_blocks=0, *py_=0, *join=0, *py_acquire;
 static PyObject *py___call__, *py___roles__, *py_AUTHENTICATED_USER;
 static PyObject *py_hasRole, *py__proxy_roles, *py_Unauthorized;
-static PyObject *py_Unauthorized_fmt, *py_read_guard;
+static PyObject *py_Unauthorized_fmt, *py_guarded_getattr;
 static PyObject *py__push, *py__pop, *py_aq_base, *py_renderNS;
 
 /* ----------------------------------------------------- */
@@ -108,7 +108,7 @@
   PyObject *inst;
   PyObject *cache;
   PyObject *namespace;
-  PyObject *read_guard;
+  PyObject *guarded_getattr;
 } InstanceDictobject;
 
 staticforward PyExtensionClass InstanceDictType;
@@ -116,18 +116,19 @@
 static PyObject *
 InstanceDict___init__(InstanceDictobject *self, PyObject *args)
 {
-  self->read_guard=NULL;
+  self->guarded_getattr=NULL;
   UNLESS(PyArg_ParseTuple(args, "OO|O",
 			  &(self->inst),
 			  &(self->namespace),
-			  &(self->read_guard)))
+			  &(self->guarded_getattr)))
     return NULL;
   Py_INCREF(self->inst);
   Py_INCREF(self->namespace);
-  if (self->read_guard)
-    Py_INCREF(self->read_guard);
+  if (self->guarded_getattr)
+    Py_INCREF(self->guarded_getattr);
   else
-    UNLESS(self->read_guard=PyObject_GetAttr(self->namespace, py_read_guard))
+    UNLESS(self->guarded_getattr=PyObject_GetAttr(self->namespace,
+                                                  py_guarded_getattr))
        return NULL;
     
   UNLESS(self->cache=PyDict_New()) return NULL;
@@ -150,7 +151,7 @@
   Py_XDECREF(self->inst);
   Py_XDECREF(self->cache);
   Py_XDECREF(self->namespace);
-  Py_XDECREF(self->read_guard);
+  Py_XDECREF(self->guarded_getattr);
   Py_DECREF(self->ob_type);
   PyMem_DEL(self);
 }
@@ -193,16 +194,13 @@
       return PyObject_Str(self->inst);
     }
   
-  if (self->read_guard != Py_None) {
-    r = PyObject_CallFunction(self->read_guard, "O", self->inst);
-    if (!r) return NULL;
+  if (self->guarded_getattr != Py_None) {
+    r = PyObject_CallFunction(self->guarded_getattr, "OO", self->inst, key);
   }
   else {
-    r = self->inst;
-    Py_INCREF(r);
+    r = PyObject_GetAttr(self->inst, key);
   }
 
-  ASSIGN(r, PyObject_GetAttr(r, key));
   if (!r) {
     PyObject *tb;
 
@@ -889,7 +887,7 @@
   UNLESS(py___roles__=PyString_FromString("__roles__")) return;
   UNLESS(py__proxy_roles=PyString_FromString("_proxy_roles")) return;
   UNLESS(py_hasRole=PyString_FromString("hasRole")) return;
-  UNLESS(py_read_guard=PyString_FromString("read_guard")) return;
+  UNLESS(py_guarded_getattr=PyString_FromString("guarded_getattr")) return;
   UNLESS(py__push=PyString_FromString("_push")) return;
   UNLESS(py__pop=PyString_FromString("_pop")) return;
   UNLESS(py_aq_base=PyString_FromString("aq_base")) return;

--- Updated File pDocumentTemplate.py in package Zope2 --
--- pDocumentTemplate.py	2001/04/27 20:27:39	1.28
+++ pDocumentTemplate.py	2001/06/21 17:45:12	1.29
@@ -117,14 +117,15 @@
 
 class InstanceDict:
 
-    read_guard=None
+    guarded_getattr=None
 
-    def __init__(self,o,namespace,read_guard=None):
+    def __init__(self,o,namespace,guarded_getattr=None):
         self.self=o
         self.cache={}
         self.namespace=namespace
-        if read_guard is None: self.read_guard=namespace.read_guard
-        else: self.read_guard=read_guard
+        if guarded_getattr is None:
+            self.guarded_getattr = namespace.guarded_getattr
+        else: self.guarded_getattr = guarded_getattr
 
     def has_key(self,key):
         return hasattr(self.self,key)
@@ -147,11 +148,11 @@
             else:
                 return str(inst)
 
-        read_guard = self.read_guard
-        if read_guard is not None:
-            inst = read_guard(inst)
+        get = self.guarded_getattr
+        if get is None:
+            get = getattr
 
-        try: r = getattr(inst, key)
+        try: r = get(inst, key)
         except AttributeError: raise KeyError, key
 
         self.cache[key]=r