python · orenmn · Oct 9, 2017 · Oct 9, 2017 · Oct 9, 2017 · Oct 9, 2017
diff --git 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
@@ -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
@@ -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 */
Original file line number	Diff line number	Diff line change
		@@ -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.