Commit 5efafc89 authored by Jason Madden's avatar Jason Madden Committed by GitHub

Merge pull request #141 from zopefoundation/issue140

Fix #140 by turning OverflowError into TypeError.
parents 65d23952 5f9ce957
......@@ -82,9 +82,11 @@ longlong_handle_overflow(PY_LONG_LONG result, int overflow)
{
if (overflow)
{
/* Python 3 tends to have an exception already set, Python 2 not so much */
if (!PyErr_Occurred())
PyErr_SetString(PyExc_OverflowError, "couldn't convert integer to C long long");
PyErr_Clear();
/* Python 3 tends to have an exception already set, Python 2 not so
much. We want to consistently raise a TypeError.
*/
PyErr_SetString(PyExc_TypeError, "couldn't convert integer to C long long");
return 0;
}
else if (result == -1 && PyErr_Occurred())
......@@ -106,7 +108,7 @@ ulonglong_check(PyObject *ob)
long tmp;
tmp = PyInt_AS_LONG(ob);
if (tmp < 0) {
PyErr_SetString(PyExc_OverflowError, "unsigned value less than 0");
PyErr_SetString(PyExc_TypeError, "unsigned value less than 0");
return 0;
}
return 1;
......@@ -166,7 +168,7 @@ ulonglong_convert(PyObject *ob, unsigned PY_LONG_LONG *value)
long tmp;
tmp = PyInt_AS_LONG(ob);
if (tmp < 0) {
PyErr_SetString(PyExc_OverflowError, "unsigned value less than 0");
PyErr_SetString(PyExc_TypeError, "unsigned value less than 0");
return 0;
}
(*value) = (unsigned PY_LONG_LONG)tmp;
......@@ -182,7 +184,14 @@ ulonglong_convert(PyObject *ob, unsigned PY_LONG_LONG *value)
val = PyLong_AsUnsignedLongLong(ob);
if (val == (unsigned long long)-1 && PyErr_Occurred())
{
if (PyErr_ExceptionMatches(PyExc_OverflowError))
{
PyErr_Clear();
PyErr_SetString(PyExc_TypeError, "overflow error converting int to C long long");
}
return 0;
}
(*value) = val;
return 1;
}
......
......@@ -1971,12 +1971,6 @@ BTree_getm(BTree *self, PyObject *args)
return d;
}
static PyObject *
BTree_has_key(BTree *self, PyObject *key)
{
return _BTree_get(self, key, 1, _BGET_REPLACE_TYPE_ERROR);
}
static PyObject *
BTree_setdefault(BTree *self, PyObject *args)
{
......@@ -2081,6 +2075,21 @@ BTree_contains(BTree *self, PyObject *key)
return result;
}
static PyObject *
BTree_has_key(BTree *self, PyObject *key)
{
int result = -1;
result = BTree_contains(self, key);
if (result == -1) {
return NULL;
}
if (result)
Py_RETURN_TRUE;
Py_RETURN_FALSE;
}
static PyObject *
BTree_addUnique(BTree *self, PyObject *args)
{
......
......@@ -1367,11 +1367,6 @@ bucket_setstate(Bucket *self, PyObject *state)
return Py_None;
}
static PyObject *
bucket_has_key(Bucket *self, PyObject *key)
{
return _bucket_get(self, key, 1);
}
static PyObject *
bucket_setdefault(Bucket *self, PyObject *args)
......@@ -1475,6 +1470,20 @@ bucket_contains(Bucket *self, PyObject *key)
return result;
}
static PyObject *
bucket_has_key(Bucket *self, PyObject *key)
{
int result = -1;
result = bucket_contains(self, key);
if (result == -1) {
return NULL;
}
if (result)
Py_RETURN_TRUE;
Py_RETURN_FALSE;
}
/*
** bucket_getm
**
......
......@@ -55,7 +55,7 @@ class DataType(object):
"""
Convert *item* into the correct format and return it.
If this cannot be done, raise an appropriate exception.
If this cannot be done, raise a :exc:`TypeError`.
"""
raise NotImplementedError
......@@ -239,7 +239,7 @@ class _AbstractNativeDataType(KeyDataType):
# PyPy can raise ValueError converting a negative number to a
# unsigned value.
if isinstance(item, int_types):
raise OverflowError("Value out of range", item)
raise TypeError("Value out of range", item)
raise TypeError(self._error_description)
return self._as_python_type(item)
......
......@@ -23,9 +23,15 @@
#define COPY_KEY_FROM_ARG(TARGET, ARG, STATUS) \
if (INT_CHECK(ARG)) { \
long vcopy = INT_AS_LONG(ARG); \
if (PyErr_Occurred()) { (STATUS)=0; (TARGET)=0; } \
if (PyErr_Occurred()) { \
if (PyErr_ExceptionMatches(PyExc_OverflowError)) { \
PyErr_Clear(); \
PyErr_SetString(PyExc_TypeError, "integer out of range"); \
} \
(STATUS)=0; (TARGET)=0; \
} \
else if ((int)vcopy != vcopy) { \
PyErr_SetString(PyExc_OverflowError, "integer out of range"); \
PyErr_SetString(PyExc_TypeError, "integer out of range"); \
(STATUS)=0; (TARGET)=0; \
} \
else TARGET = vcopy; \
......@@ -55,13 +61,19 @@
#define COPY_KEY_FROM_ARG(TARGET, ARG, STATUS) \
if (INT_CHECK(ARG)) { \
long vcopy = INT_AS_LONG(ARG); \
if (PyErr_Occurred()) { (STATUS)=0; (TARGET)=0; } \
if (PyErr_Occurred()) { \
if (PyErr_ExceptionMatches(PyExc_OverflowError)) { \
PyErr_Clear(); \
PyErr_SetString(PyExc_TypeError, "integer out of range"); \
} \
(STATUS)=0; (TARGET)=0; \
} \
else if (vcopy < 0) { \
PyErr_SetString(PyExc_OverflowError, "can't convert negative value to unsigned int"); \
PyErr_SetString(PyExc_TypeError, "can't convert negative value to unsigned int"); \
(STATUS)=0; (TARGET)=0; \
} \
else if ((unsigned int)vcopy != vcopy) { \
PyErr_SetString(PyExc_OverflowError, "integer out of range"); \
PyErr_SetString(PyExc_TypeError, "integer out of range"); \
(STATUS)=0; (TARGET)=0; \
} \
else TARGET = vcopy; \
......
......@@ -29,9 +29,15 @@
#define COPY_VALUE_FROM_ARG(TARGET, ARG, STATUS) \
if (INT_CHECK(ARG)) { \
long vcopy = INT_AS_LONG(ARG); \
if (PyErr_Occurred()) { (STATUS)=0; (TARGET)=0; } \
if (PyErr_Occurred()) { \
if (PyErr_ExceptionMatches(PyExc_OverflowError)) { \
PyErr_Clear(); \
PyErr_SetString(PyExc_TypeError, "integer out of range"); \
} \
(STATUS)=0; (TARGET)=0; \
} \
else if ((int)vcopy != vcopy) { \
PyErr_SetString(PyExc_OverflowError, "integer out of range"); \
PyErr_SetString(PyExc_TypeError, "integer out of range"); \
(STATUS)=0; (TARGET)=0; \
} \
else TARGET = vcopy; \
......@@ -62,13 +68,19 @@
#define COPY_VALUE_FROM_ARG(TARGET, ARG, STATUS) \
if (INT_CHECK(ARG)) { \
long vcopy = INT_AS_LONG(ARG); \
if (PyErr_Occurred()) { (STATUS)=0; (TARGET)=0; } \
if (PyErr_Occurred()) { \
if (PyErr_ExceptionMatches(PyExc_OverflowError)) { \
PyErr_Clear(); \
PyErr_SetString(PyExc_TypeError, "integer out of range"); \
} \
(STATUS)=0; (TARGET)=0; \
} \
else if (vcopy < 0) { \
PyErr_SetString(PyExc_OverflowError, "can't convert negative value to unsigned int"); \
PyErr_SetString(PyExc_TypeError, "can't convert negative value to unsigned int"); \
(STATUS)=0; (TARGET)=0; \
} \
} \
else if ((unsigned int)vcopy != vcopy) { \
PyErr_SetString(PyExc_OverflowError, "integer out of range"); \
PyErr_SetString(PyExc_TypeError, "integer out of range"); \
(STATUS)=0; (TARGET)=0; \
} \
else TARGET = vcopy; \
......
......@@ -390,7 +390,7 @@ class Base(SignedMixin):
self.assertFalse(issubclass(NonSub, type(t)))
self.assertFalse(isinstance(NonSub(), type(t)))
class MappingBase(Base):
class MappingBase(Base): # pylint:disable=too-many-public-methods
# Tests common to mappings (buckets, btrees)
SUPPORTS_NEGATIVE_VALUES = True
......@@ -449,6 +449,19 @@ class MappingBase(Base):
self.assertEqual(self._makeOne().get(1), None)
self.assertEqual(self._makeOne().get(1, 'foo'), 'foo')
def testGetReturnsDefaultWrongTypes(self):
self.assertIsNone(self._makeOne().get('abc'))
self.assertEqual(self._makeOne().get('abc', 'def'), 'def')
def testGetReturnsDefaultOverflowRanges(self):
too_big = 2 ** 64 + 1
self.assertIsNone(self._makeOne().get(too_big))
self.assertEqual(self._makeOne().get(too_big, 'def'), 'def')
too_small = -too_big
self.assertIsNone(self._makeOne().get(too_small))
self.assertEqual(self._makeOne().get(too_small, 'def'), 'def')
def testSetItemGetItemWorks(self):
t = self._makeOne()
t[1] = 1
......@@ -475,14 +488,24 @@ class MappingBase(Base):
self.assertEqual(len(t), len(addl), len(t))
def testHasKeyWorks(self):
from .._compat import PY2
t = self._makeOne()
t[1] = 1
if PY2:
self.assertTrue(t.has_key(1))
self.assertTrue(1 in t)
self.assertTrue(0 not in t)
self.assertTrue(2 not in t)
self.assertTrue(t.has_key(1))
self.assertIn(1, t)
self.assertNotIn(0, t)
self.assertNotIn(2, t)
def testHasKeyOverflowAndTypes(self):
t = self._makeOne()
too_big = 2 ** 64 + 1
too_small = -too_big
self.assertNotIn(too_big, t)
self.assertNotIn(too_small, t)
self.assertFalse(t.has_key(too_big))
self.assertFalse(t.has_key(too_small))
self.assertFalse(t.has_key('abc'))
def testValuesWorks(self):
t = self._makeOne()
......@@ -699,7 +722,7 @@ class MappingBase(Base):
self.assertEqual(list(keys), [])
self.assertEqual(list(t.iterkeys(max=50, min=200)), [])
def testSlicing(self):
def testSlicing(self): # pylint:disable=too-many-locals
# Test that slicing of .keys()/.values()/.items() works exactly the
# same way as slicing a Python list with the same contents.
# This tests fixes to several bugs in this area, starting with
......@@ -809,7 +832,7 @@ class MappingBase(Base):
self.assertEqual(list(t.iteritems()), list(t.items()))
@uses_negative_keys_and_values
def testRangedIterators(self):
def testRangedIterators(self): # pylint:disable=too-many-locals
t = self._makeOne()
for keys in [], [-2], [1, 4], list(range(-170, 2000, 13)):
......@@ -1051,7 +1074,6 @@ class MappingBase(Base):
self.skipTest("Needs bounded key and value")
import struct
from .._compat import PY2
good = set()
b = self._makeOne()
......@@ -1061,10 +1083,11 @@ class MappingBase(Base):
# for values that are in range on most other platforms. And on Python 2,
# PyInt_Check can fail with a TypeError starting at small values
# like 2147483648. So we look for small longs and catch those errors
# even when we think we should be in range.
# even when we think we should be in range. In all cases, our code
# catches the unexpected error (OverflowError) and turns it into TypeError.
long_is_32_bit = struct.calcsize('@l') < 8
in_range_errors = (OverflowError, TypeError) if long_is_32_bit else ()
out_of_range_errors = (OverflowError, TypeError) if long_is_32_bit and PY2 else (OverflowError,)
in_range_errors = TypeError
out_of_range_errors = TypeError
def trial(i):
i = int(i)
......@@ -1971,7 +1994,7 @@ class ModuleTest(object):
key_type = None
value_type = None
def _getModule(self):
pass
raise NotImplementedError
def testNames(self):
names = ['Bucket', 'BTree', 'Set', 'TreeSet']
......@@ -2143,11 +2166,11 @@ class TestLongIntKeys(TestLongIntSupport):
o1, o2 = self.getTwoValues()
t = self._makeOne()
k1 = SMALLEST_POSITIVE_65_BITS if self.SUPPORTS_NEGATIVE_KEYS else 2**64 + 1
with self.assertRaises(OverflowError):
with self.assertRaises(TypeError):
t[k1] = o1
t = self._makeOne()
with self.assertRaises(OverflowError):
with self.assertRaises(TypeError):
t[LARGEST_NEGATIVE_65_BITS] = o1
class TestLongIntValues(TestLongIntSupport):
......@@ -2175,11 +2198,11 @@ class TestLongIntValues(TestLongIntSupport):
k1, k2 = self.getTwoKeys()
t = self._makeOne()
v1 = SMALLEST_POSITIVE_65_BITS if self.SUPPORTS_NEGATIVE_VALUES else 2**64 + 1
with self.assertRaises(OverflowError):
with self.assertRaises(TypeError):
t[k1] = v1
t = self._makeOne()
with self.assertRaises(OverflowError):
with self.assertRaises(TypeError):
t[k1] = LARGEST_NEGATIVE_65_BITS
......@@ -2199,8 +2222,8 @@ class SetResult(object):
super(SetResult, self).setUp()
_skip_if_pure_py_and_py_test(self)
self.Akeys = [1, 3, 5, 6]
self.Bkeys = [ 2, 3, 4, 6, 7]
self.Akeys = [1, 3, 5, 6] # pylint:disable=bad-whitespace
self.Bkeys = [ 2, 3, 4, 6, 7] # pylint:disable=bad-whitespace
self.As = [makeset(self.Akeys) for makeset in self.builders()]
self.Bs = [makeset(self.Bkeys) for makeset in self.builders()]
self.emptys = [makeset() for makeset in self.builders()]
......@@ -2312,7 +2335,7 @@ class SetResult(object):
else:
self.assertEqual(list(C), want)
def testLargerInputs(self):
def testLargerInputs(self): # pylint:disable=too-many-locals
from BTrees.IIBTree import IISet # pylint:disable=no-name-in-module
from random import randint
MAXSIZE = 200
......@@ -2568,6 +2591,7 @@ class ConflictTestBase(SignedMixin, object):
# Tests common to all types: sets, buckets, and BTrees
storage = None
db = None
def setUp(self):
super(ConflictTestBase, self).setUp()
......
......@@ -114,14 +114,14 @@ class DegenerateBTree(unittest.TestCase):
t, keys = self._build_degenerate_tree()
self.assertEqual(len(t), len(keys))
self.assertEqual(list(t.keys()), keys)
# has_key actually returns the depth of a bucket.
self.assertEqual(t.has_key(1), 4)
self.assertEqual(t.has_key(3), 4)
self.assertEqual(t.has_key(5), 6)
self.assertEqual(t.has_key(7), 5)
self.assertEqual(t.has_key(11), 5)
self.assertTrue(t.has_key(1))
self.assertTrue(t.has_key(3))
self.assertTrue(t.has_key(5))
self.assertTrue(t.has_key(7))
self.assertTrue(t.has_key(11))
for i in 0, 2, 4, 6, 8, 9, 10, 12:
self.assertTrue(i not in t)
self.assertNotIn(i, t)
def _checkRanges(self, tree, keys):
self.assertEqual(len(tree), len(keys))
......
......@@ -36,7 +36,7 @@ class TestDatatypes(unittest.TestCase):
pass
def test_to_int_w_overflow(self):
self.assertRaises(OverflowError, to_int, 2**64)
self.assertRaises(TypeError, to_int, 2**64)
def test_to_int_w_invalid(self):
self.assertRaises(TypeError, to_int, ())
......@@ -60,7 +60,7 @@ class TestDatatypes(unittest.TestCase):
pass
def test_to_long_w_overflow(self):
self.assertRaises(OverflowError, to_long, 2**64)
self.assertRaises(TypeError, to_long, 2**64)
def test_to_long_w_invalid(self):
self.assertRaises(TypeError, to_long, ())
......
......@@ -5,7 +5,19 @@
4.7.2 (unreleased)
==================
- Nothing changed yet.
- Fix more cases of C and Python inconsistency. The C implementation
now behaves like the Python implementation when it comes to integer
overflow for the integer keys for ``in``, ``get`` and ``has_key``.
Now they return False, the default value, and False, respectively in
both versions if the tested value would overflow or underflow.
Previously, the C implementation would raise ``OverflowError`` or
``KeyError``, while the Python implementation functioned as
expected. See `issue 140
<https://github.com/zopefoundation/BTrees/issues/140>`_.
.. note::
The unspecified true return values of ``has_key``
have changed.
4.7.1 (2020-03-22)
......
......@@ -168,10 +168,10 @@ exclusive of the range's endpoints.
[1, 2, 3, 4]
>>> [pair for pair in t.iteritems()] # new in ZODB 3.3
[(1, 'red'), (2, 'green'), (3, 'blue'), (4, 'spades')]
>>> t.has_key(4) # returns a true value, but exactly what undefined
2
>>> t.has_key(4) # returns a true value
True
>>> t.has_key(5)
0
False
>>> 4 in t # new in ZODB 3.3
True
>>> 5 in t # new in ZODB 3.3
......@@ -256,10 +256,10 @@ example, lists supply a total ordering, and then
>>> list(s.keys()) # note that the lists are in sorted order
[[1], [2], [3]]
>>> s.has_key([3]) # and [3] is in the set
1
True
>>> L2[0] = 5 # horrible -- the set is insane now
>>> s.has_key([3]) # for example, it's insane this way
0
False
>>> s
OOSet([[1], [5], [3]])
>>>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment