From e2a3c025702086168728364a88a0d9404f80ce62 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 24 Sep 2017 20:57:14 +0300 Subject: [PATCH 1/7] bpo-31572: Get rid of PyDict_GetItem() and PyObject_HasAttrString() in sqlite3. --- Modules/_sqlite/microprotocols.c | 75 +++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/Modules/_sqlite/microprotocols.c b/Modules/_sqlite/microprotocols.c index 2261b8013ce53f..7fd4109201e873 100644 --- a/Modules/_sqlite/microprotocols.c +++ b/Modules/_sqlite/microprotocols.c @@ -75,6 +75,8 @@ pysqlite_microprotocols_add(PyTypeObject *type, PyObject *proto, PyObject *cast) PyObject * pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) { + _Py_IDENTIFIER(__adapt__); + _Py_IDENTIFIER(__conform__); PyObject *adapter, *key; /* we don't check for exact type conformance as specified in PEP 246 @@ -86,46 +88,67 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) if (!key) { return NULL; } - adapter = PyDict_GetItem(psyco_adapters, key); + adapter = PyDict_GetItemWithError(psyco_adapters, key); Py_DECREF(key); if (adapter) { PyObject *adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL); + Py_DECREF(adapter); return adapted; } + if (PyErr_Occurred()) { + return NULL; + } - /* try to have the protocol adapt this object*/ - if (PyObject_HasAttrString(proto, "__adapt__")) { - _Py_IDENTIFIER(__adapt__); - PyObject *adapted = _PyObject_CallMethodId(proto, &PyId___adapt__, "O", obj); - - if (adapted) { - if (adapted != Py_None) { - return adapted; - } else { - Py_DECREF(adapted); - } - } + /* try to have the protocol adapt this object */ + adapter = _PyObject_GetAttrId(proto, &PyId___adapt__); + if (adapter) { + PyObject *adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL); + Py_DECREF(adapter); - if (PyErr_Occurred() && !PyErr_ExceptionMatches(PyExc_TypeError)) + if (adapted == Py_None) { + Py_DECREF(adapted); + } + else if (adapted) { + return adapted; + } + else if (!PyErr_ExceptionMatches(PyExc_TypeError)) { return NULL; + } + else { + PyErr_Clear(); + } + } + else if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + return NULL; + } + else { + PyErr_Clear(); } /* and finally try to have the object adapt itself */ - if (PyObject_HasAttrString(obj, "__conform__")) { - _Py_IDENTIFIER(__conform__); - PyObject *adapted = _PyObject_CallMethodId(obj, &PyId___conform__,"O", proto); - - if (adapted) { - if (adapted != Py_None) { - return adapted; - } else { - Py_DECREF(adapted); - } - } + adapter = _PyObject_GetAttrId(obj, &PyId___conform__); + if (adapter) { + PyObject *adapted = PyObject_CallFunctionObjArgs(adapter, proto, NULL); + Py_DECREF(adapter); - if (PyErr_Occurred() && !PyErr_ExceptionMatches(PyExc_TypeError)) { + if (adapted == Py_None) { + Py_DECREF(adapted); + } + else if (adapted) { + return adapted; + } + else if (!PyErr_ExceptionMatches(PyExc_TypeError)) { return NULL; } + else { + PyErr_Clear(); + } + } + else if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + return NULL; + } + else { + PyErr_Clear(); } /* else set the right exception and return NULL */ From 423748ae46c24351caeb9e3fd470de606e70f90c Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 24 Sep 2017 22:34:00 +0300 Subject: [PATCH 2/7] Fix a refcount error. --- Modules/_sqlite/microprotocols.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Modules/_sqlite/microprotocols.c b/Modules/_sqlite/microprotocols.c index 7fd4109201e873..682a2558a6b036 100644 --- a/Modules/_sqlite/microprotocols.c +++ b/Modules/_sqlite/microprotocols.c @@ -77,7 +77,7 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) { _Py_IDENTIFIER(__adapt__); _Py_IDENTIFIER(__conform__); - PyObject *adapter, *key; + PyObject *adapter, *key, *adapted; /* we don't check for exact type conformance as specified in PEP 246 because the pysqlite_PrepareProtocolType type is abstract and there is no @@ -91,7 +91,8 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) adapter = PyDict_GetItemWithError(psyco_adapters, key); Py_DECREF(key); if (adapter) { - PyObject *adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL); + Py_INCREF(adapter); + adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL); Py_DECREF(adapter); return adapted; } @@ -102,7 +103,7 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) /* try to have the protocol adapt this object */ adapter = _PyObject_GetAttrId(proto, &PyId___adapt__); if (adapter) { - PyObject *adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL); + adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL); Py_DECREF(adapter); if (adapted == Py_None) { @@ -128,7 +129,7 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) /* and finally try to have the object adapt itself */ adapter = _PyObject_GetAttrId(obj, &PyId___conform__); if (adapter) { - PyObject *adapted = PyObject_CallFunctionObjArgs(adapter, proto, NULL); + adapted = PyObject_CallFunctionObjArgs(adapter, proto, NULL); Py_DECREF(adapter); if (adapted == Py_None) { From 4305fb71808f069f2dab47282da4fd82835a2e5d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 25 Sep 2017 12:57:52 +0300 Subject: [PATCH 3/7] Fixed more suppressed errors. --- Modules/_sqlite/cache.c | 8 ++- Modules/_sqlite/connection.c | 8 ++- Modules/_sqlite/cursor.c | 119 ++++++++++++++----------------- Modules/_sqlite/microprotocols.c | 4 ++ Modules/_sqlite/statement.c | 34 +++++---- 5 files changed, 87 insertions(+), 86 deletions(-) diff --git a/Modules/_sqlite/cache.c b/Modules/_sqlite/cache.c index 72b1f2c5614bf4..4270473ae994df 100644 --- a/Modules/_sqlite/cache.c +++ b/Modules/_sqlite/cache.c @@ -119,7 +119,7 @@ PyObject* pysqlite_cache_get(pysqlite_Cache* self, PyObject* args) pysqlite_Node* ptr; PyObject* data; - node = (pysqlite_Node*)PyDict_GetItem(self->mapping, key); + node = (pysqlite_Node*)PyDict_GetItemWithError(self->mapping, key); if (node) { /* an entry for this key already exists in the cache */ @@ -157,7 +157,11 @@ PyObject* pysqlite_cache_get(pysqlite_Cache* self, PyObject* args) } ptr->prev = node; } - } else { + } + else if (PyErr_Occurred()) { + return NULL; + } + else { /* There is no entry for this key in the cache, yet. We'll insert a new * entry in the cache, and make space if necessary by throwing the * least used item out of the cache. */ diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 27591164b82d0f..6e53c55ed9a679 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -1403,6 +1403,7 @@ pysqlite_connection_interrupt(pysqlite_Connection* self, PyObject* args) static PyObject * pysqlite_connection_iterdump(pysqlite_Connection* self, PyObject* args) { + _Py_IDENTIFIER(_iterdump); PyObject* retval = NULL; PyObject* module = NULL; PyObject* module_dict; @@ -1422,9 +1423,12 @@ pysqlite_connection_iterdump(pysqlite_Connection* self, PyObject* args) goto finally; } - pyfn_iterdump = PyDict_GetItemString(module_dict, "_iterdump"); + pyfn_iterdump = _PyDict_GetItemIdWithError(module_dict, &PyId__iterdump); if (!pyfn_iterdump) { - PyErr_SetString(pysqlite_OperationalError, "Failed to obtain _iterdump() reference"); + if (!PyErr_Occurred()) { + PyErr_SetString(pysqlite_OperationalError, + "Failed to obtain _iterdump() reference"); + } goto finally; } diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index cfe2627aacd5b7..1f5b15c61b997e 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -29,7 +29,8 @@ PyObject* pysqlite_cursor_iternext(pysqlite_Cursor* self); static const char errmsg_fetch_across_rollback[] = "Cursor needed to be reset because of commit/rollback and can no longer be fetched from."; -static int pysqlite_cursor_init(pysqlite_Cursor* self, PyObject* args, PyObject* kwargs) +static int +pysqlite_cursor_init(pysqlite_Cursor* self, PyObject* args, PyObject* kwargs) { pysqlite_Connection* connection; @@ -77,7 +78,8 @@ static int pysqlite_cursor_init(pysqlite_Cursor* self, PyObject* args, PyObject* return 0; } -static void pysqlite_cursor_dealloc(pysqlite_Cursor* self) +static void +pysqlite_cursor_dealloc(pysqlite_Cursor* self) { /* Reset the statement if the user has not closed the cursor */ if (self->statement) { @@ -99,40 +101,47 @@ static void pysqlite_cursor_dealloc(pysqlite_Cursor* self) Py_TYPE(self)->tp_free((PyObject*)self); } -PyObject* _pysqlite_get_converter(PyObject* key) +static PyObject * +_pysqlite_get_converter(const char *keystr, Py_ssize_t keylen) { - PyObject* upcase_key; - PyObject* retval; + PyObject *key; + PyObject *upcase_key; + PyObject *retval; _Py_IDENTIFIER(upper); + key = PyUnicode_FromStringAndSize(keystr, keylen); + if (!key) { + return NULL; + } upcase_key = _PyObject_CallMethodId(key, &PyId_upper, NULL); + Py_DECREF(key); if (!upcase_key) { return NULL; } - retval = PyDict_GetItem(converters, upcase_key); + retval = PyDict_GetItemWithError(converters, upcase_key); Py_DECREF(upcase_key); return retval; } -int pysqlite_build_row_cast_map(pysqlite_Cursor* self) +static int +pysqlite_build_row_cast_map(pysqlite_Cursor* self) { int i; - const char* type_start = (const char*)-1; const char* pos; - const char* colname; const char* decltype; - PyObject* py_decltype; PyObject* converter; - PyObject* key; if (!self->connection->detect_types) { return 0; } Py_XSETREF(self->row_cast_map, PyList_New(0)); + if (!self->row_cast_map) { + return -1; + } for (i = 0; i < sqlite3_column_count(self->statement->st); i++) { converter = NULL; @@ -140,20 +149,17 @@ int pysqlite_build_row_cast_map(pysqlite_Cursor* self) if (self->connection->detect_types & PARSE_COLNAMES) { colname = sqlite3_column_name(self->statement->st, i); if (colname) { + const char *type_start = NULL; for (pos = colname; *pos != 0; pos++) { if (*pos == '[') { type_start = pos + 1; - } else if (*pos == ']' && type_start != (const char*)-1) { - key = PyUnicode_FromStringAndSize(type_start, pos - type_start); - if (!key) { - /* creating a string failed, but it is too complicated - * to propagate the error here, we just assume there is - * no converter and proceed */ - break; + } + else if (*pos == ']' && type_start != NULL) { + converter = _pysqlite_get_converter(type_start, pos - type_start); + if (!converter && PyErr_Occurred()) { + Py_CLEAR(self->row_cast_map); + return -1; } - - converter = _pysqlite_get_converter(key); - Py_DECREF(key); break; } } @@ -169,16 +175,14 @@ int pysqlite_build_row_cast_map(pysqlite_Cursor* self) * 'NUMBER(10)' to be treated as 'NUMBER', for example. * In other words, it will work as people expect it to work.*/ if (*pos == ' ' || *pos == '(' || *pos == 0) { - py_decltype = PyUnicode_FromStringAndSize(decltype, pos - decltype); - if (!py_decltype) { + converter = _pysqlite_get_converter(decltype, pos - decltype); + if (!converter && PyErr_Occurred()) { + Py_CLEAR(self->row_cast_map); return -1; } break; } } - - converter = _pysqlite_get_converter(py_decltype); - Py_DECREF(py_decltype); } } @@ -187,11 +191,7 @@ int pysqlite_build_row_cast_map(pysqlite_Cursor* self) } if (PyList_Append(self->row_cast_map, converter) != 0) { - if (converter != Py_None) { - Py_DECREF(converter); - } Py_CLEAR(self->row_cast_map); - return -1; } } @@ -199,7 +199,8 @@ int pysqlite_build_row_cast_map(pysqlite_Cursor* self) return 0; } -PyObject* _pysqlite_build_column_name(const char* colname) +static PyObject * +_pysqlite_build_column_name(const char* colname) { const char* pos; @@ -223,7 +224,8 @@ PyObject* _pysqlite_build_column_name(const char* colname) * Precondidition: * - sqlite3_step() has been called before and it returned SQLITE_ROW. */ -PyObject* _pysqlite_fetch_one_row(pysqlite_Cursor* self) +static PyObject * +_pysqlite_fetch_one_row(pysqlite_Cursor* self) { int i, numcols; PyObject* row; @@ -232,12 +234,10 @@ PyObject* _pysqlite_fetch_one_row(pysqlite_Cursor* self) PyObject* converter; PyObject* converted; Py_ssize_t nbytes; - PyObject* buffer; const char* val_str; char buf[200]; const char* colname; - PyObject* buf_bytes; - PyObject* error_obj; + PyObject* error_msg; if (self->reset) { PyErr_SetString(pysqlite_InterfaceError, errmsg_fetch_across_rollback); @@ -256,6 +256,7 @@ PyObject* _pysqlite_fetch_one_row(pysqlite_Cursor* self) if (self->connection->detect_types) { converter = PyList_GetItem(self->row_cast_map, i); if (!converter) { + PyErr_Clear(); converter = Py_None; } } else { @@ -274,8 +275,6 @@ PyObject* _pysqlite_fetch_one_row(pysqlite_Cursor* self) goto error; converted = PyObject_CallFunction(converter, "O", item); Py_DECREF(item); - if (!converted) - break; } } else { Py_BEGIN_ALLOW_THREADS @@ -301,18 +300,12 @@ PyObject* _pysqlite_fetch_one_row(pysqlite_Cursor* self) } PyOS_snprintf(buf, sizeof(buf) - 1, "Could not decode to UTF-8 column '%s' with text '%s'", colname , val_str); - buf_bytes = PyByteArray_FromStringAndSize(buf, strlen(buf)); - if (!buf_bytes) { + error_msg = PyUnicode_Decode(buf, strlen(buf), "ascii", "replace"); + if (!error_msg) { PyErr_SetString(pysqlite_OperationalError, "Could not decode to UTF-8"); } else { - error_obj = PyUnicode_FromEncodedObject(buf_bytes, "ascii", "replace"); - if (!error_obj) { - PyErr_SetString(pysqlite_OperationalError, "Could not decode to UTF-8"); - } else { - PyErr_SetObject(pysqlite_OperationalError, error_obj); - Py_DECREF(error_obj); - } - Py_DECREF(buf_bytes); + PyErr_SetObject(pysqlite_OperationalError, error_msg); + Py_DECREF(error_msg); } } } else if (self->connection->text_factory == (PyObject*)&PyBytes_Type) { @@ -325,20 +318,15 @@ PyObject* _pysqlite_fetch_one_row(pysqlite_Cursor* self) } else { /* coltype == SQLITE_BLOB */ nbytes = sqlite3_column_bytes(self->statement->st, i); - buffer = PyBytes_FromStringAndSize( + converted = PyBytes_FromStringAndSize( sqlite3_column_blob(self->statement->st, i), nbytes); - if (!buffer) - break; - converted = buffer; } } - if (converted) { - PyTuple_SetItem(row, i, converted); - } else { - Py_INCREF(Py_None); - PyTuple_SetItem(row, i, Py_None); + if (!converted) { + goto error; } + PyTuple_SetItem(row, i, converted); } if (PyErr_Occurred()) @@ -356,7 +344,8 @@ PyObject* _pysqlite_fetch_one_row(pysqlite_Cursor* self) * * 0 => error; 1 => ok */ -static int check_cursor(pysqlite_Cursor* cur) +static int +check_cursor(pysqlite_Cursor* cur) { if (!cur->initialized) { PyErr_SetString(pysqlite_ProgrammingError, "Base Cursor.__init__ not called."); @@ -376,7 +365,8 @@ static int check_cursor(pysqlite_Cursor* cur) return pysqlite_check_thread(cur->connection) && pysqlite_check_connection(cur->connection); } -PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* args) +static PyObject * +_pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* args) { PyObject* operation; const char* operation_cstr; @@ -552,7 +542,7 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* } if (pysqlite_build_row_cast_map(self) != 0) { - PyErr_SetString(pysqlite_OperationalError, "Error while building row_cast_map"); + _PyErr_FormatFromCause(pysqlite_OperationalError, "Error while building row_cast_map"); goto error; } @@ -641,7 +631,8 @@ PyObject* pysqlite_cursor_executemany(pysqlite_Cursor* self, PyObject* args) return _pysqlite_query_execute(self, 1, args); } -PyObject* pysqlite_cursor_executescript(pysqlite_Cursor* self, PyObject* args) +static PyObject * +pysqlite_cursor_executescript(pysqlite_Cursor* self, PyObject* args) { PyObject* script_obj; PyObject* script_str = NULL; @@ -728,12 +719,6 @@ PyObject* pysqlite_cursor_executescript(pysqlite_Cursor* self, PyObject* args) } } -PyObject* pysqlite_cursor_getiter(pysqlite_Cursor *self) -{ - Py_INCREF(self); - return (PyObject*)self; -} - PyObject* pysqlite_cursor_iternext(pysqlite_Cursor *self) { PyObject* next_row_tuple; @@ -966,7 +951,7 @@ PyTypeObject pysqlite_CursorType = { 0, /* tp_clear */ 0, /* tp_richcompare */ offsetof(pysqlite_Cursor, in_weakreflist), /* tp_weaklistoffset */ - (getiterfunc)pysqlite_cursor_getiter, /* tp_iter */ + PyObject_SelfIter, /* tp_iter */ (iternextfunc)pysqlite_cursor_iternext, /* tp_iternext */ cursor_methods, /* tp_methods */ cursor_members, /* tp_members */ diff --git a/Modules/_sqlite/microprotocols.c b/Modules/_sqlite/microprotocols.c index 682a2558a6b036..e686d72a13b69e 100644 --- a/Modules/_sqlite/microprotocols.c +++ b/Modules/_sqlite/microprotocols.c @@ -152,6 +152,10 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) PyErr_Clear(); } + if (alt) { + Py_INCREF(alt); + return alt; + } /* else set the right exception and return NULL */ PyErr_SetString(pysqlite_ProgrammingError, "can't adapt"); return NULL; diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 38690884227e68..2f79233334d678 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -250,12 +250,10 @@ void pysqlite_statement_bind_parameters(pysqlite_Statement* self, PyObject* para if (!_need_adapt(current_param)) { adapted = current_param; } else { - adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, NULL); - if (adapted) { - Py_DECREF(current_param); - } else { - PyErr_Clear(); - adapted = current_param; + adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, current_param); + Py_DECREF(current_param); + if (!adapted) { + return; } } @@ -272,6 +270,7 @@ void pysqlite_statement_bind_parameters(pysqlite_Statement* self, PyObject* para } else if (PyDict_Check(parameters)) { /* parameters passed as dictionary */ for (i = 1; i <= num_params_needed; i++) { + PyObject *binding_name_obj; Py_BEGIN_ALLOW_THREADS binding_name = sqlite3_bind_parameter_name(self->st, i); Py_END_ALLOW_THREADS @@ -281,26 +280,31 @@ void pysqlite_statement_bind_parameters(pysqlite_Statement* self, PyObject* para } binding_name++; /* skip first char (the colon) */ + binding_name_obj = PyUnicode_FromString(binding_name); + if (!binding_name_obj) { + return; + } if (PyDict_CheckExact(parameters)) { - current_param = PyDict_GetItemString(parameters, binding_name); + current_param = PyDict_GetItemWithError(parameters, binding_name_obj); Py_XINCREF(current_param); } else { - current_param = PyMapping_GetItemString(parameters, binding_name); + current_param = PyObject_GetItem(parameters, binding_name_obj); } + Py_DECREF(binding_name_obj); if (!current_param) { - PyErr_Format(pysqlite_ProgrammingError, "You did not supply a value for binding %d.", i); + if (!PyErr_Occurred() || PyErr_ExceptionMatches(PyExc_LookupError)) { + PyErr_Format(pysqlite_ProgrammingError, "You did not supply a value for binding %d.", i); + } return; } if (!_need_adapt(current_param)) { adapted = current_param; } else { - adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, NULL); - if (adapted) { - Py_DECREF(current_param); - } else { - PyErr_Clear(); - adapted = current_param; + adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, current_param); + Py_DECREF(current_param); + if (!adapted) { + return; } } From dafc64da330f17eb99023e13eb5c3274c1435255 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 1 Feb 2018 19:57:21 +0200 Subject: [PATCH 4/7] Use _PyObject_LookupAttrId(). --- Modules/_sqlite/microprotocols.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/Modules/_sqlite/microprotocols.c b/Modules/_sqlite/microprotocols.c index c787e693db8ca5..ffc09a7a3c7c9a 100644 --- a/Modules/_sqlite/microprotocols.c +++ b/Modules/_sqlite/microprotocols.c @@ -101,7 +101,9 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) } /* try to have the protocol adapt this object */ - adapter = _PyObject_GetAttrId(proto, &PyId___adapt__); + if (_PyObject_LookupAttrId(proto, &PyId___adapt__, &adapter) < 0) { + return NULL; + } if (adapter) { adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL); Py_DECREF(adapter); @@ -119,15 +121,11 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) PyErr_Clear(); } } - else if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { - return NULL; - } - else { - PyErr_Clear(); - } /* and finally try to have the object adapt itself */ - adapter = _PyObject_GetAttrId(obj, &PyId___conform__); + if (_PyObject_LookupAttrId(obj, &PyId___conform__, &adapter) < 0) { + return NULL; + } if (adapter) { adapted = PyObject_CallFunctionObjArgs(adapter, proto, NULL); Py_DECREF(adapter); @@ -145,12 +143,6 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) PyErr_Clear(); } } - else if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { - return NULL; - } - else { - PyErr_Clear(); - } if (alt) { Py_INCREF(alt); From 4585ae81b55ac1c94c5e87a246233cb72ba88eee Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 9 Jul 2018 00:39:10 +0300 Subject: [PATCH 5/7] Minor refactoring. --- Modules/_sqlite/cursor.c | 16 ++++++++-------- Modules/_sqlite/microprotocols.c | 10 ++-------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 5a93ee569b1740..f9887ac19727f9 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -252,13 +252,13 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self) return NULL; for (i = 0; i < numcols; i++) { - if (self->connection->detect_types) { - converter = PyList_GetItem(self->row_cast_map, i); - if (!converter) { - PyErr_Clear(); - converter = Py_None; - } - } else { + if (self->connection->detect_types + && self->row_cast_map != NULL + && i < PyList_GET_SIZE(self->row_cast_map)) + { + converter = PyList_GET_ITEM(self->row_cast_map, i); + } + else { converter = Py_None; } @@ -291,7 +291,7 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self) nbytes = sqlite3_column_bytes(self->statement->st, i); if (self->connection->text_factory == (PyObject*)&PyUnicode_Type) { converted = PyUnicode_FromStringAndSize(val_str, nbytes); - if (!converted) { + if (!converted && PyErr_ExceptionMatches(PyExc_UnicodeDecodeError)) { PyErr_Clear(); colname = sqlite3_column_name(self->statement->st, i); if (!colname) { diff --git a/Modules/_sqlite/microprotocols.c b/Modules/_sqlite/microprotocols.c index ffc09a7a3c7c9a..c23b09f56b5bd1 100644 --- a/Modules/_sqlite/microprotocols.c +++ b/Modules/_sqlite/microprotocols.c @@ -111,12 +111,9 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) if (adapted == Py_None) { Py_DECREF(adapted); } - else if (adapted) { + else if (adapted || !PyErr_ExceptionMatches(PyExc_TypeError)) { return adapted; } - else if (!PyErr_ExceptionMatches(PyExc_TypeError)) { - return NULL; - } else { PyErr_Clear(); } @@ -133,12 +130,9 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) if (adapted == Py_None) { Py_DECREF(adapted); } - else if (adapted) { + else if (adapted || !PyErr_ExceptionMatches(PyExc_TypeError)) { return adapted; } - else if (!PyErr_ExceptionMatches(PyExc_TypeError)) { - return NULL; - } else { PyErr_Clear(); } From 369164a311f0d0eb63943624ae424e7cce84de04 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 23 Jul 2018 10:37:01 +0300 Subject: [PATCH 6/7] Keep the old formatting. --- Modules/_sqlite/cursor.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index c9ffac12786115..f907231a7272e7 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -29,8 +29,7 @@ PyObject* pysqlite_cursor_iternext(pysqlite_Cursor* self); static const char errmsg_fetch_across_rollback[] = "Cursor needed to be reset because of commit/rollback and can no longer be fetched from."; -static int -pysqlite_cursor_init(pysqlite_Cursor* self, PyObject* args, PyObject* kwargs) +static int pysqlite_cursor_init(pysqlite_Cursor* self, PyObject* args, PyObject* kwargs) { pysqlite_Connection* connection; @@ -77,8 +76,7 @@ pysqlite_cursor_init(pysqlite_Cursor* self, PyObject* args, PyObject* kwargs) return 0; } -static void -pysqlite_cursor_dealloc(pysqlite_Cursor* self) +static void pysqlite_cursor_dealloc(pysqlite_Cursor* self) { /* Reset the statement if the user has not closed the cursor */ if (self->statement) { @@ -343,8 +341,7 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self) * * 0 => error; 1 => ok */ -static int -check_cursor(pysqlite_Cursor* cur) +static int check_cursor(pysqlite_Cursor* cur) { if (!cur->initialized) { PyErr_SetString(pysqlite_ProgrammingError, "Base Cursor.__init__ not called."); From c5bc2a605dd12a7d274d29f437fd0c90ddaa5b87 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 23 Jul 2018 12:37:46 +0300 Subject: [PATCH 7/7] Add tests and a news entry. --- Lib/sqlite3/test/types.py | 28 +++++++++++++++++-- .../2018-07-23-12-20-02.bpo-32788.R2jSiK.rst | 3 ++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-07-23-12-20-02.bpo-32788.R2jSiK.rst diff --git a/Lib/sqlite3/test/types.py b/Lib/sqlite3/test/types.py index 6bc8d71def0e31..19ecd07500fec5 100644 --- a/Lib/sqlite3/test/types.py +++ b/Lib/sqlite3/test/types.py @@ -102,10 +102,16 @@ def __conform__(self, protocol): def __str__(self): return "<%s>" % self.val + class BadConform: + def __init__(self, exc): + self.exc = exc + def __conform__(self, protocol): + raise self.exc + def setUp(self): self.con = sqlite.connect(":memory:", detect_types=sqlite.PARSE_DECLTYPES) self.cur = self.con.cursor() - self.cur.execute("create table test(i int, s str, f float, b bool, u unicode, foo foo, bin blob, n1 number, n2 number(5))") + self.cur.execute("create table test(i int, s str, f float, b bool, u unicode, foo foo, bin blob, n1 number, n2 number(5), bad bad)") # override float, make them always return the same number sqlite.converters["FLOAT"] = lambda x: 47.2 @@ -113,6 +119,7 @@ def setUp(self): # and implement two custom ones sqlite.converters["BOOL"] = lambda x: bool(int(x)) sqlite.converters["FOO"] = DeclTypesTests.Foo + sqlite.converters["BAD"] = DeclTypesTests.BadConform sqlite.converters["WRONG"] = lambda x: "WRONG" sqlite.converters["NUMBER"] = float @@ -120,6 +127,8 @@ def tearDown(self): del sqlite.converters["FLOAT"] del sqlite.converters["BOOL"] del sqlite.converters["FOO"] + del sqlite.converters["BAD"] + del sqlite.converters["WRONG"] del sqlite.converters["NUMBER"] self.cur.close() self.con.close() @@ -159,13 +168,13 @@ def CheckBool(self): self.cur.execute("insert into test(b) values (?)", (False,)) self.cur.execute("select b from test") row = self.cur.fetchone() - self.assertEqual(row[0], False) + self.assertIs(row[0], False) self.cur.execute("delete from test") self.cur.execute("insert into test(b) values (?)", (True,)) self.cur.execute("select b from test") row = self.cur.fetchone() - self.assertEqual(row[0], True) + self.assertIs(row[0], True) def CheckUnicode(self): # default @@ -182,6 +191,19 @@ def CheckFoo(self): row = self.cur.fetchone() self.assertEqual(row[0], val) + def CheckErrorInConform(self): + val = DeclTypesTests.BadConform(TypeError) + with self.assertRaises(sqlite.InterfaceError): + self.cur.execute("insert into test(bad) values (?)", (val,)) + with self.assertRaises(sqlite.InterfaceError): + self.cur.execute("insert into test(bad) values (:val)", {"val": val}) + + val = DeclTypesTests.BadConform(KeyboardInterrupt) + with self.assertRaises(KeyboardInterrupt): + self.cur.execute("insert into test(bad) values (?)", (val,)) + with self.assertRaises(KeyboardInterrupt): + self.cur.execute("insert into test(bad) values (:val)", {"val": val}) + def CheckUnsupportedSeq(self): class Bar: pass val = Bar() diff --git a/Misc/NEWS.d/next/Library/2018-07-23-12-20-02.bpo-32788.R2jSiK.rst b/Misc/NEWS.d/next/Library/2018-07-23-12-20-02.bpo-32788.R2jSiK.rst new file mode 100644 index 00000000000000..1f3ae88b706daf --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-07-23-12-20-02.bpo-32788.R2jSiK.rst @@ -0,0 +1,3 @@ +Errors other than :exc:`TypeError` raised in methods ``__adapt__()`` and +``__conform__()`` in the :mod:`sqlite3` module are now propagated to the +user.