Skip to content

Commit

Permalink
Merge pull request #141 from zopefoundation/issue140
Browse files Browse the repository at this point in the history 
Fix #140 by turning OverflowError into TypeError.
  • Loading branch information
@jamadden
jamadden authored Apr 7, 2020
2 parents 65d2395 + 5f9ce95 commit 5efafc8
Show file tree
Hide file tree
Showing 11 changed files with 152 additions and 65 deletions.
19 changes: 14 additions & 5 deletions BTrees/BTreeModuleTemplate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
21 changes: 15 additions & 6 deletions BTrees/BTreeTemplate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down
19 changes: 14 additions & 5 deletions BTrees/BucketTemplate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
**
Expand Down
4 changes: 2 additions & 2 deletions BTrees/_datatypes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def __call__(self, item):
"""
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

Expand Down Expand Up @@ -239,7 +239,7 @@ def __call__(self, item):
# 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)
Expand Down
22 changes: 17 additions & 5 deletions BTrees/intkeymacros.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
Expand Down Expand Up @@ -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; \
Expand Down
24 changes: 18 additions & 6 deletions BTrees/intvaluemacros.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
Expand Down Expand Up @@ -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; \
Expand Down
66 changes: 45 additions & 21 deletions BTrees/tests/common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ class NonSub(object):
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

Expand Down Expand Up @@ -449,6 +449,19 @@ def testGetReturnsDefault(self):
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
Expand All @@ -475,14 +488,24 @@ def testLen(self):
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()
Expand Down Expand Up @@ -699,7 +722,7 @@ def testEmptyRangeSearches(self):
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
Expand Down Expand Up @@ -809,7 +832,7 @@ def testIterators(self):
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)):
Expand Down Expand Up @@ -1051,7 +1074,6 @@ def testKeyAndValueOverflow(self):
self.skipTest("Needs bounded key and value")

import struct
from .._compat import PY2

good = set()
b = self._makeOne()
Expand All @@ -1061,10 +1083,11 @@ def testKeyAndValueOverflow(self):
# 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)
Expand Down Expand Up @@ -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']
Expand Down Expand Up @@ -2143,11 +2166,11 @@ def testLongIntKeysOutOfRange(self):
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):
Expand Down Expand Up @@ -2175,11 +2198,11 @@ def testLongIntValuesOutOfRange(self):
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


Expand All @@ -2199,8 +2222,8 @@ def setUp(self):
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()]
Expand Down Expand Up @@ -2312,7 +2335,7 @@ def testDifference(self):
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
Expand Down Expand Up @@ -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()
Expand Down
Loading

0 comments on commit 5efafc8

Please sign in to comment.