diff options
author | Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io> | 2018-01-12 11:39:37 +0100 |
---|---|---|
committer | Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io> | 2018-01-15 15:44:33 +0000 |
commit | 1eb4ea0a6122d8e05d72b26e988f806da04261f7 (patch) | |
tree | 63e2e5ae79586c1ae753e069fd3fe2e4402753f4 /sources/shiboken2/libshiboken/basewrapper.cpp | |
parent | 4e024076fe9562b939d7936b6bf22c25507b2120 (diff) |
Prevent infinite loop due to bad __getitem__ impl
Looking at PYSIDE-441 there was an issue regarding overloading
the __getitem__ method on a class that inherits from QObject.
The bug report showed that setting an object of the same
class to be the parent of another object of the same class
ended up causing an infinite loop when trying to get the parent
element.
Overloading __getitem__ implicitly converts the class
into an iterator, and the developer *must* include a proper
implementation of the method which raises a StopIteration exception
when needed.
Commonly, people that overload this method included access
to class data structures where in most of the cases
an IndexError is raised which forces the iteration
to stop.
Since the bug report did not include this code
and also there was no access to any internal variable,
no exception was raised and ended up causing an infinite loop.
This can be replicated in python as folows:
class A(object):
def __getitem__(self, arg=None):
print("getitem called:", arg)
#raise StopIteration
a = A()
print(list(a))
This small fix avoids the infinite loop when the method __len__
is not implemented (length = -1) or when the length of the pyObj
is zero.
Without a proper implementation of __getitem__ (Raising IndexError
or StopIteration) the infinite loop will happen.
If __len__ is not implemented, then the application
will complain, but does not matter since this is never
checked during the iteration.
Change-Id: I74e7bf1755c265dbc309bb6c5a760f11643fd7ed
Task-number: PYSIDE-441
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Reviewed-by: Christian Tismer <tismer@stackless.com>
Diffstat (limited to 'sources/shiboken2/libshiboken/basewrapper.cpp')
-rw-r--r-- | sources/shiboken2/libshiboken/basewrapper.cpp | 16 |
1 files changed, 9 insertions, 7 deletions
diff --git a/sources/shiboken2/libshiboken/basewrapper.cpp b/sources/shiboken2/libshiboken/basewrapper.cpp index fc208e520..d697d0732 100644 --- a/sources/shiboken2/libshiboken/basewrapper.cpp +++ b/sources/shiboken2/libshiboken/basewrapper.cpp @@ -838,13 +838,15 @@ Py_hash_t hash(PyObject* pyObj) static void setSequenceOwnership(PyObject* pyObj, bool owner) { if (PySequence_Check(pyObj)) { - std::list<SbkObject*> objs = splitPyObject(pyObj); - std::list<SbkObject*>::const_iterator it = objs.begin(); - for(; it != objs.end(); ++it) { - if (owner) - getOwnership(*it); - else - releaseOwnership(*it); + Py_ssize_t size = PySequence_Size(pyObj); + if (size > 0) { + std::list<SbkObject*> objs = splitPyObject(pyObj); + for (auto it = objs.begin(), end = objs.end(); it != end; ++it) { + if (owner) + getOwnership(*it); + else + releaseOwnership(*it); + } } } else if (Object::checkType(pyObj)) { if (owner) |