Fix traversal behavior for 4.x (#972)

* - apply Zope master traversal fixes to 4.x * - bump version to 4.6 * - unbreak sphinx * - unbreak version number
zopefoundation · May 21, 2021 · e98d326 · e98d326
1 parent e39cbc1
commit e98d326
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 6 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -7,8 +7,11 @@ The change log for the previous version, Zope 2.13, is at
 https://zope.readthedocs.io/en/2.13/CHANGES.html
 
 
-4.5.6 (unreleased)
-------------------
+4.6 (unreleased)
+----------------
+
+- Prevent traversal to names starting with ``_`` in TAL expressions
+  and fix path expressions for the ``chameleon.tales`` expression engine.
 
 - Provide friendlier ZMI error message for the Transaction Undo form
   (`#964 <https://github.com/zopefoundation/Zope/issues/964>`_)

diff --git a/setup.py b/setup.py
@@ -29,7 +29,7 @@ def _read_file(filename):
 README = _read_file('README.rst')
 CHANGES = _read_file('CHANGES.rst')
 
-version = '4.5.6.dev0'
+version = '4.6.0.dev0'
 
 
 setup(
@@ -130,7 +130,7 @@ def _read_file(filename):
     zip_safe=False,
     extras_require={
         'docs': [
-            'Sphinx',
+            'Sphinx < 4',
             'sphinx_rtd_theme',
             'repoze.sphinx.autointerface',
             'tempstorage',

diff --git a/src/Products/PageTemplates/Expressions.py b/src/Products/PageTemplates/Expressions.py
@@ -17,6 +17,7 @@
 """
 
 import logging
+import warnings
 
 from six import binary_type
 from six import text_type
@@ -77,6 +78,14 @@ def boboAwareZopeTraverse(object, path_items, econtext):
 
     while path_items:
         name = path_items.pop()
+
+        if name == '_':
+            warnings.warn('Traversing to the name `_` is deprecated '
+                          'and will be removed in Zope 6.',
+                          DeprecationWarning)
+        elif name.startswith('_'):
+            raise NotFound(name)
+
         if OFS.interfaces.ITraversable.providedBy(object):
             object = object.restrictedTraverse(name)
         else:

diff --git a/src/Products/PageTemplates/expression.py b/src/Products/PageTemplates/expression.py
@@ -1,5 +1,6 @@
 """``chameleon.tales`` expressions."""
 
+import warnings
 from ast import NodeTransformer
 from ast import parse
 
@@ -62,8 +63,16 @@ def traverse(cls, base, request, path_items):
 
         while path_items:
             name = path_items.pop()
+
+            if name == '_':
+                warnings.warn('Traversing to the name `_` is deprecated '
+                              'and will be removed in Zope 6.',
+                              DeprecationWarning)
+            elif name.startswith('_'):
+                raise NotFound(name)
+
             if ITraversable.providedBy(base):
-                base = getattr(base, cls.traverseMethod)(name)
+                base = getattr(base, cls.traverse_method)(name)
             else:
                 base = traversePathElement(base, name, path_items,
                                            request=request)

diff --git a/src/Products/PageTemplates/tests/input/CheckPathTraverse.html b/src/Products/PageTemplates/tests/input/CheckPathTraverse.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+   <div tal:content="context/laf"></div>
+</body>
+</html>
diff --git a/src/Products/PageTemplates/tests/output/CheckPathTraverse.html b/src/Products/PageTemplates/tests/output/CheckPathTraverse.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+   <div>ok</div>
+</body>
+</html>
diff --git a/src/Products/PageTemplates/tests/testChameleonTalesExpressions.py b/src/Products/PageTemplates/tests/testChameleonTalesExpressions.py
@@ -1,3 +1,5 @@
+import unittest
+
 from ..expression import getEngine
 from . import testHTMLTests
 
@@ -19,3 +21,8 @@ def setUp(self):
     #   expressions (e.g. the ``zope.tales`` ``not`` expression
     #   returns ``int``, that of ``chameleon.tales`` ``bool``
     PREFIX = "CH_"
+
+    @unittest.skip('The test in the base class relies on a Zope context with'
+                   ' the "random" module available in expressions')
+    def test_underscore_traversal(self):
+        pass
diff --git a/src/Products/PageTemplates/tests/testExpressions.py b/src/Products/PageTemplates/tests/testExpressions.py
@@ -1,10 +1,12 @@
 # -*- coding: utf-8 -*-
 
 import unittest
+import warnings
 
 from six import text_type
 
 from AccessControl import safe_builtins
+from zExceptions import NotFound
 from zope.component.testing import PlacelessSetup
 
 
@@ -110,8 +112,12 @@ def test_evaluate_alternative_first_missing(self):
         self.assertTrue(ec.evaluate('x | nothing') is None)
 
     def test_evaluate_dict_key_as_underscore(self):
+        # Traversing to the name `_` will raise a DeprecationWarning
+        # because it will go away in Zope 6.
         ec = self._makeContext()
-        self.assertEqual(ec.evaluate('d/_'), 'under')
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore')
+            self.assertEqual(ec.evaluate('d/_'), 'under')
 
     def test_evaluate_dict_with_key_from_expansion(self):
         ec = self._makeContext()
@@ -224,6 +230,19 @@ def test_list_in_path_expr(self):
         ec = self._makeContext()
         self.assertIs(ec.evaluate('nocall: list'), safe_builtins["list"])
 
+    def test_underscore_traversal(self):
+        # Prevent traversal to names starting with an underscore (_)
+        ec = self._makeContext()
+
+        with self.assertRaises(NotFound):
+            ec.evaluate("context/__class__")
+
+        with self.assertRaises(NotFound):
+            ec.evaluate("nocall: random/_itertools/repeat")
+
+        with self.assertRaises(NotFound):
+            ec.evaluate("random/_itertools/repeat/foobar")
+
 
 class TrustedEngineTests(EngineTestsBase, unittest.TestCase):
 

diff --git a/src/Products/PageTemplates/tests/testHTMLTests.py b/src/Products/PageTemplates/tests/testHTMLTests.py
@@ -27,6 +27,7 @@
     DefaultUnicodeEncodingConflictResolver
 from Products.PageTemplates.unicodeconflictresolver import \
     PreferredCharsetResolver
+from zExceptions import NotFound
 from zope.component import provideUtility
 from zope.traversing.adapters import DefaultTraversable
 
@@ -156,6 +157,15 @@ def testPathNothing(self):
     def testPathAlt(self):
         self.assert_expected(self.folder.t, 'CheckPathAlt.html')
 
+    def testPathTraverse(self):
+        # need to perform this test with a "real" folder
+        from OFS.Folder import Folder
+        f = self.folder
+        self.folder = Folder()
+        self.folder.t, self.folder.laf = f.t, f.laf
+        self.folder.laf.write('ok')
+        self.assert_expected(self.folder.t, 'CheckPathTraverse.html')
+
     def testBatchIteration(self):
         self.assert_expected(self.folder.t, 'CheckBatchIteration.html')
 
@@ -208,3 +218,18 @@ def test_unicode_conflict_resolution(self):
         provideUtility(PreferredCharsetResolver)
         t = PageTemplate()
         self.assert_expected(t, 'UnicodeResolution.html')
+
+    def test_underscore_traversal(self):
+        t = self.folder.t
+
+        t.write('<p tal:define="p context/__class__" />')
+        with self.assertRaises(NotFound):
+            t()
+
+        t.write('<p tal:define="p nocall: random/_itertools/repeat"/>')
+        with self.assertRaises(NotFound):
+            t()
+
+        t.write('<p tal:content="random/_itertools/repeat/foobar"/>')
+        with self.assertRaises(NotFound):
+            t()