From 41cea9c7eab4288d0d3bd93e6a71abe0cad1ddde Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Wed, 27 Sep 2017 13:48:06 +0300 Subject: [PATCH 1/6] init commit --- Modules/_collectionsmodule.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index e78399ddefa43c..5d11355db06f81 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -514,6 +514,7 @@ deque_inplace_concat(dequeobject *deque, PyObject *other) static PyObject * deque_copy(PyObject *deque) { + PyObject *result; dequeobject *old_deque = (dequeobject *)deque; if (Py_TYPE(deque) == &deque_type) { dequeobject *new_deque; @@ -537,12 +538,24 @@ deque_copy(PyObject *deque) Py_DECREF(new_deque); return NULL; } - if (old_deque->maxlen < 0) - return PyObject_CallFunctionObjArgs((PyObject *)(Py_TYPE(deque)), - deque, NULL); - else - return PyObject_CallFunction((PyObject *)(Py_TYPE(deque)), "Oi", - deque, old_deque->maxlen, NULL); + if (old_deque->maxlen < 0) { + result = PyObject_CallFunctionObjArgs((PyObject *)(Py_TYPE(deque)), + deque, NULL); + } + else { + result = PyObject_CallFunction((PyObject *)(Py_TYPE(deque)), "Oi", + deque, old_deque->maxlen, NULL); + } + if (result != NULL && + !PyObject_IsInstance(result, (PyObject *)&deque_type)) + { + PyErr_Format(PyExc_TypeError, + "%.200s.__new__() must return a deque, not %.200s", + Py_TYPE(deque)->tp_name, Py_TYPE(result)->tp_name); + Py_DECREF(result); + return NULL; + } + return result; } PyDoc_STRVAR(copy_doc, "Return a shallow copy of a deque."); From 2f34cb3131cdbd161d75b8152d2b2aba747482f5 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sat, 14 Oct 2017 13:40:14 +0300 Subject: [PATCH 2/6] switched to use PyObject_TypeCheck, and fixed the error message. --- Modules/_collectionsmodule.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 5afca62426fdf2..4cd44240a42930 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -546,11 +546,9 @@ deque_copy(PyObject *deque) result = PyObject_CallFunction((PyObject *)(Py_TYPE(deque)), "Oi", deque, old_deque->maxlen, NULL); } - if (result != NULL && - !PyObject_IsInstance(result, (PyObject *)&deque_type)) - { + if (result != NULL && !PyObject_TypeCheck(result, &deque_type)) { PyErr_Format(PyExc_TypeError, - "%.200s.__new__() must return a deque, not %.200s", + "%.200s() must return a deque, not %.200s", Py_TYPE(deque)->tp_name, Py_TYPE(result)->tp_name); Py_DECREF(result); return NULL; From 07bcc04cb74eb38f0c06f06547d7ef4023b3b3cd Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 29 Oct 2017 10:43:51 +0200 Subject: [PATCH 3/6] * minimized the changes to the C code. * added a test. * added a NEWS.d item. --- Lib/test/test_deque.py | 13 +++++++++++++ .../2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst | 2 ++ Modules/_collectionsmodule.c | 6 ++---- 3 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst diff --git a/Lib/test/test_deque.py b/Lib/test/test_deque.py index e895c3c9f0df90..6199dbd2032563 100644 --- a/Lib/test/test_deque.py +++ b/Lib/test/test_deque.py @@ -892,6 +892,19 @@ def __iter__(self): d1 == d2 # not clear if this is supposed to be True or False, # but it used to give a SystemError + @support.cpython_only + def test_bug_31608(self): + # The interpreter used to crash in specific cases where a deque + # subclass returned a non-deque. + class X(deque): + pass + d = X() + def bad___new__(cls, *args, **kwargs): + return 42 + X.__new__ = bad___new__ + with self.assertRaises(TypeError): + d * 42 # shouldn't crash + class SubclassWithKwargs(deque): def __init__(self, newarg=1): diff --git a/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst b/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst new file mode 100644 index 00000000000000..1963c6a88f6a1f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst @@ -0,0 +1,2 @@ +Raise a ``TypeError`` instead of crashing, in specific cases where a +``collections.deque`` subclass returns a non-deque. Patch by Oren Milman. diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 4cd44240a42930..72d3ec56054278 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -538,14 +538,12 @@ deque_copy(PyObject *deque) Py_DECREF(new_deque); return NULL; } - if (old_deque->maxlen < 0) { + if (old_deque->maxlen < 0) result = PyObject_CallFunctionObjArgs((PyObject *)(Py_TYPE(deque)), deque, NULL); - } - else { + else result = PyObject_CallFunction((PyObject *)(Py_TYPE(deque)), "Oi", deque, old_deque->maxlen, NULL); - } if (result != NULL && !PyObject_TypeCheck(result, &deque_type)) { PyErr_Format(PyExc_TypeError, "%.200s() must return a deque, not %.200s", From 2a690e8f209a2595e04cb916e8fd2498c81d8a54 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 29 Oct 2017 15:56:13 +0200 Subject: [PATCH 4/6] make the test more realistic, and also test concatenation. --- Lib/test/test_deque.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_deque.py b/Lib/test/test_deque.py index 6199dbd2032563..921136069d7722 100644 --- a/Lib/test/test_deque.py +++ b/Lib/test/test_deque.py @@ -900,10 +900,12 @@ class X(deque): pass d = X() def bad___new__(cls, *args, **kwargs): - return 42 + return [42] X.__new__ = bad___new__ with self.assertRaises(TypeError): d * 42 # shouldn't crash + with self.assertRaises(TypeError): + d + deque([1, 2, 3]) # shouldn't crash class SubclassWithKwargs(deque): From 5c56c76bd763dea96e6c670c5bd1e951b191d778 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 11 Sep 2018 11:24:21 -0700 Subject: [PATCH 5/6] improve NEWS --- .../next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst b/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst index 1963c6a88f6a1f..d673c0b7ef9636 100644 --- a/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst +++ b/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst @@ -1,2 +1,2 @@ -Raise a ``TypeError`` instead of crashing, in specific cases where a -``collections.deque`` subclass returns a non-deque. Patch by Oren Milman. +Raise a ``TypeError`` instead of crashing if a``collections.deque`` subclass +returns a non-deque type from ``__new__``. Patch by Oren Milman. From ce1cb63c9db8fcc1bfdc0768df00cd73d9b1fde2 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 11 Sep 2018 11:31:31 -0700 Subject: [PATCH 6/6] add space --- .../next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst b/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst index d673c0b7ef9636..d657a8697361bb 100644 --- a/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst +++ b/Misc/NEWS.d/next/Library/2017-10-29-10-37-55.bpo-31608.wkp8Nw.rst @@ -1,2 +1,2 @@ -Raise a ``TypeError`` instead of crashing if a``collections.deque`` subclass -returns a non-deque type from ``__new__``. Patch by Oren Milman. +Raise a ``TypeError`` instead of crashing if a ``collections.deque`` subclass +returns a non-deque from ``__new__``. Patch by Oren Milman.