gh-146056: Fix list.__repr__() for lists containing NULLs

[serhiy-storchaka]

• Issue: Double-free in xml.etree.ElementTree.TreeBuilder #146056

[vstinner]

If you change PyUnicodeWriter_WriteRepr() to accept NULL, please apply this change to update the doc and tests:

diff --git a/Doc/c-api/unicode.rst b/Doc/c-api/unicode.rst
index 4845e0f3002..3fcfe9a41c9 100644
--- a/Doc/c-api/unicode.rst
+++ b/Doc/c-api/unicode.rst
@@ -1887,6 +1887,8 @@ object.
 
    Call :c:func:`PyObject_Repr` on *obj* and write the output into *writer*.
 
+   If *obj* is ``NULL``, write ``<NULL>`` into *writer*.
+
    On success, return ``0``.
    On error, set an exception, leave the writer unchanged, and return ``-1``.
 
diff --git a/Lib/test/test_capi/test_unicode.py b/Lib/test/test_capi/test_unicode.py
index 6a9c60f3a6d..55120448a8a 100644
--- a/Lib/test/test_capi/test_unicode.py
+++ b/Lib/test/test_capi/test_unicode.py
@@ -1779,6 +1779,13 @@ def test_basic(self):
         self.assertEqual(writer.finish(),
                          "var=long value 'repr'")
 
+    def test_repr_null(self):
+        writer = self.create_writer(0)
+        writer.write_utf8(b'var=', -1)
+        writer.write_repr(NULL)
+        self.assertEqual(writer.finish(),
+                         "var=<NULL>")
+
     def test_utf8(self):
         writer = self.create_writer(0)
         writer.write_utf8(b"ascii", -1)
diff --git a/Modules/_testcapi/unicode.c b/Modules/_testcapi/unicode.c
index 203282dd53d..668adc5085b 100644
--- a/Modules/_testcapi/unicode.c
+++ b/Modules/_testcapi/unicode.c
@@ -449,6 +449,7 @@ writer_write_repr(PyObject *self_raw, PyObject *args)
     if (!PyArg_ParseTuple(args, "O", &obj)) {
         return NULL;
     }
+    NULLABLE(obj);
 
     if (PyUnicodeWriter_WriteRepr(self->writer, obj) < 0) {
         return NULL;
diff --git a/Objects/unicode_writer.c b/Objects/unicode_writer.c
index 2b944bf1ea8..cd2688e32df 100644
--- a/Objects/unicode_writer.c
+++ b/Objects/unicode_writer.c
@@ -383,6 +383,10 @@ PyUnicodeWriter_WriteStr(PyUnicodeWriter *writer, PyObject *obj)
 int
 PyUnicodeWriter_WriteRepr(PyUnicodeWriter *writer, PyObject *obj)
 {
+    if (obj == NULL) {
+        return _PyUnicodeWriter_WriteASCIIString((_PyUnicodeWriter*)writer, "<NULL>", 6);
+    }
+
     if (Py_TYPE(obj) == &PyLong_Type) {
         return _PyLong_FormatWriter((_PyUnicodeWriter*)writer, obj, 10, 0);
     }

[vstinner]

Please add a test on tuple as well:

diff --git a/Lib/test/test_capi/test_tuple.py b/Lib/test/test_capi/test_tuple.py
index 0c27e81168f..51d26640865 100644
--- a/Lib/test/test_capi/test_tuple.py
+++ b/Lib/test/test_capi/test_tuple.py
@@ -73,6 +73,11 @@ def test_tuple_new(self):
         self.assertRaises(SystemError, tuple_new, PY_SSIZE_T_MIN)
         self.assertRaises(MemoryError, tuple_new, PY_SSIZE_T_MAX)
 
+
+    def test_uninitialized_tuple_repr(self):
+        tup = _testlimitedcapi.tuple_new(3)
+        self.assertEqual(repr(tup), '(<NULL>, <NULL>, <NULL>)')
+
     def test_tuple_fromarray(self):
         # Test PyTuple_FromArray()
         tuple_fromarray = _testcapi.tuple_fromarray

[vstinner]

LGTM. I was against this before knowing that PyObject_Repr() and Python 3.12 repr(list) already support NULL, then I changed my mind.

IMO we should fix repr(list) in Python 3.13 and 3.14. But I don't think that we should backport the PyUnicodeWriter_WriteRepr() change.

[serhiy-storchaka]

I think this was a bug in the PyUnicodeWriter C API. When the user rewrite they code to using it, they can reasonably assume that PyUnicodeWriter_WriteRepr() is equivalent to PyUnicodeWriter_WriteStr(PyObject_Repr()). This is a mine. It is better to fix it as fast as possible.

I have found few other bugs in the PyUnicodeWriter C API, working on them.

[miss-islington-app]

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

[miss-islington-app]

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 0f2246b1553f401da5ade47e0fd1c80ad7a8dfa5 3.14

[miss-islington-app]

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 0f2246b1553f401da5ade47e0fd1c80ad7a8dfa5 3.13

[bedevere-app]

GH-146155 is a backport of this pull request to the 3.14 branch.

[vstinner]

FYI I updated PyUnicodeWriter_WriteRepr() in pythoncapi-compat (backport): python/pythoncapi-compat#170.