Skip to content

Commit 33efd7f

Browse files
serhiy-storchakaserhiy-storchaka
authored
[3.11] gh-94938: Fix errror detection of unexpected keyword arguments (GH-94999) (GH-95353)
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 <[email protected]>
1 parent 547f0bb commit 33efd7f

File tree

4 files changed

+112
-85
lines changed

4 files changed

+112
-85
lines changed

‎Lib/test/test_call.py

+25
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

+27
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

+3
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

+57-85
Original file line numberDiff line numberDiff line change
@@ -1554,6 +1554,50 @@ _PyArg_VaParseTupleAndKeywordsFast_SizeT(PyObject *args, PyObject *keywords,
15541554
return retval;
15551555
}
15561556

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

18471898
return cleanreturn(1, &freelist);
@@ -2184,7 +2235,6 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
21842235
assert(IS_END_OF_FORMAT(*format) || (*format == '|') || (*format == '$'));
21852236

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

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

22412266
return cleanreturn(1, &freelist);
@@ -2409,7 +2434,6 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
24092434
}
24102435

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

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

24662465
return buf;
@@ -2589,35 +2588,8 @@ _PyArg_UnpackKeywordsWithVararg(PyObject *const *args, Py_ssize_t nargs,
25892588
}
25902589

25912590
if (nkwargs > 0) {
2592-
Py_ssize_t j;
2593-
/* make sure there are no extraneous keyword arguments */
2594-
j = 0;
2595-
while (1) {
2596-
int match;
2597-
if (kwargs != NULL) {
2598-
if (!PyDict_Next(kwargs, &j, &keyword, NULL))
2599-
break;
2600-
}
2601-
else {
2602-
if (j >= PyTuple_GET_SIZE(kwnames))
2603-
break;
2604-
keyword = PyTuple_GET_ITEM(kwnames, j);
2605-
j++;
2606-
}
2607-
2608-
match = PySequence_Contains(kwtuple, keyword);
2609-
if (match <= 0) {
2610-
if (!match) {
2611-
PyErr_Format(PyExc_TypeError,
2612-
"'%S' is an invalid keyword "
2613-
"argument for %.200s%s",
2614-
keyword,
2615-
(parser->fname == NULL) ? "this function" : parser->fname,
2616-
(parser->fname == NULL) ? "" : "()");
2617-
}
2618-
goto exit;
2619-
}
2620-
}
2591+
error_unexpected_keyword_arg(kwargs, kwnames, kwtuple, parser->fname);
2592+
goto exit;
26212593
}
26222594

26232595
return buf;

0 commit comments

Comments
 (0)