diff --git a/newsfragments/4528.added.md b/newsfragments/4528.added.md new file mode 100644 index 00000000000..0991f9a5261 --- /dev/null +++ b/newsfragments/4528.added.md @@ -0,0 +1,2 @@ +* Added bindings for `pyo3_ffi::PyWeakref_GetRef` on Python 3.13 and newer and + `py03_ffi::compat::PyWeakref_GetRef` for older Python versions. diff --git a/newsfragments/4590.changed.md b/newsfragments/4590.changed.md new file mode 100644 index 00000000000..3891da6fa9d --- /dev/null +++ b/newsfragments/4590.changed.md @@ -0,0 +1 @@ +* Deprecate `_borrowed` methods on `PyWeakRef` and `PyWeakrefProxy` (just use the owning forms). diff --git a/newsfragments/4590.fixed.md b/newsfragments/4590.fixed.md new file mode 100644 index 00000000000..afa16b6892b --- /dev/null +++ b/newsfragments/4590.fixed.md @@ -0,0 +1 @@ +* Workaround possible use-after-free in `_borrowed` methods on `PyWeakRef` and `PyWeakrefProxy` by leaking their contents. diff --git a/pyo3-ffi/src/compat/py_3_13.rs b/pyo3-ffi/src/compat/py_3_13.rs index 75c4cd101ae..23ab4918afe 100644 --- a/pyo3-ffi/src/compat/py_3_13.rs +++ b/pyo3-ffi/src/compat/py_3_13.rs @@ -37,3 +37,36 @@ compat_function!( item } ); + +compat_function!( + originally_defined_for(Py_3_13); + + #[inline] + pub unsafe fn PyWeakref_GetRef( + reference: *mut crate::PyObject, + pobj: *mut *mut crate::PyObject, + ) -> std::os::raw::c_int { + use crate::{ + compat::Py_NewRef, PyErr_SetString, PyExc_TypeError, PyWeakref_Check, + PyWeakref_GetObject, Py_None, + }; + + if !reference.is_null() && PyWeakref_Check(reference) == 0 { + *pobj = std::ptr::null_mut(); + PyErr_SetString(PyExc_TypeError, c_str!("expected a weakref").as_ptr()); + return -1; + } + let obj = PyWeakref_GetObject(reference); + if obj.is_null() { + // SystemError if reference is NULL + *pobj = std::ptr::null_mut(); + return -1; + } + if obj == Py_None() { + *pobj = std::ptr::null_mut(); + return 0; + } + *pobj = Py_NewRef(obj); + 1 + } +); diff --git a/pyo3-ffi/src/weakrefobject.rs b/pyo3-ffi/src/weakrefobject.rs index 7e11a9012e7..305dc290fa8 100644 --- a/pyo3-ffi/src/weakrefobject.rs +++ b/pyo3-ffi/src/weakrefobject.rs @@ -58,5 +58,12 @@ extern "C" { #[cfg_attr(PyPy, link_name = "PyPyWeakref_NewProxy")] pub fn PyWeakref_NewProxy(ob: *mut PyObject, callback: *mut PyObject) -> *mut PyObject; #[cfg_attr(PyPy, link_name = "PyPyWeakref_GetObject")] - pub fn PyWeakref_GetObject(_ref: *mut PyObject) -> *mut PyObject; + #[cfg_attr( + Py_3_13, + deprecated(note = "deprecated since Python 3.13. Use `PyWeakref_GetRef` instead.") + )] + pub fn PyWeakref_GetObject(reference: *mut PyObject) -> *mut PyObject; + #[cfg(Py_3_13)] + #[cfg_attr(PyPy, link_name = "PyPyWeakref_GetRef")] + pub fn PyWeakref_GetRef(reference: *mut PyObject, pobj: *mut *mut PyObject) -> c_int; } diff --git a/src/types/weakref/anyref.rs b/src/types/weakref/anyref.rs index 82e16293e62..84fdcf24dff 100644 --- a/src/types/weakref/anyref.rs +++ b/src/types/weakref/anyref.rs @@ -1,8 +1,11 @@ -use crate::err::{DowncastError, PyResult}; +use crate::err::PyResult; use crate::ffi_ptr_ext::FfiPtrExt; use crate::type_object::{PyTypeCheck, PyTypeInfo}; -use crate::types::any::{PyAny, PyAnyMethods}; -use crate::{ffi, Borrowed, Bound}; +use crate::types::{ + any::{PyAny, PyAnyMethods}, + PyNone, +}; +use crate::{ffi, Borrowed, Bound, DowncastError}; #[cfg(feature = "gil-refs")] use crate::PyNativeType; @@ -225,7 +228,7 @@ impl PyWeakref { /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| { /// let data = Bound::new(py, Foo{})?; - /// let proxy = PyWeakrefProxy::new_bound(&data)?; // Retrieve this as an PyMethods argument. + /// let proxy = PyWeakrefProxy::new_bound(&data)?; // Retrieve this as an PyMethods argument. /// let reference = proxy.downcast::()?; /// /// assert_eq!( @@ -393,7 +396,7 @@ pub trait PyWeakrefMethods<'py> { /// Upgrade the weakref to a direct Bound object reference. /// /// It is named `upgrade` to be inline with [rust's `Weak::upgrade`](std::rc::Weak::upgrade). - /// In Python it would be equivalent to [`PyWeakref_GetObject`]. + /// In Python it would be equivalent to [`PyWeakref_GetRef`]. /// /// # Example #[cfg_attr( @@ -453,7 +456,7 @@ pub trait PyWeakrefMethods<'py> { /// This function panics is the current object is invalid. /// If used propperly this is never the case. (NonNull and actually a weakref type) /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject + /// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref fn upgrade_as(&self) -> PyResult>> @@ -466,78 +469,17 @@ pub trait PyWeakrefMethods<'py> { .map_err(Into::into) } - /// Upgrade the weakref to a Borrowed object reference. - /// - /// It is named `upgrade_borrowed` to be inline with [rust's `Weak::upgrade`](std::rc::Weak::upgrade). - /// In Python it would be equivalent to [`PyWeakref_GetObject`]. - /// - /// # Example - #[cfg_attr( - not(all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9))))), - doc = "```rust,ignore" - )] - #[cfg_attr( - all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9)))), - doc = "```rust" - )] - /// use pyo3::prelude::*; - /// use pyo3::types::PyWeakrefReference; - /// - /// #[pyclass(weakref)] - /// struct Foo { /* fields omitted */ } - /// - /// #[pymethods] - /// impl Foo { - /// fn get_data(&self) -> (&str, u32) { - /// ("Dave", 10) - /// } - /// } - /// - /// fn parse_data(reference: Borrowed<'_, '_, PyWeakrefReference>) -> PyResult { - /// if let Some(data_src) = reference.upgrade_borrowed_as::()? { - /// let data = data_src.borrow(); - /// let (name, score) = data.get_data(); - /// Ok(format!("Processing '{}': score = {}", name, score)) - /// } else { - /// Ok("The supplied data reference is nolonger relavent.".to_owned()) - /// } - /// } - /// - /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| { - /// let data = Bound::new(py, Foo{})?; - /// let reference = PyWeakrefReference::new_bound(&data)?; - /// - /// assert_eq!( - /// parse_data(reference.as_borrowed())?, - /// "Processing 'Dave': score = 10" - /// ); - /// - /// drop(data); - /// - /// assert_eq!( - /// parse_data(reference.as_borrowed())?, - /// "The supplied data reference is nolonger relavent." - /// ); - /// - /// Ok(()) - /// }) - /// # } - /// ``` + /// Deprecated form of [`PyWeakrefMethods::upgrade_as`]. /// - /// # Panics - /// This function panics is the current object is invalid. - /// If used propperly this is never the case. (NonNull and actually a weakref type) - /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject - /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType - /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref? + /// Since PyO3 0.22.4 this leaks the borrowed reference to ensure the object is not + /// deallocated while in use. This is necessary to avoid possible use-after-free bugs. + #[deprecated(since = "0.22.4", note = "use `upgrade_as`")] fn upgrade_borrowed_as<'a, T>(&'a self) -> PyResult>> where T: PyTypeCheck, 'py: 'a, { - // TODO: Replace when Borrowed::downcast exists + #[allow(deprecated)] match self.upgrade_borrowed() { None => Ok(None), Some(object) if T::type_check(&object) => { @@ -550,7 +492,7 @@ pub trait PyWeakrefMethods<'py> { /// Upgrade the weakref to a direct Bound object reference unchecked. The type of the recovered object is not checked before downcasting, this could lead to unexpected behavior. Use only when absolutely certain the type can be guaranteed. The `weakref` may still return `None`. /// /// It is named `upgrade` to be inline with [rust's `Weak::upgrade`](std::rc::Weak::upgrade). - /// In Python it would be equivalent to [`PyWeakref_GetObject`]. + /// In Python it would be equivalent to [`PyWeakref_GetRef`]. /// /// # Safety /// Callers must ensure that the type is valid or risk type confusion. @@ -614,94 +556,34 @@ pub trait PyWeakrefMethods<'py> { /// This function panics is the current object is invalid. /// If used propperly this is never the case. (NonNull and actually a weakref type) /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject + /// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref unsafe fn upgrade_as_unchecked(&self) -> Option> { Some(self.upgrade()?.downcast_into_unchecked()) } - /// Upgrade the weakref to a Borrowed object reference unchecked. The type of the recovered object is not checked before downcasting, this could lead to unexpected behavior. Use only when absolutely certain the type can be guaranteed. The `weakref` may still return `None`. + /// Deprecated form of [`PyWeakrefMethods::upgrade_as_unchecked`]. /// - /// It is named `upgrade_borrowed` to be inline with [rust's `Weak::upgrade`](std::rc::Weak::upgrade). - /// In Python it would be equivalent to [`PyWeakref_GetObject`]. + /// Since PyO3 0.22.4 this leaks the borrowed reference to ensure the object is not + /// deallocated while in use. This is necessary to avoid possible use-after-free bugs. /// /// # Safety /// Callers must ensure that the type is valid or risk type confusion. /// The `weakref` is still allowed to be `None`, if the referenced object has been cleaned up. - /// - /// # Example - #[cfg_attr( - not(all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9))))), - doc = "```rust,ignore" - )] - #[cfg_attr( - all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9)))), - doc = "```rust" - )] - /// use pyo3::prelude::*; - /// use pyo3::types::PyWeakrefReference; - /// - /// #[pyclass(weakref)] - /// struct Foo { /* fields omitted */ } - /// - /// #[pymethods] - /// impl Foo { - /// fn get_data(&self) -> (&str, u32) { - /// ("Dave", 10) - /// } - /// } - /// - /// fn parse_data(reference: Borrowed<'_, '_, PyWeakrefReference>) -> String { - /// if let Some(data_src) = unsafe { reference.upgrade_borrowed_as_unchecked::() } { - /// let data = data_src.borrow(); - /// let (name, score) = data.get_data(); - /// format!("Processing '{}': score = {}", name, score) - /// } else { - /// "The supplied data reference is nolonger relavent.".to_owned() - /// } - /// } - /// - /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| { - /// let data = Bound::new(py, Foo{})?; - /// let reference = PyWeakrefReference::new_bound(&data)?; - /// - /// assert_eq!( - /// parse_data(reference.as_borrowed()), - /// "Processing 'Dave': score = 10" - /// ); - /// - /// drop(data); - /// - /// assert_eq!( - /// parse_data(reference.as_borrowed()), - /// "The supplied data reference is nolonger relavent." - /// ); - /// - /// Ok(()) - /// }) - /// # } - /// ``` - /// - /// # Panics - /// This function panics is the current object is invalid. - /// If used propperly this is never the case. (NonNull and actually a weakref type) - /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject - /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType - /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref? + #[deprecated(since = "0.22.4", note = "use `upgrade_as_uncheked`")] unsafe fn upgrade_borrowed_as_unchecked<'a, T>(&'a self) -> Option> where 'py: 'a, { + #[allow(deprecated)] Some(self.upgrade_borrowed()?.downcast_unchecked()) } /// Upgrade the weakref to a exact direct Bound object reference. /// /// It is named `upgrade` to be inline with [rust's `Weak::upgrade`](std::rc::Weak::upgrade). - /// In Python it would be equivalent to [`PyWeakref_GetObject`]. + /// In Python it would be equivalent to [`PyWeakref_GetRef`]. /// /// # Example #[cfg_attr( @@ -761,7 +643,7 @@ pub trait PyWeakrefMethods<'py> { /// This function panics is the current object is invalid. /// If used propperly this is never the case. (NonNull and actually a weakref type) /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject + /// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref fn upgrade_as_exact(&self) -> PyResult>> @@ -774,78 +656,17 @@ pub trait PyWeakrefMethods<'py> { .map_err(Into::into) } - /// Upgrade the weakref to a exact Borrowed object reference. - /// - /// It is named `upgrade_borrowed` to be inline with [rust's `Weak::upgrade`](std::rc::Weak::upgrade). - /// In Python it would be equivalent to [`PyWeakref_GetObject`]. - /// - /// # Example - #[cfg_attr( - not(all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9))))), - doc = "```rust,ignore" - )] - #[cfg_attr( - all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9)))), - doc = "```rust" - )] - /// use pyo3::prelude::*; - /// use pyo3::types::PyWeakrefReference; - /// - /// #[pyclass(weakref)] - /// struct Foo { /* fields omitted */ } - /// - /// #[pymethods] - /// impl Foo { - /// fn get_data(&self) -> (&str, u32) { - /// ("Dave", 10) - /// } - /// } - /// - /// fn parse_data(reference: Borrowed<'_, '_, PyWeakrefReference>) -> PyResult { - /// if let Some(data_src) = reference.upgrade_borrowed_as_exact::()? { - /// let data = data_src.borrow(); - /// let (name, score) = data.get_data(); - /// Ok(format!("Processing '{}': score = {}", name, score)) - /// } else { - /// Ok("The supplied data reference is nolonger relavent.".to_owned()) - /// } - /// } - /// - /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| { - /// let data = Bound::new(py, Foo{})?; - /// let reference = PyWeakrefReference::new_bound(&data)?; - /// - /// assert_eq!( - /// parse_data(reference.as_borrowed())?, - /// "Processing 'Dave': score = 10" - /// ); - /// - /// drop(data); - /// - /// assert_eq!( - /// parse_data(reference.as_borrowed())?, - /// "The supplied data reference is nolonger relavent." - /// ); - /// - /// Ok(()) - /// }) - /// # } - /// ``` - /// - /// # Panics - /// This function panics is the current object is invalid. - /// If used propperly this is never the case. (NonNull and actually a weakref type) + /// Deprecated form of [`PyWeakrefMethods::upgrade_as_exact`]. /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject - /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType - /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref? + /// Since PyO3 0.22.4 this leaks the borrowed reference to ensure the object is not + /// deallocated while in use. This is necessary to avoid possible use-after-free bugs. + #[deprecated(since = "0.22.4", note = "use `upgrade_as_exact`")] fn upgrade_borrowed_as_exact<'a, T>(&'a self) -> PyResult>> where T: PyTypeInfo, 'py: 'a, { - // TODO: Replace when Borrowed::downcast_exact exists + #[allow(deprecated)] match self.upgrade_borrowed() { None => Ok(None), Some(object) if object.is_exact_instance_of::() => { @@ -861,7 +682,7 @@ pub trait PyWeakrefMethods<'py> { /// This function returns `Some(Bound<'py, PyAny>)` if the reference still exists, otherwise `None` will be returned. /// /// This function gets the optional target of this [`weakref.ReferenceType`] (result of calling [`weakref.ref`]). - /// It produces similair results to using [`PyWeakref_GetObject`] in the C api. + /// It produces similar results to using [`PyWeakref_GetRef`] in the C api. /// /// # Example #[cfg_attr( @@ -912,7 +733,7 @@ pub trait PyWeakrefMethods<'py> { /// This function panics is the current object is invalid. /// If used propperly this is never the case. (NonNull and actually a weakref type) /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject + /// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref fn upgrade(&self) -> Option> { @@ -925,70 +746,16 @@ pub trait PyWeakrefMethods<'py> { } } - /// Upgrade the weakref to a Borrowed [`PyAny`] reference to the target object if possible. - /// - /// It is named `upgrade_borrowed` to be inline with [rust's `Weak::upgrade`](std::rc::Weak::upgrade). - /// This function returns `Some(Borrowed<'_, 'py, PyAny>)` if the reference still exists, otherwise `None` will be returned. - /// - /// This function gets the optional target of this [`weakref.ReferenceType`] (result of calling [`weakref.ref`]). - /// It produces similair results to using [`PyWeakref_GetObject`] in the C api. - /// - /// # Example - #[cfg_attr( - not(all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9))))), - doc = "```rust,ignore" - )] - #[cfg_attr( - all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9)))), - doc = "```rust" - )] - /// use pyo3::prelude::*; - /// use pyo3::types::PyWeakrefReference; - /// - /// #[pyclass(weakref)] - /// struct Foo { /* fields omitted */ } - /// - /// fn parse_data(reference: Borrowed<'_, '_, PyWeakrefReference>) -> PyResult { - /// if let Some(object) = reference.upgrade_borrowed() { - /// Ok(format!("The object '{}' refered by this reference still exists.", object.getattr("__class__")?.getattr("__qualname__")?)) - /// } else { - /// Ok("The object, which this reference refered to, no longer exists".to_owned()) - /// } - /// } - /// - /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| { - /// let data = Bound::new(py, Foo{})?; - /// let reference = PyWeakrefReference::new_bound(&data)?; - /// - /// assert_eq!( - /// parse_data(reference.as_borrowed())?, - /// "The object 'Foo' refered by this reference still exists." - /// ); - /// - /// drop(data); - /// - /// assert_eq!( - /// parse_data(reference.as_borrowed())?, - /// "The object, which this reference refered to, no longer exists" - /// ); - /// - /// Ok(()) - /// }) - /// # } - /// ``` + /// Deprecated form of [`PyWeakrefMethods::upgrade`]. /// - /// # Panics - /// This function panics is the current object is invalid. - /// If used propperly this is never the case. (NonNull and actually a weakref type) - /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject - /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType - /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref + /// Since PyO3 0.22.4 this leaks the borrowed reference to ensure the object is not + /// deallocated while in use. This is necessary to avoid possible use-after-free bugs. + #[deprecated(since = "0.22.4", note = "use `upgrade`")] fn upgrade_borrowed<'a>(&'a self) -> Option> where 'py: 'a, { + #[allow(deprecated)] let object = self.get_object_borrowed(); if object.is_none() { @@ -1003,7 +770,7 @@ pub trait PyWeakrefMethods<'py> { /// This function returns `Bound<'py, PyAny>`, which is either the object if it still exists, otherwise it will refer to [`PyNone`](crate::types::PyNone). /// /// This function gets the optional target of this [`weakref.ReferenceType`] (result of calling [`weakref.ref`]). - /// It produces similair results to using [`PyWeakref_GetObject`] in the C api. + /// It produces similar results to using [`PyWeakref_GetRef`] in the C api. /// /// # Example #[cfg_attr( @@ -1051,80 +818,40 @@ pub trait PyWeakrefMethods<'py> { /// This function panics is the current object is invalid. /// If used propperly this is never the case. (NonNull and actually a weakref type) /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject + /// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref fn get_object(&self) -> Bound<'py, PyAny> { - // PyWeakref_GetObject does some error checking, however we ensure the passed object is Non-Null and a Weakref type. + #[allow(deprecated)] self.get_object_borrowed().to_owned() } - /// Retrieve to a Borrowed object pointed to by the weakref. + /// Deprecated form of [`PyWeakrefMethods::get_object`]. /// - /// This function returns `Borrowed<'py, PyAny>`, which is either the object if it still exists, otherwise it will refer to [`PyNone`](crate::types::PyNone). - /// - /// This function gets the optional target of this [`weakref.ReferenceType`] (result of calling [`weakref.ref`]). - /// It produces similair results to using [`PyWeakref_GetObject`] in the C api. - /// - /// # Example - #[cfg_attr( - not(all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9))))), - doc = "```rust,ignore" - )] - #[cfg_attr( - all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9)))), - doc = "```rust" - )] - /// use pyo3::prelude::*; - /// use pyo3::types::PyWeakrefReference; - /// - /// #[pyclass(weakref)] - /// struct Foo { /* fields omitted */ } - /// - /// fn get_class(reference: Borrowed<'_, '_, PyWeakrefReference>) -> PyResult { - /// reference - /// .get_object_borrowed() - /// .getattr("__class__")? - /// .repr() - /// .map(|repr| repr.to_string()) - /// } - /// - /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| { - /// let object = Bound::new(py, Foo{})?; - /// let reference = PyWeakrefReference::new_bound(&object)?; - /// - /// assert_eq!( - /// get_class(reference.as_borrowed())?, - /// "" - /// ); - /// - /// drop(object); - /// - /// assert_eq!(get_class(reference.as_borrowed())?, ""); - /// - /// Ok(()) - /// }) - /// # } - /// ``` - /// - /// # Panics - /// This function panics is the current object is invalid. - /// If used propperly this is never the case. (NonNull and actually a weakref type) - /// - /// [`PyWeakref_GetObject`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetObject - /// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType - /// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref - #[track_caller] - // TODO: This function is the reason every function tracks caller, however it only panics when the weakref object is not actually a weakreference type. So is it this neccessary? + /// Since PyO3 0.22.4 this leaks the borrowed reference to ensure the object is not + /// deallocated while in use. This is necessary to avoid possible use-after-free bugs. + #[deprecated(since = "0.22.4", note = "use `get_object`")] fn get_object_borrowed(&self) -> Borrowed<'_, 'py, PyAny>; } impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakref> { + fn get_object(&self) -> Bound<'py, PyAny> { + let mut obj: *mut ffi::PyObject = std::ptr::null_mut(); + match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } { + std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref' weak reference instance should be valid (non-null and actually a weakref reference)"), + 0 => PyNone::get_bound(self.py()).to_owned().into_any(), + 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) }, + } + } + fn get_object_borrowed(&self) -> Borrowed<'_, 'py, PyAny> { - // PyWeakref_GetObject does some error checking, however we ensure the passed object is Non-Null and a Weakref type. - unsafe { ffi::PyWeakref_GetObject(self.as_ptr()).assume_borrowed_or_err(self.py()) } - .expect("The 'weakref' weak reference instance should be valid (non-null and actually a weakref reference)") + // XXX: this deliberately leaks a reference, but this is a necessary safety measure + // to ensure that the object is not deallocated while we are using it. + unsafe { + self.get_object() + .into_ptr() + .assume_borrowed_unchecked(self.py()) + } } } @@ -1198,51 +925,6 @@ mod tests { inner(new_proxy) } - #[test] - fn test_weakref_upgrade_borrowed_as() -> PyResult<()> { - fn inner( - create_reference: impl for<'py> FnOnce( - &Bound<'py, PyAny>, - ) - -> PyResult>, - ) -> PyResult<()> { - Python::with_gil(|py| { - let class = get_type(py)?; - let object = class.call0()?; - let reference = create_reference(&object)?; - - { - // This test is a bit weird but ok. - let obj = reference.upgrade_borrowed_as::(); - - assert!(obj.is_ok()); - let obj = obj.unwrap(); - - assert!(obj.is_some()); - assert!(obj.map_or(false, |obj| obj.as_ptr() == object.as_ptr() - && obj.is_exact_instance(&class))); - } - - drop(object); - - { - // This test is a bit weird but ok. - let obj = reference.upgrade_borrowed_as::(); - - assert!(obj.is_ok()); - let obj = obj.unwrap(); - - assert!(obj.is_none()); - } - - Ok(()) - }) - } - - inner(new_reference)?; - inner(new_proxy) - } - #[test] fn test_weakref_upgrade_as_unchecked() -> PyResult<()> { fn inner( @@ -1282,45 +964,6 @@ mod tests { inner(new_proxy) } - #[test] - fn test_weakref_upgrade_borrowed_as_unchecked() -> PyResult<()> { - fn inner( - create_reference: impl for<'py> FnOnce( - &Bound<'py, PyAny>, - ) - -> PyResult>, - ) -> PyResult<()> { - Python::with_gil(|py| { - let class = get_type(py)?; - let object = class.call0()?; - let reference = create_reference(&object)?; - - { - // This test is a bit weird but ok. - let obj = unsafe { reference.upgrade_borrowed_as_unchecked::() }; - - assert!(obj.is_some()); - assert!(obj.map_or(false, |obj| obj.as_ptr() == object.as_ptr() - && obj.is_exact_instance(&class))); - } - - drop(object); - - { - // This test is a bit weird but ok. - let obj = unsafe { reference.upgrade_borrowed_as_unchecked::() }; - - assert!(obj.is_none()); - } - - Ok(()) - }) - } - - inner(new_reference)?; - inner(new_proxy) - } - #[test] fn test_weakref_upgrade() -> PyResult<()> { fn inner( @@ -1354,41 +997,6 @@ mod tests { inner(new_proxy, false) } - #[test] - fn test_weakref_upgrade_borrowed() -> PyResult<()> { - fn inner( - create_reference: impl for<'py> FnOnce( - &Bound<'py, PyAny>, - ) - -> PyResult>, - call_retrievable: bool, - ) -> PyResult<()> { - let not_call_retrievable = !call_retrievable; - - Python::with_gil(|py| { - let class = get_type(py)?; - let object = class.call0()?; - let reference = create_reference(&object)?; - - assert!(not_call_retrievable || reference.call0()?.is(&object)); - assert!(reference.upgrade_borrowed().is_some()); - assert!(reference - .upgrade_borrowed() - .map_or(false, |obj| obj.is(&object))); - - drop(object); - - assert!(not_call_retrievable || reference.call0()?.is_none()); - assert!(reference.upgrade_borrowed().is_none()); - - Ok(()) - }) - } - - inner(new_reference, true)?; - inner(new_proxy, false) - } - #[test] fn test_weakref_get_object() -> PyResult<()> { fn inner( @@ -1421,38 +1029,6 @@ mod tests { inner(new_reference, true)?; inner(new_proxy, false) } - - #[test] - fn test_weakref_get_object_borrowed() -> PyResult<()> { - fn inner( - create_reference: impl for<'py> FnOnce( - &Bound<'py, PyAny>, - ) - -> PyResult>, - call_retrievable: bool, - ) -> PyResult<()> { - let not_call_retrievable = !call_retrievable; - - Python::with_gil(|py| { - let class = get_type(py)?; - let object = class.call0()?; - let reference = create_reference(&object)?; - - assert!(not_call_retrievable || reference.call0()?.is(&object)); - assert!(reference.get_object_borrowed().is(&object)); - - drop(object); - - assert!(not_call_retrievable || reference.call0()?.is_none()); - assert!(reference.get_object_borrowed().is_none()); - - Ok(()) - }) - } - - inner(new_reference, true)?; - inner(new_proxy, false) - } } // under 'abi3-py37' and 'abi3-py38' PyClass cannot be weakreferencable. @@ -1505,47 +1081,6 @@ mod tests { inner(new_proxy) } - #[test] - fn test_weakref_upgrade_borrowed_as() -> PyResult<()> { - fn inner( - create_reference: impl for<'py> FnOnce( - &Bound<'py, PyAny>, - ) - -> PyResult>, - ) -> PyResult<()> { - Python::with_gil(|py| { - let object = Py::new(py, WeakrefablePyClass {})?; - let reference = create_reference(object.bind(py))?; - - { - let obj = reference.upgrade_borrowed_as::(); - - assert!(obj.is_ok()); - let obj = obj.unwrap(); - - assert!(obj.is_some()); - assert!(obj.map_or(false, |obj| obj.as_ptr() == object.as_ptr())); - } - - drop(object); - - { - let obj = reference.upgrade_borrowed_as::(); - - assert!(obj.is_ok()); - let obj = obj.unwrap(); - - assert!(obj.is_none()); - } - - Ok(()) - }) - } - - inner(new_reference)?; - inner(new_proxy) - } - #[test] fn test_weakref_upgrade_as_unchecked() -> PyResult<()> { fn inner( @@ -1581,45 +1116,6 @@ mod tests { inner(new_proxy) } - #[test] - fn test_weakref_upgrade_borrowed_as_unchecked() -> PyResult<()> { - fn inner( - create_reference: impl for<'py> FnOnce( - &Bound<'py, PyAny>, - ) - -> PyResult>, - ) -> PyResult<()> { - Python::with_gil(|py| { - let object = Py::new(py, WeakrefablePyClass {})?; - let reference = create_reference(object.bind(py))?; - - { - let obj = unsafe { - reference.upgrade_borrowed_as_unchecked::() - }; - - assert!(obj.is_some()); - assert!(obj.map_or(false, |obj| obj.as_ptr() == object.as_ptr())); - } - - drop(object); - - { - let obj = unsafe { - reference.upgrade_borrowed_as_unchecked::() - }; - - assert!(obj.is_none()); - } - - Ok(()) - }) - } - - inner(new_reference)?; - inner(new_proxy) - } - #[test] fn test_weakref_upgrade() -> PyResult<()> { fn inner( @@ -1652,40 +1148,6 @@ mod tests { inner(new_proxy, false) } - #[test] - fn test_weakref_upgrade_borrowed() -> PyResult<()> { - fn inner( - create_reference: impl for<'py> FnOnce( - &Bound<'py, PyAny>, - ) - -> PyResult>, - call_retrievable: bool, - ) -> PyResult<()> { - let not_call_retrievable = !call_retrievable; - - Python::with_gil(|py| { - let object = Py::new(py, WeakrefablePyClass {})?; - let reference = create_reference(object.bind(py))?; - - assert!(not_call_retrievable || reference.call0()?.is(&object)); - assert!(reference.upgrade_borrowed().is_some()); - assert!(reference - .upgrade_borrowed() - .map_or(false, |obj| obj.is(&object))); - - drop(object); - - assert!(not_call_retrievable || reference.call0()?.is_none()); - assert!(reference.upgrade_borrowed().is_none()); - - Ok(()) - }) - } - - inner(new_reference, true)?; - inner(new_proxy, false) - } - #[test] fn test_weakref_get_object() -> PyResult<()> { fn inner( @@ -1717,36 +1179,5 @@ mod tests { inner(new_reference, true)?; inner(new_proxy, false) } - - #[test] - fn test_weakref_get_object_borrowed() -> PyResult<()> { - fn inner( - create_reference: impl for<'py> FnOnce( - &Bound<'py, PyAny>, - ) - -> PyResult>, - call_retrievable: bool, - ) -> PyResult<()> { - let not_call_retrievable = !call_retrievable; - - Python::with_gil(|py| { - let object = Py::new(py, WeakrefablePyClass {})?; - let reference = create_reference(object.bind(py))?; - - assert!(not_call_retrievable || reference.call0()?.is(&object)); - assert!(reference.get_object_borrowed().is(&object)); - - drop(object); - - assert!(not_call_retrievable || reference.call0()?.is_none()); - assert!(reference.get_object_borrowed().is_none()); - - Ok(()) - }) - } - - inner(new_reference, true)?; - inner(new_proxy, false) - } } } diff --git a/src/types/weakref/proxy.rs b/src/types/weakref/proxy.rs index 71334488b54..71e20d6a40a 100644 --- a/src/types/weakref/proxy.rs +++ b/src/types/weakref/proxy.rs @@ -2,7 +2,7 @@ use crate::err::PyResult; use crate::ffi_ptr_ext::FfiPtrExt; use crate::py_result_ext::PyResultExt; use crate::type_object::PyTypeCheck; -use crate::types::any::PyAny; +use crate::types::{any::PyAny, PyNone}; use crate::{ffi, Borrowed, Bound, ToPyObject}; #[cfg(feature = "gil-refs")] @@ -558,10 +558,23 @@ impl PyWeakrefProxy { } impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefProxy> { + fn get_object(&self) -> Bound<'py, PyAny> { + let mut obj: *mut ffi::PyObject = std::ptr::null_mut(); + match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } { + std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ProxyType' (or `weakref.CallableProxyType`) instance should be valid (non-null and actually a weakref reference)"), + 0 => PyNone::get_bound(self.py()).to_owned().into_any(), + 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) }, + } + } + fn get_object_borrowed(&self) -> Borrowed<'_, 'py, PyAny> { - // PyWeakref_GetObject does some error checking, however we ensure the passed object is Non-Null and a Weakref type. - unsafe { ffi::PyWeakref_GetObject(self.as_ptr()).assume_borrowed_or_err(self.py()) } - .expect("The 'weakref.ProxyType' (or `weakref.CallableProxyType`) instance should be valid (non-null and actually a weakref reference)") + // XXX: this deliberately leaks a reference, but this is a necessary safety measure + // to ensure that the object is not deallocated while we are using it. + unsafe { + self.get_object() + .into_ptr() + .assume_borrowed_unchecked(self.py()) + } } } @@ -744,6 +757,7 @@ mod tests { { // This test is a bit weird but ok. + #[allow(deprecated)] let obj = reference.upgrade_borrowed_as::(); assert!(obj.is_ok()); @@ -758,6 +772,7 @@ mod tests { { // This test is a bit weird but ok. + #[allow(deprecated)] let obj = reference.upgrade_borrowed_as::(); assert!(obj.is_ok()); @@ -808,6 +823,7 @@ mod tests { { // This test is a bit weird but ok. + #[allow(deprecated)] let obj = unsafe { reference.upgrade_borrowed_as_unchecked::() }; assert!(obj.is_some()); @@ -819,6 +835,7 @@ mod tests { { // This test is a bit weird but ok. + #[allow(deprecated)] let obj = unsafe { reference.upgrade_borrowed_as_unchecked::() }; assert!(obj.is_none()); @@ -847,6 +864,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -884,6 +902,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_get_object_borrowed() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -1006,6 +1025,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1062,6 +1082,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as_unchecked() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1108,6 +1129,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1143,6 +1165,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_get_object_borrowed() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1269,6 +1292,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -1333,6 +1357,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as_unchecked() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -1380,6 +1405,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -1417,6 +1443,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_get_object_borrowed() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -1533,6 +1560,7 @@ mod tests { }) } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1588,6 +1616,7 @@ mod tests { }) } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as_unchecked() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1634,6 +1663,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1669,6 +1699,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_get_object_borrowed() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; diff --git a/src/types/weakref/reference.rs b/src/types/weakref/reference.rs index 6cdcde3a7f7..78d6dc51709 100644 --- a/src/types/weakref/reference.rs +++ b/src/types/weakref/reference.rs @@ -1,7 +1,7 @@ use crate::err::PyResult; use crate::ffi_ptr_ext::FfiPtrExt; use crate::py_result_ext::PyResultExt; -use crate::types::any::PyAny; +use crate::types::{any::PyAny, PyNone}; use crate::{ffi, Borrowed, Bound, ToPyObject}; #[cfg(any(any(PyPy, GraalPy, Py_LIMITED_API), feature = "gil-refs"))] @@ -561,10 +561,23 @@ impl PyWeakrefReference { } impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefReference> { + fn get_object(&self) -> Bound<'py, PyAny> { + let mut obj: *mut ffi::PyObject = std::ptr::null_mut(); + match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } { + std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ReferenceType' instance should be valid (non-null and actually a weakref reference)"), + 0 => PyNone::get_bound(self.py()).to_owned().into_any(), + 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) }, + } + } + fn get_object_borrowed(&self) -> Borrowed<'_, 'py, PyAny> { - // PyWeakref_GetObject does some error checking, however we ensure the passed object is Non-Null and a Weakref type. - unsafe { ffi::PyWeakref_GetObject(self.as_ptr()).assume_borrowed_or_err(self.py()) } - .expect("The 'weakref.ReferenceType' instance should be valid (non-null and actually a weakref reference)") + // XXX: this deliberately leaks a reference, but this is a necessary safety measure + // to ensure that the object is not deallocated while we are using it. + unsafe { + self.get_object() + .into_ptr() + .assume_borrowed_unchecked(self.py()) + } } } @@ -703,6 +716,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -767,6 +781,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as_unchecked() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -816,6 +831,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -858,6 +874,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_get_object_borrowed() -> PyResult<()> { Python::with_gil(|py| { let class = get_type(py)?; @@ -957,6 +974,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1013,6 +1031,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed_as_unchecked() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1059,6 +1078,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_upgrade_borrowed() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?; @@ -1099,6 +1119,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_weakref_get_object_borrowed() -> PyResult<()> { Python::with_gil(|py| { let object = Py::new(py, WeakrefablePyClass {})?;