]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
[3.12] gh-139400: Make sure that parent parsers outlive their subparsers in `pyexpat...
authorSebastian Pipping <sebastian@pipping.org>
Tue, 7 Oct 2025 11:56:31 +0000 (13:56 +0200)
committerGitHub <noreply@github.com>
Tue, 7 Oct 2025 11:56:31 +0000 (13:56 +0200)
Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2ddb5f3695cf4938979d645f31d7fba43ec8)

Lib/test/test_pyexpat.py
Misc/NEWS.d/next/Security/2025-09-29-00-01-28.gh-issue-139400.X2T-jO.rst [new file with mode: 0644]
Modules/pyexpat.c

index 43cbd27151d5a2f9c0b9744cd9a3a4b815bf30ec..4dc1d1991b2b100074ca27817e2bb9b6aa75285c 100644 (file)
@@ -758,6 +758,42 @@ class ForeignDTDTests(unittest.TestCase):
         self.assertEqual(handler_call_args, [("bar", "baz")])
 
 
+class ParentParserLifetimeTest(unittest.TestCase):
+    """
+    Subparsers make use of their parent XML_Parser inside of Expat.
+    As a result, parent parsers need to outlive subparsers.
+
+    See https://github.com/python/cpython/issues/139400.
+    """
+
+    def test_parent_parser_outlives_its_subparsers__single(self):
+        parser = expat.ParserCreate()
+        subparser = parser.ExternalEntityParserCreate(None)
+
+        # Now try to cause garbage collection of the parent parser
+        # while it's still being referenced by a related subparser.
+        del parser
+
+    def test_parent_parser_outlives_its_subparsers__multiple(self):
+        parser = expat.ParserCreate()
+        subparser_one = parser.ExternalEntityParserCreate(None)
+        subparser_two = parser.ExternalEntityParserCreate(None)
+
+        # Now try to cause garbage collection of the parent parser
+        # while it's still being referenced by a related subparser.
+        del parser
+
+    def test_parent_parser_outlives_its_subparsers__chain(self):
+        parser = expat.ParserCreate()
+        subparser = parser.ExternalEntityParserCreate(None)
+        subsubparser = subparser.ExternalEntityParserCreate(None)
+
+        # Now try to cause garbage collection of the parent parsers
+        # while they are still being referenced by a related subparser.
+        del parser
+        del subparser
+
+
 class ReparseDeferralTest(unittest.TestCase):
     def test_getter_setter_round_trip(self):
         parser = expat.ParserCreate()
diff --git a/Misc/NEWS.d/next/Security/2025-09-29-00-01-28.gh-issue-139400.X2T-jO.rst b/Misc/NEWS.d/next/Security/2025-09-29-00-01-28.gh-issue-139400.X2T-jO.rst
new file mode 100644 (file)
index 0000000..a5dea3b
--- /dev/null
@@ -0,0 +1,4 @@
+:mod:`xml.parsers.expat`: Make sure that parent Expat parsers are only
+garbage-collected once they are no longer referenced by subparsers created
+by :meth:`~xml.parsers.expat.xmlparser.ExternalEntityParserCreate`.
+Patch by Sebastian Pipping.
index b354a86e7f9fee7586dbb36806a46f2e06fa9bc1..d941d25b79be89c9d8a6a863aadc58ed38621913 100644 (file)
@@ -69,6 +69,15 @@ typedef struct {
     PyObject_HEAD
 
     XML_Parser itself;
+    /*
+     * Strong reference to a parent `xmlparseobject` if this parser
+     * is a child parser. Set to NULL if this parser is a root parser.
+     * This is needed to keep the parent parser alive as long as it has
+     * at least one child parser.
+     *
+     * See https://github.com/python/cpython/issues/139400 for details.
+     */
+    PyObject *parent;
     int ordered_attributes;     /* Return attributes as a list. */
     int specified_attributes;   /* Report only specified attributes. */
     int in_callback;            /* Is a callback active? */
@@ -990,6 +999,11 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
         return NULL;
     }
 
+    // The new subparser will make use of the parent XML_Parser inside of Expat.
+    // So we need to take subparsers into account with the reference counting
+    // of their parent parser.
+    Py_INCREF(self);
+
     new_parser->buffer_size = self->buffer_size;
     new_parser->buffer_used = 0;
     new_parser->buffer = NULL;
@@ -999,6 +1013,7 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
     new_parser->ns_prefixes = self->ns_prefixes;
     new_parser->itself = XML_ExternalEntityParserCreate(self->itself, context,
                                                         encoding);
+    new_parser->parent = (PyObject *)self;
     new_parser->handlers = 0;
     new_parser->intern = Py_XNewRef(self->intern);
 
@@ -1006,11 +1021,13 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
         new_parser->buffer = PyMem_Malloc(new_parser->buffer_size);
         if (new_parser->buffer == NULL) {
             Py_DECREF(new_parser);
+            Py_DECREF(self);
             return PyErr_NoMemory();
         }
     }
     if (!new_parser->itself) {
         Py_DECREF(new_parser);
+        Py_DECREF(self);
         return PyErr_NoMemory();
     }
 
@@ -1023,6 +1040,7 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
     new_parser->handlers = PyMem_New(PyObject *, i);
     if (!new_parser->handlers) {
         Py_DECREF(new_parser);
+        Py_DECREF(self);
         return PyErr_NoMemory();
     }
     clear_handlers(new_parser, 1);
@@ -1212,6 +1230,7 @@ newxmlparseobject(pyexpat_state *state, const char *encoding,
     /* namespace_separator is either NULL or contains one char + \0 */
     self->itself = XML_ParserCreate_MM(encoding, &ExpatMemoryHandler,
                                        namespace_separator);
+    self->parent = NULL;
     if (self->itself == NULL) {
         PyErr_SetString(PyExc_RuntimeError,
                         "XML_ParserCreate failed");
@@ -1247,6 +1266,7 @@ xmlparse_traverse(xmlparseobject *op, visitproc visit, void *arg)
     for (int i = 0; handler_info[i].name != NULL; i++) {
         Py_VISIT(op->handlers[i]);
     }
+    Py_VISIT(op->parent);
     Py_VISIT(Py_TYPE(op));
     return 0;
 }
@@ -1256,6 +1276,10 @@ xmlparse_clear(xmlparseobject *op)
 {
     clear_handlers(op, 0);
     Py_CLEAR(op->intern);
+    // NOTE: We cannot call Py_CLEAR(op->parent) prior to calling
+    //       XML_ParserFree(op->itself), or a subparser could lose its parent
+    //       XML_Parser while still making use of it internally.
+    //       https://github.com/python/cpython/issues/139400
     return 0;
 }
 
@@ -1267,6 +1291,7 @@ xmlparse_dealloc(xmlparseobject *self)
     if (self->itself != NULL)
         XML_ParserFree(self->itself);
     self->itself = NULL;
+    Py_CLEAR(self->parent);
 
     if (self->handlers != NULL) {
         PyMem_Free(self->handlers);