Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

3 changes: 0 additions & 3 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
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()`.
/// Adding a patient will keep it alive up until the enclosing function returns.
class loader_life_support {
Expand Down
15 changes: 14 additions & 1 deletion include/pybind11/detail/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,20 @@ inline std::string get_fully_qualified_tp_name(PyTypeObject *type) {
#if !defined(PYPY_VERSION)
return type->tp_name;
#else
return handle((PyObject *) type).attr("__module__").cast<std::string>() + "." + type->tp_name;
auto module_name = handle((PyObject *) type).attr("__module__").cast<std::string>();
bool is_builtin_module = module_name ==
# if PY_MAJOR_VERSION >= 3
YannickJadoul marked this conversation as resolved.
Show resolved Hide resolved
"builtins"
# else
"__builtin__"
# endif
;
if (is_builtin_module) {
return type->tp_name;
}
else {
return std::move(module_name) + "." + type->tp_name;
}
#endif
}

Expand Down
6 changes: 3 additions & 3 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -853,8 +853,8 @@ class any_container {
const std::vector<T> *operator->() const { return &v; }
};

PYBIND11_NAMESPACE_END(detail)


// Forward-declaration; see detail/class.h
std::string get_fully_qualified_tp_name(PyTypeObject*);

PYBIND11_NAMESPACE_END(detail)
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
11 changes: 9 additions & 2 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -812,11 +812,18 @@ PYBIND11_NAMESPACE_END(detail)
: Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
{ 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) \
PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
/* This is deliberately not 'explicit' to allow implicit conversion from object: */ \
Name(const object &o) : Parent(o) { } \
Name(object &&o) : Parent(std::move(o)) { }
Name(const object &o) : Parent(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) \
PYBIND11_OBJECT(Name, Parent, CheckFun) \
Expand Down
6 changes: 1 addition & 5 deletions tests/test_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,7 @@ TEST_SUBMODULE(class_, m) {
});

m.def("as_type", [](py::object ob) {
auto tp = py::type(ob);
if (py::isinstance<py::type>(ob))
return tp;
else
throw std::runtime_error("Invalid type");
return py::type(ob);
});

// test_mismatched_holder
Expand Down
4 changes: 2 additions & 2 deletions tests/test_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ def test_type_of_py_nodelete():
def test_as_type_py():
assert m.as_type(int) == int

with pytest.raises(RuntimeError):
with pytest.raises(TypeError):
assert m.as_type(1) == int

with pytest.raises(RuntimeError):
with pytest.raises(TypeError):
assert m.as_type(m.DerivedClass1()) == m.DerivedClass1


Expand Down
13 changes: 13 additions & 0 deletions tests/test_pytypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,19 @@ TEST_SUBMODULE(pytypes, m) {

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", []() {
py::dict d;
d["char*_i1"] = "abc";
Expand Down
14 changes: 14 additions & 0 deletions tests/test_pytypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,20 @@ def test_constructors():
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():
# specifically to exercise pybind11::str::raw_str
cvt = m.convert_to_pybind11_str
Expand Down