FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-155733: Validate keyword argument keys in operator.methodcaller and functools.partial by pranavchoudhary-tech · Pull Request #155779 · python/cpython · GitHub

/ cpython Public
3 changes: 3 additions & 0 deletions Lib/functools.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,9 @@ def __setstate__(self, state):
(namespace is not None and not isinstance(namespace, dict))):
raise TypeError("invalid partial state")

if kwds is not None and any(not isinstance(k, str) for k in kwds):
raise TypeError("keywords must be strings")
Comment on lines +420 to +421

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Not sure if you really need this check. You'll get the error when calling the function anyway. For instance, we don't check that Placeholder isn't passed as a keyword value either.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Makes sense. Should I remove the explicit check from Lib/functools.py and rely on the standard unpacking error?


if args and args[-1] is Placeholder:
raise TypeError("trailing Placeholders are not allowed")
phcount, merger = _partial_prepare_merger(args)
Expand Down
11 changes: 11 additions & 0 deletions Lib/test/test_functools.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,17 @@ def test_keyword(self):
empty, got = p(x=None)
self.assertTrue(expected == got and empty == ())

def test_non_string_keywords(self):
with self.assertRaisesRegex(TypeError, "keywords must be strings"):
self.partial(capture, **{1: 'x'})
p = self.partial(capture)
p.keywords[1] = 'x'
with self.assertRaisesRegex(TypeError, "keywords must be strings"):
p()
p2 = self.partial(capture)
with self.assertRaisesRegex(TypeError, "keywords must be strings"):
p2.__setstate__((capture, (), {1: 'x'}, None))

def test_no_side_effects(self):
# make sure there are no side effects that affect subsequent calls
p = self.partial(capture, 0, a=1)
Expand Down
5 changes: 5 additions & 0 deletions Lib/test/test_operator.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,11 @@ def return_arguments(self, *args, **kwds):
f = operator.methodcaller('return_arguments', *many_positional_arguments, **many_kw_arguments)
self.assertEqual(f(a), (many_positional_arguments, many_kw_arguments))

def test_non_string_keywords(self):
operator = self.module
with self.assertRaisesRegex(TypeError, "keywords must be strings"):
operator.methodcaller('x', **{1: 'x'})

def test_inplace(self):
operator = self.module
class C(object):
Expand Down
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Validate keyword arguments to :func:`operator.methodcaller` and :func:`functools.partial` to raise a :exc:`TypeError` instead of crashing with non-string keys.
32 changes: 29 additions & 3 deletions Modules/_functoolsmodule.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,16 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw)
return NULL;
}

/* keyword Placeholder prohibition */
/* keyword Placeholder prohibition and key type validation */
if (kw != NULL) {
PyObject *key, *val;
Py_ssize_t pos = 0;
while (PyDict_Next(kw, &pos, &key, &val)) {
if (!PyUnicode_Check(key)) {
PyErr_SetString(PyExc_TypeError,
"keywords must be strings");
return NULL;
}
if (val == phold) {
PyErr_SetString(PyExc_TypeError,
"Placeholder cannot be passed as a keyword argument");
Expand Down Expand Up @@ -500,18 +505,29 @@ partial_vectorcall(PyObject *self, PyObject *const *args,
PyTuple_SET_ITEM(tot_kwnames, pto_nkwds + i, key);
}

/* Copy pto_keywords with overlapping call keywords merged
* Note, tail is already coppied. */
Comment on lines -503 to -504

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is there a reason why we can remove this comment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Restored, thanks for catching that!

/* Copy pto_keywords with overlapping call keywords merged.
* Note, tail is already copied. */
Py_ssize_t pos = 0, i = 0;
PyObject *keyword_dict = n_merges ? pto_kw_merged : partial_keywords;
int valid_kwargs = 1;
Py_BEGIN_CRITICAL_SECTION(keyword_dict);
while (PyDict_Next(keyword_dict, &pos, &key, &val)) {
if (!PyUnicode_Check(key)) {
valid_kwargs = 0;
break;
}
assert(i < pto_nkwds);
PyTuple_SET_ITEM(tot_kwnames, i, Py_NewRef(key));
stack[tot_nargs + i] = val;
i++;
}
Py_END_CRITICAL_SECTION();
if (!valid_kwargs) {
PyErr_SetString(PyExc_TypeError, "keywords must be strings");
Py_XDECREF(pto_kw_merged);
Py_DECREF(tot_kwnames);
goto clean_stack;
}
assert(i == pto_nkwds);
Py_XDECREF(pto_kw_merged);

Expand Down Expand Up @@ -816,6 +832,16 @@ partial_setstate(PyObject *self, PyObject *state)
PyErr_SetString(PyExc_TypeError, "invalid partial state");
return NULL;
}
if (kw != Py_None) {
Py_ssize_t pos = 0;
PyObject *key, *val;
while (PyDict_Next(kw, &pos, &key, &val)) {
if (!PyUnicode_Check(key)) {
PyErr_SetString(PyExc_TypeError, "keywords must be strings");
return NULL;
}
}
}

Py_ssize_t nargs = PyTuple_GET_SIZE(fnargs);
if (nargs && PyTuple_GET_ITEM(fnargs, nargs - 1) == pto->placeholder) {
Expand Down
12 changes: 12 additions & 0 deletions Modules/_operator.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -1705,6 +1705,18 @@ methodcaller_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
return NULL;
}

if (kwds != NULL && PyDict_Check(kwds)) {
PyObject *key, *value;
Py_ssize_t pos = 0;
while (PyDict_Next(kwds, &pos, &key, &value)) {
if (!PyUnicode_Check(key)) {
PyErr_SetString(PyExc_TypeError,
"keywords must be strings");
return NULL;
}
}
}

_operator_state *state = _PyType_GetModuleState(type);
/* create methodcallerobject structure */
mc = PyObject_GC_New(methodcallerobject, (PyTypeObject *)state->methodcaller_type);
Expand Down
Loading

Back | FazBrowse Home | New Git URL