Skip to content

Commit ebad53a

Browse files
serhiy-storchakaserhiy-storchaka
authored
pythongh-94938: Fix errror detection of unexpected keyword arguments (pythonGH-94999)
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.
1 parent 0fe645d commit ebad53a

4 files changed

Lines changed: 112 additions & 85 deletions

File tree

‎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):
@@ -145,6 +158,18 @@ def test_varargs17_kw(self):
145158
self.assertRaisesRegex(TypeError, msg,
146159
print, 0, sep=1, end=2, file=3, flush=4, foo=5)
147160

161+
def test_varargs18_kw(self):
162+
# _PyArg_UnpackKeywordsWithVararg()
163+
msg = r"invalid keyword argument for print\(\)$"
164+
with self.assertRaisesRegex(TypeError, msg):
165+
print(0, 1, **{BadStr('foo'): ','})
166+
167+
def test_varargs19_kw(self):
168+
# _PyArg_UnpackKeywords()
169+
msg = r"invalid keyword argument for round\(\)$"
170+
with self.assertRaisesRegex(TypeError, msg):
171+
round(1.75, **{BadStr('foo'): 1})
172+
148173
def test_oldargs0_1(self):
149174
msg = r"keys\(\) takes no arguments \(1 given\)"
150175
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
@@ -746,6 +746,33 @@ def test_surrogate_keyword(self):
746746
"'\udc80' is an invalid keyword argument for this function"):
747747
getargs_keyword_only(1, 2, **{'\uDC80': 10})
748748

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

750777
class PositionalOnlyAndKeywords_TestCase(unittest.TestCase):
751778
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: 57 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,6 +1502,50 @@ _PyArg_VaParseTupleAndKeywordsFast_SizeT(PyObject *args, PyObject *keywords,
15021502
return retval;
15031503
}
15041504

1505+
static void
1506+
error_unexpected_keyword_arg(PyObject *kwargs, PyObject *kwnames, PyObject *kwtuple, const char *fname)
1507+
{
1508+
/* make sure there are no extraneous keyword arguments */
1509+
Py_ssize_t j = 0;
1510+
while (1) {
1511+
PyObject *keyword;
1512+
if (kwargs != NULL) {
1513+
if (!PyDict_Next(kwargs, &j, &keyword, NULL))
1514+
break;
1515+
}
1516+
else {
1517+
if (j >= PyTuple_GET_SIZE(kwnames))
1518+
break;
1519+
keyword = PyTuple_GET_ITEM(kwnames, j);
1520+
j++;
1521+
}
1522+
if (!PyUnicode_Check(keyword)) {
1523+
PyErr_SetString(PyExc_TypeError,
1524+
"keywords must be strings");
1525+
return;
1526+
}
1527+
1528+
int match = PySequence_Contains(kwtuple, keyword);
1529+
if (match <= 0) {
1530+
if (!match) {
1531+
PyErr_Format(PyExc_TypeError,
1532+
"'%S' is an invalid keyword "
1533+
"argument for %.200s%s",
1534+
keyword,
1535+
(fname == NULL) ? "this function" : fname,
1536+
(fname == NULL) ? "" : "()");
1537+
}
1538+
return;
1539+
}
1540+
}
1541+
/* Something wrong happened. There are extraneous keyword arguments,
1542+
* but we don't know what. And we don't bother. */
1543+
PyErr_Format(PyExc_TypeError,
1544+
"invalid keyword argument for %.200s%s",
1545+
(fname == NULL) ? "this function" : fname,
1546+
(fname == NULL) ? "" : "()");
1547+
}
1548+
15051549
int
15061550
PyArg_ValidateKeywordArguments(PyObject *kwargs)
15071551
{
@@ -1790,6 +1834,13 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
17901834
return cleanreturn(0, &freelist);
17911835
}
17921836
}
1837+
/* Something wrong happened. There are extraneous keyword arguments,
1838+
* but we don't know what. And we don't bother. */
1839+
PyErr_Format(PyExc_TypeError,
1840+
"invalid keyword argument for %.200s%s",
1841+
(fname == NULL) ? "this function" : fname,
1842+
(fname == NULL) ? "" : "()");
1843+
return cleanreturn(0, &freelist);
17931844
}
17941845

17951846
return cleanreturn(1, &freelist);
@@ -2132,7 +2183,6 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
21322183
assert(IS_END_OF_FORMAT(*format) || (*format == '|') || (*format == '$'));
21332184

21342185
if (nkwargs > 0) {
2135-
Py_ssize_t j;
21362186
/* make sure there are no arguments given by name and position */
21372187
for (i = pos; i < nargs; i++) {
21382188
keyword = PyTuple_GET_ITEM(kwtuple, i - pos);
@@ -2156,34 +2206,9 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
21562206
return cleanreturn(0, &freelist);
21572207
}
21582208
}
2159-
/* make sure there are no extraneous keyword arguments */
2160-
j = 0;
2161-
while (1) {
2162-
int match;
2163-
if (kwargs != NULL) {
2164-
if (!PyDict_Next(kwargs, &j, &keyword, NULL))
2165-
break;
2166-
}
2167-
else {
2168-
if (j >= PyTuple_GET_SIZE(kwnames))
2169-
break;
2170-
keyword = PyTuple_GET_ITEM(kwnames, j);
2171-
j++;
2172-
}
21732209

2174-
match = PySequence_Contains(kwtuple, keyword);
2175-
if (match <= 0) {
2176-
if (!match) {
2177-
PyErr_Format(PyExc_TypeError,
2178-
"'%S' is an invalid keyword "
2179-
"argument for %.200s%s",
2180-
keyword,
2181-
(parser->fname == NULL) ? "this function" : parser->fname,
2182-
(parser->fname == NULL) ? "" : "()");
2183-
}
2184-
return cleanreturn(0, &freelist);
2185-
}
2186-
}
2210+
error_unexpected_keyword_arg(kwargs, kwnames, kwtuple, parser->fname);
2211+
return cleanreturn(0, &freelist);
21872212
}
21882213

21892214
return cleanreturn(1, &freelist);
@@ -2357,7 +2382,6 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
23572382
}
23582383

23592384
if (nkwargs > 0) {
2360-
Py_ssize_t j;
23612385
/* make sure there are no arguments given by name and position */
23622386
for (i = posonly; i < nargs; i++) {
23632387
keyword = PyTuple_GET_ITEM(kwtuple, i - posonly);
@@ -2381,34 +2405,9 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
23812405
return NULL;
23822406
}
23832407
}
2384-
/* make sure there are no extraneous keyword arguments */
2385-
j = 0;
2386-
while (1) {
2387-
int match;
2388-
if (kwargs != NULL) {
2389-
if (!PyDict_Next(kwargs, &j, &keyword, NULL))
2390-
break;
2391-
}
2392-
else {
2393-
if (j >= PyTuple_GET_SIZE(kwnames))
2394-
break;
2395-
keyword = PyTuple_GET_ITEM(kwnames, j);
2396-
j++;
2397-
}
23982408

2399-
match = PySequence_Contains(kwtuple, keyword);
2400-
if (match <= 0) {
2401-
if (!match) {
2402-
PyErr_Format(PyExc_TypeError,
2403-
"'%S' is an invalid keyword "
2404-
"argument for %.200s%s",
2405-
keyword,
2406-
(parser->fname == NULL) ? "this function" : parser->fname,
2407-
(parser->fname == NULL) ? "" : "()");
2408-
}
2409-
return NULL;
2410-
}
2411-
}
2409+
error_unexpected_keyword_arg(kwargs, kwnames, kwtuple, parser->fname);
2410+
return NULL;
24122411
}
24132412

24142413
return buf;
@@ -2537,35 +2536,8 @@ _PyArg_UnpackKeywordsWithVararg(PyObject *const *args, Py_ssize_t nargs,
25372536
}
25382537

25392538
if (nkwargs > 0) {
2540-
Py_ssize_t j;
2541-
/* make sure there are no extraneous keyword arguments */
2542-
j = 0;
2543-
while (1) {
2544-
int match;
2545-
if (kwargs != NULL) {
2546-
if (!PyDict_Next(kwargs, &j, &keyword, NULL))
2547-
break;
2548-
}
2549-
else {
2550-
if (j >= PyTuple_GET_SIZE(kwnames))
2551-
break;
2552-
keyword = PyTuple_GET_ITEM(kwnames, j);
2553-
j++;
2554-
}
2555-
2556-
match = PySequence_Contains(kwtuple, keyword);
2557-
if (match <= 0) {
2558-
if (!match) {
2559-
PyErr_Format(PyExc_TypeError,
2560-
"'%S' is an invalid keyword "
2561-
"argument for %.200s%s",
2562-
keyword,
2563-
(parser->fname == NULL) ? "this function" : parser->fname,
2564-
(parser->fname == NULL) ? "" : "()");
2565-
}
2566-
goto exit;
2567-
}
2568-
}
2539+
error_unexpected_keyword_arg(kwargs, kwnames, kwtuple, parser->fname);
2540+
goto exit;
25692541
}
25702542

25712543
return buf;

0 commit comments

Comments
 (0)