Fail on passing py::object with wrong Python type to py::object subclass using PYBIND11_OBJECT macro (#2349)

* Fail on passing py::object with wrong Python type to py::object subclass using PYBIND11_OBJECT macro

* Split off test_non_converting_constructors from test_constructors

* Fix test_as_type, as py::type constructor now throws an error itself if the argument is not a type

* Replace tp_name access by pybind11::detail::get_fully_qualified_tp_name

* Move forward-declaration of get_fully_qualified_tp_name to detail/common.h

* Don't add the builtins module name in get_fully_qualified_tp_name for PyPy

* Add PYBIND11_BUILTINS_MODULE macro, and use it in get_fully_qualified_tp_name
This commit is contained in:
Yannick Jadoul 2020-10-05 22:48:54 +02:00 committed by GitHub
parent 2a2f52201d
commit f537093a2f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 49 additions and 16 deletions

View File

@ -39,9 +39,6 @@
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail) PYBIND11_NAMESPACE_BEGIN(detail)
// Forward-declaration; see detail/class.h
std::string get_fully_qualified_tp_name(PyTypeObject*);
/// A life support system for temporary objects created by `type_caster::load()`. /// A life support system for temporary objects created by `type_caster::load()`.
/// Adding a patient will keep it alive up until the enclosing function returns. /// Adding a patient will keep it alive up until the enclosing function returns.
class loader_life_support { class loader_life_support {

View File

@ -28,7 +28,11 @@ inline std::string get_fully_qualified_tp_name(PyTypeObject *type) {
#if !defined(PYPY_VERSION) #if !defined(PYPY_VERSION)
return type->tp_name; return type->tp_name;
#else #else
return handle((PyObject *) type).attr("__module__").cast<std::string>() + "." + type->tp_name; auto module_name = handle((PyObject *) type).attr("__module__").cast<std::string>();
if (module_name == PYBIND11_BUILTINS_MODULE)
return type->tp_name;
else
return std::move(module_name) + "." + type->tp_name;
#endif #endif
} }

View File

@ -182,6 +182,7 @@
#define PYBIND11_STR_TYPE ::pybind11::str #define PYBIND11_STR_TYPE ::pybind11::str
#define PYBIND11_BOOL_ATTR "__bool__" #define PYBIND11_BOOL_ATTR "__bool__"
#define PYBIND11_NB_BOOL(ptr) ((ptr)->nb_bool) #define PYBIND11_NB_BOOL(ptr) ((ptr)->nb_bool)
#define PYBIND11_BUILTINS_MODULE "builtins"
// Providing a separate declaration to make Clang's -Wmissing-prototypes happy. // Providing a separate declaration to make Clang's -Wmissing-prototypes happy.
// See comment for PYBIND11_MODULE below for why this is marked "maybe unused". // See comment for PYBIND11_MODULE below for why this is marked "maybe unused".
#define PYBIND11_PLUGIN_IMPL(name) \ #define PYBIND11_PLUGIN_IMPL(name) \
@ -209,6 +210,7 @@
#define PYBIND11_STR_TYPE ::pybind11::bytes #define PYBIND11_STR_TYPE ::pybind11::bytes
#define PYBIND11_BOOL_ATTR "__nonzero__" #define PYBIND11_BOOL_ATTR "__nonzero__"
#define PYBIND11_NB_BOOL(ptr) ((ptr)->nb_nonzero) #define PYBIND11_NB_BOOL(ptr) ((ptr)->nb_nonzero)
#define PYBIND11_BUILTINS_MODULE "__builtin__"
// Providing a separate PyInit decl to make Clang's -Wmissing-prototypes happy. // Providing a separate PyInit decl to make Clang's -Wmissing-prototypes happy.
// See comment for PYBIND11_MODULE below for why this is marked "maybe unused". // See comment for PYBIND11_MODULE below for why this is marked "maybe unused".
#define PYBIND11_PLUGIN_IMPL(name) \ #define PYBIND11_PLUGIN_IMPL(name) \
@ -853,8 +855,8 @@ public:
const std::vector<T> *operator->() const { return &v; } const std::vector<T> *operator->() const { return &v; }
}; };
// Forward-declaration; see detail/class.h
std::string get_fully_qualified_tp_name(PyTypeObject*);
PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(detail)
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

View File

@ -812,11 +812,18 @@ PYBIND11_NAMESPACE_END(detail)
: Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \ : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
{ if (!m_ptr) throw error_already_set(); } { if (!m_ptr) throw error_already_set(); }
#define PYBIND11_OBJECT_CHECK_FAILED(Name, o) \
type_error("Object of type '" + \
pybind11::detail::get_fully_qualified_tp_name(Py_TYPE(o.ptr())) + \
"' is not an instance of '" #Name "'")
#define PYBIND11_OBJECT(Name, Parent, CheckFun) \ #define PYBIND11_OBJECT(Name, Parent, CheckFun) \
PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
/* This is deliberately not 'explicit' to allow implicit conversion from object: */ \ /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \
Name(const object &o) : Parent(o) { } \ Name(const object &o) : Parent(o) \
Name(object &&o) : Parent(std::move(o)) { } { if (o && !check_(o)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, o); } \
Name(object &&o) : Parent(std::move(o)) \
{ if (o && !check_(o)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, o); }
#define PYBIND11_OBJECT_DEFAULT(Name, Parent, CheckFun) \ #define PYBIND11_OBJECT_DEFAULT(Name, Parent, CheckFun) \
PYBIND11_OBJECT(Name, Parent, CheckFun) \ PYBIND11_OBJECT(Name, Parent, CheckFun) \

View File

@ -157,11 +157,7 @@ TEST_SUBMODULE(class_, m) {
}); });
m.def("as_type", [](py::object ob) { m.def("as_type", [](py::object ob) {
auto tp = py::type(ob); return py::type(ob);
if (py::isinstance<py::type>(ob))
return tp;
else
throw std::runtime_error("Invalid type");
}); });
// test_mismatched_holder // test_mismatched_holder

View File

@ -59,10 +59,10 @@ def test_type_of_py_nodelete():
def test_as_type_py(): def test_as_type_py():
assert m.as_type(int) == int assert m.as_type(int) == int
with pytest.raises(RuntimeError): with pytest.raises(TypeError):
assert m.as_type(1) == int assert m.as_type(1) == int
with pytest.raises(RuntimeError): with pytest.raises(TypeError):
assert m.as_type(m.DerivedClass1()) == m.DerivedClass1 assert m.as_type(m.DerivedClass1()) == m.DerivedClass1

View File

@ -243,6 +243,19 @@ TEST_SUBMODULE(pytypes, m) {
m.def("convert_to_pybind11_str", [](py::object o) { return py::str(o); }); m.def("convert_to_pybind11_str", [](py::object o) { return py::str(o); });
m.def("nonconverting_constructor", [](std::string type, py::object value) -> py::object {
if (type == "bytes") {
return py::bytes(value);
}
else if (type == "none") {
return py::none(value);
}
else if (type == "ellipsis") {
return py::ellipsis(value);
}
throw std::runtime_error("Invalid type");
});
m.def("get_implicit_casting", []() { m.def("get_implicit_casting", []() {
py::dict d; py::dict d;
d["char*_i1"] = "abc"; d["char*_i1"] = "abc";

View File

@ -245,6 +245,20 @@ def test_constructors():
assert noconv2[k] is expected[k] assert noconv2[k] is expected[k]
def test_non_converting_constructors():
non_converting_test_cases = [
("bytes", range(10)),
("none", 42),
("ellipsis", 42),
]
for t, v in non_converting_test_cases:
with pytest.raises(TypeError) as excinfo:
m.nonconverting_constructor(t, v)
expected_error = "Object of type '{}' is not an instance of '{}'".format(
type(v).__name__, t)
assert str(excinfo.value) == expected_error
def test_pybind11_str_raw_str(): def test_pybind11_str_raw_str():
# specifically to exercise pybind11::str::raw_str # specifically to exercise pybind11::str::raw_str
cvt = m.convert_to_pybind11_str cvt = m.convert_to_pybind11_str