From 0d54c803babe4512c1f5af6658ac15c116174e10 Mon Sep 17 00:00:00 2001
From: Serhiy Storchaka <storchaka@gmail.com>
Date: Fri, 1 Sep 2023 23:06:55 +0300
Subject: [PATCH] gh-106307: Fix PyMapping_GetOptionalItemString()

The resulting pointer was not set to NULL if the creation of a temporary
string object was failed.

The tests were also missed due to oversight.
---
 Lib/test/test_capi/test_abstract.py | 37 +++++++++++++++++++++++++++++
 Modules/_testcapi/abstract.c        |  8 +++----
 Modules/_testcapi/dict.c            |  4 ++--
 Modules/_testcapi/util.h            |  2 ++
 Objects/abstract.c                  |  2 ++
 5 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/Lib/test/test_capi/test_abstract.py b/Lib/test/test_capi/test_abstract.py
index 3f51e5b28104d9..671f62b977d2aa 100644
--- a/Lib/test/test_capi/test_abstract.py
+++ b/Lib/test/test_capi/test_abstract.py
@@ -265,6 +265,43 @@ def test_mapping_getitemstring(self):
         self.assertRaises(TypeError, getitemstring, [], b'a')
         self.assertRaises(SystemError, getitemstring, NULL, b'a')
 
+    def test_mapping_getoptionalitem(self):
+        getitem = _testcapi.mapping_getoptionalitem
+        dct = {'a': 1, '\U0001f40d': 2}
+        self.assertEqual(getitem(dct, 'a'), 1)
+        self.assertEqual(getitem(dct, 'b'), KeyError)
+        self.assertEqual(getitem(dct, '\U0001f40d'), 2)
+
+        dct2 = ProxyGetItem(dct)
+        self.assertEqual(getitem(dct2, 'a'), 1)
+        self.assertEqual(getitem(dct2, 'b'), KeyError)
+
+        self.assertEqual(getitem(['a', 'b', 'c'], 1), 'b')
+
+        self.assertRaises(TypeError, getitem, 42, 'a')
+        self.assertRaises(TypeError, getitem, {}, [])  # unhashable
+        self.assertRaises(IndexError, getitem, [], 1)
+        self.assertRaises(TypeError, getitem, [], 'a')
+        # CRASHES getitem({}, NULL)
+        # CRASHES getitem(NULL, 'a')
+
+    def test_mapping_getoptionalitemstring(self):
+        getitemstring = _testcapi.mapping_getoptionalitemstring
+        dct = {'a': 1, '\U0001f40d': 2}
+        self.assertEqual(getitemstring(dct, b'a'), 1)
+        self.assertEqual(getitemstring(dct, b'b'), KeyError)
+        self.assertEqual(getitemstring(dct, '\U0001f40d'.encode()), 2)
+
+        dct2 = ProxyGetItem(dct)
+        self.assertEqual(getitemstring(dct2, b'a'), 1)
+        self.assertEqual(getitemstring(dct2, b'b'), KeyError)
+
+        self.assertRaises(TypeError, getitemstring, 42, b'a')
+        self.assertRaises(UnicodeDecodeError, getitemstring, {}, b'\xff')
+        self.assertRaises(SystemError, getitemstring, {}, NULL)
+        self.assertRaises(TypeError, getitemstring, [], b'a')
+        # CRASHES getitemstring(NULL, b'a')
+
     def test_mapping_haskey(self):
         haskey = _testcapi.mapping_haskey
         dct = {'a': 1, '\U0001f40d': 2}
diff --git a/Modules/_testcapi/abstract.c b/Modules/_testcapi/abstract.c
index 91a1ee3aaafc02..bde0d2848954ed 100644
--- a/Modules/_testcapi/abstract.c
+++ b/Modules/_testcapi/abstract.c
@@ -32,7 +32,7 @@ object_getattrstring(PyObject *self, PyObject *args)
 static PyObject *
 object_getoptionalattr(PyObject *self, PyObject *args)
 {
-    PyObject *obj, *attr_name, *value;
+    PyObject *obj, *attr_name, *value = UNINITIALIZED_PTR;
     if (!PyArg_ParseTuple(args, "OO", &obj, &attr_name)) {
         return NULL;
     }
@@ -57,7 +57,7 @@ object_getoptionalattr(PyObject *self, PyObject *args)
 static PyObject *
 object_getoptionalattrstring(PyObject *self, PyObject *args)
 {
-    PyObject *obj, *value;
+    PyObject *obj, *value = UNINITIALIZED_PTR;
     const char *attr_name;
     Py_ssize_t size;
     if (!PyArg_ParseTuple(args, "Oz#", &obj, &attr_name, &size)) {
@@ -207,7 +207,7 @@ mapping_getitemstring(PyObject *self, PyObject *args)
 static PyObject *
 mapping_getoptionalitem(PyObject *self, PyObject *args)
 {
-    PyObject *obj, *attr_name, *value;
+    PyObject *obj, *attr_name, *value = UNINITIALIZED_PTR;
     if (!PyArg_ParseTuple(args, "OO", &obj, &attr_name)) {
         return NULL;
     }
@@ -232,7 +232,7 @@ mapping_getoptionalitem(PyObject *self, PyObject *args)
 static PyObject *
 mapping_getoptionalitemstring(PyObject *self, PyObject *args)
 {
-    PyObject *obj, *value;
+    PyObject *obj, *value = UNINITIALIZED_PTR;
     const char *attr_name;
     Py_ssize_t size;
     if (!PyArg_ParseTuple(args, "Oz#", &obj, &attr_name, &size)) {
diff --git a/Modules/_testcapi/dict.c b/Modules/_testcapi/dict.c
index 6720f0437401ef..b2cbdbe3432cb7 100644
--- a/Modules/_testcapi/dict.c
+++ b/Modules/_testcapi/dict.c
@@ -139,7 +139,7 @@ dict_getitemwitherror(PyObject *self, PyObject *args)
 static PyObject *
 dict_getitemref(PyObject *self, PyObject *args)
 {
-    PyObject *obj, *attr_name, *value;
+    PyObject *obj, *attr_name, *value = UNINITIALIZED_PTR;
     if (!PyArg_ParseTuple(args, "OO", &obj, &attr_name)) {
         return NULL;
     }
@@ -164,7 +164,7 @@ dict_getitemref(PyObject *self, PyObject *args)
 static PyObject *
 dict_getitemstringref(PyObject *self, PyObject *args)
 {
-    PyObject *obj, *value;
+    PyObject *obj, *value = UNINITIALIZED_PTR;
     const char *attr_name;
     Py_ssize_t size;
     if (!PyArg_ParseTuple(args, "Oz#", &obj, &attr_name, &size)) {
diff --git a/Modules/_testcapi/util.h b/Modules/_testcapi/util.h
index 05ccb12a4444f8..0a4ea841e9518f 100644
--- a/Modules/_testcapi/util.h
+++ b/Modules/_testcapi/util.h
@@ -23,3 +23,5 @@
         assert(!PyErr_Occurred());          \
         return PyLong_FromSsize_t(_ret);    \
     } while (0)
+
+#define UNINITIALIZED_PTR ((void *)"uninitialized")
diff --git a/Objects/abstract.c b/Objects/abstract.c
index c113364a88a26a..bb605d35ec954c 100644
--- a/Objects/abstract.c
+++ b/Objects/abstract.c
@@ -2394,11 +2394,13 @@ int
 PyMapping_GetOptionalItemString(PyObject *obj, const char *key, PyObject **result)
 {
     if (key == NULL) {
+        *result = NULL;
         null_error();
         return -1;
     }
     PyObject *okey = PyUnicode_FromString(key);
     if (okey == NULL) {
+        *result = NULL;
         return -1;
     }
     int rc = PyMapping_GetOptionalItem(obj, okey, result);