From fa494fd88384acc52cf9292d0c89e2961c8f747f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 30 May 2015 17:45:22 +0300 Subject: [PATCH] Issue #24115: Update uses of PyObject_IsTrue(), PyObject_Not(), PyObject_IsInstance(), PyObject_RichCompareBool() and _PyDict_Contains() to check for and handle errors correctly. --- Misc/NEWS | 4 ++++ Modules/_json.c | 24 +++++++++++++++++------- Modules/_threadmodule.c | 18 ++++++++++++------ Modules/faulthandler.c | 4 ++-- Objects/bytearrayobject.c | 10 +++++++--- Objects/bytesobject.c | 20 +++++++++++++------- Objects/rangeobject.c | 5 ++++- Objects/setobject.c | 17 +++++++++++++---- Python/codecs.c | 11 ++++++----- Python/import.c | 6 +++++- 10 files changed, 83 insertions(+), 36 deletions(-) diff --git a/Misc/NEWS b/Misc/NEWS index e4e5064a4e6..0a4eadb7089 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,10 @@ Release date: tba Core and Builtins ----------------- +- Issue #24115: Update uses of PyObject_IsTrue(), PyObject_Not(), + PyObject_IsInstance(), PyObject_RichCompareBool() and _PyDict_Contains() + to check for and handle errors correctly. + - Issue #24257: Fixed system error in the comparison of faked types.SimpleNamespace. diff --git a/Modules/_json.c b/Modules/_json.c index f87d680db63..2f42c3459cb 100644 --- a/Modules/_json.c +++ b/Modules/_json.c @@ -595,6 +595,9 @@ _parse_object_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ss int has_pairs_hook = (s->object_pairs_hook != Py_None); Py_ssize_t next_idx; + if (strict < 0) + return NULL; + if (PyUnicode_READY(pystr) == -1) return NULL; @@ -940,6 +943,7 @@ scan_once_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ssize_ void *str; int kind; Py_ssize_t length; + int strict; if (PyUnicode_READY(pystr) == -1) return NULL; @@ -960,9 +964,10 @@ scan_once_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ssize_ switch (PyUnicode_READ(kind, str, idx)) { case '"': /* string */ - return scanstring_unicode(pystr, idx + 1, - PyObject_IsTrue(s->strict), - next_idx_ptr); + strict = PyObject_IsTrue(s->strict); + if (strict < 0) + return NULL; + return scanstring_unicode(pystr, idx + 1, strict, next_idx_ptr); case '{': /* object */ if (Py_EnterRecursiveCall(" while decoding a JSON object " @@ -1212,12 +1217,13 @@ encoder_init(PyObject *self, PyObject *args, PyObject *kwds) PyEncoderObject *s; PyObject *markers, *defaultfn, *encoder, *indent, *key_separator; - PyObject *item_separator, *sort_keys, *skipkeys, *allow_nan; + PyObject *item_separator, *sort_keys, *skipkeys; + int allow_nan; assert(PyEncoder_Check(self)); s = (PyEncoderObject *)self; - if (!PyArg_ParseTupleAndKeywords(args, kwds, "OOOOOOOOO:make_encoder", kwlist, + if (!PyArg_ParseTupleAndKeywords(args, kwds, "OOOOOOOOp:make_encoder", kwlist, &markers, &defaultfn, &encoder, &indent, &key_separator, &item_separator, &sort_keys, &skipkeys, &allow_nan)) return -1; @@ -1231,7 +1237,7 @@ encoder_init(PyObject *self, PyObject *args, PyObject *kwds) s->sort_keys = sort_keys; s->skipkeys = skipkeys; s->fast_encode = (PyCFunction_Check(s->encoder) && PyCFunction_GetFunction(s->encoder) == (PyCFunction)py_encode_basestring_ascii); - s->allow_nan = PyObject_IsTrue(allow_nan); + s->allow_nan = allow_nan; Py_INCREF(s->markers); Py_INCREF(s->defaultfn); @@ -1500,6 +1506,7 @@ encoder_listencode_dict(PyEncoderObject *s, _PyAccu *acc, PyObject *items; PyObject *item = NULL; int skipkeys; + int sortkeys; Py_ssize_t idx; if (open_dict == NULL || close_dict == NULL || empty_dict == NULL) { @@ -1544,13 +1551,16 @@ encoder_listencode_dict(PyEncoderObject *s, _PyAccu *acc, items = PyMapping_Items(dct); if (items == NULL) goto bail; - if (PyObject_IsTrue(s->sort_keys) && PyList_Sort(items) < 0) + sortkeys = PyObject_IsTrue(s->sort_keys); + if (sortkeys < 0 || (sortkeys && PyList_Sort(items) < 0)) goto bail; it = PyObject_GetIter(items); Py_DECREF(items); if (it == NULL) goto bail; skipkeys = PyObject_IsTrue(s->skipkeys); + if (skipkeys < 0) + goto bail; idx = 0; while ((item = PyIter_Next(it)) != NULL) { PyObject *encoded, *key, *value; diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 9925b0e7ec1..f80f76a6380 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -718,12 +718,18 @@ local_new(PyTypeObject *type, PyObject *args, PyObject *kw) "_localdummy_destroyed", (PyCFunction) _localdummy_destroyed, METH_O }; - if (type->tp_init == PyBaseObject_Type.tp_init - && ((args && PyObject_IsTrue(args)) - || (kw && PyObject_IsTrue(kw)))) { - PyErr_SetString(PyExc_TypeError, - "Initialization arguments are not supported"); - return NULL; + if (type->tp_init == PyBaseObject_Type.tp_init) { + int rc = 0; + if (args != NULL) + rc = PyObject_IsTrue(args); + if (rc == 0 && kw != NULL) + rc = PyObject_IsTrue(kw); + if (rc != 0) { + if (rc > 0) + PyErr_SetString(PyExc_TypeError, + "Initialization arguments are not supported"); + return NULL; + } } self = (localobject *)type->tp_alloc(type, 0); diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c index c729414a497..1493f8d8859 100644 --- a/Modules/faulthandler.c +++ b/Modules/faulthandler.c @@ -1087,8 +1087,8 @@ faulthandler_env_options(void) has_key = PyDict_Contains(xoptions, key); Py_DECREF(key); - if (!has_key) - return 0; + if (has_key <= 0) + return has_key; } module = PyImport_ImportModule("faulthandler"); diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 8629ab7772e..2e47a1c47c9 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -974,13 +974,17 @@ bytearray_richcompare(PyObject *self, PyObject *other, int op) Py_buffer self_bytes, other_bytes; PyObject *res; Py_ssize_t minsize; - int cmp; + int cmp, rc; /* Bytes can be compared to anything that supports the (binary) buffer API. Except that a comparison with Unicode is always an error, even if the comparison is for equality. */ - if (PyObject_IsInstance(self, (PyObject*)&PyUnicode_Type) || - PyObject_IsInstance(other, (PyObject*)&PyUnicode_Type)) { + rc = PyObject_IsInstance(self, (PyObject*)&PyUnicode_Type); + if (!rc) + rc = PyObject_IsInstance(other, (PyObject*)&PyUnicode_Type); + if (rc < 0) + return NULL; + if (rc) { if (Py_BytesWarningFlag && (op == Py_EQ || op == Py_NE)) { if (PyErr_WarnEx(PyExc_BytesWarning, "Comparison between bytearray and string", 1)) diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index b9b49acebec..57681548b15 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -816,17 +816,23 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op) Py_ssize_t len_a, len_b; Py_ssize_t min_len; PyObject *result; + int rc; /* Make sure both arguments are strings. */ if (!(PyBytes_Check(a) && PyBytes_Check(b))) { - if (Py_BytesWarningFlag && (op == Py_EQ || op == Py_NE) && - (PyObject_IsInstance((PyObject*)a, - (PyObject*)&PyUnicode_Type) || - PyObject_IsInstance((PyObject*)b, - (PyObject*)&PyUnicode_Type))) { - if (PyErr_WarnEx(PyExc_BytesWarning, - "Comparison between bytes and string", 1)) + if (Py_BytesWarningFlag && (op == Py_EQ || op == Py_NE)) { + rc = PyObject_IsInstance((PyObject*)a, + (PyObject*)&PyUnicode_Type); + if (!rc) + rc = PyObject_IsInstance((PyObject*)b, + (PyObject*)&PyUnicode_Type); + if (rc < 0) return NULL; + if (rc) { + if (PyErr_WarnEx(PyExc_BytesWarning, + "Comparison between bytes and string", 1)) + return NULL; + } } result = Py_NotImplemented; } diff --git a/Objects/rangeobject.c b/Objects/rangeobject.c index ca66a45d5fe..c4ba715018a 100644 --- a/Objects/rangeobject.c +++ b/Objects/rangeobject.c @@ -194,8 +194,11 @@ compute_range_length(PyObject *start, PyObject *stop, PyObject *step) } /* if (lo >= hi), return length of 0. */ - if (PyObject_RichCompareBool(lo, hi, Py_GE) == 1) { + cmp_result = PyObject_RichCompareBool(lo, hi, Py_GE); + if (cmp_result != 0) { Py_XDECREF(step); + if (cmp_result < 0) + return NULL; return PyLong_FromLong(0); } diff --git a/Objects/setobject.c b/Objects/setobject.c index 34e43b92de0..304519c5d47 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1569,9 +1569,15 @@ set_difference(PySetObject *so, PyObject *other) if (PyDict_CheckExact(other)) { while (set_next(so, &pos, &entry)) { setentry entrycopy; + int rv; entrycopy.hash = entry->hash; entrycopy.key = entry->key; - if (!_PyDict_Contains(other, entry->key, entry->hash)) { + rv = _PyDict_Contains(other, entry->key, entry->hash); + if (rv < 0) { + Py_DECREF(result); + return NULL; + } + if (!rv) { if (set_add_entry((PySetObject *)result, &entrycopy) == -1) { Py_DECREF(result); return NULL; @@ -1807,7 +1813,8 @@ PyDoc_STRVAR(issuperset_doc, "Report whether this set contains another set."); static PyObject * set_richcompare(PySetObject *v, PyObject *w, int op) { - PyObject *r1, *r2; + PyObject *r1; + int r2; if(!PyAnySet_Check(w)) Py_RETURN_NOTIMPLEMENTED; @@ -1825,9 +1832,11 @@ set_richcompare(PySetObject *v, PyObject *w, int op) r1 = set_richcompare(v, w, Py_EQ); if (r1 == NULL) return NULL; - r2 = PyBool_FromLong(PyObject_Not(r1)); + r2 = PyObject_IsTrue(r1); Py_DECREF(r1); - return r2; + if (r2 < 0) + return NULL; + return PyBool_FromLong(!r2); case Py_LE: return set_issubset(v, w); case Py_GE: diff --git a/Python/codecs.c b/Python/codecs.c index 74445b03dc5..27f2aebf827 100644 --- a/Python/codecs.c +++ b/Python/codecs.c @@ -549,12 +549,13 @@ PyObject * _PyCodec_LookupTextEncoding(const char *encoding, } else { is_text_codec = PyObject_IsTrue(attr); Py_DECREF(attr); - if (!is_text_codec) { + if (is_text_codec <= 0) { Py_DECREF(codec); - PyErr_Format(PyExc_LookupError, - "'%.400s' is not a text encoding; " - "use %s to handle arbitrary codecs", - encoding, alternate_command); + if (!is_text_codec) + PyErr_Format(PyExc_LookupError, + "'%.400s' is not a text encoding; " + "use %s to handle arbitrary codecs", + encoding, alternate_command); return NULL; } } diff --git a/Python/import.c b/Python/import.c index 34f4fd504d8..4f0765a13a0 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1416,6 +1416,7 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *given_globals, PyObject *globals = NULL; PyObject *fromlist = NULL; PyInterpreterState *interp = PyThreadState_GET()->interp; + int has_from; /* Make sure to use default values so as to not have PyObject_CallMethodObjArgs() truncate the parameter list because of a @@ -1646,7 +1647,10 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *given_globals, } /* From now on we don't hold the import lock anymore. */ - if (PyObject_Not(fromlist)) { + has_from = PyObject_IsTrue(fromlist); + if (has_from < 0) + goto error; + if (!has_from) { if (level == 0 || PyUnicode_GET_LENGTH(name) > 0) { PyObject *front = NULL; PyObject *partition = NULL;