https://github.com/python/cpython/commit/713be70175a2dad477a1cf5e7c00bab0edda04ad commit: 713be70175a2dad477a1cf5e7c00bab0edda04ad branch: main author: Victor Stinner <[email protected]> committer: vstinner <[email protected]> date: 2026-03-11T17:05:09+01:00 summary:
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 <[email protected]> files: M Lib/test/test_capi/test_dict.py M Objects/dictobject.c diff --git a/Lib/test/test_capi/test_dict.py b/Lib/test/test_capi/test_dict.py index cd46fea5476ca6..4cf404e9f56327 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 @@ def test_dict_contains_string(self): # 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 @@ def test_dict_setitem(self): 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 @@ def test_dict_setitemstring(self): 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 @@ def test_dict_delitem(self): 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 @@ def test_dict_delitemstring(self): 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 @@ def test_dict_setdefault(self): 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 @@ def test_dict_setdefaultref(self): 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 @@ def test_dict_update(self): 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 @@ def test_dict_merge(self): 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 @@ 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) - 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 @@ def test_dict_pop(self): 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 @@ def test_dict_popstring(self): 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 b5f2a682c54982..842d9be73b8792 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; } _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
