deque: add bounds check for __getitem__ / __setitem__ / __delitem__

Author: cosminbasca

graalpython/com.oracle.graal.python.test/src/graalpytest.py

@@ -154,22 +154,23 @@ def assertSequenceEqual(self, expected, actual, msg=None):
     class assertRaises():
 
         def __init__(self, exc_type, function=None, *args, **kwargs):
-            if function is None:
+            self.function = function
+            if self.function is None:
                 self.exc_type = exc_type
             else:
                 try:
-                    function(*args, **kwargs)
+                    self.function(*args, **kwargs)
                 except exc_type:
                     pass
                 else:
-                    assert False, "expected '%r' to raise '%r'" % (function, exc_type)
+                    assert False, "expected '%r' to raise '%r'" % (self.function, exc_type)
 
         def __enter__(self):
             return self
 
         def __exit__(self, exc_type, exc, traceback):
             if not exc_type:
-                assert False, "expected '%r' to raise '%r'" % (function, exc_type)
+                assert False, "expected '%r' to raise '%r'" % (self.function, exc_type)
             elif self.exc_type in exc_type.mro():
                 self.exception = exc
                 return True

graalpython/com.oracle.graal.python.test/src/tests/test_deque.py

@@ -37,7 +37,6 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 import copy
-import pickle
 import random
 import unittest
 from collections import deque
@@ -290,7 +289,6 @@ def test_index(self):
 
         # Test start and stop arguments behavior matches list.index()
         elements = 'ABCDEFGHI'
-        nonelement = 'Z'
         d = deque(elements * 2)
         s = list(elements * 2)
         for start in range(-5 - len(s)*2, 5 + len(s) * 2):
@@ -299,8 +297,7 @@ def test_index(self):
                     try:
                         target = s.index(element, start, stop)
                     except ValueError:
-                        with self.assertRaises(ValueError):
-                            d.index(element, start, stop)
+                        self.assertRaises(ValueError, lambda: d.index(element, start, stop))
                     else:
                         self.assertEqual(d.index(element, start, stop), target)
 
@@ -309,31 +306,6 @@ def test_index_bug_24913(self):
         with self.assertRaises(ValueError):
             i = d.index("Hello world", 0, 4)
 
-    def test_insert(self):
-        # Test to make sure insert behaves like lists
-        elements = 'ABCDEFGHI'
-        for i in range(-5 - len(elements)*2, 5 + len(elements) * 2):
-            d = deque('ABCDEFGHI')
-            s = list('ABCDEFGHI')
-            d.insert(i, 'Z')
-            s.insert(i, 'Z')
-            self.assertEqual(list(d), s)
-
-    def test_insert_bug_26194(self):
-        data = 'ABC'
-        d = deque(data, maxlen=len(data))
-        with self.assertRaises(IndexError):
-            d.insert(2, None)
-
-        elements = 'ABCDEFGHI'
-        for i in range(-len(elements), len(elements)):
-            d = deque(elements, maxlen=len(elements)+1)
-            d.insert(i, 'Z')
-            if i >= 0:
-                self.assertEqual(d[i], 'Z')
-            else:
-                self.assertEqual(d[i-1], 'Z')
-
     def test_imul(self):
         for n in (-10, -1, 0, 1, 2, 10, 1000):
             d = deque()
@@ -411,9 +383,9 @@ def test_delitem(self):
             self.assertEqual(len(d), n-i)
             j = random.randrange(-len(d), len(d))
             val = d[j]
-            self.assertIn(val, d)
+            self.assertTrue(val in d)
             del d[j]
-            self.assertNotIn(val, d)
+            self.assertTrue(val not in d)
         self.assertEqual(len(d), 0)
 
     def test_reverse(self):
@@ -614,74 +586,6 @@ def test_roundtrip_iter_init(self):
         self.assertNotEqual(id(d), id(e))
         self.assertEqual(list(d), list(e))
 
-    def test_pickle(self):
-        for d in deque(range(200)), deque(range(200), 100):
-            for i in range(pickle.HIGHEST_PROTOCOL + 1):
-                s = pickle.dumps(d, i)
-                e = pickle.loads(s)
-                self.assertNotEqual(id(e), id(d))
-                self.assertEqual(list(e), list(d))
-                self.assertEqual(e.maxlen, d.maxlen)
-
-    def test_pickle_recursive(self):
-        for d in deque('abc'), deque('abc', 3):
-            d.append(d)
-            for i in range(pickle.HIGHEST_PROTOCOL + 1):
-                e = pickle.loads(pickle.dumps(d, i))
-                self.assertNotEqual(id(e), id(d))
-                self.assertEqual(id(e[-1]), id(e))
-                self.assertEqual(e.maxlen, d.maxlen)
-
-    def test_iterator_pickle(self):
-        orig = deque(range(200))
-        data = [i*1.01 for i in orig]
-        for proto in range(pickle.HIGHEST_PROTOCOL + 1):
-            # initial iterator
-            itorg = iter(orig)
-            dump = pickle.dumps((itorg, orig), proto)
-            it, d = pickle.loads(dump)
-            for i, x in enumerate(data):
-                d[i] = x
-            self.assertEqual(type(it), type(itorg))
-            self.assertEqual(list(it), data)
-
-            # running iterator
-            next(itorg)
-            dump = pickle.dumps((itorg, orig), proto)
-            it, d = pickle.loads(dump)
-            for i, x in enumerate(data):
-                d[i] = x
-            self.assertEqual(type(it), type(itorg))
-            self.assertEqual(list(it), data[1:])
-
-            # empty iterator
-            for i in range(1, len(data)):
-                next(itorg)
-            dump = pickle.dumps((itorg, orig), proto)
-            it, d = pickle.loads(dump)
-            for i, x in enumerate(data):
-                d[i] = x
-            self.assertEqual(type(it), type(itorg))
-            self.assertEqual(list(it), [])
-
-            # exhausted iterator
-            self.assertRaises(StopIteration, next, itorg)
-            dump = pickle.dumps((itorg, orig), proto)
-            it, d = pickle.loads(dump)
-            for i, x in enumerate(data):
-                d[i] = x
-            self.assertEqual(type(it), type(itorg))
-            self.assertEqual(list(it), [])
-
-    def test_deepcopy(self):
-        mut = [10]
-        d = deque([mut])
-        e = copy.deepcopy(d)
-        self.assertEqual(list(d), list(e))
-        mut[0] = 11
-        self.assertNotEqual(id(d), id(e))
-        self.assertNotEqual(list(d), list(e))
-
     def test_copy(self):
         mut = [10]
         d = deque([mut])

graalpython/lib-graalpython/_collections.py

@@ -279,12 +279,14 @@ def remove(self, x):
         """Remove first occurrence of value."""
         block = self.leftblock
         index = self.leftindex
-        lock = self._getlock()
-        for i in range(self.len):
+        n = self.len
+        for i in range(n):
             item = block.data[index]
-            self._checklock(lock)
+            if self.len != n:
+                raise IndexError("deque mutated during remove().")
+
             if item == x:
-                self._delitem(i)
+                self.delitem(i)
                 return
             # Advance the block/index pair
             index += 1
@@ -395,7 +397,7 @@ def __contains__(self, v):
         index = self.leftindex
         b = self.leftblock
 
-        while n >= 0:
+        while (n - 1) >= 0:
             n -= 1
             assert b is not None
             item = b.data[index]
@@ -411,36 +413,45 @@ def __contains__(self, v):
 
         return False
 
-    def index(self, v, start=0, stop=-1):
-        lock = self._getlock()
-        if start < 0:
-            start += self.len
-            if start < 0:
-                start = 0
+    def _norm_index(self, idx, force_index_to_zero=True):
+        if idx < 0:
+            idx += self.len
+            if idx < 0 and force_index_to_zero:
+                idx = 0
+        return idx
 
-        if stop < 0:
-            stop += self.len
-            if stop < 0:
-                stop = 0
+    def _check_index(self, idx):
+        if idx < 0 or idx >= self.len:
+            raise IndexError("deque index out of range")
+
+    def index(self, v, start=0, stop=None):
+        if stop is None:
+            stop = self.len
+
+        lock = self._getlock()
+        start = self._norm_index(start)
+        stop = self._norm_index(stop)
 
         if stop > self.len:
             stop = self.len
 
         if start > stop:
             start = stop
 
+        assert 0 <= start <= stop <= self.len
+
         index = self.leftindex
         b = self.leftblock
-
         i = 0
+
         for i in range(start):
             index += 1
             if index == BLOCKLEN:
                 b = b.rightlink
                 index = 0
 
-        n = stop - i
-        while n >= 0:
+        n = stop - start
+        while (n - 1) >= 0:
             n -= 1
             assert b is not None
             item = b.data[index]
@@ -491,23 +502,29 @@ def _locate(self, i):
         assert i >= 0
         return b, i
 
-    def _delitem(self, i):
+    def delitem(self, i):
         # delitem() implemented in terms of rotate for simplicity and
         # reasonable performance near the end points.
         self.rotate(-i)
         self.popleft()
         self.rotate(i)
 
     def __getitem__(self, idx):
+        idx = self._norm_index(idx)
+        self._check_index(idx)
         b, i = self._locate(idx)
         return b.data[i]
 
     def __setitem__(self, idx, value):
+        idx = self._norm_index(idx, force_index_to_zero=False)
+        self._check_index(idx)
         b, i = self._locate(idx)
         b.data[i] = value
 
     def __delitem__(self, idx):
-        self._delitem(idx)
+        idx = self._norm_index(idx, force_index_to_zero=False)
+        self._check_index(idx)
+        self.delitem(idx)
 
     def copy(self):
         """Return a shallow copy of a deque."""