diff --git a/newsfragments/4454.fixed.md b/newsfragments/4454.fixed.md new file mode 100644 index 00000000000..e7faf3e690d --- /dev/null +++ b/newsfragments/4454.fixed.md @@ -0,0 +1 @@ +Fix a soundness bug with `PyClassInitializer`: from now you cannot initialize a `PyClassInitializer` with `PyClassInitializer::from(Py).add_subclass(SubClass)`. \ No newline at end of file diff --git a/src/pyclass_init.rs b/src/pyclass_init.rs index 01983c79b13..0aae495d856 100644 --- a/src/pyclass_init.rs +++ b/src/pyclass_init.rs @@ -27,6 +27,10 @@ pub trait PyObjectInit: Sized { py: Python<'_>, subtype: *mut PyTypeObject, ) -> PyResult<*mut ffi::PyObject>; + + #[doc(hidden)] + fn can_be_subclassed(&self) -> bool; + private_decl! {} } @@ -81,6 +85,11 @@ impl PyObjectInit for PyNativeTypeInitializer { inner(py, type_object, subtype) } + #[inline] + fn can_be_subclassed(&self) -> bool { + true + } + private_impl! {} } @@ -147,7 +156,14 @@ impl PyClassInitializer { /// Constructs a new initializer from value `T` and base class' initializer. /// /// It is recommended to use `add_subclass` instead of this method for most usage. + #[track_caller] + #[inline] pub fn new(init: T, super_init: ::Initializer) -> Self { + // This is unsound; see https://github.com/PyO3/pyo3/issues/4452. + assert!( + super_init.can_be_subclassed(), + "you cannot add a subclass to an existing value", + ); Self(PyClassInitializerImpl::New { init, super_init }) } @@ -197,6 +213,8 @@ impl PyClassInitializer { /// }) /// } /// ``` + #[track_caller] + #[inline] pub fn add_subclass(self, subclass_value: S) -> PyClassInitializer where S: PyClass, @@ -268,6 +286,11 @@ impl PyObjectInit for PyClassInitializer { .map(Bound::into_ptr) } + #[inline] + fn can_be_subclassed(&self) -> bool { + !matches!(self.0, PyClassInitializerImpl::Existing(..)) + } + private_impl! {} } @@ -288,6 +311,8 @@ where B: PyClass, B::BaseType: PyClassBaseType>, { + #[track_caller] + #[inline] fn from(sub_and_base: (S, B)) -> PyClassInitializer { let (sub, base) = sub_and_base; PyClassInitializer::from(base).add_subclass(sub) @@ -320,3 +345,27 @@ where Ok(self.into()) } } + +#[cfg(test)] +mod tests { + //! See https://github.com/PyO3/pyo3/issues/4452. + + use crate::prelude::*; + + #[pyclass(crate = "crate", subclass)] + struct BaseClass {} + + #[pyclass(crate = "crate", extends=BaseClass)] + struct SubClass { + _data: i32, + } + + #[test] + #[should_panic] + fn add_subclass_to_py_is_unsound() { + Python::with_gil(|py| { + let base = Py::new(py, BaseClass {}).unwrap(); + let _subclass = PyClassInitializer::from(base).add_subclass(SubClass { _data: 42 }); + }); + } +}