fix: unicode surrogate character in Python exception message. (#4297)

* Fix & test for issue #4288 (unicode surrogate character in Python exception message). * DRY `message_unavailable_exc` * fix: add a constexpr Co-authored-by: Aaron Gokaslan <skylion.aaron@gmail.com> * style: pre-commit fixes Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com> Co-authored-by: Aaron Gokaslan <skylion.aaron@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
pybind · Nov 18, 2022 · c3987b1 · c3987b1
1 parent 6d3a0fc
commit c3987b1
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 7 deletions.
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
@@ -501,11 +501,29 @@ struct error_fetch_and_normalize {
         std::string message_error_string;
         if (m_value) {
             auto value_str = reinterpret_steal<object>(PyObject_Str(m_value.ptr()));
+            constexpr const char *message_unavailable_exc
+                = "<MESSAGE UNAVAILABLE DUE TO ANOTHER EXCEPTION>";
             if (!value_str) {
                 message_error_string = detail::error_string();
-                result = "<MESSAGE UNAVAILABLE DUE TO ANOTHER EXCEPTION>";
+                result = message_unavailable_exc;
             } else {
-                result = value_str.cast<std::string>();
+                // Not using `value_str.cast<std::string>()`, to not potentially throw a secondary
+                // error_already_set that will then result in process termination (#4288).
+                auto value_bytes = reinterpret_steal<object>(
+                    PyUnicode_AsEncodedString(value_str.ptr(), "utf-8", "backslashreplace"));
+                if (!value_bytes) {
+                    message_error_string = detail::error_string();
+                    result = message_unavailable_exc;
+                } else {
+                    char *buffer = nullptr;
+                    Py_ssize_t length = 0;
+                    if (PyBytes_AsStringAndSize(value_bytes.ptr(), &buffer, &length) == -1) {
+                        message_error_string = detail::error_string();
+                        result = message_unavailable_exc;
+                    } else {
+                        result = std::string(buffer, static_cast<std::size_t>(length));
+                    }
+                }
             }
         } else {
             result = "<MESSAGE UNAVAILABLE>";

diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp
@@ -105,11 +105,6 @@ struct PythonAlreadySetInDestructor {
     py::str s;
 };
 
-std::string error_already_set_what(const py::object &exc_type, const py::object &exc_value) {
-    PyErr_SetObject(exc_type.ptr(), exc_value.ptr());
-    return py::error_already_set().what();
-}
-
 TEST_SUBMODULE(exceptions, m) {
     m.def("throw_std_exception",
           []() { throw std::runtime_error("This exception was intentionally thrown."); });

diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py
@@ -275,6 +275,20 @@ def test_local_translator(msg):
     assert msg(excinfo.value) == "this mod"
 
 
+def test_error_already_set_message_with_unicode_surrogate():  # Issue #4288
+    assert m.error_already_set_what(RuntimeError, "\ud927") == (
+        "RuntimeError: \\ud927",
+        False,
+    )
+
+
+def test_error_already_set_message_with_malformed_utf8():
+    assert m.error_already_set_what(RuntimeError, b"\x80") == (
+        "RuntimeError: b'\\x80'",
+        False,
+    )
+
+
 class FlakyException(Exception):
     def __init__(self, failure_point):
         if failure_point == "failure_point_init":