From f1ec3cefad4639797c37eaa8c074830188fa0a44 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 12 Jan 2019 10:12:24 +0200 Subject: [PATCH] bpo-35634: Raise an error when first passed kwargs contains duplicated keys. (GH-11438) --- Lib/test/test_extcall.py | 46 +++++++++ .../2019-01-05-18-39-49.bpo-35634.nVP_gs.rst | 3 + Python/ceval.c | 93 +++++++++---------- 3 files changed, 94 insertions(+), 48 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-01-05-18-39-49.bpo-35634.nVP_gs.rst diff --git a/Lib/test/test_extcall.py b/Lib/test/test_extcall.py index 2c1848337b2..a3ff4413eee 100644 --- a/Lib/test/test_extcall.py +++ b/Lib/test/test_extcall.py @@ -316,6 +316,52 @@ not function ... TypeError: dir() got multiple values for keyword argument 'b' +Test a kwargs mapping with duplicated keys. + + >>> from collections.abc import Mapping + >>> class MultiDict(Mapping): + ... def __init__(self, items): + ... self._items = items + ... + ... def __iter__(self): + ... return (k for k, v in self._items) + ... + ... def __getitem__(self, key): + ... for k, v in self._items: + ... if k == key: + ... return v + ... raise KeyError(key) + ... + ... def __len__(self): + ... return len(self._items) + ... + ... def keys(self): + ... return [k for k, v in self._items] + ... + ... def values(self): + ... return [v for k, v in self._items] + ... + ... def items(self): + ... return [(k, v) for k, v in self._items] + ... + >>> g(**MultiDict([('x', 1), ('y', 2)])) + 1 () {'y': 2} + + >>> g(**MultiDict([('x', 1), ('x', 2)])) + Traceback (most recent call last): + ... + TypeError: g() got multiple values for keyword argument 'x' + + >>> g(a=3, **MultiDict([('x', 1), ('x', 2)])) + Traceback (most recent call last): + ... + TypeError: g() got multiple values for keyword argument 'x' + + >>> g(**MultiDict([('a', 3)]), **MultiDict([('x', 1), ('x', 2)])) + Traceback (most recent call last): + ... + TypeError: g() got multiple values for keyword argument 'x' + Another helper function >>> def f2(*a, **b): diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-01-05-18-39-49.bpo-35634.nVP_gs.rst b/Misc/NEWS.d/next/Core and Builtins/2019-01-05-18-39-49.bpo-35634.nVP_gs.rst new file mode 100644 index 00000000000..24e57896065 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-01-05-18-39-49.bpo-35634.nVP_gs.rst @@ -0,0 +1,3 @@ +``func(**kwargs)`` will now raise an error when ``kwargs`` is a mapping +containing multiple entries with the same key. An error was already raised +when other keyword arguments are passed before ``**kwargs`` since Python 3.6. diff --git a/Python/ceval.c b/Python/ceval.c index 3e82ceb9525..3db7c7c92a0 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -70,7 +70,7 @@ static PyObject * unicode_concatenate(PyObject *, PyObject *, PyFrameObject *, const _Py_CODEUNIT *); static PyObject * special_lookup(PyObject *, _Py_Identifier *); static int check_args_iterable(PyObject *func, PyObject *vararg); -static void format_kwargs_mapping_error(PyObject *func, PyObject *kwargs); +static void format_kwargs_error(PyObject *func, PyObject *kwargs); static void format_awaitable_error(PyTypeObject *, int); #define NAME_ERROR_MSG \ @@ -2660,37 +2660,8 @@ main_loop: for (i = oparg; i > 0; i--) { PyObject *arg = PEEK(i); if (_PyDict_MergeEx(sum, arg, 2) < 0) { - PyObject *func = PEEK(2 + oparg); - if (PyErr_ExceptionMatches(PyExc_AttributeError)) { - format_kwargs_mapping_error(func, arg); - } - else if (PyErr_ExceptionMatches(PyExc_KeyError)) { - PyObject *exc, *val, *tb; - PyErr_Fetch(&exc, &val, &tb); - if (val && PyTuple_Check(val) && PyTuple_GET_SIZE(val) == 1) { - PyObject *key = PyTuple_GET_ITEM(val, 0); - if (!PyUnicode_Check(key)) { - PyErr_Format(PyExc_TypeError, - "%.200s%.200s keywords must be strings", - PyEval_GetFuncName(func), - PyEval_GetFuncDesc(func)); - } else { - PyErr_Format(PyExc_TypeError, - "%.200s%.200s got multiple " - "values for keyword argument '%U'", - PyEval_GetFuncName(func), - PyEval_GetFuncDesc(func), - key); - } - Py_XDECREF(exc); - Py_XDECREF(val); - Py_XDECREF(tb); - } - else { - PyErr_Restore(exc, val, tb); - } - } Py_DECREF(sum); + format_kwargs_error(PEEK(2 + oparg), arg); goto error; } } @@ -3286,17 +3257,9 @@ main_loop: PyObject *d = PyDict_New(); if (d == NULL) goto error; - if (PyDict_Update(d, kwargs) != 0) { + if (_PyDict_MergeEx(d, kwargs, 2) < 0) { Py_DECREF(d); - /* PyDict_Update raises attribute - * error (percolated from an attempt - * to get 'keys' attribute) instead of - * a type error if its second argument - * is not a mapping. - */ - if (PyErr_ExceptionMatches(PyExc_AttributeError)) { - format_kwargs_mapping_error(SECOND(), kwargs); - } + format_kwargs_error(SECOND(), kwargs); Py_DECREF(kwargs); goto error; } @@ -5063,14 +5026,48 @@ check_args_iterable(PyObject *func, PyObject *args) } static void -format_kwargs_mapping_error(PyObject *func, PyObject *kwargs) +format_kwargs_error(PyObject *func, PyObject *kwargs) { - PyErr_Format(PyExc_TypeError, - "%.200s%.200s argument after ** " - "must be a mapping, not %.200s", - PyEval_GetFuncName(func), - PyEval_GetFuncDesc(func), - kwargs->ob_type->tp_name); + /* _PyDict_MergeEx raises attribute + * error (percolated from an attempt + * to get 'keys' attribute) instead of + * a type error if its second argument + * is not a mapping. + */ + if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Format(PyExc_TypeError, + "%.200s%.200s argument after ** " + "must be a mapping, not %.200s", + PyEval_GetFuncName(func), + PyEval_GetFuncDesc(func), + kwargs->ob_type->tp_name); + } + else if (PyErr_ExceptionMatches(PyExc_KeyError)) { + PyObject *exc, *val, *tb; + PyErr_Fetch(&exc, &val, &tb); + if (val && PyTuple_Check(val) && PyTuple_GET_SIZE(val) == 1) { + PyObject *key = PyTuple_GET_ITEM(val, 0); + if (!PyUnicode_Check(key)) { + PyErr_Format(PyExc_TypeError, + "%.200s%.200s keywords must be strings", + PyEval_GetFuncName(func), + PyEval_GetFuncDesc(func)); + } else { + PyErr_Format(PyExc_TypeError, + "%.200s%.200s got multiple " + "values for keyword argument '%U'", + PyEval_GetFuncName(func), + PyEval_GetFuncDesc(func), + key); + } + Py_XDECREF(exc); + Py_XDECREF(val); + Py_XDECREF(tb); + } + else { + PyErr_Restore(exc, val, tb); + } + } } static void