Skip to content

Commit d8effd4

Browse files
vstinnermohsinm-dev
vstinner
and
mohsinm-dev
committed
gh-141510: Raise TypeError in PyDict_SetItem() on frozendict
If the following functions get an unexpected frozendict, raise a TypeError with an useful error message: * 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 <mohsin.mdev@gmail.com>
1 parent 0c29f83 commit d8effd4

File tree

2 files changed

+124
-32
lines changed

2 files changed

+124
-32
lines changed

‎Lib/test/test_capi/test_dict.py‎

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import contextlib
12
import unittest
23
from collections import OrderedDict, UserDict
34
from types import MappingProxyType
@@ -258,6 +259,12 @@ def test_dict_contains_string(self):
258259
# CRASHES contains({}, NULL)
259260
# CRASHES contains(NULL, b'a')
260261

262+
@contextlib.contextmanager
263+
def frozendict_does_not_support(self, what):
264+
errmsg = f'frozendict object does not support item {what}'
265+
with self.assertRaisesRegex(TypeError, errmsg):
266+
yield
267+
261268
def test_dict_setitem(self):
262269
# Test PyDict_SetItem()
263270
setitem = _testlimitedcapi.dict_setitem
@@ -269,7 +276,10 @@ def test_dict_setitem(self):
269276
self.assertEqual(dct, {'a': 5, '\U0001f40d': 8})
270277

271278
self.assertRaises(TypeError, setitem, {}, [], 5) # unhashable
272-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
279+
for test_type in FROZENDICT_TYPES:
280+
with self.frozendict_does_not_support('assignment'):
281+
setitem(test_type(), 'a', 5)
282+
for test_type in MAPPING_TYPES + OTHER_TYPES:
273283
self.assertRaises(SystemError, setitem, test_type(), 'a', 5)
274284
# CRASHES setitem({}, NULL, 5)
275285
# CRASHES setitem({}, 'a', NULL)
@@ -286,7 +296,10 @@ def test_dict_setitemstring(self):
286296
self.assertEqual(dct, {'a': 5, '\U0001f40d': 8})
287297

288298
self.assertRaises(UnicodeDecodeError, setitemstring, {}, INVALID_UTF8, 5)
289-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
299+
for test_type in FROZENDICT_TYPES:
300+
with self.frozendict_does_not_support('assignment'):
301+
setitemstring(test_type(), b'a', 5)
302+
for test_type in MAPPING_TYPES + OTHER_TYPES:
290303
self.assertRaises(SystemError, setitemstring, test_type(), b'a', 5)
291304
# CRASHES setitemstring({}, NULL, 5)
292305
# CRASHES setitemstring({}, b'a', NULL)
@@ -304,7 +317,10 @@ def test_dict_delitem(self):
304317
self.assertEqual(dct, {'c': 2})
305318

306319
self.assertRaises(TypeError, delitem, {}, []) # unhashable
307-
for test_type in NOT_DICT_TYPES:
320+
for test_type in FROZENDICT_TYPES:
321+
with self.frozendict_does_not_support('deletion'):
322+
delitem(test_type({'a': 1}), 'a')
323+
for test_type in MAPPING_TYPES:
308324
self.assertRaises(SystemError, delitem, test_type({'a': 1}), 'a')
309325
for test_type in OTHER_TYPES:
310326
self.assertRaises(SystemError, delitem, test_type(), 'a')
@@ -323,7 +339,10 @@ def test_dict_delitemstring(self):
323339
self.assertEqual(dct, {'c': 2})
324340

325341
self.assertRaises(UnicodeDecodeError, delitemstring, {}, INVALID_UTF8)
326-
for test_type in NOT_DICT_TYPES:
342+
for test_type in FROZENDICT_TYPES:
343+
with self.frozendict_does_not_support('deletion'):
344+
delitemstring(test_type({'a': 1}), b'a')
345+
for test_type in MAPPING_TYPES:
327346
self.assertRaises(SystemError, delitemstring, test_type({'a': 1}), b'a')
328347
for test_type in OTHER_TYPES:
329348
self.assertRaises(SystemError, delitemstring, test_type(), b'a')
@@ -341,7 +360,10 @@ def test_dict_setdefault(self):
341360
self.assertEqual(dct, {'a': 5})
342361

343362
self.assertRaises(TypeError, setdefault, {}, [], 5) # unhashable
344-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
363+
for test_type in FROZENDICT_TYPES:
364+
with self.frozendict_does_not_support('assignment'):
365+
setdefault(test_type(), 'a', 5)
366+
for test_type in MAPPING_TYPES + OTHER_TYPES:
345367
self.assertRaises(SystemError, setdefault, test_type(), 'a', 5)
346368
# CRASHES setdefault({}, NULL, 5)
347369
# CRASHES setdefault({}, 'a', NULL)
@@ -358,7 +380,10 @@ def test_dict_setdefaultref(self):
358380
self.assertEqual(dct, {'a': 5})
359381

360382
self.assertRaises(TypeError, setdefault, {}, [], 5) # unhashable
361-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
383+
for test_type in FROZENDICT_TYPES:
384+
with self.frozendict_does_not_support('assignment'):
385+
setdefault(test_type(), 'a', 5)
386+
for test_type in MAPPING_TYPES + OTHER_TYPES:
362387
self.assertRaises(SystemError, setdefault, test_type(), 'a', 5)
363388
# CRASHES setdefault({}, NULL, 5)
364389
# CRASHES setdefault({}, 'a', NULL)
@@ -424,7 +449,10 @@ def test_dict_update(self):
424449

425450
self.assertRaises(AttributeError, update, {}, [])
426451
self.assertRaises(AttributeError, update, {}, 42)
427-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
452+
for test_type in FROZENDICT_TYPES:
453+
with self.frozendict_does_not_support('assignment'):
454+
update(test_type(), {})
455+
for test_type in MAPPING_TYPES + OTHER_TYPES:
428456
self.assertRaises(SystemError, update, test_type(), {})
429457
self.assertRaises(SystemError, update, {}, NULL)
430458
self.assertRaises(SystemError, update, NULL, {})
@@ -443,7 +471,10 @@ def test_dict_merge(self):
443471

444472
self.assertRaises(AttributeError, merge, {}, [], 0)
445473
self.assertRaises(AttributeError, merge, {}, 42, 0)
446-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
474+
for test_type in FROZENDICT_TYPES:
475+
with self.frozendict_does_not_support('assignment'):
476+
merge(test_type(), {}, 0)
477+
for test_type in MAPPING_TYPES + OTHER_TYPES:
447478
self.assertRaises(SystemError, merge, test_type(), {}, 0)
448479
self.assertRaises(SystemError, merge, {}, NULL, 0)
449480
self.assertRaises(SystemError, merge, NULL, {}, 0)
@@ -464,7 +495,10 @@ def test_dict_mergefromseq2(self):
464495
self.assertRaises(ValueError, mergefromseq2, {}, [(1, 2, 3)], 0)
465496
self.assertRaises(TypeError, mergefromseq2, {}, [1], 0)
466497
self.assertRaises(TypeError, mergefromseq2, {}, 42, 0)
467-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
498+
for test_type in FROZENDICT_TYPES:
499+
with self.frozendict_does_not_support('assignment'):
500+
mergefromseq2(test_type(), [], 0)
501+
for test_type in MAPPING_TYPES + OTHER_TYPES:
468502
self.assertRaises(SystemError, mergefromseq2, test_type(), [], 0)
469503
# CRASHES mergefromseq2({}, NULL, 0)
470504
# CRASHES mergefromseq2(NULL, {}, 0)
@@ -511,7 +545,10 @@ def test_dict_pop(self):
511545
dict_pop(mydict, not_hashable_key)
512546

513547
# wrong dict type
514-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
548+
for test_type in FROZENDICT_TYPES:
549+
with self.frozendict_does_not_support('deletion'):
550+
dict_pop(test_type(), "key")
551+
for test_type in MAPPING_TYPES + OTHER_TYPES:
515552
not_dict = test_type()
516553
self.assertRaises(SystemError, dict_pop, not_dict, "key")
517554
self.assertRaises(SystemError, dict_pop_null, not_dict, "key")
@@ -560,7 +597,10 @@ def test_dict_popstring(self):
560597
self.assertRaises(UnicodeDecodeError, dict_popstring_null, mydict, INVALID_UTF8)
561598

562599
# wrong dict type
563-
for test_type in NOT_DICT_TYPES + OTHER_TYPES:
600+
for test_type in FROZENDICT_TYPES:
601+
with self.frozendict_does_not_support('deletion'):
602+
dict_popstring(test_type(), "key")
603+
for test_type in MAPPING_TYPES + OTHER_TYPES:
564604
not_dict = test_type()
565605
self.assertRaises(SystemError, dict_popstring, not_dict, "key")
566606
self.assertRaises(SystemError, dict_popstring_null, not_dict, "key")

‎Objects/dictobject.c‎

Lines changed: 73 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2719,6 +2719,10 @@ _PyDict_LoadBuiltinsFromGlobals(PyObject *globals)
27192719
return builtins;
27202720
}
27212721

2722+
#define frozendict_does_not_support(WHAT) \
2723+
PyErr_SetString(PyExc_TypeError, "frozendict object does " \
2724+
"not support item " WHAT)
2725+
27222726
/* Consumes references to key and value */
27232727
static int
27242728
setitem_take2_lock_held(PyDictObject *mp, PyObject *key, PyObject *value)
@@ -2762,12 +2766,19 @@ _PyDict_SetItem_Take2(PyDictObject *mp, PyObject *key, PyObject *value)
27622766
int
27632767
PyDict_SetItem(PyObject *op, PyObject *key, PyObject *value)
27642768
{
2769+
assert(key);
2770+
assert(value);
2771+
27652772
if (!PyDict_Check(op)) {
2766-
PyErr_BadInternalCall();
2773+
if (PyFrozenDict_Check(op)) {
2774+
frozendict_does_not_support("assignment");
2775+
}
2776+
else {
2777+
PyErr_BadInternalCall();
2778+
}
27672779
return -1;
27682780
}
2769-
assert(key);
2770-
assert(value);
2781+
27712782
return _PyDict_SetItem_Take2((PyDictObject *)op,
27722783
Py_NewRef(key), Py_NewRef(value));
27732784
}
@@ -2807,14 +2818,20 @@ int
28072818
_PyDict_SetItem_KnownHash(PyObject *op, PyObject *key, PyObject *value,
28082819
Py_hash_t hash)
28092820
{
2810-
if (!PyDict_Check(op)) {
2811-
PyErr_BadInternalCall();
2812-
return -1;
2813-
}
28142821
assert(key);
28152822
assert(value);
28162823
assert(hash != -1);
28172824

2825+
if (!PyDict_Check(op)) {
2826+
if (PyFrozenDict_Check(op)) {
2827+
frozendict_does_not_support("assignment");
2828+
}
2829+
else {
2830+
PyErr_BadInternalCall();
2831+
}
2832+
return -1;
2833+
}
2834+
28182835
int res;
28192836
Py_BEGIN_CRITICAL_SECTION(op);
28202837
res = _PyDict_SetItem_KnownHash_LockHeld((PyDictObject *)op, key, value, hash);
@@ -2899,13 +2916,18 @@ PyDict_DelItem(PyObject *op, PyObject *key)
28992916
int
29002917
_PyDict_DelItem_KnownHash_LockHeld(PyObject *op, PyObject *key, Py_hash_t hash)
29012918
{
2902-
Py_ssize_t ix;
2903-
PyObject *old_value;
2904-
29052919
if (!PyDict_Check(op)) {
2906-
PyErr_BadInternalCall();
2920+
if (PyFrozenDict_Check(op)) {
2921+
frozendict_does_not_support("deletion");
2922+
}
2923+
else {
2924+
PyErr_BadInternalCall();
2925+
}
29072926
return -1;
29082927
}
2928+
2929+
Py_ssize_t ix;
2930+
PyObject *old_value;
29092931
PyDictObject *mp = (PyDictObject *)op;
29102932
assert(can_modify_dict(mp));
29112933

@@ -3206,7 +3228,12 @@ pop_lock_held(PyObject *op, PyObject *key, PyObject **result)
32063228
if (result) {
32073229
*result = NULL;
32083230
}
3209-
PyErr_BadInternalCall();
3231+
if (PyFrozenDict_Check(op)) {
3232+
frozendict_does_not_support("deletion");
3233+
}
3234+
else {
3235+
PyErr_BadInternalCall();
3236+
}
32103237
return -1;
32113238
}
32123239
PyDictObject *dict = (PyDictObject *)op;
@@ -4017,7 +4044,12 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
40174044
assert(d != NULL);
40184045
assert(seq2 != NULL);
40194046
if (!PyDict_Check(d)) {
4020-
PyErr_BadInternalCall();
4047+
if (PyFrozenDict_Check(d)) {
4048+
frozendict_does_not_support("assignment");
4049+
}
4050+
else {
4051+
PyErr_BadInternalCall();
4052+
}
40214053
return -1;
40224054
}
40234055

@@ -4220,7 +4252,12 @@ dict_merge_api(PyObject *a, PyObject *b, int override)
42204252
* PyMapping_Keys() and PyObject_GetItem() be supported.
42214253
*/
42224254
if (a == NULL || !PyDict_Check(a) || b == NULL) {
4223-
PyErr_BadInternalCall();
4255+
if (a != NULL && PyFrozenDict_Check(a)) {
4256+
frozendict_does_not_support("assignment");
4257+
}
4258+
else {
4259+
PyErr_BadInternalCall();
4260+
}
42244261
return -1;
42254262
}
42264263
return dict_merge(a, b, override);
@@ -4596,20 +4633,25 @@ static int
45964633
dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_value,
45974634
PyObject **result, int incref_result)
45984635
{
4599-
PyDictObject *mp = (PyDictObject *)d;
4600-
PyObject *value;
4601-
Py_hash_t hash;
4602-
Py_ssize_t ix;
4603-
46044636
if (!PyDict_Check(d)) {
4605-
PyErr_BadInternalCall();
4637+
if (PyFrozenDict_Check(d)) {
4638+
frozendict_does_not_support("assignment");
4639+
}
4640+
else {
4641+
PyErr_BadInternalCall();
4642+
}
46064643
if (result) {
46074644
*result = NULL;
46084645
}
46094646
return -1;
46104647
}
46114648
assert(can_modify_dict((PyDictObject*)d));
46124649

4650+
PyDictObject *mp = (PyDictObject *)d;
4651+
PyObject *value;
4652+
Py_hash_t hash;
4653+
Py_ssize_t ix;
4654+
46134655
hash = _PyObject_HashFast(key);
46144656
if (hash == -1) {
46154657
dict_unhashable_type(d, key);
@@ -7122,7 +7164,17 @@ int
71227164
_PyDict_SetItem_LockHeld(PyDictObject *dict, PyObject *name, PyObject *value)
71237165
{
71247166
if (!PyDict_Check(dict)) {
7125-
PyErr_BadInternalCall();
7167+
if (PyFrozenDict_Check(dict)) {
7168+
if (value == NULL) {
7169+
frozendict_does_not_support("deletion");
7170+
}
7171+
else {
7172+
frozendict_does_not_support("assignment");
7173+
}
7174+
}
7175+
else {
7176+
PyErr_BadInternalCall();
7177+
}
71267178
return -1;
71277179
}
71287180

0 commit comments

Comments
 (0)