[Zodb-checkins] SVN: ZODB/branches/3.10/src/ - BTrees allowed object keys with insane comparison. (Comparison

Jim Fulton jim at zope.com
Tue Oct 26 18:17:25 EDT 2010


Log message for revision 117940:
  - BTrees allowed object keys with insane comparison. (Comparison
    inherited from object, which compares based on in-process address.)
    Now BTrees warn if an attempt is made to save a key with
    comparison inherited from object. (This doesn't apply to old-style
    class instances.) This will become an error in ZODB 3.11.
  

Changed:
  U   ZODB/branches/3.10/src/BTrees/BTreeModuleTemplate.c
  U   ZODB/branches/3.10/src/BTrees/objectkeymacros.h
  U   ZODB/branches/3.10/src/BTrees/tests/testBTrees.py
  U   ZODB/branches/3.10/src/CHANGES.txt
  U   ZODB/branches/3.10/src/ZODB/ConflictResolution.txt

-=-
Modified: ZODB/branches/3.10/src/BTrees/BTreeModuleTemplate.c
===================================================================
--- ZODB/branches/3.10/src/BTrees/BTreeModuleTemplate.c	2010-10-26 21:55:08 UTC (rev 117939)
+++ ZODB/branches/3.10/src/BTrees/BTreeModuleTemplate.c	2010-10-26 22:17:24 UTC (rev 117940)
@@ -465,6 +465,12 @@
 {
     PyObject *m, *d, *c;
 
+#ifdef KEY_TYPE_IS_PYOBJECT
+    object_ = PyTuple_GetItem(Py_None->ob_type->tp_bases, 0);
+    if (object_ == NULL)
+      return;
+#endif
+
     sort_str = PyString_InternFromString("sort");
     if (!sort_str)
 	return;

Modified: ZODB/branches/3.10/src/BTrees/objectkeymacros.h
===================================================================
--- ZODB/branches/3.10/src/BTrees/objectkeymacros.h	2010-10-26 21:55:08 UTC (rev 117939)
+++ ZODB/branches/3.10/src/BTrees/objectkeymacros.h	2010-10-26 22:17:24 UTC (rev 117940)
@@ -1,9 +1,33 @@
 #define KEYMACROS_H "$Id$\n"
 #define KEY_TYPE PyObject *
 #define KEY_TYPE_IS_PYOBJECT
+
+#include "Python.h"
+
+static PyObject *object_;
+
+static int
+check_argument_cmp(PyObject *arg)
+{
+  if (arg->ob_type->tp_richcompare == NULL
+      &&
+      arg->ob_type->tp_compare ==
+      ((PyTypeObject *)object_)->ob_type->tp_compare
+      )
+    {
+      return PyErr_WarnEx(
+         PyExc_UserWarning,
+         "Object has default comparison! This will error in 3.11",
+         1) >= 0;
+    }
+  return 1;
+}
+
 #define TEST_KEY_SET_OR(V, KEY, TARGET) if ( ( (V) = PyObject_Compare((KEY),(TARGET)) ), PyErr_Occurred() )
 #define INCREF_KEY(k) Py_INCREF(k)
 #define DECREF_KEY(KEY) Py_DECREF(KEY)
 #define COPY_KEY(KEY, E) KEY=(E)
 #define COPY_KEY_TO_OBJECT(O, K) O=(K); Py_INCREF(O)
-#define COPY_KEY_FROM_ARG(TARGET, ARG, S) TARGET=(ARG)
+#define COPY_KEY_FROM_ARG(TARGET, ARG, S) \
+    TARGET=(ARG); \
+    (S) = check_argument_cmp(ARG); 

Modified: ZODB/branches/3.10/src/BTrees/tests/testBTrees.py
===================================================================
--- ZODB/branches/3.10/src/BTrees/tests/testBTrees.py	2010-10-26 21:55:08 UTC (rev 117939)
+++ ZODB/branches/3.10/src/BTrees/tests/testBTrees.py	2010-10-26 22:17:24 UTC (rev 117940)
@@ -15,6 +15,7 @@
 import pickle
 import random
 import StringIO
+import sys
 from unittest import TestCase, TestSuite, TextTestRunner, makeSuite
 from types import ClassType
 import zope.interface.verify
@@ -1884,6 +1885,55 @@
     def setUp(self):
         self.t = OOBTree()
 
+    def testRejectDefaultComparison(self):
+        # Check that passing int keys w default comparison fails.
+        # Only applies to new-style class instances. Old-style
+        # instances are too hard to introspect.
+
+        # This is white box because we know that the check is being
+        # used in a function that's used in lots of places.
+        # Otherwise, there are many permutations that would have to be
+        # checked.
+
+        import warnings
+
+        class C(object):
+            pass
+
+        with warnings.catch_warnings(record=True) as w:
+            # Cause all warnings to always be triggered.
+            warnings.simplefilter("always")
+
+            self.t[C()] = 1
+
+            # Verify some things
+            n = len(w) # somewhat unpredictable #
+
+            for m in w:
+                self.assertEqual(m.category, UserWarning)
+                self.assert_("Object has default comparison" in str(m.message))
+
+            self.t.clear()
+
+            class C(object):
+                def __cmp__(*args):
+                    return 1
+
+            self.t[C()] = 1
+            self.t.clear()
+
+            class C(object):
+                def __lt__(*args):
+                    return 1
+
+            self.t[C()] = 1
+            self.t.clear()
+
+            self.assertEqual(len(w), n)
+
+    if sys.version_info[:2] < (2, 6):
+        del testRejectDefaultComparison
+
 if using64bits:
     class IIBTreeTest(BTreeTests, TestLongIntKeys, TestLongIntValues):
         def setUp(self):
@@ -1922,9 +1972,6 @@
         self.t = OLBTree()
     def getTwoKeys(self):
         return object(), object()
-class OOBTreeTest(BTreeTests):
-    def setUp(self):
-        self.t = OOBTree()
 
 
 # cmp error propagation tests

Modified: ZODB/branches/3.10/src/CHANGES.txt
===================================================================
--- ZODB/branches/3.10/src/CHANGES.txt	2010-10-26 21:55:08 UTC (rev 117939)
+++ ZODB/branches/3.10/src/CHANGES.txt	2010-10-26 22:17:24 UTC (rev 117940)
@@ -2,7 +2,7 @@
  Change History
 ================
 
-3.10.1 (2010-10-??)
+3.10.1 (2010-10-27)
 ===================
 
 Bugs Fixed
@@ -28,6 +28,12 @@
   don't send invalidations. There's no reason to send them when an
   external garbage collector is used.
 
+- BTrees allowed object keys with insane comparison. (Comparison
+  inherited from object, which compares based on in-process address.)
+  Now BTrees warn if an attempt is made to save a key with
+  comparison inherited from object. (This doesn't apply to old-style
+  class instances.) This will become an error in ZODB 3.11.
+
 - ZEO client cache simulation misshandled invalidations
   causing incorrect statistics and errors.
 

Modified: ZODB/branches/3.10/src/ZODB/ConflictResolution.txt
===================================================================
--- ZODB/branches/3.10/src/ZODB/ConflictResolution.txt	2010-10-26 21:55:08 UTC (rev 117939)
+++ ZODB/branches/3.10/src/ZODB/ConflictResolution.txt	2010-10-26 22:17:24 UTC (rev 117940)
@@ -65,6 +65,11 @@
             from persistent import Persistent
             class PCounter(Persistent):
                 '`value` is readonly; increment it with `inc`.'
+
+                def __cmp__(self, other):
+                    'Fool BTree checks for sane comparison :/'
+                    return object.__cmp__(self, other)
+
                 _val = 0
                 def inc(self):
                     self._val += 1



More information about the Zodb-checkins mailing list