diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index 7dd0050528f8fa..8fd75702274022 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -24,6 +24,7 @@ import datetime import unittest import sqlite3 as sqlite +from test import support class RegressionTests(unittest.TestCase): def setUp(self): @@ -376,6 +377,25 @@ def CheckCommitCursorReset(self): counter += 1 self.assertEqual(counter, 3, "should have returned exactly three rows") + @support.cpython_only + def CheckUninitializedCache(self): + # bpo-31734: Cache.get() shouldn't crash in case the Cache object is + # uninitialized. + cache = sqlite.Cache.__new__(sqlite.Cache) + self.assertRaises(ValueError, cache.get, None) + + @support.cpython_only + def Check__init__Fail(self): + # bpo-31734: A failure of the __init__() method of an already + # initialized Cache object shouldn't cause the Cache object to be + # partially initialized, and its get() method to raise a SystemError. + cache = sqlite.Cache(str) + try: + cache.__init__() # invalid number of arguments + except TypeError: + pass + self.assertRaises(ValueError, cache.get, None) + def suite(): regression_suite = unittest.makeSuite(RegressionTests, "Check") diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-10-09-16-58-33.bpo-31734.HCa6O5.rst b/Misc/NEWS.d/next/Core and Builtins/2017-10-09-16-58-33.bpo-31734.HCa6O5.rst new file mode 100644 index 00000000000000..d0c261ec75281d --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-10-09-16-58-33.bpo-31734.HCa6O5.rst @@ -0,0 +1,2 @@ +Raise a `ValueError` instead of crashing, when the `get()` method of an +uninitialized `sqlite3.Cache` object is called. Patch by Oren Milman. diff --git a/Modules/_sqlite/cache.c b/Modules/_sqlite/cache.c index 72b1f2c5614bf4..1cb883240752dd 100644 --- a/Modules/_sqlite/cache.c +++ b/Modules/_sqlite/cache.c @@ -54,12 +54,37 @@ void pysqlite_node_dealloc(pysqlite_Node* self) Py_TYPE(self)->tp_free((PyObject*)self); } +/* Iterate over all nodes and deallocate them. If there aren't any nodes, do + nothing. */ +static void deallocate_nodes(pysqlite_Cache* self) +{ + pysqlite_Node* node; + pysqlite_Node* delete_node; + + node = self->first; + self->first = NULL; + self->last = NULL; + while (node) { + delete_node = node; + node = node->next; + Py_DECREF(delete_node); + } +} + int pysqlite_cache_init(pysqlite_Cache* self, PyObject* args, PyObject* kwargs) { PyObject* factory; int size = 10; - self->factory = NULL; + if (self->decref_factory) { + self->decref_factory = 0; + Py_CLEAR(self->factory); + } + else { + self->factory = NULL; + } + deallocate_nodes(self); + Py_CLEAR(self->mapping); if (!PyArg_ParseTuple(args, "O|i", &factory, &size)) { return -1; @@ -70,8 +95,6 @@ int pysqlite_cache_init(pysqlite_Cache* self, PyObject* args, PyObject* kwargs) size = 5; } self->size = size; - self->first = NULL; - self->last = NULL; self->mapping = PyDict_New(); if (!self->mapping) { @@ -88,26 +111,13 @@ int pysqlite_cache_init(pysqlite_Cache* self, PyObject* args, PyObject* kwargs) void pysqlite_cache_dealloc(pysqlite_Cache* self) { - pysqlite_Node* node; - pysqlite_Node* delete_node; - - if (!self->factory) { - /* constructor failed, just get out of here */ - return; - } - - /* iterate over all nodes and deallocate them */ - node = self->first; - while (node) { - delete_node = node; - node = node->next; - Py_DECREF(delete_node); - } - - if (self->decref_factory) { - Py_DECREF(self->factory); + if (self->factory != NULL) { + deallocate_nodes(self); + if (self->decref_factory) { + Py_DECREF(self->factory); + } + Py_DECREF(self->mapping); } - Py_DECREF(self->mapping); Py_TYPE(self)->tp_free((PyObject*)self); } @@ -119,6 +129,11 @@ PyObject* pysqlite_cache_get(pysqlite_Cache* self, PyObject* args) pysqlite_Node* ptr; PyObject* data; + if (self->factory == NULL) { + PyErr_SetString(PyExc_ValueError, + MODULE_NAME ".Cache.__init__() not called"); + return NULL; + } node = (pysqlite_Node*)PyDict_GetItem(self->mapping, key); if (node) { /* an entry for this key already exists in the cache */