From 4260f7632519e27e9f0f2b9599823355c6111693 Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Sat, 25 Jul 2026 16:17:55 +0800 Subject: [PATCH 1/4] fix(structseq): validate PyStructSequence_Desc field invariants `count_members()` now raises `SystemError` (returning `-1`) when a struct sequence descriptor places an unnamed field outside the visible sequence fields, or when `n_in_sequence` exceeds the total number of fields; such descriptors previously built a malformed type. The failure is propagated by `PyStructSequence_NewType()`, `PyStructSequence_InitType()` and `PyStructSequence_InitType2()`. Add `_testcapi` helpers and regression tests for both rejected cases, and document the invariants in the C API reference. --- Doc/c-api/tuple.rst | 16 +++++- Lib/test/test_structseq.py | 14 ++++- ...-07-25-15-28-37.gh-issue-154387.cR09uA.rst | 4 ++ Modules/_testcapimodule.c | 55 +++++++++++++++++++ Objects/structseq.c | 25 +++++++++ 5 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/C_API/2026-07-25-15-28-37.gh-issue-154387.cR09uA.rst diff --git a/Doc/c-api/tuple.rst b/Doc/c-api/tuple.rst index e8be4762dc33a1..46f77c65959c2a 100644 --- a/Doc/c-api/tuple.rst +++ b/Doc/c-api/tuple.rst @@ -165,6 +165,11 @@ type. Return ``NULL`` with an exception set on failure. + .. versionchanged:: next + Raise :exc:`SystemError` if *desc* places an unnamed field outside the visible + sequence fields, or if :c:member:`~PyStructSequence_Desc.n_in_sequence` exceeds + the total number of fields. + .. c:function:: void PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc) @@ -178,6 +183,10 @@ type. .. versionadded:: 3.4 + .. versionchanged:: next + Raise :exc:`SystemError` for an invalid *desc*, as described in + :c:func:`PyStructSequence_NewType`. + .. c:type:: PyStructSequence_Desc @@ -198,7 +207,8 @@ type. .. c:member:: int n_in_sequence - Number of fields visible to the Python side (if used as tuple). + Number of fields visible to the Python side (if used as tuple). Must not + exceed the total number of fields. .. c:type:: PyStructSequence_Field @@ -221,7 +231,9 @@ type. .. c:var:: const char * const PyStructSequence_UnnamedField - Special value for a field name to leave it unnamed. + Special value for a field name to leave it unnamed. An unnamed field must be + one of the visible sequence fields, that is, its index must be less than + :c:member:`~PyStructSequence_Desc.n_in_sequence`. .. versionchanged:: 3.9 The type was changed from ``char *``. diff --git a/Lib/test/test_structseq.py b/Lib/test/test_structseq.py index 74506fc54de50e..279c9a290eb061 100644 --- a/Lib/test/test_structseq.py +++ b/Lib/test/test_structseq.py @@ -6,7 +6,7 @@ import textwrap import time import unittest -from test.support import script_helper +from test.support import import_helper, script_helper class StructSeqTest(unittest.TestCase): @@ -48,6 +48,18 @@ def test_repr(self): self.assertIn("st_ino=", rep) self.assertIn("st_dev=", rep) + def test_newtype_rejects_unnamed_hidden_field(self): + # gh-154387: an unnamed field must be a visible sequence field. + _testcapi = import_helper.import_module("_testcapi") + with self.assertRaises(SystemError): + _testcapi.test_structseq_newtype_unnamed_hidden_field() + + def test_newtype_rejects_n_in_sequence_over_n_fields(self): + # gh-154387: n_in_sequence must not exceed the number of fields. + _testcapi = import_helper.import_module("_testcapi") + with self.assertRaises(SystemError): + _testcapi.test_structseq_newtype_too_many_visible_fields() + def test_concat(self): t1 = time.gmtime() t2 = t1 + tuple(t1) diff --git a/Misc/NEWS.d/next/C_API/2026-07-25-15-28-37.gh-issue-154387.cR09uA.rst b/Misc/NEWS.d/next/C_API/2026-07-25-15-28-37.gh-issue-154387.cR09uA.rst new file mode 100644 index 00000000000000..23813840a194b2 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2026-07-25-15-28-37.gh-issue-154387.cR09uA.rst @@ -0,0 +1,4 @@ +:c:func:`PyStructSequence_NewType` and :c:func:`PyStructSequence_InitType2` +now raise :exc:`SystemError` when the :c:type:`PyStructSequence_Desc` places an +unnamed field outside the visible sequence fields, or sets ``n_in_sequence`` +larger than the total number of fields. Patch by Xuehai Pan. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index fb18a866e62812..23c39fff07b00b 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1304,6 +1304,57 @@ test_structseq_newtype_null_descr_doc(PyObject *Py_UNUSED(self), Py_RETURN_NONE; } +static PyObject * +test_structseq_newtype_unnamed_hidden_field(PyObject *Py_UNUSED(self), + PyObject *Py_UNUSED(args)) +{ + // gh-154387: an unnamed field is only allowed among the visible sequence + // fields. Placing one at a hidden index must be rejected. + PyStructSequence_Field descr_fields[] = { + {"visible", "a visible field"}, + {PyStructSequence_UnnamedField, "an unnamed hidden field"}, + {NULL, NULL}, + }; + PyStructSequence_Desc descr = { + "_testcapi.bad_unnamed", NULL, descr_fields, 1, + }; + + PyTypeObject *type = PyStructSequence_NewType(&descr); + if (type != NULL) { + Py_DECREF(type); + PyErr_SetString(PyExc_AssertionError, + "unnamed field at a hidden index was not rejected"); + return NULL; + } + // NewType() failed and left the expected SystemError set; propagate it. + return NULL; +} + +static PyObject * +test_structseq_newtype_too_many_visible_fields(PyObject *Py_UNUSED(self), + PyObject *Py_UNUSED(args)) +{ + // gh-154387: n_in_sequence must not exceed the total number of fields. + PyStructSequence_Field descr_fields[] = { + {"a", "field a"}, + {"b", "field b"}, + {NULL, NULL}, + }; + PyStructSequence_Desc descr = { + "_testcapi.too_many_visible", NULL, descr_fields, 3, + }; + + PyTypeObject *type = PyStructSequence_NewType(&descr); + if (type != NULL) { + Py_DECREF(type); + PyErr_SetString(PyExc_AssertionError, + "n_in_sequence exceeding the field count was not rejected"); + return NULL; + } + // NewType() failed and left the expected SystemError set; propagate it. + return NULL; +} + typedef struct { PyThread_type_lock start_event; PyThread_type_lock exit_event; @@ -3007,6 +3058,10 @@ static PyMethodDef TestMethods[] = { test_structseq_newtype_doesnt_leak, METH_NOARGS}, {"test_structseq_newtype_null_descr_doc", test_structseq_newtype_null_descr_doc, METH_NOARGS}, + {"test_structseq_newtype_unnamed_hidden_field", + test_structseq_newtype_unnamed_hidden_field, METH_NOARGS}, + {"test_structseq_newtype_too_many_visible_fields", + test_structseq_newtype_too_many_visible_fields, METH_NOARGS}, {"pyobject_repr_from_null", pyobject_repr_from_null, METH_NOARGS}, {"pyobject_str_from_null", pyobject_str_from_null, METH_NOARGS}, {"pyobject_bytes_from_null", pyobject_bytes_from_null, METH_NOARGS}, diff --git a/Objects/structseq.c b/Objects/structseq.c index 9130fe6a133b1e..7c521d9e12b38f 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -472,9 +472,25 @@ count_members(PyStructSequence_Desc *desc, Py_ssize_t *n_unnamed_members) { *n_unnamed_members = 0; for (i = 0; desc->fields[i].name != NULL; ++i) { if (desc->fields[i].name == PyStructSequence_UnnamedField) { + // Unnamed fields must be visible sequence fields. + if (i >= desc->n_in_sequence) { + PyErr_Format(PyExc_SystemError, + "%s: unnamed field %zd is not a visible sequence field " + "(n_in_sequence=%zd)", + desc->name, i, desc->n_in_sequence); + return -1; + } (*n_unnamed_members)++; } } + if (desc->n_in_sequence > i) { + PyErr_Format(PyExc_SystemError, + "%s: n_in_sequence=%zd exceeds the total number of fields %zd", + desc->name, desc->n_in_sequence, i); + return -1; + } + // Implied by the per-field check above: unnamed fields are all visible. + assert(*n_unnamed_members <= desc->n_in_sequence); return i; } @@ -621,6 +637,9 @@ _PyStructSequence_InitBuiltinWithFlags(PyInterpreterState *interp, } Py_ssize_t n_unnamed_members; Py_ssize_t n_members = count_members(desc, &n_unnamed_members); + if (n_members < 0) { + return -1; + } PyMemberDef *members = NULL; if ((type->tp_flags & Py_TPFLAGS_READY) == 0) { @@ -691,6 +710,9 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) } n_members = count_members(desc, &n_unnamed_members); + if (n_members < 0) { + return -1; + } members = initialize_members(desc, n_members, n_unnamed_members); if (members == NULL) { return -1; @@ -752,6 +774,9 @@ _PyStructSequence_NewType(PyStructSequence_Desc *desc, unsigned long tp_flags) /* Initialize MemberDefs */ n_members = count_members(desc, &n_unnamed_members); + if (n_members < 0) { + return NULL; + } members = initialize_members(desc, n_members, n_unnamed_members); if (members == NULL) { return NULL; From a06b34929dd636fa5a1397d6d88c2e117d1efc13 Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Sat, 25 Jul 2026 17:45:16 +0800 Subject: [PATCH 2/4] fix(structseq): reject negative n_in_sequence and fix int format specifiers count_members() now rejects a PyStructSequence_Desc whose n_in_sequence is negative, completing the bounds check (only n_in_sequence <= n_fields was enforced before). A negative value otherwise inflated tp_basicsize and failed later with a cryptic SystemError from PyTuple_New(). Also fix the two existing error messages: n_in_sequence is an int but was printed with %zd (Py_ssize_t), which is undefined behavior in a variadic call. Add a _testcapi helper and regression test for the negative case, and document the non-negative constraint. --- Doc/c-api/tuple.rst | 8 ++--- Lib/test/test_structseq.py | 30 +++++++++++-------- ...-07-25-15-28-37.gh-issue-154387.cR09uA.rst | 5 ++-- Modules/_testcapimodule.c | 26 ++++++++++++++++ Objects/structseq.c | 9 ++++-- 5 files changed, 58 insertions(+), 20 deletions(-) diff --git a/Doc/c-api/tuple.rst b/Doc/c-api/tuple.rst index 46f77c65959c2a..90f566f050cce5 100644 --- a/Doc/c-api/tuple.rst +++ b/Doc/c-api/tuple.rst @@ -167,8 +167,8 @@ type. .. versionchanged:: next Raise :exc:`SystemError` if *desc* places an unnamed field outside the visible - sequence fields, or if :c:member:`~PyStructSequence_Desc.n_in_sequence` exceeds - the total number of fields. + sequence fields, or if :c:member:`~PyStructSequence_Desc.n_in_sequence` is + negative or exceeds the total number of fields. .. c:function:: void PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc) @@ -207,8 +207,8 @@ type. .. c:member:: int n_in_sequence - Number of fields visible to the Python side (if used as tuple). Must not - exceed the total number of fields. + Number of fields visible to the Python side (if used as tuple). + Must be non-negative and must not exceed the total number of fields. .. c:type:: PyStructSequence_Field diff --git a/Lib/test/test_structseq.py b/Lib/test/test_structseq.py index 279c9a290eb061..c1e703c6245de5 100644 --- a/Lib/test/test_structseq.py +++ b/Lib/test/test_structseq.py @@ -11,6 +11,24 @@ class StructSeqTest(unittest.TestCase): + def test_newtype_rejects_negative_n_in_sequence(self): + # gh-154387: n_in_sequence must not be negative. + _testcapi = import_helper.import_module("_testcapi") + with self.assertRaises(SystemError): + _testcapi.test_structseq_newtype_negative_n_in_sequence() + + def test_newtype_rejects_unnamed_hidden_field(self): + # gh-154387: an unnamed field must be a visible sequence field. + _testcapi = import_helper.import_module("_testcapi") + with self.assertRaises(SystemError): + _testcapi.test_structseq_newtype_unnamed_hidden_field() + + def test_newtype_rejects_n_in_sequence_over_n_fields(self): + # gh-154387: n_in_sequence must not exceed the number of fields. + _testcapi = import_helper.import_module("_testcapi") + with self.assertRaises(SystemError): + _testcapi.test_structseq_newtype_too_many_visible_fields() + def test_tuple(self): t = time.gmtime() self.assertIsInstance(t, tuple) @@ -48,18 +66,6 @@ def test_repr(self): self.assertIn("st_ino=", rep) self.assertIn("st_dev=", rep) - def test_newtype_rejects_unnamed_hidden_field(self): - # gh-154387: an unnamed field must be a visible sequence field. - _testcapi = import_helper.import_module("_testcapi") - with self.assertRaises(SystemError): - _testcapi.test_structseq_newtype_unnamed_hidden_field() - - def test_newtype_rejects_n_in_sequence_over_n_fields(self): - # gh-154387: n_in_sequence must not exceed the number of fields. - _testcapi = import_helper.import_module("_testcapi") - with self.assertRaises(SystemError): - _testcapi.test_structseq_newtype_too_many_visible_fields() - def test_concat(self): t1 = time.gmtime() t2 = t1 + tuple(t1) diff --git a/Misc/NEWS.d/next/C_API/2026-07-25-15-28-37.gh-issue-154387.cR09uA.rst b/Misc/NEWS.d/next/C_API/2026-07-25-15-28-37.gh-issue-154387.cR09uA.rst index 23813840a194b2..cae5b00f6e589d 100644 --- a/Misc/NEWS.d/next/C_API/2026-07-25-15-28-37.gh-issue-154387.cR09uA.rst +++ b/Misc/NEWS.d/next/C_API/2026-07-25-15-28-37.gh-issue-154387.cR09uA.rst @@ -1,4 +1,5 @@ :c:func:`PyStructSequence_NewType` and :c:func:`PyStructSequence_InitType2` now raise :exc:`SystemError` when the :c:type:`PyStructSequence_Desc` places an -unnamed field outside the visible sequence fields, or sets ``n_in_sequence`` -larger than the total number of fields. Patch by Xuehai Pan. +unnamed field outside the visible sequence fields, or sets a negative +``n_in_sequence`` or one larger than the total number of fields. +Patch by Xuehai Pan. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 23c39fff07b00b..f37d0a714eed30 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1304,6 +1304,30 @@ test_structseq_newtype_null_descr_doc(PyObject *Py_UNUSED(self), Py_RETURN_NONE; } +static PyObject * +test_structseq_newtype_negative_n_in_sequence(PyObject *Py_UNUSED(self), + PyObject *Py_UNUSED(args)) +{ + // gh-154387: n_in_sequence must not be negative. + PyStructSequence_Field descr_fields[] = { + {"a", "field a"}, + {NULL, NULL}, + }; + PyStructSequence_Desc descr = { + "_testcapi.negative_n_in_sequence", NULL, descr_fields, -1, + }; + + PyTypeObject *type = PyStructSequence_NewType(&descr); + if (type != NULL) { + Py_DECREF(type); + PyErr_SetString(PyExc_AssertionError, + "negative n_in_sequence was not rejected"); + return NULL; + } + // NewType() failed and left the expected SystemError set; propagate it. + return NULL; +} + static PyObject * test_structseq_newtype_unnamed_hidden_field(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) @@ -3058,6 +3082,8 @@ static PyMethodDef TestMethods[] = { test_structseq_newtype_doesnt_leak, METH_NOARGS}, {"test_structseq_newtype_null_descr_doc", test_structseq_newtype_null_descr_doc, METH_NOARGS}, + {"test_structseq_newtype_negative_n_in_sequence", + test_structseq_newtype_negative_n_in_sequence, METH_NOARGS}, {"test_structseq_newtype_unnamed_hidden_field", test_structseq_newtype_unnamed_hidden_field, METH_NOARGS}, {"test_structseq_newtype_too_many_visible_fields", diff --git a/Objects/structseq.c b/Objects/structseq.c index 7c521d9e12b38f..f5d2195851e05d 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -469,6 +469,11 @@ static Py_ssize_t count_members(PyStructSequence_Desc *desc, Py_ssize_t *n_unnamed_members) { Py_ssize_t i; + if (desc->n_in_sequence < 0) { + PyErr_Format(PyExc_SystemError, + "%s: n_in_sequence=%d is negative", desc->name, desc->n_in_sequence); + return -1; + } *n_unnamed_members = 0; for (i = 0; desc->fields[i].name != NULL; ++i) { if (desc->fields[i].name == PyStructSequence_UnnamedField) { @@ -476,7 +481,7 @@ count_members(PyStructSequence_Desc *desc, Py_ssize_t *n_unnamed_members) { if (i >= desc->n_in_sequence) { PyErr_Format(PyExc_SystemError, "%s: unnamed field %zd is not a visible sequence field " - "(n_in_sequence=%zd)", + "(n_in_sequence=%d)", desc->name, i, desc->n_in_sequence); return -1; } @@ -485,7 +490,7 @@ count_members(PyStructSequence_Desc *desc, Py_ssize_t *n_unnamed_members) { } if (desc->n_in_sequence > i) { PyErr_Format(PyExc_SystemError, - "%s: n_in_sequence=%zd exceeds the total number of fields %zd", + "%s: n_in_sequence=%d exceeds the total number of fields %zd", desc->name, desc->n_in_sequence, i); return -1; } From 29c4c2e77de7b80c04e2adc966a6dd6f8cf910e5 Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Sat, 25 Jul 2026 22:00:28 +0800 Subject: [PATCH 3/4] fix: remove `test_` prefix for functions in `_testcapi` --- Lib/test/test_structseq.py | 6 +++--- Modules/_testcapimodule.c | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_structseq.py b/Lib/test/test_structseq.py index c1e703c6245de5..583e4b96bd654f 100644 --- a/Lib/test/test_structseq.py +++ b/Lib/test/test_structseq.py @@ -15,19 +15,19 @@ def test_newtype_rejects_negative_n_in_sequence(self): # gh-154387: n_in_sequence must not be negative. _testcapi = import_helper.import_module("_testcapi") with self.assertRaises(SystemError): - _testcapi.test_structseq_newtype_negative_n_in_sequence() + _testcapi.structseq_newtype_negative_n_in_sequence() def test_newtype_rejects_unnamed_hidden_field(self): # gh-154387: an unnamed field must be a visible sequence field. _testcapi = import_helper.import_module("_testcapi") with self.assertRaises(SystemError): - _testcapi.test_structseq_newtype_unnamed_hidden_field() + _testcapi.structseq_newtype_unnamed_hidden_field() def test_newtype_rejects_n_in_sequence_over_n_fields(self): # gh-154387: n_in_sequence must not exceed the number of fields. _testcapi = import_helper.import_module("_testcapi") with self.assertRaises(SystemError): - _testcapi.test_structseq_newtype_too_many_visible_fields() + _testcapi.structseq_newtype_too_many_visible_fields() def test_tuple(self): t = time.gmtime() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index f37d0a714eed30..235a0eae2d1161 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1305,7 +1305,7 @@ test_structseq_newtype_null_descr_doc(PyObject *Py_UNUSED(self), } static PyObject * -test_structseq_newtype_negative_n_in_sequence(PyObject *Py_UNUSED(self), +structseq_newtype_negative_n_in_sequence(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) { // gh-154387: n_in_sequence must not be negative. @@ -1329,7 +1329,7 @@ test_structseq_newtype_negative_n_in_sequence(PyObject *Py_UNUSED(self), } static PyObject * -test_structseq_newtype_unnamed_hidden_field(PyObject *Py_UNUSED(self), +structseq_newtype_unnamed_hidden_field(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) { // gh-154387: an unnamed field is only allowed among the visible sequence @@ -1355,7 +1355,7 @@ test_structseq_newtype_unnamed_hidden_field(PyObject *Py_UNUSED(self), } static PyObject * -test_structseq_newtype_too_many_visible_fields(PyObject *Py_UNUSED(self), +structseq_newtype_too_many_visible_fields(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) { // gh-154387: n_in_sequence must not exceed the total number of fields. @@ -3082,12 +3082,12 @@ static PyMethodDef TestMethods[] = { test_structseq_newtype_doesnt_leak, METH_NOARGS}, {"test_structseq_newtype_null_descr_doc", test_structseq_newtype_null_descr_doc, METH_NOARGS}, - {"test_structseq_newtype_negative_n_in_sequence", - test_structseq_newtype_negative_n_in_sequence, METH_NOARGS}, - {"test_structseq_newtype_unnamed_hidden_field", - test_structseq_newtype_unnamed_hidden_field, METH_NOARGS}, - {"test_structseq_newtype_too_many_visible_fields", - test_structseq_newtype_too_many_visible_fields, METH_NOARGS}, + {"structseq_newtype_negative_n_in_sequence", + structseq_newtype_negative_n_in_sequence, METH_NOARGS}, + {"structseq_newtype_unnamed_hidden_field", + structseq_newtype_unnamed_hidden_field, METH_NOARGS}, + {"structseq_newtype_too_many_visible_fields", + structseq_newtype_too_many_visible_fields, METH_NOARGS}, {"pyobject_repr_from_null", pyobject_repr_from_null, METH_NOARGS}, {"pyobject_str_from_null", pyobject_str_from_null, METH_NOARGS}, {"pyobject_bytes_from_null", pyobject_bytes_from_null, METH_NOARGS}, From 57095f02ba8a115d370bf009ba848d9d3847487f Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Mon, 27 Jul 2026 01:10:57 +0800 Subject: [PATCH 4/4] ci: try fix ci --- Lib/test/test_class.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_class.py b/Lib/test/test_class.py index 62d8806b75d9db..08778eb9355c5f 100644 --- a/Lib/test/test_class.py +++ b/Lib/test/test_class.py @@ -1019,6 +1019,8 @@ def test_detach_materialized_dict_no_memory(self): import test.support import _testcapi + set_nomemory = _testcapi.set_nomemory # eagerly load the function + class A: def __init__(self): self.a = 1 @@ -1026,7 +1028,7 @@ def __init__(self): a = A() d = a.__dict__ with test.support.catch_unraisable_exception() as ex: - _testcapi.set_nomemory(0, 1) + set_nomemory(0, 1) del a assert ex.unraisable.exc_type is MemoryError try: