Raise ValueError if non-text name passed to adapter registry methods.75-prevent-non-text-name-errors

Prevents corruption of lookup cache.

Alternative to fix proposed in #75.

5 files changed, 84 insertions, 4 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index ab99ed3..38f7134 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,7 +4,8 @@ Changes
 4.4.1 (unreleased)
 ------------------
 
-- TBD
+- Raise ``ValueError`` when non-text names are passed to adapter registry
+  methods:  prevents corruption of lookup caches.
 
 4.4.0 (2017-04-21)
 ------------------
diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c
index 94a6c67..a524945 100644
--- a/src/zope/interface/_zope_interface_coptimizations.c
+++ b/src/zope/interface/_zope_interface_coptimizations.c
@@ -884,6 +884,16 @@ _lookup(lookup *self,
 {
   PyObject *result, *key, *cache;
 
+#ifdef PY3K
+  if ( name && !PyUnicode_Check(name) )
+#else
+  if ( name && !PyString_Check(name) && !PyUnicode_Check(name) )
+#endif
+  {
+    PyErr_SetString(PyExc_ValueError,
+                    "name is not a string or unicode");
+    return NULL;
+  }
   cache = _getcache(self, provided, name);
   if (cache == NULL)
     return NULL;
@@ -965,6 +975,17 @@ _lookup1(lookup *self,
 {
   PyObject *result, *cache;
 
+#ifdef PY3K
+  if ( name && !PyUnicode_Check(name) )
+#else
+  if ( name && !PyString_Check(name) && !PyUnicode_Check(name) )
+#endif
+  {
+    PyErr_SetString(PyExc_ValueError,
+                    "name is not a string or unicode");
+    return NULL;
+  }
+
   cache = _getcache(self, provided, name);
   if (cache == NULL)
     return NULL;
@@ -1028,6 +1049,17 @@ _adapter_hook(lookup *self,
 {
   PyObject *required, *factory, *result;
 
+#ifdef PY3K
+  if ( name && !PyUnicode_Check(name) )
+#else
+  if ( name && !PyString_Check(name) && !PyUnicode_Check(name) )
+#endif
+  {
+    PyErr_SetString(PyExc_ValueError,
+                    "name is not a string or unicode");
+    return NULL;
+  }
+
   required = providedBy(NULL, object);
   if (required == NULL)
     return NULL;
diff --git a/src/zope/interface/adapter.py b/src/zope/interface/adapter.py
index f8fdb00..df37f42 100644
--- a/src/zope/interface/adapter.py
+++ b/src/zope/interface/adapter.py
@@ -22,6 +22,7 @@ from zope.interface import ro
 from zope.interface.interfaces import IAdapterRegistry
 
 from zope.interface._compat import _normalize_name
+from zope.interface._compat import STRING_TYPES
 
 _BLANK = u''
 
@@ -102,6 +103,8 @@ class BaseAdapterRegistry(object):
         self._v_lookup.changed(originally_changed)
 
     def register(self, required, provided, name, value):
+        if not isinstance(name, STRING_TYPES):
+            raise ValueError('name is not a string')
         if value is None:
             self.unregister(required, provided, name, value)
             return
@@ -321,6 +324,8 @@ class LookupBaseFallback(object):
         return cache
 
     def lookup(self, required, provided, name=_BLANK, default=None):
+        if not isinstance(name, STRING_TYPES):
+            raise ValueError('name is not a string')
         cache = self._getcache(provided, name)
         required = tuple(required)
         if len(required) == 1:
@@ -341,6 +346,8 @@ class LookupBaseFallback(object):
         return result
 
     def lookup1(self, required, provided, name=_BLANK, default=None):
+        if not isinstance(name, STRING_TYPES):
+            raise ValueError('name is not a string')
         cache = self._getcache(provided, name)
         result = cache.get(required, _not_in_mapping)
         if result is _not_in_mapping:
@@ -355,6 +362,8 @@ class LookupBaseFallback(object):
         return self.adapter_hook(provided, object, name, default)
 
     def adapter_hook(self, provided, object, name=_BLANK, default=None):
+        if not isinstance(name, STRING_TYPES):
+            raise ValueError('name is not a string')
         required = providedBy(object)
         cache = self._getcache(provided, name)
         factory = cache.get(required, _not_in_mapping)
diff --git a/src/zope/interface/interfaces.py b/src/zope/interface/interfaces.py
index 0523e93..860a52e 100644
--- a/src/zope/interface/interfaces.py
+++ b/src/zope/interface/interfaces.py
@@ -676,12 +676,14 @@ class IAdapterRegistry(Interface):
         """Register a value
 
         A value is registered for a *sequence* of required specifications, a
-        provided interface, and a name.
+        provided interface, and a name, which must be text.
         """
 
     def registered(required, provided, name=_BLANK):
         """Return the component registered for the given interfaces and name
 
+        name must be text.
+
         Unlike the lookup method, this methods won't retrieve
         components registered for more specific required interfaces or
         less specific provided interfaces.
@@ -695,7 +697,8 @@ class IAdapterRegistry(Interface):
         """Lookup a value
 
         A value is looked up based on a *sequence* of required
-        specifications, a provided interface, and a name.
+        specifications, a provided interface, and a name, which must be
+        text.
         """
 
     def queryMultiAdapter(objects, provided, name=_BLANK, default=None):
@@ -706,7 +709,8 @@ class IAdapterRegistry(Interface):
         """Lookup a value using a single required interface
 
         A value is looked up based on a single required
-        specifications, a provided interface, and a name.
+        specifications, a provided interface, and a name, which must be
+        text.
         """
 
     def queryAdapter(object, provided, name=_BLANK, default=None):
@@ -715,6 +719,8 @@ class IAdapterRegistry(Interface):
 
     def adapter_hook(provided, object, name=_BLANK, default=None):
         """Adapt an object using a registered adapter factory.
+
+        name must be text.
         """
 
     def lookupAll(required, provided):
diff --git a/src/zope/interface/tests/test_adapter.py b/src/zope/interface/tests/test_adapter.py
index 68074f9..fe1ca95 100644
--- a/src/zope/interface/tests/test_adapter.py
+++ b/src/zope/interface/tests/test_adapter.py
@@ -91,6 +91,12 @@ class BaseAdapterRegistryTests(unittest.TestCase):
         self.assertEqual(len(registry._adapters), 2) #order 0 and order 1
         self.assertEqual(registry._generation, 2)
 
+    def test_register_with_invalid_name(self):
+        IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces()
+        registry = self._makeOne()
+        with self.assertRaises(ValueError):
+            registry.register([IB0], IR0, object(), 'A1')
+
     def test_register_with_value_None_unregisters(self):
         IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces()
         registry = self._makeOne()
@@ -253,6 +259,16 @@ class LookupBaseFallbackTests(unittest.TestCase):
             _uncached_subscriptions = uc_subscriptions
         return Derived()
 
+    def test_lookup_w_invalid_name(self):
+        _called_with = []
+        def _lookup(self, required, provided, name):
+            _called_with.append((required, provided, name))
+            return None
+        lb = self._makeOne(uc_lookup=_lookup)
+        with self.assertRaises(ValueError):
+            lb.lookup(('A',), 'B', object())
+        self.assertEqual(_called_with, [])
+
     def test_lookup_miss_no_default(self):
         _called_with = []
         def _lookup(self, required, provided, name):
@@ -344,6 +360,16 @@ class LookupBaseFallbackTests(unittest.TestCase):
                          [(('A',), 'B', 'C'), (('A',), 'B', 'C')])
         self.assertEqual(_results, [c])
 
+    def test_lookup1_w_invalid_name(self):
+        _called_with = []
+        def _lookup(self, required, provided, name):
+            _called_with.append((required, provided, name))
+            return None
+        lb = self._makeOne(uc_lookup=_lookup)
+        with self.assertRaises(ValueError):
+            lb.lookup1('A', 'B', object())
+        self.assertEqual(_called_with, [])
+
     def test_lookup1_miss_no_default(self):
         _called_with = []
         def _lookup(self, required, provided, name):
@@ -421,6 +447,12 @@ class LookupBaseFallbackTests(unittest.TestCase):
                          [(('A',), 'B', 'C'), (('A',), 'B', 'C')])
         self.assertEqual(_results, [c])
 
+    def test_adapter_hook_w_invalid_name(self):
+        req, prv = object(), object()
+        lb = self._makeOne()
+        with self.assertRaises(ValueError):
+            lb.adapter_hook(prv, req, object())
+
     def test_adapter_hook_miss_no_default(self):
         req, prv = object(), object()
         lb = self._makeOne()