serhiy-storchaka · Nov 11, 2026
diff --git a/Doc/library/xml.dom.minidom.rst b/Doc/library/xml.dom.minidom.rst
@@ -147,6 +147,15 @@ module documentation.  This section lists the differences between the API and
    otherwise it is set to ``"no"``.
    Not stating the argument will omit the declaration from the document.
 
+   :exc:`ValueError` is raised if the content of the node cannot be
+   represented in XML, for example if a comment contains ``"--"`` or
+   a CDATA section contains ``"]]>"``.
+   The characters are not checked:
+   the text, the attribute values and the content of comments,
+   processing instructions and CDATA sections are written as is.
+   If they can contain characters which are not allowed in XML,
+   check them with :func:`xml.is_valid_text` before serialization.
+
    .. versionchanged:: 3.8
       The :meth:`writexml` method now preserves the attribute order specified
       by the user.

diff --git a/Lib/test/test_minidom.py b/Lib/test/test_minidom.py
@@ -2228,10 +2228,71 @@ def testDeepcopiedDocument(self):
         doc2 = copy.deepcopy(doc)
         self.assert_recursive_equal(doc, doc2)
 
-    def testSerializeCommentNodeWithDoubleHyphen(self):
-        doc = create_doc_without_doctype()
-        doc.appendChild(doc.createComment("foo--bar"))
-        self.assertRaises(ValueError, doc.toxml)
+    def testWriteXMLDoctypeIdentifiers(self):
+        impl = getDOMImplementation()
+        def serialize(publicId, systemId):
+            doctype = impl.createDocumentType("r", publicId, systemId)
+            return impl.createDocument(None, "r", doctype).toxml()[22:]
+        self.assertEqual(serialize(None, None), '<!DOCTYPE r><r/>')
+        self.assertEqual(serialize(None, "s"), "<!DOCTYPE r  SYSTEM 's'><r/>")
+        self.assertEqual(serialize("p", "s"),
+                         "<!DOCTYPE r  PUBLIC 'p'  's'><r/>")
+        # the quote which is not in the literal is used
+        self.assertEqual(serialize("it's", "sys'tem"),
+                         '<!DOCTYPE r  PUBLIC "it\'s"  "sys\'tem"><r/>')
+        # a system literal cannot contain both quotes
+        with self.assertRaisesRegex(ValueError, "both quotes"):
+            serialize(None, 'a"b\'c')
+        # a public identifier can only contain PubidChars
+        for publicId in ['pub"lic', 'p>', 'p<', 'p&', 'p[', '\xe9']:
+            with self.subTest(publicId=publicId):
+                with self.assertRaisesRegex(ValueError,
+                                            "invalid public identifier"):
+                    serialize(publicId, "s")
+        # a public identifier is meaningless without a system identifier
+        with self.assertRaisesRegex(ValueError, "requires a system"):
+            serialize("p", None)
+
+    def testSerializeComment(self):
+        impl = getDOMImplementation()
+        def serialize(data):
+            doc = impl.createDocument(None, "r", None)
+            doc.documentElement.appendChild(doc.createComment(data))
+            return doc.toxml()[22:]
+        self.assertEqual(serialize("foo"), "<r><!--foo--></r>")
+        self.assertEqual(serialize(""), "<r><!----></r>")
+        self.assertEqual(serialize("-foo"), "<r><!---foo--></r>")
+        with self.assertRaisesRegex(ValueError, "'--' is not allowed"):
+            serialize("foo--bar")
+        # a trailing hyphen would be fused with the closing "-->"
+        with self.assertRaisesRegex(ValueError, "cannot end with"):
+            serialize("foo-")
+
+    def testSerializeProcessingInstruction(self):
+        impl = getDOMImplementation()
+        def serialize(target, data):
+            doc = impl.createDocument(None, "r", None)
+            pi = doc.createProcessingInstruction(target, data)
+            doc.documentElement.appendChild(pi)
+            return doc.toxml()[22:]
+        self.assertEqual(serialize("p", "a"), "<r><?p a?></r>")
+        self.assertEqual(serialize("p", ""), "<r><?p ?></r>")
+        self.assertEqual(serialize("xml-stylesheet", 'href="a"'),
+                         '<r><?xml-stylesheet href="a"?></r>')
+        # "?>" would close the processing instruction
+        with self.assertRaisesRegex(ValueError, "'\\?>' is not allowed"):
+            serialize("p", "a?>b")
+        # the target "xml" is reserved
+        for target in ["xml", "XML", "XmL"]:
+            with self.subTest(target=target):
+                with self.assertRaisesRegex(ValueError, "reserved"):
+                    serialize(target, "a")
+        # the target is a name
+        doc = impl.createDocument(None, "r", None)
+        pi = doc.createProcessingInstruction("p", "a")
+        doc.documentElement.appendChild(pi)
+        pi.target = "a b"
+        self.assertRaises(xml.dom.InvalidCharacterErr, doc.toxml)
 
 
     def testEmptyXMLNSValue(self):

diff --git a/Lib/xml/dom/minidom.py b/Lib/xml/dom/minidom.py
@@ -300,6 +300,27 @@ def __exit__(self, et, ev, tb):
 defproperty(Node, "localName",  doc="Namespace-local name of this node.")
 
 
+# PubidChar (see XML 1.0, 2.3, [13]).
+_PUBID_CHARS = frozenset(
+    " \r\n-'()+,./:=?;!*#@$_%"
+    "0123456789"
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+    "abcdefghijklmnopqrstuvwxyz")
+
+
+def _quote_literal(text):
+    """Quote a system literal or a public identifier.
+
+    A literal is delimited with a quote which it does not contain
+    (see XML 1.0, 2.3, [11] and [12]).
+    """
+    if "'" not in text:
+        return "'%s'" % text
+    if '"' not in text:
+        return '"%s"' % text
+    raise ValueError("a literal cannot contain both quotes: %r" % (text,))
+
+
 def _check_name(name):
     if not xml.is_valid_name(name):
         raise xml.dom.InvalidCharacterErr(
@@ -1228,7 +1249,20 @@ def _set_nodeName(self, value):
     nodeName = property(_get_nodeName, _set_nodeName)
 
     def writexml(self, writer, indent="", addindent="", newl=""):
-        writer.write("%s<?%s %s?>%s" % (indent,self.target, self.data, newl))
+        target = self.target
+        # PITarget is a Name, except the names matching "xml" in any case
+        # (see XML 1.0, 2.6, [17]).
+        _check_name(target)
+        if target.lower() == "xml":
+            raise ValueError("%r is a reserved processing instruction target"
+                             % (target,))
+        data = self.data
+        # The data cannot contain "?>", which ends the instruction
+        # (see XML 1.0, 2.6, [16]).
+        if "?>" in data:
+            raise ValueError("'?>' is not allowed in a processing "
+                             "instruction node")
+        writer.write("%s<?%s %s?>%s" % (indent, target, data, newl))
 
 
 class CharacterData(Childless, Node):
@@ -1423,9 +1457,14 @@ def __init__(self, data):
         self._data = data
 
     def writexml(self, writer, indent="", addindent="", newl=""):
-        if "--" in self.data:
+        data = self.data
+        # The data cannot contain "--", and cannot end with "-", which
+        # would be fused with the closing "-->" (see XML 1.0, 2.5, [15]).
+        if "--" in data:
             raise ValueError("'--' is not allowed in a comment node")
-        writer.write("%s<!--%s-->%s" % (indent, self.data, newl))
+        if data.endswith("-"):
+            raise ValueError("a comment node cannot end with '-'")
+        writer.write("%s<!--%s-->%s" % (indent, data, newl))
 
 
 class CDATASection(Text):
@@ -1571,10 +1610,20 @@ def writexml(self, writer, indent="", addindent="", newl=""):
         writer.write("<!DOCTYPE ")
         writer.write(self.name)
         if self.publicId:
-            writer.write("%s  PUBLIC '%s'%s  '%s'"
-                         % (newl, self.publicId, newl, self.systemId))
+            if not _PUBID_CHARS.issuperset(self.publicId):
+                raise ValueError("invalid public identifier: %r"
+                                 % (self.publicId,))
+            # ExternalID with a public identifier always includes
+            # a system identifier (see XML 1.0, 4.2.2, [75]).
+            if not self.systemId:
+                raise ValueError("a public identifier requires "
+                                 "a system identifier")
+            writer.write("%s  PUBLIC %s%s  %s"
+                         % (newl, _quote_literal(self.publicId),
+                            newl, _quote_literal(self.systemId)))
         elif self.systemId:
-            writer.write("%s  SYSTEM '%s'" % (newl, self.systemId))
+            writer.write("%s  SYSTEM %s" % (newl,
+                                            _quote_literal(self.systemId)))
         if self.internalSubset is not None:
             writer.write(" [")
             writer.write(self.internalSubset)

diff --git a/Misc/NEWS.d/next/Library/2026-11-11-20-50-03.gh-issue-156812.nqPw30.rst b/Misc/NEWS.d/next/Library/2026-11-11-20-50-03.gh-issue-156812.nqPw30.rst
@@ -0,0 +1,10 @@
+Serializing a document type declaration, a comment or a processing
+instruction in :mod:`xml.dom.minidom` now checks the structural rules of XML.
+A system literal is now quoted with the quote which it does not contain,
+and :exc:`ValueError` is raised for an invalid public identifier,
+for a literal containing both quotes,
+for a public identifier without a system identifier,
+for a comment ending with ``-``,
+and for a processing instruction containing ``?>`` in the data
+or using the reserved target ``xml``.
+The characters are still not checked.