diff options
author | Jason Madden <jamadden@gmail.com> | 2017-09-12 12:06:06 -0500 |
---|---|---|
committer | Jason Madden <jamadden@gmail.com> | 2017-09-12 12:06:06 -0500 |
commit | 720b67f120316ae7930a001334098e90e4eb23e9 (patch) | |
tree | d6ec69423c53714b6f618091330dbda5bfe6af39 | |
parent | 4d430612493cb22611efcefe4465b1464962d7a0 (diff) | |
download | zope-proxy-issue21.tar.gz |
Fix indexing pure-Python proxies on Py3, and restore __getslice__ on Py2issue21
Fixes #21.
This removes the special cases for lists and tuples in the C code (but
requires an #if Python 2 block). Tests are added (and generalized for
Python 3). I verified that this fixes the issues observed in the
zope.security tests.
-rw-r--r-- | CHANGES.rst | 7 | ||||
-rw-r--r-- | src/zope/proxy/__init__.py | 27 | ||||
-rw-r--r-- | src/zope/proxy/_zope_proxy_proxy.c | 23 | ||||
-rw-r--r-- | src/zope/proxy/tests/test_proxy.py | 113 | ||||
-rw-r--r-- | tox.ini | 12 |
5 files changed, 132 insertions, 50 deletions
diff --git a/CHANGES.rst b/CHANGES.rst index 70ad85b..48f0a61 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,13 @@ Changes - 100% test coverage. +- Fix indexing pure-Python proxies with slices under Python 3, and + restore the use of ``__getslice__`` (if implemented by the target's + type) under Python 2. Previously, pure-Python proxies would fail + with an AttributeError when given a slice on Python 3, and on Python + 2, a custom ``__getslice__`` was ignored. See `issue 21 + <https://github.com/zopefoundation/zope.proxy/issues/21>`_. + 4.2.1 (2017-04-23) ------------------ diff --git a/src/zope/proxy/__init__.py b/src/zope/proxy/__init__.py index 0c142bd..620e565 100644 --- a/src/zope/proxy/__init__.py +++ b/src/zope/proxy/__init__.py @@ -216,22 +216,23 @@ class AbstractPyProxyBase(object): def __len__(self): return len(self._wrapped) + def __getslice__(self, start, stop): + try: + getslice = type(self._wrapped).__getslice__ + except AttributeError: + return self.__getitem__(slice(start, stop)) + return getslice(self._wrapped, start, stop) + def __getitem__(self, key): - if isinstance(key, slice): - if isinstance(self._wrapped, (list, tuple)): - return self._wrapped[key] - start, stop = key.start, key.stop - if start is None: - start = 0 - if start < 0: - start += len(self._wrapped) - if stop is None: - stop = sys.maxint - if stop < 0: - stop += len(self._wrapped) - return operator.getslice(self._wrapped, start, stop) return self._wrapped[key] + def __setslice__(self, start, stop, value): + try: + setslice = type(self._wrapped).__setslice__ + except AttributeError: + return self.__setitem__(slice(start, stop), value) + return setslice(self._wrapped, start, stop, value) + def __setitem__(self, key, value): self._wrapped[key] = value diff --git a/src/zope/proxy/_zope_proxy_proxy.c b/src/zope/proxy/_zope_proxy_proxy.c index ccb563e..d1c4478 100644 --- a/src/zope/proxy/_zope_proxy_proxy.c +++ b/src/zope/proxy/_zope_proxy_proxy.c @@ -705,16 +705,22 @@ wrap_length(PyObject *self) static PyObject * wrap_slice(PyObject *self, Py_ssize_t start, Py_ssize_t end) { + /* + * Note that we have arrived here through PySequence_GetSlice + * once already, which on Python 2 adjusted indices. We can't call + * PySequence_GetSlice again or they will be wrong. So we directly + * call the slice method the type provides. + */ PyObject *obj = Proxy_GET_OBJECT(self); - if (PyList_Check(obj)) { - return PyList_GetSlice(obj, start, end); - } - else if (PyTuple_Check(obj)) { - return PyTuple_GetSlice(obj, start, end); - } - else { - return PySequence_GetSlice(obj, start, end); +#if PY_MAJOR_VERSION < 3 + PySequenceMethods *m; + + m = obj->ob_type->tp_as_sequence; + if (m && m->sq_slice) { + return m->sq_slice(obj, start, end); } +#endif + return PySequence_GetSlice(obj, start, end); } static int @@ -1251,4 +1257,3 @@ MOD_INIT(_zope_proxy_proxy) return MOD_SUCCESS_VAL(m); } - diff --git a/src/zope/proxy/tests/test_proxy.py b/src/zope/proxy/tests/test_proxy.py index 85d844c..c96492d 100644 --- a/src/zope/proxy/tests/test_proxy.py +++ b/src/zope/proxy/tests/test_proxy.py @@ -35,6 +35,11 @@ class ModuleConformanceCase(unittest.TestCase): class PyProxyBaseTestCase(unittest.TestCase): + # Names of special methods + getslice = '__getitem__' if PY3 else '__getslice__' + setslice = '__setitem__' if PY3 else '__setslice__' + + def _getTargetClass(self): from zope.proxy import PyProxyBase return PyProxyBase @@ -338,24 +343,35 @@ class PyProxyBaseTestCase(unittest.TestCase): self.assertEqual(pTuple[-3:], (1, 2)) def test___getitem__w_slice_against_derived_list(self): - # This behavior should be true for all list- and tuple-derived classes. - # It even passes on Py3, even though there is no __getslice__ + data = [1, 2] class DerivedList(list): - def __getslice__(self, start, end, step=None): - raise AssertionError("not called") + def __getslice__(self, start, stop): + return list.__getslice__(self, start, stop) - pList = self._makeOne(DerivedList([1, 2])) - self.assertEqual(pList[-1:], [2]) - self.assertEqual(pList[-2:], [1, 2]) - self.assertEqual(pList[-3:], [1, 2]) + pList = self._makeOne(DerivedList(data)) + + self.assertEqual(pList[-1:], data[-1:]) + self.assertEqual(pList[-2:], data[-2:]) + self.assertEqual(pList[-3:], data[-3:]) - @unittest.skipIf(PY3, "No __getslice__ in Py3") def test___getitem__w_slice_against_class_w_custom___getslice__(self): + import sys + test = self class Slicer(object): def __len__(self): return 2 - def __getslice__(self, start, end, step=None): - return (start, end, step) + + def __getslice__(self, start, end): + return (start, end) + + def __getitem__(self, a_slice): # pragma: no cover + test.assertTrue(PY3) + # On Python 3, we basically just return what the test expects. + # Mostly that's the computed indices (yay!) but there are + # a few special cases. + indices = a_slice.indices(len(self)) + return (indices[0] if a_slice.start != -3 else -1, + indices[-1] if a_slice.stop is not None else sys.maxsize) pSlicer = self._makeOne(Slicer()) self.assertEqual(pSlicer[:1][0], 0) @@ -364,9 +380,36 @@ class PyProxyBaseTestCase(unittest.TestCase): self.assertEqual(pSlicer[:-1][1], 1) self.assertEqual(pSlicer[-1:][0], 1) self.assertEqual(pSlicer[-2:][0], 0) - # Note that for non-lists and non-tuples the slice is computed - # differently - self.assertEqual(pSlicer[-3:][0], 1) + self.assertEqual(pSlicer[-3:], (-1, sys.maxsize)) + + def test___getslice___dne_uses_getitem(self): + class Missing(Exception): + pass + class Get(object): + def __getitem__(self, x): + raise Missing('__getitem__') + + target = Get() + proxy = self._makeOne(target) + with self.assertRaisesRegexp(Missing, + '__getitem__'): + proxy[1:2] + + def test___getslice___error_propagates(self): + test = self + class Missing(Exception): + pass + class Get(object): + def __getitem__(self, x): # pragma: no cover (only py3) + test.assertTrue(PY3) + raise Missing('__getitem__') + def __getslice__(self, start, stop): + raise Missing("__getslice__") + target = Get() + proxy = self._makeOne(target) + with self.assertRaisesRegexp(Missing, + self.getslice): + proxy[1:2] def test___setslice___against_list(self): # Lists have special slicing bahvior for assignment as well. @@ -395,6 +438,34 @@ class PyProxyBaseTestCase(unittest.TestCase): pList[-3:] = [3, 4] self.assertEqual(pList, [3, 4]) + def test___setslice___error_propagates(self): + class Missing(Exception): + pass + class Set(object): + def __setitem__(self, k, v): + raise Missing('__setitem__') # pragma: no cover (only py3) + def __setslice__(self, start, stop, value): + raise Missing("__setslice__") + target = Set() + proxy = self._makeOne(target) + with self.assertRaisesRegexp(Missing, + self.setslice): + proxy[1:2] = 1 + + def test___setslice___dne_uses_setitem(self): + class Missing(Exception): + pass + class Set(object): + def __setitem__(self, k, v): + raise Missing('__setitem__') + + target = Set() + proxy = self._makeOne(target) + with self.assertRaisesRegexp(Missing, + '__setitem__'): + proxy[1:2] = 1 + + def test___iter___w_wrapped_iterable(self): a = [1, 2, 3] b = [] @@ -573,7 +644,7 @@ class PyProxyBaseTestCase(unittest.TestCase): a, b = coerce(x, y) self.assertTrue(isinstance(a, float)) # a was coerced self.assertFalse(a is x) - self.assertEqual(a, float(x)) + self.assertEqual(a, float(x)) self.assertTrue(b is y) x = self._makeOne(1.1) @@ -582,7 +653,7 @@ class PyProxyBaseTestCase(unittest.TestCase): self.assertTrue(a is x) self.assertTrue(isinstance(b, float)) # b was coerced self.assertFalse(b is y) - self.assertEqual(b, float(y)) + self.assertEqual(b, float(y)) x = self._makeOne(1) y = 2 @@ -595,7 +666,7 @@ class PyProxyBaseTestCase(unittest.TestCase): a, b = coerce(x, y) self.assertTrue(isinstance(a, float)) # a was coerced self.assertFalse(a is x) - self.assertEqual(a, float(x)) + self.assertEqual(a, float(x)) self.assertTrue(b is y) x = self._makeOne(1.1) @@ -604,7 +675,7 @@ class PyProxyBaseTestCase(unittest.TestCase): self.assertTrue(a is x) self.assertTrue(isinstance(b, float)) # b was coerced self.assertFalse(b is y) - self.assertEqual(b, float(y)) + self.assertEqual(b,float(y)) x = 1 y = self._makeOne(2) @@ -742,7 +813,7 @@ class PyProxyBaseTestCase(unittest.TestCase): self._check_wrapping_builtin_returns_correct_provided_by(Proxy, builtin_type) # Our new class did not gain an __implemented__ attribute, unless we're # the pure-python version - if hasattr(Proxy, '__implemented__'): + if hasattr(Proxy, '__implemented__'): # pragma: no cover from zope.proxy import PyProxyBase self.assertTrue(self._getTargetClass() is PyProxyBase) @@ -842,14 +913,14 @@ class Test__module(Test_py__module): class Test_py_subclass__module(Test_py__module): def _getTargetClass(self): - class ProxySubclass(super(Test_py_subclass__module,self)._getTargetClass()): + class ProxySubclass(super(Test_py_subclass__module, self)._getTargetClass()): pass return ProxySubclass class Test_subclass__module(Test__module): def _getTargetClass(self): - class ProxySubclass(super(Test_subclass__module,self)._getTargetClass()): + class ProxySubclass(super(Test_subclass__module, self)._getTargetClass()): pass return ProxySubclass @@ -10,17 +10,15 @@ commands = sphinx-build -b doctest -d {envdir}/doctrees docs {envdir}/doctest [testenv:coverage] +usedevelop = true basepython = python2.7 commands = -# The installed version messes up nose's test discovery / coverage reporting -# So, we uninstall that from the environment, and then install the editable -# version, before running nosetests. - pip uninstall -y zope.proxy - pip install -e . - coverage run -m zope.testrunner --test-path=src + coverage run -m zope.testrunner --test-path=src [] + coverage run -a -m sphinx -b doctest -d {envdir}/.cache/doctrees docs {envdir}/.cache/doctest + coverage report --fail-under=100 deps = - .[test] + {[testenv]deps} coverage [testenv:docs] |