SVN: ZODB/trunk/src/ Passing keys or values outside the range of 32-bit ints on 64-bit

Jim Fulton <[email protected]>
Newsgroups gmane.comp.python.zope.zodb.cvs
Message-ID <20100711142527.3C39E941C9__11332.0132754327$1278858332$gmane$org@cvs.zope.org>
Log message for revision 114600:
  Passing keys or values outside the range of 32-bit ints on 64-bit
  platforms led to undetected overflow errors. Now these cases cause
  Overflow errors to be raised.
  
  https://bugs.launchpad.net/zodb/+bug/143237
  

Changed:
  U   ZODB/trunk/src/BTrees/intkeymacros.h
  U   ZODB/trunk/src/BTrees/intvaluemacros.h
  U   ZODB/trunk/src/BTrees/tests/testBTrees.py
  U   ZODB/trunk/src/CHANGES.txt

-=-
Modified: ZODB/trunk/src/BTrees/intkeymacros.h
===================================================================
--- ZODB/trunk/src/BTrees/intkeymacros.h	2010-07-11 13:14:35 UTC (rev 114599)
+++ ZODB/trunk/src/BTrees/intkeymacros.h	2010-07-11 14:25:26 UTC (rev 114600)
@@ -21,9 +21,16 @@
 #define KEY_TYPE int
 #define KEY_CHECK PyInt_Check
 #define COPY_KEY_TO_OBJECT(O, K) O=PyInt_FromLong(K)
-#define COPY_KEY_FROM_ARG(TARGET, ARG, STATUS) \
-  if (PyInt_Check(ARG)) TARGET=PyInt_AS_LONG(ARG); else { \
-      PyErr_SetString(PyExc_TypeError, "expected integer key"); \
+#define COPY_KEY_FROM_ARG(TARGET, ARG, STATUS)                    \
+  if (PyInt_Check(ARG)) {                                         \
+      long vcopy = PyInt_AS_LONG(ARG);                            \
+      if ((int)vcopy != vcopy) {                                  \
+        PyErr_SetObject(PyExc_OverflowError, ARG);                \
+        (STATUS)=0; (TARGET)=0;                                   \
+      }                                                           \
+      else TARGET = vcopy;                                        \
+  } else {                                                        \
+      PyErr_SetString(PyExc_TypeError, "expected integer key");   \
       (STATUS)=0; (TARGET)=0; }
 #endif
 

Modified: ZODB/trunk/src/BTrees/intvaluemacros.h
===================================================================
--- ZODB/trunk/src/BTrees/intvaluemacros.h	2010-07-11 13:14:35 UTC (rev 114599)
+++ ZODB/trunk/src/BTrees/intvaluemacros.h	2010-07-11 14:25:26 UTC (rev 114600)
@@ -18,11 +18,20 @@
 #else
 #define VALUE_TYPE int
 #define VALUE_PARSE "i"
-#define COPY_VALUE_TO_OBJECT(O, K) O=PyInt_FromLong(K) 
-#define COPY_VALUE_FROM_ARG(TARGET, ARG, STATUS) \
-  if (PyInt_Check(ARG)) TARGET=PyInt_AsLong(ARG); else { \
-      PyErr_SetString(PyExc_TypeError, "expected integer value"); \
-      (STATUS)=0; (TARGET)=0; } 
+#define COPY_VALUE_TO_OBJECT(O, K) O=PyInt_FromLong(K)
+
+#define COPY_VALUE_FROM_ARG(TARGET, ARG, STATUS)                  \
+  if (PyInt_Check(ARG)) {                                         \
+      long vcopy = PyInt_AS_LONG(ARG);                            \
+      if ((int)vcopy != vcopy) {                                  \
+        PyErr_SetObject(PyExc_OverflowError, ARG);                \
+        (STATUS)=0; (TARGET)=0;                                   \
+      }                                                           \
+      else TARGET = vcopy;                                        \
+  } else {                                                        \
+      PyErr_SetString(PyExc_TypeError, "expected integer key");   \
+      (STATUS)=0; (TARGET)=0; }
+
 #endif
 
 #undef VALUE_TYPE_IS_PYOBJECT

Modified: ZODB/trunk/src/BTrees/tests/testBTrees.py
===================================================================
--- ZODB/trunk/src/BTrees/tests/testBTrees.py	2010-07-11 13:14:35 UTC (rev 114599)
+++ ZODB/trunk/src/BTrees/tests/testBTrees.py	2010-07-11 14:25:26 UTC (rev 114600)
@@ -1805,6 +1805,28 @@
 class IIBTreeTest(BTreeTests):
     def setUp(self):
         self.t = IIBTree()
+
+    def testIIBTreeOverflow(self):
+        good = set()
+        b = self.t
+
+        def trial(i):
+            try:
+                b[i] = 0
+            except (OverflowError, TypeError), v:
+                self.assertRaises(v.__class__, b.__setitem__, 0, i)
+            else:
+                good.add(i)
+                b[0] = i
+                self.assertEqual(b[0], i)
+
+        for i in range((1<<31) - 3, (1<<31) + 3):
+            trial(i)
+            trial(-i)
+
+        del b[0]
+        self.assertEqual(sorted(good), sorted(b))
+
 class IFBTreeTest(BTreeTests):
     def setUp(self):
         self.t = IFBTree()
@@ -1934,14 +1956,9 @@
             self.assertRaises(TypeError, s.insert, big)
             self.assertRaises(TypeError, s.insert, BTrees.family32.minint - 1)
         else: # 64 bit Python
-            s.insert(BTrees.family32.maxint + 1)
-            self.assert_(BTrees.family32.maxint + 1 not in list(s))
-            # yeah, it's len of 1 now, and rolled over to the minint...
-            # don't look...don't look...
-            s = IOTreeSet()
-            s.insert(BTrees.family32.minint - 1)
-            self.assert_(BTrees.family32.minint - 1 not in list(s))
-            # similarly, this is a len of 1, rolling over to the maxint...
+            self.assertRaises(OverflowError, s.insert, big)
+            self.assertRaises(OverflowError, s.insert,
+                              BTrees.family32.minint - 1)
         self.check_pickling(BTrees.family32)
 
     def test64(self):

Modified: ZODB/trunk/src/CHANGES.txt
===================================================================
--- ZODB/trunk/src/CHANGES.txt	2010-07-11 13:14:35 UTC (rev 114599)
+++ ZODB/trunk/src/CHANGES.txt	2010-07-11 14:25:26 UTC (rev 114600)
@@ -20,6 +20,12 @@
 - DemoStorages didn't close their changes databases when they were
   created temporarily (not passed to the constructor).
 
+- Passing keys or values outside the range of 32-bit ints on 64-bit
+  platforms led to undetected overflow errors. Now these cases cause
+  Overflow errors to be raised.
+
+  https://bugs.launchpad.net/zodb/+bug/143237
+
 3.10.0b1 (2010-05-18)
 =====================
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.