[3.10] gh-94938: Fix errror detection of unexpected keyword arguments (GH-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..
(cherry picked from commit ebad53a4dc1bb591820724a22cef9b8459185b5f)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py
index 3f459222748b3a86c88489ceafa7a554bf7744b8..91ef9c94695c1e8d1917ac1bad9208cd4e5ac878 100644 (file)
--- a/Lib/test/test_call.py
+++ b/Lib/test/test_call.py
@@ -11,6 +11,19 @@ import gc
 import contextlib
 
 
+class BadStr(str):
+    def __eq__(self, other):
+        return True
+    def __hash__(self):
+        # Guaranteed different hash
+        return str.__hash__(self) ^ 3
+
+    def __eq__(self, other):
+        return False
+    def __hash__(self):
+        return str.__hash__(self)
+
+
 class FunctionCalls(unittest.TestCase):
 
     def test_kwargs_order(self):
@@ -133,6 +146,18 @@ class CFunctionCallsErrorMessages(unittest.TestCase):
         self.assertRaisesRegex(TypeError, msg,
                                print, 0, sep=1, end=2, file=3, flush=4, foo=5)
 
+    def test_varargs18_kw(self):
+        # _PyArg_UnpackKeywordsWithVararg()
+        msg = r"invalid keyword argument for print\(\)$"
+        with self.assertRaisesRegex(TypeError, msg):
+            print(0, 1, **{BadStr('foo'): ','})
+
+    def test_varargs19_kw(self):
+        # _PyArg_UnpackKeywords()
+        msg = r"invalid keyword argument for round\(\)$"
+        with self.assertRaisesRegex(TypeError, msg):
+            round(1.75, **{BadStr('foo'): 1})
+
     def test_oldargs0_1(self):
         msg = r"keys\(\) takes no arguments \(1 given\)"
         self.assertRaisesRegex(TypeError, msg, {}.keys, 0)

diff --git a/Lib/test/test_getargs2.py b/Lib/test/test_getargs2.py
index fd1952c5845db73da57b40badaa3fa3eb9c0fe42..72b6d64a49863a5b250956b16d72516054c90f63 100644 (file)
--- a/Lib/test/test_getargs2.py
+++ b/Lib/test/test_getargs2.py
@@ -747,6 +747,33 @@ class KeywordOnly_TestCase(unittest.TestCase):
             "'\udc80' is an invalid keyword argument for this function"):
             getargs_keyword_only(1, 2, **{'\uDC80': 10})
 
+    def test_weird_str_subclass(self):
+        class BadStr(str):
+            def __eq__(self, other):
+                return True
+            def __hash__(self):
+                # Guaranteed different hash
+                return str.__hash__(self) ^ 3
+        with self.assertRaisesRegex(TypeError,
+            "invalid keyword argument for this function"):
+            getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3})
+        with self.assertRaisesRegex(TypeError,
+            "invalid keyword argument for this function"):
+            getargs_keyword_only(1, 2, **{BadStr("monster"): 666})
+
+    def test_weird_str_subclass2(self):
+        class BadStr(str):
+            def __eq__(self, other):
+                return False
+            def __hash__(self):
+                return str.__hash__(self)
+        with self.assertRaisesRegex(TypeError,
+            "invalid keyword argument for this function"):
+            getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3})
+        with self.assertRaisesRegex(TypeError,
+            "invalid keyword argument for this function"):
+            getargs_keyword_only(1, 2, **{BadStr("monster"): 666})
+
 
 class PositionalOnlyAndKeywords_TestCase(unittest.TestCase):
     from _testcapi import getargs_positional_only_and_keywords as getargs

diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-07-19-09-41-55.gh-issue-94938.xYBlM7.rst b/Misc/NEWS.d/next/Core and Builtins/2022-07-19-09-41-55.gh-issue-94938.xYBlM7.rst
new file mode 100644 (file)
index 0000000..cc4feae
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2022-07-19-09-41-55.gh-issue-94938.xYBlM7.rst
@@ -0,0 +1,3 @@
+Fix error detection in some builtin functions when keyword argument name is
+an instance of a str subclass with overloaded ``__eq__`` and ``__hash__``.
+Previously it could cause SystemError or other undesired behavior.

diff --git a/Python/getargs.c b/Python/getargs.c
index d92ea2d8e6b463125e525d350bfe5ce63ad1bb18..fd49ee0f05d4fb057d099e0cd9cf14a7c6aa9b6f 100644 (file)
--- a/Python/getargs.c
+++ b/Python/getargs.c
@@ -1553,6 +1553,50 @@ _PyArg_VaParseTupleAndKeywordsFast_SizeT(PyObject *args, PyObject *keywords,
     return retval;
 }
 
+static void
+error_unexpected_keyword_arg(PyObject *kwargs, PyObject *kwnames, PyObject *kwtuple, const char *fname)
+{
+    /* make sure there are no extraneous keyword arguments */
+    Py_ssize_t j = 0;
+    while (1) {
+        PyObject *keyword;
+        if (kwargs != NULL) {
+            if (!PyDict_Next(kwargs, &j, &keyword, NULL))
+                break;
+        }
+        else {
+            if (j >= PyTuple_GET_SIZE(kwnames))
+                break;
+            keyword = PyTuple_GET_ITEM(kwnames, j);
+            j++;
+        }
+        if (!PyUnicode_Check(keyword)) {
+            PyErr_SetString(PyExc_TypeError,
+                            "keywords must be strings");
+            return;
+        }
+
+        int match = PySequence_Contains(kwtuple, keyword);
+        if (match <= 0) {
+            if (!match) {
+                PyErr_Format(PyExc_TypeError,
+                             "'%S' is an invalid keyword "
+                             "argument for %.200s%s",
+                             keyword,
+                             (fname == NULL) ? "this function" : fname,
+                             (fname == NULL) ? "" : "()");
+            }
+            return;
+        }
+    }
+    /* Something wrong happened. There are extraneous keyword arguments,
+     * but we don't know what. And we don't bother. */
+    PyErr_Format(PyExc_TypeError,
+                 "invalid keyword argument for %.200s%s",
+                 (fname == NULL) ? "this function" : fname,
+                 (fname == NULL) ? "" : "()");
+}
+
 int
 PyArg_ValidateKeywordArguments(PyObject *kwargs)
 {
@@ -1841,6 +1885,13 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
                 return cleanreturn(0, &freelist);
             }
         }
+        /* Something wrong happened. There are extraneous keyword arguments,
+         * but we don't know what. And we don't bother. */
+        PyErr_Format(PyExc_TypeError,
+                     "invalid keyword argument for %.200s%s",
+                     (fname == NULL) ? "this function" : fname,
+                     (fname == NULL) ? "" : "()");
+        return cleanreturn(0, &freelist);
     }
 
     return cleanreturn(1, &freelist);
@@ -2183,7 +2234,6 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
     assert(IS_END_OF_FORMAT(*format) || (*format == '|') || (*format == '$'));
 
     if (nkwargs > 0) {
-        Py_ssize_t j;
         /* make sure there are no arguments given by name and position */
         for (i = pos; i < nargs; i++) {
             keyword = PyTuple_GET_ITEM(kwtuple, i - pos);
@@ -2207,34 +2257,9 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
                 return cleanreturn(0, &freelist);
             }
         }
-        /* make sure there are no extraneous keyword arguments */
-        j = 0;
-        while (1) {
-            int match;
-            if (kwargs != NULL) {
-                if (!PyDict_Next(kwargs, &j, &keyword, NULL))
-                    break;
-            }
-            else {
-                if (j >= PyTuple_GET_SIZE(kwnames))
-                    break;
-                keyword = PyTuple_GET_ITEM(kwnames, j);
-                j++;
-            }
 
-            match = PySequence_Contains(kwtuple, keyword);
-            if (match <= 0) {
-                if (!match) {
-                    PyErr_Format(PyExc_TypeError,
-                                 "'%S' is an invalid keyword "
-                                 "argument for %.200s%s",
-                                 keyword,
-                                 (parser->fname == NULL) ? "this function" : parser->fname,
-                                 (parser->fname == NULL) ? "" : "()");
-                }
-                return cleanreturn(0, &freelist);
-            }
-        }
+        error_unexpected_keyword_arg(kwargs, kwnames, kwtuple, parser->fname);
+        return cleanreturn(0, &freelist);
     }
 
     return cleanreturn(1, &freelist);
@@ -2408,7 +2433,6 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
     }
 
     if (nkwargs > 0) {
-        Py_ssize_t j;
         /* make sure there are no arguments given by name and position */
         for (i = posonly; i < nargs; i++) {
             keyword = PyTuple_GET_ITEM(kwtuple, i - posonly);
@@ -2432,34 +2456,9 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
                 return NULL;
             }
         }
-        /* make sure there are no extraneous keyword arguments */
-        j = 0;
-        while (1) {
-            int match;
-            if (kwargs != NULL) {
-                if (!PyDict_Next(kwargs, &j, &keyword, NULL))
-                    break;
-            }
-            else {
-                if (j >= PyTuple_GET_SIZE(kwnames))
-                    break;
-                keyword = PyTuple_GET_ITEM(kwnames, j);
-                j++;
-            }
 
-            match = PySequence_Contains(kwtuple, keyword);
-            if (match <= 0) {
-                if (!match) {
-                    PyErr_Format(PyExc_TypeError,
-                                 "'%S' is an invalid keyword "
-                                 "argument for %.200s%s",
-                                 keyword,
-                                 (parser->fname == NULL) ? "this function" : parser->fname,
-                                 (parser->fname == NULL) ? "" : "()");
-                }
-                return NULL;
-            }
-        }
+        error_unexpected_keyword_arg(kwargs, kwnames, kwtuple, parser->fname);
+        return NULL;
     }
 
     return buf;