Skip to content

Commit 563f058

Browse files
serhiy-storchakaserhiy-storchaka
authored
[3.10] gh-94938: Fix errror detection of unexpected keyword arguments (GH-94999) (GH-95354)
When keyword argument name is an instance of a str subclass with overloaded methods __eq__ and __hash__, the former code could not find the name of an extraneous keyword argument to report an error, and _PyArg_UnpackKeywords() returned success without setting the corresponding cell in the linearized arguments array. But since the number of expected initialized cells is determined as the total number of passed arguments, this lead to reading NULL as a keyword parameter value, that caused SystemError or crash or other undesired behavior.. (cherry picked from commit ebad53a) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent fb42214 commit 563f058

File tree

4 files changed

+110
-56
lines changed

4 files changed

+110
-56
lines changed

‎Lib/test/test_call.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,19 @@
1111
import contextlib
1212

1313

14+
class BadStr(str):
15+
def __eq__(self, other):
16+
return True
17+
def __hash__(self):
18+
# Guaranteed different hash
19+
return str.__hash__(self) ^ 3
20+
21+
def __eq__(self, other):
22+
return False
23+
def __hash__(self):
24+
return str.__hash__(self)
25+
26+
1427
class FunctionCalls(unittest.TestCase):
1528

1629
def test_kwargs_order(self):
@@ -133,6 +146,18 @@ def test_varargs17_kw(self):
133146
self.assertRaisesRegex(TypeError, msg,
134147
print, 0, sep=1, end=2, file=3, flush=4, foo=5)
135148

149+
def test_varargs18_kw(self):
150+
# _PyArg_UnpackKeywordsWithVararg()
151+
msg = r"invalid keyword argument for print\(\)$"
152+
with self.assertRaisesRegex(TypeError, msg):
153+
print(0, 1, **{BadStr('foo'): ','})
154+
155+
def test_varargs19_kw(self):
156+
# _PyArg_UnpackKeywords()
157+
msg = r"invalid keyword argument for round\(\)$"
158+
with self.assertRaisesRegex(TypeError, msg):
159+
round(1.75, **{BadStr('foo'): 1})
160+
136161
def test_oldargs0_1(self):
137162
msg = r"keys\(\) takes no arguments \(1 given\)"
138163
self.assertRaisesRegex(TypeError, msg, {}.keys, 0)

‎Lib/test/test_getargs2.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,33 @@ def test_surrogate_keyword(self):
747747
"'\udc80' is an invalid keyword argument for this function"):
748748
getargs_keyword_only(1, 2, **{'\uDC80': 10})
749749

750+
def test_weird_str_subclass(self):
751+
class BadStr(str):
752+
def __eq__(self, other):
753+
return True
754+
def __hash__(self):
755+
# Guaranteed different hash
756+
return str.__hash__(self) ^ 3
757+
with self.assertRaisesRegex(TypeError,
758+
"invalid keyword argument for this function"):
759+
getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3})
760+
with self.assertRaisesRegex(TypeError,
761+
"invalid keyword argument for this function"):
762+
getargs_keyword_only(1, 2, **{BadStr("monster"): 666})
763+
764+
def test_weird_str_subclass2(self):
765+
class BadStr(str):
766+
def __eq__(self, other):
767+
return False
768+
def __hash__(self):
769+
return str.__hash__(self)
770+
with self.assertRaisesRegex(TypeError,
771+
"invalid keyword argument for this function"):
772+
getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3})
773+
with self.assertRaisesRegex(TypeError,
774+
"invalid keyword argument for this function"):
775+
getargs_keyword_only(1, 2, **{BadStr("monster"): 666})
776+
750777

751778
class PositionalOnlyAndKeywords_TestCase(unittest.TestCase):
752779
from _testcapi import getargs_positional_only_and_keywords as getargs

‎Misc/NEWS.d/next/Core and Builtins/2022-07-19-09-41-55.gh-issue-94938.xYBlM7.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix error detection in some builtin functions when keyword argument name is
2+
an instance of a str subclass with overloaded ``__eq__`` and ``__hash__``.
3+
Previously it could cause SystemError or other undesired behavior.

‎Python/getargs.c‎

Lines changed: 55 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,6 +1553,50 @@ _PyArg_VaParseTupleAndKeywordsFast_SizeT(PyObject *args, PyObject *keywords,
15531553
return retval;
15541554
}
15551555

1556+
static void
1557+
error_unexpected_keyword_arg(PyObject *kwargs, PyObject *kwnames, PyObject *kwtuple, const char *fname)
1558+
{
1559+
/* make sure there are no extraneous keyword arguments */
1560+
Py_ssize_t j = 0;
1561+
while (1) {
1562+
PyObject *keyword;
1563+
if (kwargs != NULL) {
1564+
if (!PyDict_Next(kwargs, &j, &keyword, NULL))
1565+
break;
1566+
}
1567+
else {
1568+
if (j >= PyTuple_GET_SIZE(kwnames))
1569+
break;
1570+
keyword = PyTuple_GET_ITEM(kwnames, j);
1571+
j++;
1572+
}
1573+
if (!PyUnicode_Check(keyword)) {
1574+
PyErr_SetString(PyExc_TypeError,
1575+
"keywords must be strings");
1576+
return;
1577+
}
1578+
1579+
int match = PySequence_Contains(kwtuple, keyword);
1580+
if (match <= 0) {
1581+
if (!match) {
1582+
PyErr_Format(PyExc_TypeError,
1583+
"'%S' is an invalid keyword "
1584+
"argument for %.200s%s",
1585+
keyword,
1586+
(fname == NULL) ? "this function" : fname,
1587+
(fname == NULL) ? "" : "()");
1588+
}
1589+
return;
1590+
}
1591+
}
1592+
/* Something wrong happened. There are extraneous keyword arguments,
1593+
* but we don't know what. And we don't bother. */
1594+
PyErr_Format(PyExc_TypeError,
1595+
"invalid keyword argument for %.200s%s",
1596+
(fname == NULL) ? "this function" : fname,
1597+
(fname == NULL) ? "" : "()");
1598+
}
1599+
15561600
int
15571601
PyArg_ValidateKeywordArguments(PyObject *kwargs)
15581602
{
@@ -1841,6 +1885,13 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
18411885
return cleanreturn(0, &freelist);
18421886
}
18431887
}
1888+
/* Something wrong happened. There are extraneous keyword arguments,
1889+
* but we don't know what. And we don't bother. */
1890+
PyErr_Format(PyExc_TypeError,
1891+
"invalid keyword argument for %.200s%s",
1892+
(fname == NULL) ? "this function" : fname,
1893+
(fname == NULL) ? "" : "()");
1894+
return cleanreturn(0, &freelist);
18441895
}
18451896

18461897
return cleanreturn(1, &freelist);
@@ -2183,7 +2234,6 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
21832234
assert(IS_END_OF_FORMAT(*format) || (*format == '|') || (*format == '$'));
21842235

21852236
if (nkwargs > 0) {
2186-
Py_ssize_t j;
21872237
/* make sure there are no arguments given by name and position */
21882238
for (i = pos; i < nargs; i++) {
21892239
keyword = PyTuple_GET_ITEM(kwtuple, i - pos);
@@ -2207,34 +2257,9 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
22072257
return cleanreturn(0, &freelist);
22082258
}
22092259
}
2210-
/* make sure there are no extraneous keyword arguments */
2211-
j = 0;
2212-
while (1) {
2213-
int match;
2214-
if (kwargs != NULL) {
2215-
if (!PyDict_Next(kwargs, &j, &keyword, NULL))
2216-
break;
2217-
}
2218-
else {
2219-
if (j >= PyTuple_GET_SIZE(kwnames))
2220-
break;
2221-
keyword = PyTuple_GET_ITEM(kwnames, j);
2222-
j++;
2223-
}
22242260

2225-
match = PySequence_Contains(kwtuple, keyword);
2226-
if (match <= 0) {
2227-
if (!match) {
2228-
PyErr_Format(PyExc_TypeError,
2229-
"'%S' is an invalid keyword "
2230-
"argument for %.200s%s",
2231-
keyword,
2232-
(parser->fname == NULL) ? "this function" : parser->fname,
2233-
(parser->fname == NULL) ? "" : "()");
2234-
}
2235-
return cleanreturn(0, &freelist);
2236-
}
2237-
}
2261+
error_unexpected_keyword_arg(kwargs, kwnames, kwtuple, parser->fname);
2262+
return cleanreturn(0, &freelist);
22382263
}
22392264

22402265
return cleanreturn(1, &freelist);
@@ -2408,7 +2433,6 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
24082433
}
24092434

24102435
if (nkwargs > 0) {
2411-
Py_ssize_t j;
24122436
/* make sure there are no arguments given by name and position */
24132437
for (i = posonly; i < nargs; i++) {
24142438
keyword = PyTuple_GET_ITEM(kwtuple, i - posonly);
@@ -2432,34 +2456,9 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
24322456
return NULL;
24332457
}
24342458
}
2435-
/* make sure there are no extraneous keyword arguments */
2436-
j = 0;
2437-
while (1) {
2438-
int match;
2439-
if (kwargs != NULL) {
2440-
if (!PyDict_Next(kwargs, &j, &keyword, NULL))
2441-
break;
2442-
}
2443-
else {
2444-
if (j >= PyTuple_GET_SIZE(kwnames))
2445-
break;
2446-
keyword = PyTuple_GET_ITEM(kwnames, j);
2447-
j++;
2448-
}
24492459

2450-
match = PySequence_Contains(kwtuple, keyword);
2451-
if (match <= 0) {
2452-
if (!match) {
2453-
PyErr_Format(PyExc_TypeError,
2454-
"'%S' is an invalid keyword "
2455-
"argument for %.200s%s",
2456-
keyword,
2457-
(parser->fname == NULL) ? "this function" : parser->fname,
2458-
(parser->fname == NULL) ? "" : "()");
2459-
}
2460-
return NULL;
2461-
}
2462-
}
2460+
error_unexpected_keyword_arg(kwargs, kwnames, kwtuple, parser->fname);
2461+
return NULL;
24632462
}
24642463

24652464
return buf;

0 commit comments

Comments
 (0)