diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 27bd0535..22eceb05 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -929,31 +929,27 @@ public: py_value = (py_type) PyFloat_AsDouble(src.ptr()); else return false; - } else if (sizeof(T) <= sizeof(long)) { - if (PyFloat_Check(src.ptr())) - return false; - if (std::is_signed::value) - py_value = (py_type) PyLong_AsLong(src.ptr()); - else - py_value = (py_type) PyLong_AsUnsignedLong(src.ptr()); - } else { - if (PyFloat_Check(src.ptr())) - return false; - if (std::is_signed::value) - py_value = (py_type) PYBIND11_LONG_AS_LONGLONG(src.ptr()); - else - py_value = (py_type) PYBIND11_LONG_AS_UNSIGNED_LONGLONG(src.ptr()); + } else if (PyFloat_Check(src.ptr())) { + return false; + } else if (std::is_unsigned::value) { + py_value = as_unsigned(src.ptr()); + } else { // signed integer: + py_value = sizeof(T) <= sizeof(long) + ? (py_type) PyLong_AsLong(src.ptr()) + : (py_type) PYBIND11_LONG_AS_LONGLONG(src.ptr()); } - if ((py_value == (py_type) -1 && PyErr_Occurred()) || - (std::is_integral::value && sizeof(py_type) != sizeof(T) && - (py_value < (py_type) std::numeric_limits::min() || - py_value > (py_type) std::numeric_limits::max()))) { + bool py_err = py_value == (py_type) -1 && PyErr_Occurred(); + if (py_err || (std::is_integral::value && sizeof(py_type) != sizeof(T) && + (py_value < (py_type) std::numeric_limits::min() || + py_value > (py_type) std::numeric_limits::max()))) { + bool type_error = py_err && PyErr_ExceptionMatches( #if PY_VERSION_HEX < 0x03000000 && !defined(PYPY_VERSION) - bool type_error = PyErr_ExceptionMatches(PyExc_SystemError); + PyExc_SystemError #else - bool type_error = PyErr_ExceptionMatches(PyExc_TypeError); + PyExc_TypeError #endif + ); PyErr_Clear(); if (type_error && convert && PyNumber_Check(src.ptr())) { auto tmp = reinterpret_borrow(std::is_floating_point::value diff --git a/include/pybind11/common.h b/include/pybind11/common.h index af0a8d0e..1a5f7fa4 100644 --- a/include/pybind11/common.h +++ b/include/pybind11/common.h @@ -147,7 +147,6 @@ #define PYBIND11_BYTES_SIZE PyBytes_Size #define PYBIND11_LONG_CHECK(o) PyLong_Check(o) #define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o) -#define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) PyLong_AsUnsignedLongLong(o) #define PYBIND11_BYTES_NAME "bytes" #define PYBIND11_STRING_NAME "str" #define PYBIND11_SLICE_OBJECT PyObject @@ -167,7 +166,6 @@ #define PYBIND11_BYTES_SIZE PyString_Size #define PYBIND11_LONG_CHECK(o) (PyInt_Check(o) || PyLong_Check(o)) #define PYBIND11_LONG_AS_LONGLONG(o) (PyInt_Check(o) ? (long long) PyLong_AsLong(o) : PyLong_AsLongLong(o)) -#define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) (PyInt_Check(o) ? (unsigned long long) PyLong_AsUnsignedLong(o) : PyLong_AsUnsignedLongLong(o)) #define PYBIND11_BYTES_NAME "str" #define PYBIND11_STRING_NAME "unicode" #define PYBIND11_SLICE_OBJECT PySliceObject diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index a5aff662..cc48bbbf 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -931,6 +931,28 @@ private: } }; +NAMESPACE_BEGIN(detail) +// Converts a value to the given unsigned type. If an error occurs, you get back (Unsigned) -1; +// otherwise you get back the unsigned long or unsigned long long value cast to (Unsigned). +// (The distinction is critically important when casting a returned -1 error value to some other +// unsigned type: (A)-1 != (B)-1 when A and B are unsigned types of different sizes). +template +Unsigned as_unsigned(PyObject *o) { + if (sizeof(Unsigned) <= sizeof(unsigned long) +#if PY_VERSION_HEX < 0x03000000 + || PyInt_Check(o) +#endif + ) { + unsigned long v = PyLong_AsUnsignedLong(o); + return v == (unsigned long) -1 && PyErr_Occurred() ? (Unsigned) -1 : (Unsigned) v; + } + else { + unsigned long long v = PyLong_AsUnsignedLongLong(o); + return v == (unsigned long long) -1 && PyErr_Occurred() ? (Unsigned) -1 : (Unsigned) v; + } +} +NAMESPACE_END(detail) + class int_ : public object { public: PYBIND11_OBJECT_CVT(int_, object, PYBIND11_LONG_CHECK, PyNumber_Long) @@ -956,17 +978,11 @@ public: template ::value, int> = 0> operator T() const { - if (sizeof(T) <= sizeof(long)) { - if (std::is_signed::value) - return (T) PyLong_AsLong(m_ptr); - else - return (T) PyLong_AsUnsignedLong(m_ptr); - } else { - if (std::is_signed::value) - return (T) PYBIND11_LONG_AS_LONGLONG(m_ptr); - else - return (T) PYBIND11_LONG_AS_UNSIGNED_LONGLONG(m_ptr); - } + return std::is_unsigned::value + ? detail::as_unsigned(m_ptr) + : sizeof(T) <= sizeof(long) + ? (T) PyLong_AsLong(m_ptr) + : (T) PYBIND11_LONG_AS_LONGLONG(m_ptr); } }; diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index 33fe689a..55269bac 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -72,6 +72,12 @@ TEST_SUBMODULE(builtin_casters, m) { m.def("string_view32_return", []() { return std::u32string_view(U"utf32 secret \U0001f382"); }); #endif + // test_integer_casting + m.def("i32_str", [](std::int32_t v) { return std::to_string(v); }); + m.def("u32_str", [](std::uint32_t v) { return std::to_string(v); }); + m.def("i64_str", [](std::int64_t v) { return std::to_string(v); }); + m.def("u64_str", [](std::uint64_t v) { return std::to_string(v); }); + // test_tuple m.def("pair_passthrough", [](std::pair input) { return std::make_pair(input.second, input.first); diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 59af0ee9..a6f9b57a 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -143,6 +143,44 @@ def test_string_view(capture): """ +def test_integer_casting(): + """Issue #929 - out-of-range integer values shouldn't be accepted""" + import sys + assert m.i32_str(-1) == "-1" + assert m.i64_str(-1) == "-1" + assert m.i32_str(2000000000) == "2000000000" + assert m.u32_str(2000000000) == "2000000000" + if sys.version_info < (3,): + assert m.i32_str(long(-1)) == "-1" + assert m.i64_str(long(-1)) == "-1" + assert m.i64_str(long(-999999999999)) == "-999999999999" + assert m.u64_str(long(999999999999)) == "999999999999" + else: + assert m.i64_str(-999999999999) == "-999999999999" + assert m.u64_str(999999999999) == "999999999999" + + with pytest.raises(TypeError) as excinfo: + m.u32_str(-1) + assert "incompatible function arguments" in str(excinfo.value) + with pytest.raises(TypeError) as excinfo: + m.u64_str(-1) + assert "incompatible function arguments" in str(excinfo.value) + with pytest.raises(TypeError) as excinfo: + m.i32_str(-3000000000) + assert "incompatible function arguments" in str(excinfo.value) + with pytest.raises(TypeError) as excinfo: + m.i32_str(3000000000) + assert "incompatible function arguments" in str(excinfo.value) + + if sys.version_info < (3,): + with pytest.raises(TypeError) as excinfo: + m.u32_str(long(-1)) + assert "incompatible function arguments" in str(excinfo.value) + with pytest.raises(TypeError) as excinfo: + m.u64_str(long(-1)) + assert "incompatible function arguments" in str(excinfo.value) + + def test_tuple(doc): """std::pair <-> tuple & std::tuple <-> tuple""" assert m.pair_passthrough((True, "test")) == ("test", True)