Skip to content

Commit fb5db7e

Browse files
serhiy-storchakaserhiy-storchaka
authored
bpo-42006: Stop using PyDict_GetItem, PyDict_GetItemString and _PyDict_GetItemId. (pythonGH-22648)
These functions are considered not safe because they suppress all internal errors and can return wrong result. PyDict_GetItemString and _PyDict_GetItemId can also silence current exception in rare cases. Remove no longer used _PyDict_GetItemId. Add _PyDict_ContainsId and rename _PyDict_Contains into _PyDict_Contains_KnownHash.
1 parent 96a9eed commit fb5db7e

File tree

17 files changed

+254
-137
lines changed

17 files changed

+254
-137
lines changed

‎Include/cpython/dictobject.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ PyAPI_FUNC(int) _PyDict_Next(
4646

4747
/* Get the number of items of a dictionary. */
4848
#define PyDict_GET_SIZE(mp) (assert(PyDict_Check(mp)),((PyDictObject *)mp)->ma_used)
49-
PyAPI_FUNC(int) _PyDict_Contains(PyObject *mp, PyObject *key, Py_hash_t hash);
49+
PyAPI_FUNC(int) _PyDict_Contains_KnownHash(PyObject *, PyObject *, Py_hash_t);
50+
PyAPI_FUNC(int) _PyDict_ContainsId(PyObject *, struct _Py_Identifier *);
5051
PyAPI_FUNC(PyObject *) _PyDict_NewPresized(Py_ssize_t minused);
5152
PyAPI_FUNC(void) _PyDict_MaybeUntrack(PyObject *mp);
5253
PyAPI_FUNC(int) _PyDict_HasOnlyStringKeys(PyObject *mp);
@@ -63,7 +64,6 @@ PyObject *_PyDict_FromKeys(PyObject *, PyObject *, PyObject *);
6364
argument is raised.
6465
*/
6566
PyAPI_FUNC(int) _PyDict_MergeEx(PyObject *mp, PyObject *other, int override);
66-
PyAPI_FUNC(PyObject *) _PyDict_GetItemId(PyObject *dp, struct _Py_Identifier *key);
6767
PyAPI_FUNC(int) _PyDict_SetItemId(PyObject *dp, struct _Py_Identifier *key, PyObject *item);
6868

6969
PyAPI_FUNC(int) _PyDict_DelItemId(PyObject *mp, struct _Py_Identifier *key);

‎Modules/_decimal/_decimal.c‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3186,6 +3186,31 @@ dotsep_as_utf8(const char *s)
31863186
return utf8;
31873187
}
31883188

3189+
static int
3190+
dict_get_item_string(PyObject *dict, const char *key, PyObject **valueobj, const char **valuestr)
3191+
{
3192+
*valueobj = NULL;
3193+
PyObject *keyobj = PyUnicode_FromString(key);
3194+
if (keyobj == NULL) {
3195+
return -1;
3196+
}
3197+
PyObject *value = PyDict_GetItemWithError(dict, keyobj);
3198+
Py_DECREF(keyobj);
3199+
if (value == NULL) {
3200+
if (PyErr_Occurred()) {
3201+
return -1;
3202+
}
3203+
return 0;
3204+
}
3205+
value = PyUnicode_AsUTF8String(value);
3206+
if (value == NULL) {
3207+
return -1;
3208+
}
3209+
*valueobj = value;
3210+
*valuestr = PyBytes_AS_STRING(value);
3211+
return 0;
3212+
}
3213+
31893214
/* Formatted representation of a PyDecObject. */
31903215
static PyObject *
31913216
dec_format(PyObject *dec, PyObject *args)
@@ -3256,23 +3281,11 @@ dec_format(PyObject *dec, PyObject *args)
32563281
"optional argument must be a dict");
32573282
goto finish;
32583283
}
3259-
if ((dot = PyDict_GetItemString(override, "decimal_point"))) {
3260-
if ((dot = PyUnicode_AsUTF8String(dot)) == NULL) {
3261-
goto finish;
3262-
}
3263-
spec.dot = PyBytes_AS_STRING(dot);
3264-
}
3265-
if ((sep = PyDict_GetItemString(override, "thousands_sep"))) {
3266-
if ((sep = PyUnicode_AsUTF8String(sep)) == NULL) {
3267-
goto finish;
3268-
}
3269-
spec.sep = PyBytes_AS_STRING(sep);
3270-
}
3271-
if ((grouping = PyDict_GetItemString(override, "grouping"))) {
3272-
if ((grouping = PyUnicode_AsUTF8String(grouping)) == NULL) {
3273-
goto finish;
3274-
}
3275-
spec.grouping = PyBytes_AS_STRING(grouping);
3284+
if (dict_get_item_string(override, "decimal_point", &dot, &spec.dot) ||
3285+
dict_get_item_string(override, "thousands_sep", &sep, &spec.sep) ||
3286+
dict_get_item_string(override, "grouping", &grouping, &spec.grouping))
3287+
{
3288+
goto finish;
32763289
}
32773290
if (mpd_validate_lconv(&spec) < 0) {
32783291
PyErr_SetString(PyExc_ValueError,

‎Modules/_threadmodule.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -816,10 +816,14 @@ local_clear(localobject *self)
816816
for(tstate = PyInterpreterState_ThreadHead(tstate->interp);
817817
tstate;
818818
tstate = PyThreadState_Next(tstate))
819-
if (tstate->dict && PyDict_GetItem(tstate->dict, self->key)) {
820-
if (PyDict_DelItem(tstate->dict, self->key)) {
819+
if (tstate->dict) {
820+
PyObject *v = _PyDict_Pop(tstate->dict, self->key, Py_None);
821+
if (v == NULL) {
821822
PyErr_Clear();
822823
}
824+
else {
825+
Py_DECREF(v);
826+
}
823827
}
824828
}
825829
return 0;

‎Modules/_zoneinfo.c‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -722,17 +722,16 @@ zoneinfo__unpickle(PyTypeObject *cls, PyObject *args)
722722
static PyObject *
723723
load_timedelta(long seconds)
724724
{
725-
PyObject *rv = NULL;
725+
PyObject *rv;
726726
PyObject *pyoffset = PyLong_FromLong(seconds);
727727
if (pyoffset == NULL) {
728728
return NULL;
729729
}
730-
int contains = PyDict_Contains(TIMEDELTA_CACHE, pyoffset);
731-
if (contains == -1) {
732-
goto error;
733-
}
734-
735-
if (!contains) {
730+
rv = PyDict_GetItemWithError(TIMEDELTA_CACHE, pyoffset);
731+
if (rv == NULL) {
732+
if (PyErr_Occurred()) {
733+
goto error;
734+
}
736735
PyObject *tmp = PyDateTimeAPI->Delta_FromDelta(
737736
0, seconds, 0, 1, PyDateTimeAPI->DeltaType);
738737

@@ -743,12 +742,9 @@ load_timedelta(long seconds)
743742
rv = PyDict_SetDefault(TIMEDELTA_CACHE, pyoffset, tmp);
744743
Py_DECREF(tmp);
745744
}
746-
else {
747-
rv = PyDict_GetItem(TIMEDELTA_CACHE, pyoffset);
748-
}
749745

746+
Py_XINCREF(rv);
750747
Py_DECREF(pyoffset);
751-
Py_INCREF(rv);
752748
return rv;
753749
error:
754750
Py_DECREF(pyoffset);

‎Modules/signalmodule.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,10 +1427,9 @@ signal_exec(PyObject *m)
14271427
return -1;
14281428
#endif
14291429

1430-
IntHandler = PyDict_GetItemString(d, "default_int_handler");
1430+
IntHandler = PyMapping_GetItemString(d, "default_int_handler");
14311431
if (!IntHandler)
14321432
return -1;
1433-
Py_INCREF(IntHandler);
14341433

14351434
_Py_atomic_store_relaxed(&Handlers[0].tripped, 0);
14361435
for (int i = 1; i < NSIG; i++) {

‎Modules/socketmodule.c‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ static FlagRuntimeInfo win_runtime_flags[] = {
324324
{14393, "TCP_FASTOPEN"}
325325
};
326326

327-
static void
327+
static int
328328
remove_unusable_flags(PyObject *m)
329329
{
330330
PyObject *dict;
@@ -333,7 +333,7 @@ remove_unusable_flags(PyObject *m)
333333

334334
dict = PyModule_GetDict(m);
335335
if (dict == NULL) {
336-
return;
336+
return -1;
337337
}
338338

339339
/* set to Windows 10, except BuildNumber. */
@@ -359,19 +359,19 @@ remove_unusable_flags(PyObject *m)
359359
break;
360360
}
361361
else {
362-
if (PyDict_GetItemString(
363-
dict,
364-
win_runtime_flags[i].flag_name) != NULL)
365-
{
366-
if (PyDict_DelItemString(
367-
dict,
368-
win_runtime_flags[i].flag_name))
369-
{
370-
PyErr_Clear();
371-
}
362+
PyObject *flag_name = PyUnicode_FromString(win_runtime_flags[i].flag_name);
363+
if (flag_name == NULL) {
364+
return -1;
365+
}
366+
PyObject *v = _PyDict_Pop(dict, flag_name, Py_None);
367+
Py_DECREF(flag_name);
368+
if (v == NULL) {
369+
return -1;
372370
}
371+
Py_DECREF(v);
373372
}
374373
}
374+
return 0;
375375
}
376376

377377
#endif
@@ -8382,7 +8382,10 @@ PyInit__socket(void)
83828382

83838383
#ifdef MS_WINDOWS
83848384
/* remove some flags on older version Windows during run-time */
8385-
remove_unusable_flags(m);
8385+
if (remove_unusable_flags(m) < 0) {
8386+
Py_DECREF(m);
8387+
return NULL;
8388+
}
83868389
#endif
83878390

83888391
return m;

‎Objects/dictobject.c‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3434,7 +3434,7 @@ PyDict_Contains(PyObject *op, PyObject *key)
34343434

34353435
/* Internal version of PyDict_Contains used when the hash value is already known */
34363436
int
3437-
_PyDict_Contains(PyObject *op, PyObject *key, Py_hash_t hash)
3437+
_PyDict_Contains_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash)
34383438
{
34393439
PyDictObject *mp = (PyDictObject *)op;
34403440
PyObject *value;
@@ -3446,6 +3446,16 @@ _PyDict_Contains(PyObject *op, PyObject *key, Py_hash_t hash)
34463446
return (ix != DKIX_EMPTY && value != NULL);
34473447
}
34483448

3449+
int
3450+
_PyDict_ContainsId(PyObject *op, struct _Py_Identifier *key)
3451+
{
3452+
PyObject *kv = _PyUnicode_FromId(key); /* borrowed */
3453+
if (kv == NULL) {
3454+
return -1;
3455+
}
3456+
return PyDict_Contains(op, kv);
3457+
}
3458+
34493459
/* Hack to implement "key in dict" */
34503460
static PySequenceMethods dict_as_sequence = {
34513461
0, /* sq_length */
@@ -3590,18 +3600,6 @@ PyTypeObject PyDict_Type = {
35903600
.tp_vectorcall = dict_vectorcall,
35913601
};
35923602

3593-
PyObject *
3594-
_PyDict_GetItemId(PyObject *dp, struct _Py_Identifier *key)
3595-
{
3596-
PyObject *kv;
3597-
kv = _PyUnicode_FromId(key); /* borrowed */
3598-
if (kv == NULL) {
3599-
PyErr_Clear();
3600-
return NULL;
3601-
}
3602-
return PyDict_GetItem(dp, kv);
3603-
}
3604-
36053603
/* For backward compatibility with old dictionary interface */
36063604

36073605
PyObject *

‎Objects/moduleobject.c‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -477,10 +477,12 @@ PyModule_GetNameObject(PyObject *m)
477477
}
478478
d = ((PyModuleObject *)m)->md_dict;
479479
if (d == NULL ||
480-
(name = _PyDict_GetItemId(d, &PyId___name__)) == NULL ||
480+
(name = _PyDict_GetItemIdWithError(d, &PyId___name__)) == NULL ||
481481
!PyUnicode_Check(name))
482482
{
483-
PyErr_SetString(PyExc_SystemError, "nameless module");
483+
if (!PyErr_Occurred()) {
484+
PyErr_SetString(PyExc_SystemError, "nameless module");
485+
}
484486
return NULL;
485487
}
486488
Py_INCREF(name);
@@ -509,10 +511,12 @@ PyModule_GetFilenameObject(PyObject *m)
509511
}
510512
d = ((PyModuleObject *)m)->md_dict;
511513
if (d == NULL ||
512-
(fileobj = _PyDict_GetItemId(d, &PyId___file__)) == NULL ||
514+
(fileobj = _PyDict_GetItemIdWithError(d, &PyId___file__)) == NULL ||
513515
!PyUnicode_Check(fileobj))
514516
{
515-
PyErr_SetString(PyExc_SystemError, "module filename missing");
517+
if (!PyErr_Occurred()) {
518+
PyErr_SetString(PyExc_SystemError, "module filename missing");
519+
}
516520
return NULL;
517521
}
518522
Py_INCREF(fileobj);
@@ -721,14 +725,21 @@ module_getattro(PyModuleObject *m, PyObject *name)
721725
PyErr_Clear();
722726
if (m->md_dict) {
723727
_Py_IDENTIFIER(__getattr__);
724-
getattr = _PyDict_GetItemId(m->md_dict, &PyId___getattr__);
728+
getattr = _PyDict_GetItemIdWithError(m->md_dict, &PyId___getattr__);
725729
if (getattr) {
726730
return PyObject_CallOneArg(getattr, name);
727731
}
728-
mod_name = _PyDict_GetItemId(m->md_dict, &PyId___name__);
732+
if (PyErr_Occurred()) {
733+
return NULL;
734+
}
735+
mod_name = _PyDict_GetItemIdWithError(m->md_dict, &PyId___name__);
729736
if (mod_name && PyUnicode_Check(mod_name)) {
730737
Py_INCREF(mod_name);
731-
PyObject *spec = _PyDict_GetItemId(m->md_dict, &PyId___spec__);
738+
PyObject *spec = _PyDict_GetItemIdWithError(m->md_dict, &PyId___spec__);
739+
if (spec == NULL && PyErr_Occurred()) {
740+
Py_DECREF(mod_name);
741+
return NULL;
742+
}
732743
Py_XINCREF(spec);
733744
if (_PyModuleSpec_IsInitializing(spec)) {
734745
PyErr_Format(PyExc_AttributeError,
@@ -746,6 +757,9 @@ module_getattro(PyModuleObject *m, PyObject *name)
746757
Py_DECREF(mod_name);
747758
return NULL;
748759
}
760+
else if (PyErr_Occurred()) {
761+
return NULL;
762+
}
749763
}
750764
PyErr_Format(PyExc_AttributeError,
751765
"module has no attribute '%U'", name);

‎Objects/setobject.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1498,7 +1498,7 @@ set_difference(PySetObject *so, PyObject *other)
14981498
while (set_next(so, &pos, &entry)) {
14991499
key = entry->key;
15001500
hash = entry->hash;
1501-
rv = _PyDict_Contains(other, key, hash);
1501+
rv = _PyDict_Contains_KnownHash(other, key, hash);
15021502
if (rv < 0) {
15031503
Py_DECREF(result);
15041504
return NULL;

0 commit comments

Comments
 (0)