From 548ce0923b9ef93b1c1df59f8febc4bb3daff28a Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 13 Oct 2023 16:05:01 +0300 Subject: [PATCH] gh-110815: Improve tests for PyArg_ParseTupleAndKeywords() (GH-110817) --- Lib/test/test_capi/test_getargs.py | 36 ++++++++++++++++++--- Modules/_testcapi/getargs.c | 52 ++++++++++++++++++++++-------- 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_capi/test_getargs.py b/Lib/test/test_capi/test_getargs.py index e10f679eeb71c8..7fc25f82597399 100644 --- a/Lib/test/test_capi/test_getargs.py +++ b/Lib/test/test_capi/test_getargs.py @@ -55,6 +55,8 @@ LLONG_MIN = -2**63 ULLONG_MAX = 2**64-1 +NULL = None + class Index: def __index__(self): return 99 @@ -1160,6 +1162,27 @@ def test_parse_tuple_and_keywords(self): self.assertRaises(ValueError, _testcapi.parse_tuple_and_keywords, (), {}, '', [42]) + def test_basic(self): + parse = _testcapi.parse_tuple_and_keywords + + self.assertEqual(parse((), {'a': 1}, 'O', ['a']), (1,)) + self.assertEqual(parse((), {}, '|O', ['a']), (NULL,)) + self.assertEqual(parse((1, 2), {}, 'OO', ['a', 'b']), (1, 2)) + self.assertEqual(parse((1,), {'b': 2}, 'OO', ['a', 'b']), (1, 2)) + self.assertEqual(parse((), {'a': 1, 'b': 2}, 'OO', ['a', 'b']), (1, 2)) + self.assertEqual(parse((), {'b': 2}, '|OO', ['a', 'b']), (NULL, 2)) + + with self.assertRaisesRegex(TypeError, + "function missing required argument 'a'"): + parse((), {}, 'O', ['a']) + with self.assertRaisesRegex(TypeError, + "'b' is an invalid keyword argument"): + parse((), {'b': 1}, '|O', ['a']) + with self.assertRaisesRegex(TypeError, + fr"argument for function given by name \('a'\) " + fr"and position \(1\)"): + parse((1,), {'a': 2}, 'O|O', ['a', 'b']) + def test_bad_use(self): # Test handling invalid format and keywords in # PyArg_ParseTupleAndKeywords() @@ -1187,20 +1210,23 @@ def test_bad_use(self): def test_positional_only(self): parse = _testcapi.parse_tuple_and_keywords - parse((1, 2, 3), {}, 'OOO', ['', '', 'a']) - parse((1, 2), {'a': 3}, 'OOO', ['', '', 'a']) + self.assertEqual(parse((1, 2, 3), {}, 'OOO', ['', '', 'a']), (1, 2, 3)) + self.assertEqual(parse((1, 2), {'a': 3}, 'OOO', ['', '', 'a']), (1, 2, 3)) with self.assertRaisesRegex(TypeError, r'function takes at least 2 positional arguments \(1 given\)'): parse((1,), {'a': 3}, 'OOO', ['', '', 'a']) - parse((1,), {}, 'O|OO', ['', '', 'a']) + self.assertEqual(parse((1,), {}, 'O|OO', ['', '', 'a']), + (1, NULL, NULL)) with self.assertRaisesRegex(TypeError, r'function takes at least 1 positional argument \(0 given\)'): parse((), {}, 'O|OO', ['', '', 'a']) - parse((1, 2), {'a': 3}, 'OO$O', ['', '', 'a']) + self.assertEqual(parse((1, 2), {'a': 3}, 'OO$O', ['', '', 'a']), + (1, 2, 3)) with self.assertRaisesRegex(TypeError, r'function takes exactly 2 positional arguments \(1 given\)'): parse((1,), {'a': 3}, 'OO$O', ['', '', 'a']) - parse((1,), {}, 'O|O$O', ['', '', 'a']) + self.assertEqual(parse((1,), {}, 'O|O$O', ['', '', 'a']), + (1, NULL, NULL)) with self.assertRaisesRegex(TypeError, r'function takes at least 1 positional argument \(0 given\)'): parse((), {}, 'O|O$O', ['', '', 'a']) diff --git a/Modules/_testcapi/getargs.c b/Modules/_testcapi/getargs.c index 5f4a6dc8ca7672..86d8f5984dd3e5 100644 --- a/Modules/_testcapi/getargs.c +++ b/Modules/_testcapi/getargs.c @@ -13,9 +13,9 @@ parse_tuple_and_keywords(PyObject *self, PyObject *args) const char *sub_format; PyObject *sub_keywords; - double buffers[8][4]; /* double ensures alignment where necessary */ - PyObject *converted[8]; - char *keywords[8 + 1]; /* space for NULL at end */ +#define MAX_PARAMS 8 + double buffers[MAX_PARAMS][4]; /* double ensures alignment where necessary */ + char *keywords[MAX_PARAMS + 1]; /* space for NULL at end */ PyObject *return_value = NULL; @@ -35,11 +35,10 @@ parse_tuple_and_keywords(PyObject *self, PyObject *args) } memset(buffers, 0, sizeof(buffers)); - memset(converted, 0, sizeof(converted)); memset(keywords, 0, sizeof(keywords)); Py_ssize_t size = PySequence_Fast_GET_SIZE(sub_keywords); - if (size > 8) { + if (size > MAX_PARAMS) { PyErr_SetString(PyExc_ValueError, "parse_tuple_and_keywords: too many keywords in sub_keywords"); goto exit; @@ -47,29 +46,56 @@ parse_tuple_and_keywords(PyObject *self, PyObject *args) for (Py_ssize_t i = 0; i < size; i++) { PyObject *o = PySequence_Fast_GET_ITEM(sub_keywords, i); - if (!PyUnicode_FSConverter(o, (void *)(converted + i))) { + if (PyUnicode_Check(o)) { + keywords[i] = (char *)PyUnicode_AsUTF8(o); + if (keywords[i] == NULL) { + goto exit; + } + } + else if (PyBytes_Check(o)) { + keywords[i] = PyBytes_AS_STRING(o); + } + else { PyErr_Format(PyExc_ValueError, "parse_tuple_and_keywords: " - "could not convert keywords[%zd] to narrow string", i); + "keywords must be str or bytes", i); goto exit; } - keywords[i] = PyBytes_AS_STRING(converted[i]); } + assert(MAX_PARAMS == 8); int result = PyArg_ParseTupleAndKeywords(sub_args, sub_kwargs, sub_format, keywords, buffers + 0, buffers + 1, buffers + 2, buffers + 3, buffers + 4, buffers + 5, buffers + 6, buffers + 7); if (result) { - return_value = Py_NewRef(Py_None); + int objects_only = 1; + for (const char *f = sub_format; *f; f++) { + if (Py_ISALNUM(*f) && strchr("OSUY", *f) == NULL) { + objects_only = 0; + break; + } + } + if (objects_only) { + return_value = PyTuple_New(size); + if (return_value == NULL) { + goto exit; + } + for (Py_ssize_t i = 0; i < size; i++) { + PyObject *arg = *(PyObject **)(buffers + i); + if (arg == NULL) { + arg = Py_None; + } + PyTuple_SET_ITEM(return_value, i, Py_NewRef(arg)); + } + } + else { + return_value = Py_NewRef(Py_None); + } } exit: - size = sizeof(converted) / sizeof(converted[0]); - for (Py_ssize_t i = 0; i < size; i++) { - Py_XDECREF(converted[i]); - } return return_value; }