[Python-checkins] gh-94930: skipitem() in getargs.c should return non-NULL on error (GH-94931)

[serhiy-storchaka]

https://github.com/python/cpython/commit/067f0da33506f70c36a67d5f3d8d011c8dae10c9
commit: 067f0da33506f70c36a67d5f3d8d011c8dae10c9
branch: main
author: Serhiy Storchaka <storchaka at gmail.com>
committer: serhiy-storchaka <storchaka at gmail.com>
date: 2022-07-18T18:07:31+03:00
summary:

gh-94930: skipitem() in getargs.c should return non-NULL on error (GH-94931)

files:
A Misc/NEWS.d/next/C API/2022-07-17-18-21-40.gh-issue-94930.gPFGDL.rst
M Lib/test/test_getargs2.py
M Modules/_testcapimodule.c
M Python/getargs.c

diff --git a/Lib/test/test_getargs2.py b/Lib/test/test_getargs2.py
index 7c11c6cd33a84..899e5463315e1 100644
--- a/Lib/test/test_getargs2.py
+++ b/Lib/test/test_getargs2.py
@@ -877,9 +877,19 @@ def test_s_hash(self):
     def test_s_hash_int(self):
         # "s#" without PY_SSIZE_T_CLEAN defined.
         from _testcapi import getargs_s_hash_int
-        self.assertRaises(SystemError, getargs_s_hash_int, "abc")
-        self.assertRaises(SystemError, getargs_s_hash_int, x=42)
-        # getargs_s_hash_int() don't raise SystemError because skipitem() is not called.
+        from _testcapi import getargs_s_hash_int2
+        buf = bytearray([1, 2])
+        self.assertRaises(SystemError, getargs_s_hash_int, buf, "abc")
+        self.assertRaises(SystemError, getargs_s_hash_int, buf, x=42)
+        self.assertRaises(SystemError, getargs_s_hash_int, buf, x="abc")
+        self.assertRaises(SystemError, getargs_s_hash_int2, buf, ("abc",))
+        self.assertRaises(SystemError, getargs_s_hash_int2, buf, x=42)
+        self.assertRaises(SystemError, getargs_s_hash_int2, buf, x="abc")
+        buf.append(3)  # still mutable -- not locked by a buffer export
+        # getargs_s_hash_int(buf) may not raise SystemError because skipitem()
+        # is not called. But it is an implementation detail.
+        # getargs_s_hash_int(buf)
+        # getargs_s_hash_int2(buf)
 
     def test_z(self):
         from _testcapi import getargs_z
diff --git a/Misc/NEWS.d/next/C API/2022-07-17-18-21-40.gh-issue-94930.gPFGDL.rst b/Misc/NEWS.d/next/C API/2022-07-17-18-21-40.gh-issue-94930.gPFGDL.rst
new file mode 100644
index 0000000000000..5b79d757b1e4a
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2022-07-17-18-21-40.gh-issue-94930.gPFGDL.rst	
@@ -0,0 +1,2 @@
+Fix ``SystemError`` raised when :c:func:`PyArg_ParseTupleAndKeywords` is
+used with ``#`` in ``(...)`` but without ``PY_SSIZE_T_CLEAN`` defined.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 436c701a7a3fa..6b9afe2aa2415 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -6035,6 +6035,7 @@ test_macros(PyObject *self, PyObject *Py_UNUSED(args))
 static PyObject *negative_dictoffset(PyObject *, PyObject *);
 static PyObject *test_buildvalue_issue38913(PyObject *, PyObject *);
 static PyObject *getargs_s_hash_int(PyObject *, PyObject *, PyObject*);
+static PyObject *getargs_s_hash_int2(PyObject *, PyObject *, PyObject*);
 
 static PyMethodDef TestMethods[] = {
     {"raise_exception",         raise_exception,                 METH_VARARGS},
@@ -6157,6 +6158,8 @@ static PyMethodDef TestMethods[] = {
     {"getargs_s_hash",          getargs_s_hash,                  METH_VARARGS},
     {"getargs_s_hash_int",      _PyCFunction_CAST(getargs_s_hash_int),
       METH_VARARGS|METH_KEYWORDS},
+    {"getargs_s_hash_int2",      _PyCFunction_CAST(getargs_s_hash_int2),
+      METH_VARARGS|METH_KEYWORDS},
     {"getargs_z",               getargs_z,                       METH_VARARGS},
     {"getargs_z_star",          getargs_z_star,                  METH_VARARGS},
     {"getargs_z_hash",          getargs_z_hash,                  METH_VARARGS},
@@ -7794,11 +7797,27 @@ PyAPI_FUNC(int) PyArg_ParseTupleAndKeywords(PyObject *, PyObject *,
 static PyObject *
 getargs_s_hash_int(PyObject *self, PyObject *args, PyObject *kwargs)
 {
-    static char *keywords[] = {"", "x", NULL};
+    static char *keywords[] = {"", "", "x", NULL};
+    Py_buffer buf = {NULL};
+    const char *s;
+    int len;
+    int i = 0;
+    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "w*|s#i", keywords, &buf, &s, &len, &i))
+        return NULL;
+    PyBuffer_Release(&buf);
+    Py_RETURN_NONE;
+}
+
+static PyObject *
+getargs_s_hash_int2(PyObject *self, PyObject *args, PyObject *kwargs)
+{
+    static char *keywords[] = {"", "", "x", NULL};
+    Py_buffer buf = {NULL};
     const char *s;
     int len;
     int i = 0;
-    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|s#i", keywords, &s, &len, &i))
+    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "w*|(s#)i", keywords, &buf, &s, &len, &i))
         return NULL;
+    PyBuffer_Release(&buf);
     Py_RETURN_NONE;
 }
diff --git a/Python/getargs.c b/Python/getargs.c
index fb4a5124beab8..41881162ff2fb 100644
--- a/Python/getargs.c
+++ b/Python/getargs.c
@@ -2641,9 +2641,7 @@ skipitem(const char **p_format, va_list *p_va, int flags)
             if (*format == '#') {
                 if (p_va != NULL) {
                     if (!(flags & FLAG_SIZE_T)) {
-                        PyErr_SetString(PyExc_SystemError,
-                                "PY_SSIZE_T_CLEAN macro must be defined for '#' formats");
-                        return NULL;
+                        return "PY_SSIZE_T_CLEAN macro must be defined for '#' formats";
                     }
                     (void) va_arg(*p_va, Py_ssize_t *);
                 }