Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion Doc/c-api/tuple.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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` is
negative or exceeds the total number of fields.


.. c:function:: void PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc)

Expand All @@ -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

Expand All @@ -199,6 +208,7 @@ type.
.. c:member:: int n_in_sequence

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
Expand All @@ -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 *``.
Expand Down
20 changes: 19 additions & 1 deletion Lib/test/test_structseq.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,29 @@
import textwrap
import time
import unittest
from test.support import script_helper
from test.support import import_helper, script_helper


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.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.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.structseq_newtype_too_many_visible_fields()

def test_tuple(self):
t = time.gmtime()
self.assertIsInstance(t, tuple)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +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 a negative
``n_in_sequence`` or one larger than the total number of fields.
Patch by Xuehai Pan.
81 changes: 81 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1304,6 +1304,81 @@ test_structseq_newtype_null_descr_doc(PyObject *Py_UNUSED(self),
Py_RETURN_NONE;
}

static PyObject *
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 *
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 *
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;
Expand Down Expand Up @@ -3007,6 +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},
{"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},
Expand Down
30 changes: 30 additions & 0 deletions Objects/structseq.c
Original file line number Diff line number Diff line change
Expand Up @@ -469,12 +469,33 @@ 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) {
// 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=%d)",
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=%d 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;
}

Expand Down Expand Up @@ -621,6 +642,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) {
Expand Down Expand Up @@ -691,6 +715,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;
Expand Down Expand Up @@ -752,6 +779,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;
Expand Down
Loading