From 97b527ac26494c01fe244b12c3c6be68e2d0f2f0 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 30 Oct 2022 17:06:53 -0700 Subject: [PATCH 1/4] Fix & test for issue #4288 (unicode surrogate character in Python exception message). --- include/pybind11/pytypes.h | 18 +++++++++++++++++- tests/test_exceptions.cpp | 5 ----- tests/test_exceptions.py | 14 ++++++++++++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 80a2e2228e..7e3ee06588 100644 --- 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 = ""; } else { - result = value_str.cast(); + // Not using `value_str.cast()`, to not potentially throw a secondary + // error_already_set that will then result in process termination (#4288). + auto value_bytes = reinterpret_steal( + PyUnicode_AsEncodedString(value_str.ptr(), "utf-8", "backslashreplace")); + if (!value_bytes) { + message_error_string = detail::error_string(); + result = ""; + } 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 = ""; + } else { + result = std::string(buffer, static_cast(length)); + } + } } } else { result = ""; diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 3583f22a50..f57e095068 100644 --- 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 index 70b6ffea95..7eb1a9d62c 100644 --- 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": From e3bf61f9d1671ae884d942b5fc1db0fcb56fae31 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 30 Oct 2022 17:32:35 -0700 Subject: [PATCH 2/4] DRY `message_unavailable_exc` --- include/pybind11/pytypes.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 7e3ee06588..ecfd8f3c0e 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -501,9 +501,10 @@ struct error_fetch_and_normalize { std::string message_error_string; if (m_value) { auto value_str = reinterpret_steal(PyObject_Str(m_value.ptr())); + const char *message_unavailable_exc = ""; if (!value_str) { message_error_string = detail::error_string(); - result = ""; + result = message_unavailable_exc; } else { // Not using `value_str.cast()`, to not potentially throw a secondary // error_already_set that will then result in process termination (#4288). @@ -511,13 +512,13 @@ struct error_fetch_and_normalize { PyUnicode_AsEncodedString(value_str.ptr(), "utf-8", "backslashreplace")); if (!value_bytes) { message_error_string = detail::error_string(); - result = ""; + 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 = ""; + result = message_unavailable_exc; } else { result = std::string(buffer, static_cast(length)); } From 108ec221028dcd661dbda9767b318af810333dd7 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 31 Oct 2022 11:04:40 -0400 Subject: [PATCH 3/4] fix: add a constexpr Co-authored-by: Aaron Gokaslan --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index ecfd8f3c0e..b77db961b4 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -501,7 +501,7 @@ struct error_fetch_and_normalize { std::string message_error_string; if (m_value) { auto value_str = reinterpret_steal(PyObject_Str(m_value.ptr())); - const char *message_unavailable_exc = ""; + constexpr const char *message_unavailable_exc = ""; if (!value_str) { message_error_string = detail::error_string(); result = message_unavailable_exc; From 35f87f733e5053610808b3e0a0b7fab81cdb2f55 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 31 Oct 2022 15:06:21 +0000 Subject: [PATCH 4/4] style: pre-commit fixes --- include/pybind11/pytypes.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index b77db961b4..91cbaaba28 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -501,7 +501,8 @@ struct error_fetch_and_normalize { std::string message_error_string; if (m_value) { auto value_str = reinterpret_steal(PyObject_Str(m_value.ptr())); - constexpr const char *message_unavailable_exc = ""; + constexpr const char *message_unavailable_exc + = ""; if (!value_str) { message_error_string = detail::error_string(); result = message_unavailable_exc;