From: Victor Stinner Date: Wed, 11 Mar 2026 16:05:09 +0000 (+0100) Subject: gh-141510: Raise TypeError in PyDict_SetItem() on frozendict (#145564) X-Git-Tag: v3.15.0a8~348 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=713be70175a2dad477a1cf5e7c00bab0edda04ad;p=thirdparty%2FPython%2Fcpython.git gh-141510: Raise TypeError in PyDict_SetItem() on frozendict (#145564) If the following functions get an unexpected frozendict, raise TypeError instead of SystemError: * PyDict_DelItem() * PyDict_DelItemString() * PyDict_Merge() * PyDict_MergeFromSeq2() * PyDict_Pop() * PyDict_PopString() * PyDict_SetDefault() * PyDict_SetDefaultRef() * PyDict_SetItem() * PyDict_SetItemString() * _PyDict_SetItem_KnownHash() * PyDict_Update() Co-authored-by: mohsinm-dev --- diff --git a/Lib/test/test_capi/test_dict.py b/Lib/test/test_capi/test_dict.py index cd46fea5476c..4cf404e9f563 100644 --- a/Lib/test/test_capi/test_dict.py +++ b/Lib/test/test_capi/test_dict.py @@ -1,3 +1,4 @@ +import contextlib import unittest from collections import OrderedDict, UserDict from types import MappingProxyType @@ -258,6 +259,12 @@ class CAPITest(unittest.TestCase): # CRASHES contains({}, NULL) # CRASHES contains(NULL, b'a') + @contextlib.contextmanager + def frozendict_does_not_support(self, what): + errmsg = f'frozendict object does not support item {what}' + with self.assertRaisesRegex(TypeError, errmsg): + yield + def test_dict_setitem(self): # Test PyDict_SetItem() setitem = _testlimitedcapi.dict_setitem @@ -269,7 +276,10 @@ class CAPITest(unittest.TestCase): self.assertEqual(dct, {'a': 5, '\U0001f40d': 8}) self.assertRaises(TypeError, setitem, {}, [], 5) # unhashable - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('assignment'): + setitem(test_type(), 'a', 5) + for test_type in MAPPING_TYPES + OTHER_TYPES: self.assertRaises(SystemError, setitem, test_type(), 'a', 5) # CRASHES setitem({}, NULL, 5) # CRASHES setitem({}, 'a', NULL) @@ -286,7 +296,10 @@ class CAPITest(unittest.TestCase): self.assertEqual(dct, {'a': 5, '\U0001f40d': 8}) self.assertRaises(UnicodeDecodeError, setitemstring, {}, INVALID_UTF8, 5) - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('assignment'): + setitemstring(test_type(), b'a', 5) + for test_type in MAPPING_TYPES + OTHER_TYPES: self.assertRaises(SystemError, setitemstring, test_type(), b'a', 5) # CRASHES setitemstring({}, NULL, 5) # CRASHES setitemstring({}, b'a', NULL) @@ -304,7 +317,10 @@ class CAPITest(unittest.TestCase): self.assertEqual(dct, {'c': 2}) self.assertRaises(TypeError, delitem, {}, []) # unhashable - for test_type in NOT_DICT_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('deletion'): + delitem(test_type({'a': 1}), 'a') + for test_type in MAPPING_TYPES: self.assertRaises(SystemError, delitem, test_type({'a': 1}), 'a') for test_type in OTHER_TYPES: self.assertRaises(SystemError, delitem, test_type(), 'a') @@ -323,7 +339,10 @@ class CAPITest(unittest.TestCase): self.assertEqual(dct, {'c': 2}) self.assertRaises(UnicodeDecodeError, delitemstring, {}, INVALID_UTF8) - for test_type in NOT_DICT_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('deletion'): + delitemstring(test_type({'a': 1}), b'a') + for test_type in MAPPING_TYPES: self.assertRaises(SystemError, delitemstring, test_type({'a': 1}), b'a') for test_type in OTHER_TYPES: self.assertRaises(SystemError, delitemstring, test_type(), b'a') @@ -341,7 +360,10 @@ class CAPITest(unittest.TestCase): self.assertEqual(dct, {'a': 5}) self.assertRaises(TypeError, setdefault, {}, [], 5) # unhashable - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('assignment'): + setdefault(test_type(), 'a', 5) + for test_type in MAPPING_TYPES + OTHER_TYPES: self.assertRaises(SystemError, setdefault, test_type(), 'a', 5) # CRASHES setdefault({}, NULL, 5) # CRASHES setdefault({}, 'a', NULL) @@ -358,7 +380,10 @@ class CAPITest(unittest.TestCase): self.assertEqual(dct, {'a': 5}) self.assertRaises(TypeError, setdefault, {}, [], 5) # unhashable - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('assignment'): + setdefault(test_type(), 'a', 5) + for test_type in MAPPING_TYPES + OTHER_TYPES: self.assertRaises(SystemError, setdefault, test_type(), 'a', 5) # CRASHES setdefault({}, NULL, 5) # CRASHES setdefault({}, 'a', NULL) @@ -424,7 +449,10 @@ class CAPITest(unittest.TestCase): self.assertRaises(AttributeError, update, {}, []) self.assertRaises(AttributeError, update, {}, 42) - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('assignment'): + update(test_type(), {}) + for test_type in MAPPING_TYPES + OTHER_TYPES: self.assertRaises(SystemError, update, test_type(), {}) self.assertRaises(SystemError, update, {}, NULL) self.assertRaises(SystemError, update, NULL, {}) @@ -443,7 +471,10 @@ class CAPITest(unittest.TestCase): self.assertRaises(AttributeError, merge, {}, [], 0) self.assertRaises(AttributeError, merge, {}, 42, 0) - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('assignment'): + merge(test_type(), {}, 0) + for test_type in MAPPING_TYPES + OTHER_TYPES: self.assertRaises(SystemError, merge, test_type(), {}, 0) self.assertRaises(SystemError, merge, {}, NULL, 0) self.assertRaises(SystemError, merge, NULL, {}, 0) @@ -464,7 +495,10 @@ class CAPITest(unittest.TestCase): self.assertRaises(ValueError, mergefromseq2, {}, [(1, 2, 3)], 0) self.assertRaises(TypeError, mergefromseq2, {}, [1], 0) self.assertRaises(TypeError, mergefromseq2, {}, 42, 0) - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('assignment'): + mergefromseq2(test_type(), [], 0) + for test_type in MAPPING_TYPES + OTHER_TYPES: self.assertRaises(SystemError, mergefromseq2, test_type(), [], 0) # CRASHES mergefromseq2({}, NULL, 0) # CRASHES mergefromseq2(NULL, {}, 0) @@ -511,7 +545,10 @@ class CAPITest(unittest.TestCase): dict_pop(mydict, not_hashable_key) # wrong dict type - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('deletion'): + dict_pop(test_type(), "key") + for test_type in MAPPING_TYPES + OTHER_TYPES: not_dict = test_type() self.assertRaises(SystemError, dict_pop, not_dict, "key") self.assertRaises(SystemError, dict_pop_null, not_dict, "key") @@ -560,7 +597,10 @@ class CAPITest(unittest.TestCase): self.assertRaises(UnicodeDecodeError, dict_popstring_null, mydict, INVALID_UTF8) # wrong dict type - for test_type in NOT_DICT_TYPES + OTHER_TYPES: + for test_type in FROZENDICT_TYPES: + with self.frozendict_does_not_support('deletion'): + dict_popstring(test_type(), "key") + for test_type in MAPPING_TYPES + OTHER_TYPES: not_dict = test_type() self.assertRaises(SystemError, dict_popstring, not_dict, "key") self.assertRaises(SystemError, dict_popstring_null, not_dict, "key") diff --git a/Objects/dictobject.c b/Objects/dictobject.c index b5f2a682c549..842d9be73b87 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2719,6 +2719,10 @@ _PyDict_LoadBuiltinsFromGlobals(PyObject *globals) return builtins; } +#define frozendict_does_not_support(WHAT) \ + PyErr_SetString(PyExc_TypeError, "frozendict object does " \ + "not support item " WHAT) + /* Consumes references to key and value */ static int setitem_take2_lock_held(PyDictObject *mp, PyObject *key, PyObject *value) @@ -2762,12 +2766,19 @@ _PyDict_SetItem_Take2(PyDictObject *mp, PyObject *key, PyObject *value) int PyDict_SetItem(PyObject *op, PyObject *key, PyObject *value) { + assert(key); + assert(value); + if (!PyDict_Check(op)) { - PyErr_BadInternalCall(); + if (PyFrozenDict_Check(op)) { + frozendict_does_not_support("assignment"); + } + else { + PyErr_BadInternalCall(); + } return -1; } - assert(key); - assert(value); + return _PyDict_SetItem_Take2((PyDictObject *)op, Py_NewRef(key), Py_NewRef(value)); } @@ -2807,14 +2818,20 @@ int _PyDict_SetItem_KnownHash(PyObject *op, PyObject *key, PyObject *value, Py_hash_t hash) { - if (!PyDict_Check(op)) { - PyErr_BadInternalCall(); - return -1; - } assert(key); assert(value); assert(hash != -1); + if (!PyDict_Check(op)) { + if (PyFrozenDict_Check(op)) { + frozendict_does_not_support("assignment"); + } + else { + PyErr_BadInternalCall(); + } + return -1; + } + int res; Py_BEGIN_CRITICAL_SECTION(op); res = _PyDict_SetItem_KnownHash_LockHeld((PyDictObject *)op, key, value, hash); @@ -2899,13 +2916,18 @@ PyDict_DelItem(PyObject *op, PyObject *key) int _PyDict_DelItem_KnownHash_LockHeld(PyObject *op, PyObject *key, Py_hash_t hash) { - Py_ssize_t ix; - PyObject *old_value; - if (!PyDict_Check(op)) { - PyErr_BadInternalCall(); + if (PyFrozenDict_Check(op)) { + frozendict_does_not_support("deletion"); + } + else { + PyErr_BadInternalCall(); + } return -1; } + + Py_ssize_t ix; + PyObject *old_value; PyDictObject *mp = (PyDictObject *)op; assert(can_modify_dict(mp)); @@ -3206,7 +3228,12 @@ pop_lock_held(PyObject *op, PyObject *key, PyObject **result) if (result) { *result = NULL; } - PyErr_BadInternalCall(); + if (PyFrozenDict_Check(op)) { + frozendict_does_not_support("deletion"); + } + else { + PyErr_BadInternalCall(); + } return -1; } PyDictObject *dict = (PyDictObject *)op; @@ -4017,7 +4044,12 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override) assert(d != NULL); assert(seq2 != NULL); if (!PyDict_Check(d)) { - PyErr_BadInternalCall(); + if (PyFrozenDict_Check(d)) { + frozendict_does_not_support("assignment"); + } + else { + PyErr_BadInternalCall(); + } return -1; } @@ -4220,7 +4252,12 @@ dict_merge_api(PyObject *a, PyObject *b, int override) * PyMapping_Keys() and PyObject_GetItem() be supported. */ if (a == NULL || !PyDict_Check(a) || b == NULL) { - PyErr_BadInternalCall(); + if (a != NULL && PyFrozenDict_Check(a)) { + frozendict_does_not_support("assignment"); + } + else { + PyErr_BadInternalCall(); + } return -1; } return dict_merge(a, b, override); @@ -4596,13 +4633,13 @@ static int dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_value, PyObject **result, int incref_result) { - PyDictObject *mp = (PyDictObject *)d; - PyObject *value; - Py_hash_t hash; - Py_ssize_t ix; - if (!PyDict_Check(d)) { - PyErr_BadInternalCall(); + if (PyFrozenDict_Check(d)) { + frozendict_does_not_support("assignment"); + } + else { + PyErr_BadInternalCall(); + } if (result) { *result = NULL; } @@ -4610,6 +4647,11 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu } assert(can_modify_dict((PyDictObject*)d)); + PyDictObject *mp = (PyDictObject *)d; + PyObject *value; + Py_hash_t hash; + Py_ssize_t ix; + hash = _PyObject_HashFast(key); if (hash == -1) { dict_unhashable_type(d, key); @@ -7154,7 +7196,17 @@ int _PyDict_SetItem_LockHeld(PyDictObject *dict, PyObject *name, PyObject *value) { if (!PyDict_Check(dict)) { - PyErr_BadInternalCall(); + if (PyFrozenDict_Check(dict)) { + if (value == NULL) { + frozendict_does_not_support("deletion"); + } + else { + frozendict_does_not_support("assignment"); + } + } + else { + PyErr_BadInternalCall(); + } return -1; }