Skip to content

gh-68114: Fix handling for removed PyArg_ParseTuple formatters #8204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
encukou merged 11 commits into from
Apr 23, 2024
Merged

gh-68114: Fix handling for removed PyArg_ParseTuple formatters #8204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5a275c8
Fix handling for removed PyArg_ParseTuple formatters
Jul 9, 2018
cbc1897
Merge in the main branch
encukou Mar 18, 2024
65aec54
Merge comments
encukou Mar 18, 2024
6c948fb
Recreate the blurb
encukou Mar 18, 2024
c2a63a2
Fix the test
encukou Mar 18, 2024
4fbed15
Fix style
encukou Mar 18, 2024
3aff049
Ensure the invalid args result in SystemError
encukou Mar 18, 2024
ae9b1e2
Fix functionality, and add additional test
encukou Mar 18, 2024
30e7def
Unmark 'w#' as supported in test_skipitem_with_suffix
encukou Mar 19, 2024
2aee71c
Don't omit parameter names
encukou Mar 19, 2024
11eca72
Merge branch 'main' into remove-old-formatters
encukou Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 21 additions & 17 deletions Lib/test/test_capi/test_getargs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,20 +856,24 @@ def test_y_hash(self):

def test_w_star(self):
# getargs_w_star() modifies first and last byte
from _testcapi import getargs_w_star
self.assertRaises(TypeError, getargs_w_star, 'abc\xe9')
self.assertRaises(TypeError, getargs_w_star, b'bytes')
self.assertRaises(TypeError, getargs_w_star, b'nul:\0')
self.assertRaises(TypeError, getargs_w_star, memoryview(b'bytes'))
buf = bytearray(b'bytearray')
self.assertEqual(getargs_w_star(buf), b'[ytearra]')
self.assertEqual(buf, bytearray(b'[ytearra]'))
buf = bytearray(b'memoryview')
self.assertEqual(getargs_w_star(memoryview(buf)), b'[emoryvie]')
self.assertEqual(buf, bytearray(b'[emoryvie]'))
self.assertRaises(TypeError, getargs_w_star, None)
self.assertRaises(TypeError, getargs_w_star, NONCONTIG_WRITABLE)
self.assertRaises(TypeError, getargs_w_star, NONCONTIG_READONLY)
# getargs_w_star_opt() takes additional optional args: with one
# argument it should behave the same as getargs_w_star
from _testcapi import getargs_w_star, getargs_w_star_opt
for func in (getargs_w_star, getargs_w_star_opt):
with self.subTest(func=func):
self.assertRaises(TypeError, func, 'abc\xe9')
self.assertRaises(TypeError, func, b'bytes')
self.assertRaises(TypeError, func, b'nul:\0')
self.assertRaises(TypeError, func, memoryview(b'bytes'))
buf = bytearray(b'bytearray')
self.assertEqual(func(buf), b'[ytearra]')
self.assertEqual(buf, bytearray(b'[ytearra]'))
buf = bytearray(b'memoryview')
self.assertEqual(func(memoryview(buf)), b'[emoryvie]')
self.assertEqual(buf, bytearray(b'[emoryvie]'))
self.assertRaises(TypeError, func, None)
self.assertRaises(TypeError, func, NONCONTIG_WRITABLE)
self.assertRaises(TypeError, func, NONCONTIG_READONLY)

def test_getargs_empty(self):
from _testcapi import getargs_empty
Expand Down Expand Up @@ -1112,9 +1116,9 @@ def test_skipitem(self):
c = chr(i)

# skip parentheses, the error reporting is inconsistent about them
# skip 'e', it's always a two-character code
# skip 'e' and 'w', they're always two-character codes
# skip '|' and '$', they don't represent arguments anyway
if c in '()e|$':
if c in '()ew|$':
continue

# test the format unit when not skipped
Expand Down Expand Up @@ -1152,7 +1156,7 @@ def test_skipitem_with_suffix(self):
dict_b = {'b':1}
keywords = ["a", "b"]

supported = ('s#', 's*', 'z#', 'z*', 'y#', 'y*', 'w#', 'w*')
supported = ('s#', 's*', 'z#', 'z*', 'y#', 'y*', 'w*')
for c in string.ascii_letters:
for c2 in '#*':
f = c + c2
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/C API/2024-03-18-17-29-52.gh-issue-68114.W7R_lI.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed skipitem()'s handling of the old 'w' and 'w#' formatters. These are
no longer supported and now raise an exception if used.
118 changes: 118 additions & 0 deletions Modules/_testcapi/getargs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,122 @@ getargs_w_star(PyObject *self, PyObject *args)
return result;
}

static PyObject *
getargs_w_star_opt(PyObject *self, PyObject *args)
{
Py_buffer buffer;
Py_buffer buf2;
int number = 1;

if (!PyArg_ParseTuple(args, "w*|w*i:getargs_w_star",
&buffer, &buf2, &number)) {
return NULL;
}

if (2 <= buffer.len) {
char *str = buffer.buf;
str[0] = '[';
str[buffer.len-1] = ']';
}

PyObject *result = PyBytes_FromStringAndSize(buffer.buf, buffer.len);
PyBuffer_Release(&buffer);
return result;
}

/* Test the old w and w# codes that no longer work */
static PyObject *
test_w_code_invalid(PyObject *self, PyObject *arg)
{
static const char * const keywords[] = {"a", "b", "c", "d", NULL};
char *formats_3[] = {"O|w#$O",
"O|w$O",
"O|w#O",
"O|wO",
NULL};
char *formats_4[] = {"O|w#O$O",
"O|wO$O",
"O|Ow#O",
"O|OwO",
"O|Ow#$O",
"O|Ow$O",
NULL};
size_t n;
PyObject *args;
PyObject *kwargs;
PyObject *tmp;

if (!(args = PyTuple_Pack(1, Py_None))) {
return NULL;
}

kwargs = PyDict_New();
if (!kwargs) {
Py_DECREF(args);
return NULL;
}

if (PyDict_SetItemString(kwargs, "c", Py_None)) {
Py_DECREF(args);
Py_XDECREF(kwargs);
return NULL;
}

for (n = 0; formats_3[n]; ++n) {
if (PyArg_ParseTupleAndKeywords(args, kwargs, formats_3[n],
(char**) keywords,
&tmp, &tmp, &tmp)) {
Py_DECREF(args);
Py_DECREF(kwargs);
PyErr_Format(PyExc_AssertionError,
"test_w_code_invalid_suffix: %s",
formats_3[n]);
return NULL;
}
else {
if (!PyErr_ExceptionMatches(PyExc_SystemError)) {
Py_DECREF(args);
Py_DECREF(kwargs);
return NULL;
}
PyErr_Clear();
}
}

if (PyDict_DelItemString(kwargs, "c") ||
PyDict_SetItemString(kwargs, "d", Py_None)) {

Py_DECREF(kwargs);
Py_DECREF(args);
return NULL;
}

for (n = 0; formats_4[n]; ++n) {
if (PyArg_ParseTupleAndKeywords(args, kwargs, formats_4[n],
(char**) keywords,
&tmp, &tmp, &tmp, &tmp)) {
Py_DECREF(args);
Py_DECREF(kwargs);
PyErr_Format(PyExc_AssertionError,
"test_w_code_invalid_suffix: %s",
formats_4[n]);
return NULL;
}
else {
if (!PyErr_ExceptionMatches(PyExc_SystemError)) {
Py_DECREF(args);
Py_DECREF(kwargs);
return NULL;
}
PyErr_Clear();
}
}

Py_DECREF(args);
Py_DECREF(kwargs);
Py_RETURN_NONE;
}

static PyObject *
getargs_empty(PyObject *self, PyObject *args, PyObject *kwargs)
{
Expand Down Expand Up @@ -684,6 +800,7 @@ static PyMethodDef test_methods[] = {
{"getargs_s_star", getargs_s_star, METH_VARARGS},
{"getargs_tuple", getargs_tuple, METH_VARARGS},
{"getargs_w_star", getargs_w_star, METH_VARARGS},
{"getargs_w_star_opt", getargs_w_star_opt, METH_VARARGS},
{"getargs_empty", _PyCFunction_CAST(getargs_empty), METH_VARARGS|METH_KEYWORDS},
{"getargs_y", getargs_y, METH_VARARGS},
{"getargs_y_hash", getargs_y_hash, METH_VARARGS},
Expand All @@ -693,6 +810,7 @@ static PyMethodDef test_methods[] = {
{"getargs_z_star", getargs_z_star, METH_VARARGS},
{"parse_tuple_and_keywords", parse_tuple_and_keywords, METH_VARARGS},
{"gh_99240_clear_args", gh_99240_clear_args, METH_VARARGS},
{"test_w_code_invalid", test_w_code_invalid, METH_NOARGS},
{NULL},
};

Expand Down
5 changes: 5 additions & 0 deletions Python/getargs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2641,6 +2641,11 @@ skipitem(const char **p_format, va_list *p_va, int flags)
if (p_va != NULL) {
(void) va_arg(*p_va, char **);
}
if (c == 'w' && *format != '*')
{
/* after 'w', only '*' is allowed */
goto err;
}
if (*format == '#') {
if (p_va != NULL) {
(void) va_arg(*p_va, Py_ssize_t *);
Expand Down