Merge pull request #1520 from sontek/catch_bad_request_method_name

Catch bad `name` and raise a `ValueError`

4 files changed, 92 insertions, 5 deletions

diff --git a/pyramid/config/factories.py b/pyramid/config/factories.py
index 15cfb796f..10678df55 100644
--- a/pyramid/config/factories.py
+++ b/pyramid/config/factories.py
@@ -15,8 +15,10 @@ from pyramid.traversal import DefaultRootFactory
 from pyramid.util import (
     action_method,
     InstancePropertyMixin,
+    get_callable_name,
     )
 
+
 class FactoriesConfiguratorMixin(object):
     @action_method
     def set_root_factory(self, factory):
@@ -33,9 +35,10 @@ class FactoriesConfiguratorMixin(object):
         factory = self.maybe_dotted(factory)
         if factory is None:
             factory = DefaultRootFactory
+
         def register():
             self.registry.registerUtility(factory, IRootFactory)
-            self.registry.registerUtility(factory, IDefaultRootFactory) # b/c
+            self.registry.registerUtility(factory, IDefaultRootFactory)  # b/c
 
         intr = self.introspectable('root factories',
                                    None,
@@ -44,7 +47,7 @@ class FactoriesConfiguratorMixin(object):
         intr['factory'] = factory
         self.action(IRootFactory, register, introspectables=(intr,))
 
-    _set_root_factory = set_root_factory # bw compat
+    _set_root_factory = set_root_factory  # bw compat
 
     @action_method
     def set_session_factory(self, factory):
@@ -60,6 +63,7 @@ class FactoriesConfiguratorMixin(object):
            achieve the same purpose.
         """
         factory = self.maybe_dotted(factory)
+
         def register():
             self.registry.registerUtility(factory, ISessionFactory)
         intr = self.introspectable('session factory', None,
@@ -89,6 +93,7 @@ class FactoriesConfiguratorMixin(object):
            can be used to achieve the same purpose.
         """
         factory = self.maybe_dotted(factory)
+
         def register():
             self.registry.registerUtility(factory, IRequestFactory)
         intr = self.introspectable('request factory', None,
@@ -173,6 +178,8 @@ class FactoriesConfiguratorMixin(object):
                 callable, name=name, reify=reify)
         elif name is None:
             name = callable.__name__
+        else:
+            name = get_callable_name(name)
 
         def register():
             exts = self.registry.queryUtility(IRequestExtensions)
@@ -224,9 +231,9 @@ class FactoriesConfiguratorMixin(object):
         'set_request_propery() is deprecated as of Pyramid 1.5; use '
         'add_request_method() with the property=True argument instead')
 
+
 @implementer(IRequestExtensions)
 class _RequestExtensions(object):
     def __init__(self):
         self.descriptors = {}
         self.methods = {}
-
diff --git a/pyramid/tests/test_config/test_factories.py b/pyramid/tests/test_config/test_factories.py
index 0bd5336ff..42bb5accc 100644
--- a/pyramid/tests/test_config/test_factories.py
+++ b/pyramid/tests/test_config/test_factories.py
@@ -126,6 +126,23 @@ class TestFactoriesMixin(unittest.TestCase):
         config = self._makeOne(autocommit=True)
         self.assertRaises(AttributeError, config.add_request_method)
 
+    def test_add_request_method_with_text_type_name(self):
+        from pyramid.interfaces import IRequestExtensions
+        from pyramid.compat import text_, PY3
+        from pyramid.exceptions import ConfigurationError
+
+        config = self._makeOne(autocommit=True)
+        def boomshaka(r): pass
+
+        def get_bad_name():
+            if PY3:  # pragma: nocover
+                name = b'La Pe\xc3\xb1a'
+            else:  # pragma: nocover
+                name = text_(b'La Pe\xc3\xb1a', 'utf-8')
+
+            config.add_request_method(boomshaka, name=name)
+
+        self.assertRaises(ConfigurationError, get_bad_name)
 
 class TestDeprecatedFactoriesMixinMethods(unittest.TestCase):
     def setUp(self):
@@ -135,7 +152,7 @@ class TestDeprecatedFactoriesMixinMethods(unittest.TestCase):
     def tearDown(self):
         from zope.deprecation import __show__
         __show__.on()
-        
+
     def _makeOne(self, *arg, **kw):
         from pyramid.config import Configurator
         config = Configurator(*arg, **kw)
diff --git a/pyramid/tests/test_util.py b/pyramid/tests/test_util.py
index ac5ea0683..371cd8703 100644
--- a/pyramid/tests/test_util.py
+++ b/pyramid/tests/test_util.py
@@ -1,9 +1,11 @@
 import unittest
 from pyramid.compat import PY3
 
+
 class Test_InstancePropertyMixin(unittest.TestCase):
     def _makeOne(self):
         cls = self._getTargetClass()
+
         class Foo(cls):
             pass
         return Foo()
@@ -124,6 +126,21 @@ class Test_InstancePropertyMixin(unittest.TestCase):
         self.assertEqual(1, foo.x)
         self.assertEqual(2, foo.y)
 
+    def test__make_property_unicode(self):
+        from pyramid.compat import text_
+        from pyramid.exceptions import ConfigurationError
+
+        cls = self._getTargetClass()
+        if PY3:  # pragma: nocover
+            name = b'La Pe\xc3\xb1a'
+        else:  # pragma: nocover
+            name = text_(b'La Pe\xc3\xb1a', 'utf-8')
+
+        def make_bad_name():
+            cls._make_property(lambda x: 1, name=name, reify=True)
+
+        self.assertRaises(ConfigurationError, make_bad_name)
+
     def test__set_properties_with_dict(self):
         foo = self._makeOne()
         x_name, x_fn = foo._make_property(lambda _: 1, name='x', reify=True)
@@ -619,7 +636,36 @@ class TestActionInfo(unittest.TestCase):
                          "Line 0 of file filename:\n       linerepr  ")
 
 
+class TestCallableName(unittest.TestCase):
+    def test_valid_ascii(self):
+        from pyramid.util import get_callable_name
+        from pyramid.compat import text_, PY3
+
+        if PY3:  # pragma: nocover
+            name = b'hello world'
+        else:  # pragma: nocover
+            name = text_(b'hello world', 'utf-8')
+
+        self.assertEquals(get_callable_name(name), 'hello world')
+
+    def test_invalid_ascii(self):
+        from pyramid.util import get_callable_name
+        from pyramid.compat import text_, PY3
+        from pyramid.exceptions import ConfigurationError
+
+        def get_bad_name():
+            if PY3:  # pragma: nocover
+                name = b'La Pe\xc3\xb1a'
+            else:  # pragma: nocover
+                name = text_(b'La Pe\xc3\xb1a', 'utf-8')
+
+            get_callable_name(name)
+
+        self.assertRaises(ConfigurationError, get_bad_name)
+
+
 def dummyfunc(): pass
 
+
 class Dummy(object):
     pass
diff --git a/pyramid/util.py b/pyramid/util.py
index 18cef4602..7e8535aaf 100644
--- a/pyramid/util.py
+++ b/pyramid/util.py
@@ -22,6 +22,7 @@ from pyramid.compat import (
     string_types,
     text_,
     PY3,
+    native_
     )
 
 from pyramid.interfaces import IActionInfo
@@ -55,7 +56,7 @@ class InstancePropertyMixin(object):
                 raise ValueError('cannot reify a property')
         elif name is not None:
             fn = lambda this: callable(this)
-            fn.__name__ = name
+            fn.__name__ = get_callable_name(name)
             fn.__doc__ = callable.__doc__
         else:
             name = callable.__name__
@@ -112,6 +113,7 @@ class InstancePropertyMixin(object):
         for name, fn in iteritems_(extensions.methods):
             method = fn.__get__(self, self.__class__)
             setattr(self, name, method)
+
         self._set_properties(extensions.descriptors)
 
     def set_property(self, callable, name=None, reify=False):
@@ -550,3 +552,18 @@ def action_method(wrapped):
         functools.update_wrapper(wrapper, wrapped)
     wrapper.__docobj__ = wrapped
     return wrapper
+
+
+def get_callable_name(name):
+    """
+    Verifies that the ``name`` is ascii and will raise a ``ConfigurationError``
+    if it is not.
+    """
+    try:
+        return native_(name, 'ascii')
+    except (UnicodeEncodeError, UnicodeDecodeError):
+        msg = (
+            '`name="%s"` is invalid. `name` must be ascii because it is '
+            'used on __name__ of the method'
+        )
+        raise ConfigurationError(msg % name)