gh-141510: Check argument in PyDict_MergeFromSeq2() (#145082)

PyDict_MergeFromSeq2() now fails with SystemError if the first
argument is not a dict or a dict subclass.

PyDict_Update(), PyDict_Merge() and _PyDict_MergeEx() no longer
accept frozendict.

Author: vstinner

Lib/test/test_capi/test_dict.py

@@ -419,6 +419,7 @@ def test_dict_next(self):
         # CRASHES dict_next(NULL, 0)
 
     def test_dict_update(self):
+        # Test PyDict_Update()
         update = _testlimitedcapi.dict_update
         for cls1 in dict, DictSubclass:
             for cls2 in dict, DictSubclass, UserDict:
@@ -429,11 +430,13 @@ def test_dict_update(self):
         self.assertRaises(AttributeError, update, {}, [])
         self.assertRaises(AttributeError, update, {}, 42)
         self.assertRaises(SystemError, update, UserDict(), {})
+        self.assertRaises(SystemError, update, frozendict(), {})
         self.assertRaises(SystemError, update, 42, {})
         self.assertRaises(SystemError, update, {}, NULL)
         self.assertRaises(SystemError, update, NULL, {})
 
     def test_dict_merge(self):
+        # Test PyDict_Merge()
         merge = _testlimitedcapi.dict_merge
         for cls1 in dict, DictSubclass:
             for cls2 in dict, DictSubclass, UserDict:
@@ -447,11 +450,13 @@ def test_dict_merge(self):
         self.assertRaises(AttributeError, merge, {}, [], 0)
         self.assertRaises(AttributeError, merge, {}, 42, 0)
         self.assertRaises(SystemError, merge, UserDict(), {}, 0)
+        self.assertRaises(SystemError, merge, frozendict(), {}, 0)
         self.assertRaises(SystemError, merge, 42, {}, 0)
         self.assertRaises(SystemError, merge, {}, NULL, 0)
         self.assertRaises(SystemError, merge, NULL, {}, 0)
 
     def test_dict_mergefromseq2(self):
+        # Test PyDict_MergeFromSeq2()
         mergefromseq2 = _testlimitedcapi.dict_mergefromseq2
         for cls1 in dict, DictSubclass:
             for cls2 in list, iter:
@@ -466,8 +471,8 @@ def test_dict_mergefromseq2(self):
         self.assertRaises(ValueError, mergefromseq2, {}, [(1, 2, 3)], 0)
         self.assertRaises(TypeError, mergefromseq2, {}, [1], 0)
         self.assertRaises(TypeError, mergefromseq2, {}, 42, 0)
-        # CRASHES mergefromseq2(UserDict(), [], 0)
-        # CRASHES mergefromseq2(42, [], 0)
+        self.assertRaises(SystemError, mergefromseq2, UserDict(), [], 0)
+        self.assertRaises(SystemError, mergefromseq2, 42, [], 0)
         # CRASHES mergefromseq2({}, NULL, 0)
         # CRASHES mergefromseq2(NULL, {}, 0)
 

Objects/dictobject.c

@@ -140,6 +140,7 @@ static PyObject* frozendict_new(PyTypeObject *type, PyObject *args,
                                 PyObject *kwds);
 static PyObject* dict_new(PyTypeObject *type, PyObject *args, PyObject *kwds);
 static int dict_merge(PyObject *a, PyObject *b, int override);
+static int dict_merge_from_seq2(PyObject *d, PyObject *seq2, int override);
 
 
 /*[clinic input]
@@ -3818,16 +3819,16 @@ static int
 dict_update_arg(PyObject *self, PyObject *arg)
 {
     if (PyAnyDict_CheckExact(arg)) {
-        return PyDict_Merge(self, arg, 1);
+        return dict_merge(self, arg, 1);
     }
     int has_keys = PyObject_HasAttrWithError(arg, &_Py_ID(keys));
     if (has_keys < 0) {
         return -1;
     }
     if (has_keys) {
-        return PyDict_Merge(self, arg, 1);
+        return dict_merge(self, arg, 1);
     }
-    return PyDict_MergeFromSeq2(self, arg, 1);
+    return dict_merge_from_seq2(self, arg, 1);
 }
 
 static int
@@ -3846,7 +3847,7 @@ dict_update_common(PyObject *self, PyObject *args, PyObject *kwds,
 
     if (result == 0 && kwds != NULL) {
         if (PyArg_ValidateKeywordArguments(kwds))
-            result = PyDict_Merge(self, kwds, 1);
+            result = dict_merge(self, kwds, 1);
         else
             result = -1;
     }
@@ -3960,8 +3961,8 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
     return Py_SAFE_DOWNCAST(i, Py_ssize_t, int);
 }
 
-int
-PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
+static int
+dict_merge_from_seq2(PyObject *d, PyObject *seq2, int override)
 {
     int res;
     Py_BEGIN_CRITICAL_SECTION(d);
@@ -3971,6 +3972,19 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
     return res;
 }
 
+int
+PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
+{
+    assert(d != NULL);
+    assert(seq2 != NULL);
+    if (!PyDict_Check(d)) {
+        PyErr_BadInternalCall();
+        return -1;
+    }
+
+    return dict_merge_from_seq2(d, seq2, override);
+}
+
 static int
 dict_dict_merge(PyDictObject *mp, PyDictObject *other, int override)
 {
@@ -4070,23 +4084,14 @@ dict_dict_merge(PyDictObject *mp, PyDictObject *other, int override)
 static int
 dict_merge(PyObject *a, PyObject *b, int override)
 {
-    PyDictObject *mp, *other;
-
+    assert(a != NULL);
+    assert(b != NULL);
     assert(0 <= override && override <= 2);
 
-    /* We accept for the argument either a concrete dictionary object,
-     * or an abstract "mapping" object.  For the former, we can do
-     * things quite efficiently.  For the latter, we only require that
-     * PyMapping_Keys() and PyObject_GetItem() be supported.
-     */
-    if (a == NULL || !PyAnyDict_Check(a) || b == NULL) {
-        PyErr_BadInternalCall();
-        return -1;
-    }
-    mp = (PyDictObject*)a;
+    PyDictObject *mp = _PyAnyDict_CAST(a);
     int res = 0;
     if (PyAnyDict_Check(b) && (Py_TYPE(b)->tp_iter == dict_iter)) {
-        other = (PyDictObject*)b;
+        PyDictObject *other = (PyDictObject*)b;
         int res;
         Py_BEGIN_CRITICAL_SECTION2(a, b);
         res = dict_dict_merge((PyDictObject *)a, other, override);
@@ -4167,23 +4172,38 @@ dict_merge(PyObject *a, PyObject *b, int override)
     }
 }
 
+static int
+dict_merge_api(PyObject *a, PyObject *b, int override)
+{
+    /* We accept for the argument either a concrete dictionary object,
+     * or an abstract "mapping" object.  For the former, we can do
+     * things quite efficiently.  For the latter, we only require that
+     * PyMapping_Keys() and PyObject_GetItem() be supported.
+     */
+    if (a == NULL || !PyDict_Check(a) || b == NULL) {
+        PyErr_BadInternalCall();
+        return -1;
+    }
+    return dict_merge(a, b, override);
+}
+
 int
 PyDict_Update(PyObject *a, PyObject *b)
 {
-    return dict_merge(a, b, 1);
+    return dict_merge_api(a, b, 1);
 }
 
 int
 PyDict_Merge(PyObject *a, PyObject *b, int override)
 {
     /* XXX Deprecate override not in (0, 1). */
-    return dict_merge(a, b, override != 0);
+    return dict_merge_api(a, b, override != 0);
 }
 
 int
 _PyDict_MergeEx(PyObject *a, PyObject *b, int override)
 {
-    return dict_merge(a, b, override);
+    return dict_merge_api(a, b, override);
 }
 
 /*[clinic input]