RestrictedPython, TALES Expressions and CMF
(sorry for the cross-post) I'm currently facing an issue that seems to be a result of a bad interaction between CMF, TALES and Restricted Python. The issue currently happens when: 1. A TALES 'Path Expression' in 'Caching Policy Manager' is evaluated 2. The result of evaluating a sub-expression is a Python Script (eg: object/modified where 'modified' is a Python Script) 3. The context as built by CMF doesn't define 'here' What happens in this case is that the call will end up in PageTemplates/ZRPythonExpr.py:call_with_ns, which is reproduced here for your pleasure: def call_with_ns(f, ns, arg=1): td = Rtd() td.this = ns['here'] td._push(ns['request']) td._push(InstanceDict(td.this, td)) td._push(ns) try: if arg==2: return f(None, td) else: return f(td) finally: td._pop(3) Now, given that there has been a decision of deprecating 'here' in favor of 'context', I'm not exactly sure about the fix. CMF seems to create expression contexts in two places off the top of my head: In 'CMFCore/Expression.py' and 'CMFCore/CachingPolicyManager.py'. None of those define 'here' or 'context' but instead just 'object'. In 'Products/PageTemplates/TALES.py', in the 'translate' function, 'here' is also hardcoded, but that should be less of an issue as code reaching into that function *should* have a proper expression context. The question then is, which code is wrong? PageTemplates for relying on 'here' being defined, or CMF for not defining 'here'? I volunteer to provide a fix with tests as soon as someone clarifies which one needs to be fixed. As a reminder, 'actions' as defined by 'portal_actions' tool and anything that derives from 'ActionProviderBase' suffer from the same issue. -- Sidnei da Silva Enfold Systems, LLC. http://enfoldsystems.com
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Sidnei da Silva wrote:
(sorry for the cross-post)
I'm currently facing an issue that seems to be a result of a bad interaction between CMF, TALES and Restricted Python.
The issue currently happens when:
1. A TALES 'Path Expression' in 'Caching Policy Manager' is evaluated 2. The result of evaluating a sub-expression is a Python Script (eg: object/modified where 'modified' is a Python Script) 3. The context as built by CMF doessn't define 'here'
What happens in this case is that the call will end up in PageTemplates/ZRPythonExpr.py:call_with_ns, which is reproduced here for your pleasure:
def call_with_ns(f, ns, arg=1): td = Rtd() td.this = ns['here'] td._push(ns['request']) td._push(InstanceDict(td.this, td)) td._push(ns) try: if arg==2: return f(None, td) else: return f(td) finally: td._pop(3)
'call_with_ns' should be invoked only for objects with either a '__render_with_namespace__' attribute (PythonScripts fit here) or those with 'isDocTemp' true. Its only real purpose is to set up the namespace before calling a DTMLMethod; I don't even understand why PythonScripts have such a method. I'm guessing that you get to this point via CMFCore.Expression's __call__ method, which calls a compiled path expression.
Now, given that there has been a decision of deprecating 'here' in favor of 'context', I'm not exactly sure about the fix.
CMF seems to create expression contexts in two places off the top of my head: In 'CMFCore/Expression.py' and 'CMFCore/CachingPolicyManager.py'. None of those define 'here' or 'context' but instead just 'object'.
In 'Products/PageTemplates/TALES.py', in the 'translate' function, 'here' is also hardcoded, but that should be less of an issue as code reaching into that function *should* have a proper expression context.
'here' is *never* the proper key to expect; I would class that a bug.
The question then is, which code is wrong? PageTemplates for relying on 'here' being defined, or CMF for not defining 'here'? I volunteer to provide a fix with tests as soon as someone clarifies which one needs to be fixed.
The 'call_with_ns' bit is *definitely* the wrong code path for PythonScripts, which should be simple callables (I think). I think PythonScripts get their '__render_with_namespace__' attribute by way of 'Shared.DC.Script.Bindings', which is only in order to support a really *goofy* use case (being able to get at the DTML '_' namespace when it is bound). I would be willing to bet that *nobody* does this who actually needs it. If we *must* go through that path, then the 'call_with_ns' code needs to be hardened two ways: - It should prefer 'context' to 'here' ('here' is deprectated in Zope). - It should be willing to supply 'None' if the 'context'/'here' keys are not present in the namespace.
As a reminder, 'actions' as defined by 'portal_actions' tool and anything that derives from 'ActionProviderBase' suffer from the same issue.
We could work around the issue in the CMF by supplying a (temporary, deprecated) 'here' key in the context we create. The problem with Zope assuming that 'here' or 'context' are mandatory namespace keys is that, in our case, we have multiple candidates for the role, and therefore wish to use unambiguous name. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.5 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDS7bg+gerLs4ltQ4RArczAJ47bJIYNRFb40klCPDJBuuXSgJjgQCgh85v c9al2avb9+rBcqP2GiQIfqw= =uIWu -----END PGP SIGNATURE-----
On Tue, Oct 11, 2005 at 08:58:08AM -0400, Tres Seaver wrote: | 'call_with_ns' should be invoked only for objects with either a | '__render_with_namespace__' attribute (PythonScripts fit here) or those | with 'isDocTemp' true. Its only real purpose is to set up the namespace | before calling a DTMLMethod; I don't even understand why PythonScripts | have such a method. | | I'm guessing that you get to this point via CMFCore.Expression's | __call__ method, which calls a compiled path expression. That is correct. | > Now, given that there has been a decision of deprecating 'here' in | > favor of 'context', I'm not exactly sure about the fix. | > | > CMF seems to create expression contexts in two places off the top of | > my head: In 'CMFCore/Expression.py' and | > 'CMFCore/CachingPolicyManager.py'. None of those define 'here' or | > 'context' but instead just 'object'. | > | > In 'Products/PageTemplates/TALES.py', in the 'translate' function, | > 'here' is also hardcoded, but that should be less of an issue as code | > reaching into that function *should* have a proper expression context. | | 'here' is *never* the proper key to expect; I would class that a bug. Bingo. | > The question then is, which code is wrong? PageTemplates for relying | > on 'here' being defined, or CMF for not defining 'here'? I volunteer | > to provide a fix with tests as soon as someone clarifies which one | > needs to be fixed. | | The 'call_with_ns' bit is *definitely* the wrong code path for | PythonScripts, which should be simple callables (I think). I think | PythonScripts get their '__render_with_namespace__' attribute by way of | 'Shared.DC.Script.Bindings', which is only in order to support a really | *goofy* use case (being able to get at the DTML '_' namespace when it is | bound). I would be willing to bet that *nobody* does this who actually | needs it. | | If we *must* go through that path, then the 'call_with_ns' code needs | to be hardened two ways: | | - It should prefer 'context' to 'here' ('here' is deprectated in | Zope). | | - It should be willing to supply 'None' if the 'context'/'here' keys | are not present in the namespace.
From glancing at the code it looks like this is the right thing to do. I wonder who's the person that gets the last word on that dark corner of Zope. Would that be Fred?
| > As a reminder, 'actions' as defined by 'portal_actions' tool and | > anything that derives from 'ActionProviderBase' suffer from the same | > issue. | | We could work around the issue in the CMF by supplying a (temporary, | deprecated) 'here' key in the context we create. The problem with Zope | assuming that 'here' or 'context' are mandatory namespace keys is that, | in our case, we have multiple candidates for the role, and therefore | wish to use unambiguous name. Yeah, I would like to avoid that one, but considering all Zope versions out there have the issue, we might need to provide the temporary fix for a while, to be deprecated in a couple versions. -- Sidnei da Silva Enfold Systems, LLC. http://enfoldsystems.com
Tres Seaver wrote at 2005-10-11 08:58 -0400:
... 'call_with_ns' should be invoked only for objects with either a '__render_with_namespace__' attribute (PythonScripts fit here) or those with 'isDocTemp' true. Its only real purpose is to set up the namespace before calling a DTMLMethod; I don't even understand why PythonScripts have such a method.
Probably, because they can bind the DTML namespace... -- Dieter
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dieter Maurer wrote:
Tres Seaver wrote at 2005-10-11 08:58 -0400:
... 'call_with_ns' should be invoked only for objects with either a '__render_with_namespace__' attribute (PythonScripts fit here) or those with 'isDocTemp' true. Its only real purpose is to set up the namespace before calling a DTMLMethod; I don't even understand why PythonScripts have such a method.
Probably, because they can bind the DTML namespace...
I knew that they *could* bind it; it just don't understand why anyone would *want* that feature, given the availability of the other, non-ambiguous bindings. I would argue that it is a misfeature, especially given the bug which it surfaces in 'render' / 'call_with_ns'. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDUppZ+gerLs4ltQ4RAhTCAJ4hxAKWvo9BCdDEjqMynsZAu+pHCACfSvoP nXcFWDzofq01ofVW8BpR4UI= =nIvs -----END PGP SIGNATURE-----
Tres Seaver wrote at 2005-10-16 14:22 -0400:
...
Probably, because they can bind the DTML namespace...
I knew that they *could* bind it; it just don't understand why anyone would *want* that feature, given the availability of the other, non-ambiguous bindings.
I know that I used it intensively in the past (for a former employer). Now, I almost dropped DTML and with it the DTML namespace bindung of Python Scripts, although it works as well with the ZPT namespace...
I would argue that it is a misfeature, especially given the bug which it surfaces in 'render' / 'call_with_ns'.
I have seen this several times: When a bug comes to the surface, a feature is reclassified as a misfeature... I do not need this feature (unlike other reclassified things) but maybe, fixing the bug is also a solution?
=nIvs -----END PGP SIGNATURE-----
-- Dieter
participants (3)
-
Dieter Maurer -
Sidnei da Silva -
Tres Seaver