Fix & test for issue pybind#4288 (unicode surrogate character in Pyth…

…on exception message).
rwgk · Oct 31, 2022 · 97b527a · 97b527a
1 parent 5bc0943
commit 97b527a
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 6 deletions.
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
@@ -505,7 +505,23 @@ struct error_fetch_and_normalize {
                 message_error_string = detail::error_string();
                 result = "<MESSAGE UNAVAILABLE DUE TO ANOTHER EXCEPTION>";
             } 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 DUE TO ANOTHER EXCEPTION>";
+                } 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 DUE TO ANOTHER EXCEPTION>";
+                    } 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":