diff --git a/Cargo.toml b/Cargo.toml index 49cc3d9497..2cb733efb6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ categories = ["api-bindings"] [dependencies] bitflags = "1.2.1" +parking_lot = "0.11.2" ext-php-rs-derive = { version = "=0.4.0", path = "./ext-php-rs-derive" } [build-dependencies] diff --git a/build.rs b/build.rs index 7931874172..a83a0bf49b 100644 --- a/build.rs +++ b/build.rs @@ -85,6 +85,7 @@ fn main() { .parse_callbacks(Box::new(bindgen::CargoCallbacks)) .rustfmt_bindings(true) .no_copy("_zend_value") + .no_copy("_zend_string") .layout_tests(env::var("EXT_PHP_RS_TEST").is_ok()); for binding in ALLOWED_BINDINGS.iter() { diff --git a/example/skel/src/lib.rs b/example/skel/src/lib.rs index 6a2855f5bb..246881a5db 100644 --- a/example/skel/src/lib.rs +++ b/example/skel/src/lib.rs @@ -112,5 +112,10 @@ pub fn closure_count() -> Closure { #[php_module] pub fn module(module: ModuleBuilder) -> ModuleBuilder { - module + module.function( + FunctionBuilder::new("test_zval", test_zval) + .arg(Arg::new("test", DataType::Array)) + .build() + .unwrap(), + ) } diff --git a/example/skel/test.php b/example/skel/test.php index cf76234230..7e950f635c 100644 --- a/example/skel/test.php +++ b/example/skel/test.php @@ -1,6 +1,18 @@ 'world']); +======= +test_zval(); +>>>>>>> 712aded56 (Call zval destructor when changing zval type and dropping) +======= +test_zval(['hello' => 'world']); +>>>>>>> 3646da741 (Remove `ZendHashTable` wrapper) +======= +test_zval([]); +>>>>>>> 7ba9ee9bf (Refactor `ZendString` into a borrowed and owned variant) //include 'vendor/autoload.php'; //$ext = new ReflectionExtension('skel'); diff --git a/src/php/class.rs b/src/php/class.rs index 776d6a722a..5eccfd2c9b 100644 --- a/src/php/class.rs +++ b/src/php/class.rs @@ -38,10 +38,10 @@ impl ClassEntry { /// not be found or the class table has not been initialized. pub fn try_find(name: &str) -> Option<&'static Self> { ExecutorGlobals::get().class_table()?; - let name = ZendString::new(name, false).ok()?; + let mut name = ZendString::new(name, false).ok()?; unsafe { - crate::bindings::zend_lookup_class_ex(name.borrow_ptr(), std::ptr::null_mut(), 0) + crate::bindings::zend_lookup_class_ex(name.as_mut_zend_str(), std::ptr::null_mut(), 0) .as_ref() } } @@ -115,8 +115,7 @@ impl ClassEntry { if self.flags().contains(ClassFlags::ResolvedParent) { unsafe { self.__bindgen_anon_1.parent.as_ref() } } else { - let name = - unsafe { ZendString::from_ptr(self.__bindgen_anon_1.parent_name, false) }.ok()?; + let name = unsafe { self.__bindgen_anon_1.parent_name.as_ref()? }; Self::try_find(name.as_str()?) } } @@ -288,7 +287,7 @@ impl ClassBuilder { /// /// Returns an [`Error`] variant if the class could not be registered. pub fn build(mut self) -> Result<&'static mut ClassEntry> { - self.ptr.name = ZendString::new_interned(&self.name)?.release(); + self.ptr.name = ZendString::new_interned(&self.name, true)?.into_inner(); self.methods.push(FunctionEntry::end()); let func = Box::into_raw(self.methods.into_boxed_slice()) as *const FunctionEntry; @@ -349,9 +348,9 @@ impl ClassBuilder { impl Debug for ClassEntry { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let name: String = unsafe { ZendString::from_ptr(self.name, false) } - .and_then(|str| str.try_into()) - .map_err(|_| std::fmt::Error)?; + let name: String = unsafe { self.name.as_ref() } + .and_then(|s| s.try_into().ok()) + .ok_or(std::fmt::Error)?; f.debug_struct("ClassEntry") .field("name", &name) diff --git a/src/php/types/array.rs b/src/php/types/array.rs index baa034b8bb..1be16ee908 100644 --- a/src/php/types/array.rs +++ b/src/php/types/array.rs @@ -24,10 +24,7 @@ use crate::{ php::enums::DataType, }; -use super::{ - string::ZendString, - zval::{FromZval, IntoZval, Zval}, -}; +use super::zval::{FromZval, IntoZval, Zval}; /// PHP array, which is represented in memory as a hashtable. pub use crate::bindings::HashTable; @@ -239,9 +236,7 @@ impl<'a> Iterator for Iter<'a> { } let bucket = unsafe { pos.as_ref() }; - let key = unsafe { ZendString::from_ptr(bucket.key, false) } - .and_then(|s| s.try_into()) - .ok(); + let key = unsafe { bucket.key.as_ref() }.and_then(|s| s.try_into().ok()); self.pos = NonNull::new(unsafe { pos.as_ptr().offset(1) }); @@ -272,9 +267,7 @@ impl<'a> DoubleEndedIterator for Iter<'a> { let new_end = NonNull::new(unsafe { end.as_ptr().offset(-1) })?; let bucket = unsafe { new_end.as_ref() }; - let key = unsafe { ZendString::from_ptr(bucket.key, false) } - .and_then(|s| s.try_into()) - .ok(); + let key = unsafe { bucket.key.as_ref() }.and_then(|s| s.try_into().ok()); self.end = Some(new_end); Some((bucket.h, key, &bucket.val)) @@ -370,7 +363,11 @@ impl OwnedHashTable { } } - /// Returns the inner pointer to the hashtable, without destroying the + /// Converts the owned Zend hashtable into the internal pointer, bypassing the [`Drop`] + /// implementation. + /// + /// The caller is responsible for freeing the resulting pointer using the `zend_array_destroy` + /// function. pub fn into_inner(self) -> *mut HashTable { let this = ManuallyDrop::new(self); this.ptr.as_ptr() diff --git a/src/php/types/object.rs b/src/php/types/object.rs index dc709f3f47..e6c443d25b 100644 --- a/src/php/types/object.rs +++ b/src/php/types/object.rs @@ -45,14 +45,13 @@ pub enum PropertyQuery { impl ZendObject { /// Attempts to retrieve the class name of the object. pub fn get_class_name(&self) -> Result { - let name = unsafe { - ZendString::from_ptr( - self.handlers()?.get_class_name.ok_or(Error::InvalidScope)?(self), - false, - ) - }?; - - name.try_into() + unsafe { + self.handlers()? + .get_class_name + .and_then(|f| f(self).as_ref()) + .ok_or(Error::InvalidScope) + .and_then(|s| s.try_into()) + } } /// Checks if the given object is an instance of a registered class with Rust @@ -74,13 +73,13 @@ impl ZendObject { return Err(Error::InvalidProperty); } - let name = ZendString::new(name, false)?; + let mut name = ZendString::new(name, false)?; let mut rv = Zval::new(); unsafe { self.handlers()?.read_property.ok_or(Error::InvalidScope)?( self.mut_ptr(), - name.borrow_ptr(), + name.as_mut_zend_str(), 1, std::ptr::null_mut(), &mut rv, @@ -98,13 +97,13 @@ impl ZendObject { /// * `name` - The name of the property. /// * `value` - The value to set the property to. pub fn set_property(&mut self, name: &str, value: impl IntoZval) -> Result<&Zval> { - let name = ZendString::new(name, false)?; + let mut name = ZendString::new(name, false)?; let mut value = value.into_zval(false)?; unsafe { self.handlers()?.write_property.ok_or(Error::InvalidScope)?( self, - name.borrow_ptr(), + name.as_mut_zend_str(), &mut value, std::ptr::null_mut(), ) @@ -127,7 +126,7 @@ impl ZendObject { Ok(unsafe { self.handlers()?.has_property.ok_or(Error::InvalidScope)?( self.mut_ptr(), - name.borrow_ptr(), + name.deref() as *const _ as *mut _, query as _, std::ptr::null_mut(), ) diff --git a/src/php/types/string.rs b/src/php/types/string.rs index 95093d0fd5..aae92f4392 100644 --- a/src/php/types/string.rs +++ b/src/php/types/string.rs @@ -1,7 +1,21 @@ //! Represents a string in the PHP world. Similar to a C string, but is reference counted and //! contains the length of the string, meaning the string can contain the NUL character. -use std::{convert::TryFrom, ffi::CString, fmt::Debug}; +use std::{ + borrow::{Borrow, Cow}, + convert::TryFrom, + ffi::{CStr, CString}, + fmt::Debug, + mem::ManuallyDrop, + ops::Deref, + ptr::NonNull, + slice, +}; + +use parking_lot::{ + lock_api::{Mutex, RawMutex}, + RawMutex as RawMutexStruct, +}; use crate::{ bindings::{ @@ -11,135 +25,307 @@ use crate::{ errors::{Error, Result}, }; -/// A wrapper around the [`zend_string`] used within the Zend API. Essentially a C string, except -/// that the structure contains the length of the string as well as the string being refcounted. +/// A borrowed Zend-string. +/// +/// Although this object does implement [`Sized`], it is in fact not sized. As C cannot represent unsized +/// types, an array of size 1 is used at the end of the type to represent the contents of the string, therefore +/// this type is actually unsized and has no valid constructors. See the owned variant [`ZendString`] to +/// create an owned version of a [`ZendStr`]. +/// +/// Once the `ptr_metadata` feature lands in stable rust, this type can potentially be changed to a DST using +/// slices and metadata. See the tracking issue here: +pub type ZendStr = zend_string; + +// Clippy complains about there being no `is_empty` function when implementing on the alias `ZendStr` :( +// +#[allow(clippy::len_without_is_empty)] +impl ZendStr { + /// Returns the length of the string. + pub fn len(&self) -> usize { + self.len as usize + } + + /// Returns true if the string is empty, false otherwise. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Returns a reference to the underlying [`CStr`] inside the Zend string. + pub fn as_c_str(&self) -> &CStr { + // SAFETY: Zend strings store their readable length in a fat pointer. + unsafe { + let slice = slice::from_raw_parts(self.val.as_ptr() as *const u8, self.len()); + CStr::from_bytes_with_nul_unchecked(slice) + } + } + + /// Attempts to return a reference to the underlying [`str`] inside the Zend string. + /// + /// Returns the [`None`] variant if the [`CStr`] contains non-UTF-8 characters. + pub fn as_str(&self) -> Option<&str> { + self.as_c_str().to_str().ok() + } +} + +impl Debug for ZendStr { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.as_c_str().fmt(f) + } +} + +impl ToOwned for ZendStr { + type Owned = ZendString; + + fn to_owned(&self) -> Self::Owned { + Self::Owned::from_c_str(self.as_c_str(), false) + } +} + +impl PartialEq for ZendStr { + #[inline] + fn eq(&self, other: &Self) -> bool { + self.as_c_str().eq(other.as_c_str()) + } +} + +impl<'a> From<&'a ZendStr> for &'a CStr { + fn from(value: &'a ZendStr) -> Self { + value.as_c_str() + } +} + +impl<'a> TryFrom<&'a ZendStr> for &'a str { + type Error = Error; + + fn try_from(value: &'a ZendStr) -> Result { + value.as_str().ok_or(Error::InvalidCString) + } +} + +impl<'a> TryFrom<&ZendStr> for String { + type Error = Error; + + fn try_from(value: &ZendStr) -> Result { + value + .as_str() + .map(|s| s.to_string()) + .ok_or(Error::InvalidCString) + } +} + +impl<'a> From<&'a ZendStr> for Cow<'a, ZendStr> { + fn from(value: &'a ZendStr) -> Self { + Cow::Borrowed(value) + } +} + +/// A type representing an owned Zend string, commonly used throughout the PHP API. +/// +/// The type contains an inner pointer to a [`ZendStr`], which is the DST that contains the contents +/// of the string. This type simply provides the required functions to handle the creation and deletion +/// of the internal string. pub struct ZendString { - ptr: *mut zend_string, - free: bool, + inner: NonNull, } +// Adding to the Zend interned string hashtable is not atomic and can be contested when PHP is compiled with ZTS, +// so an empty mutex is used to ensure no collisions occur on the Rust side. Not much we can do about collisions +// on the PHP side. +static INTERNED_LOCK: Mutex = Mutex::const_new(RawMutex::INIT, ()); + impl ZendString { - /// Creates a new Zend string. Returns a result containin the string. + /// Creates a new Zend string from a [`str`]. /// /// # Parameters /// - /// * `str_` - The string to create a Zend string from. - /// * `persistent` - Whether the request should relive the request boundary. + /// * `str` - String content. + /// * `persistent` - Whether the string should persist through the request boundary. + /// + /// # Returns + /// + /// Returns a result containing the Zend string if successful. Returns an error if the given + /// string contains NUL bytes, which cannot be contained inside a C string. + /// + /// # Panics + /// + /// Panics if the function was unable to allocate memory for the Zend string. pub fn new(str: &str, persistent: bool) -> Result { - Ok(Self { - ptr: unsafe { - ext_php_rs_zend_string_init(CString::new(str)?.as_ptr(), str.len() as _, persistent) - }, - free: true, - }) + Ok(Self::from_c_str(&CString::new(str)?, persistent)) + } + + /// Creates a new Zend string from a [`CStr`]. + /// + /// # Parameters + /// + /// * `str` - String content. + /// * `persistent` - Whether the string should persist through the request boundary. + /// + /// # Panics + /// + /// Panics if the function was unable to allocate memory for the Zend string. + pub fn from_c_str(str: &CStr, persistent: bool) -> Self { + let ptr = unsafe { + ext_php_rs_zend_string_init(str.as_ptr(), str.to_bytes().len() as _, persistent) + }; + + Self { + inner: NonNull::new(ptr).expect("Failed to allocate for Zend string"), + } } - /// Creates a new interned Zend string. Returns a result containing the interned string. + /// Creates a new interned Zend string from a [`str`]. + /// + /// An interned string is only ever stored once and is immutable. PHP stores the string in an + /// internal hashtable which stores the interned strings. + /// + /// As Zend hashtables are not thread-safe, a mutex is used to prevent two interned strings from + /// being created at the same time. /// /// # Parameters /// - /// * `str_` - The string to create a Zend string from. - #[allow(clippy::unwrap_used)] - pub fn new_interned(str_: &str) -> Result { - // Unwrap is OK here - `zend_string_init_interned` will be a valid function ptr by the time - // our extension is loaded. - Ok(Self { - ptr: unsafe { - zend_string_init_interned.unwrap()( - CString::new(str_)?.as_ptr(), - str_.len() as _, - true, - ) - }, - free: true, - }) - } - - /// Creates a new [`ZendString`] wrapper from a raw pointer to a [`zend_string`]. + /// * `str` - String content. + /// * `persistent` - Whether the string should persist through the request boundary. + /// + /// # Returns + /// + /// Returns a result containing the Zend string if successful. Returns an error if the given + /// string contains NUL bytes, which cannot be contained inside a C string. + /// + /// # Panics + /// + /// Panics if the function was unable to allocate memory for the Zend string. + pub fn new_interned(str: &str, persistent: bool) -> Result { + Ok(Self::interned_from_c_str(&CString::new(str)?, persistent)) + } + + /// Creates a new interned Zend string from a [`CStr`]. + /// + /// An interned string is only ever stored once and is immutable. PHP stores the string in an + /// internal hashtable which stores the interned strings. + /// + /// As Zend hashtables are not thread-safe, a mutex is used to prevent two interned strings from + /// being created at the same time. /// /// # Parameters /// - /// * `ptr` - A raw pointer to a [`zend_string`]. - /// * `free` - Whether the pointer should be freed when the resulting [`ZendString`] goes - /// out of scope. + /// * `str` - String content. + /// * `persistent` - Whether the string should persist through the request boundary. /// - /// # Safety + /// # Panics /// - /// As a raw pointer is given this function is unsafe, you must ensure the pointer is valid when calling - /// the function. A simple null check is done but this is not sufficient in most places. - pub unsafe fn from_ptr(ptr: *mut zend_string, free: bool) -> Result { - if ptr.is_null() { - return Err(Error::InvalidPointer); - } + /// Panics under the following circumstances: + /// + /// * The function used to create interned strings has not been set. + /// * The function could not allocate enough memory for the Zend string. + pub fn interned_from_c_str(str: &CStr, persistent: bool) -> Self { + let _lock = INTERNED_LOCK.lock(); + let ptr = unsafe { + zend_string_init_interned.expect("`zend_string_init_interned` not ready")( + str.as_ptr(), + str.to_bytes().len() as _, + persistent, + ) + }; - Ok(Self { ptr, free }) + Self { + inner: NonNull::new(ptr).expect("Failed to allocate for Zend string"), + } } - /// Releases the Zend string, returning the raw pointer to the `zend_string` object - /// and consuming the internal Rust [`ZendString`] container. - pub fn release(mut self) -> *mut zend_string { - self.free = false; - self.ptr + /// Returns a reference to the internal [`ZendStr`]. + pub fn as_zend_str(&self) -> &ZendStr { + // SAFETY: All constructors ensure a valid internal pointer. + unsafe { self.inner.as_ref() } } - /// Extracts a string slice containing the contents of the [`ZendString`]. - pub fn as_str(&self) -> Option<&str> { - // SAFETY: Zend strings have a length that we know we can read. - // By reading this many bytes we should not run into any issues. - // The value of the string is represented in C as a `char` array of - // length 1, but the data can be read up to `ptr.len` bytes. - unsafe { - let ptr = self.ptr.as_ref()?; - let slice = std::slice::from_raw_parts(ptr.val.as_ptr() as *const u8, ptr.len as _); - std::str::from_utf8(slice).ok() - } + /// Returns a reference to the internal [`ZendStr`]. + pub(crate) fn as_mut_zend_str(&mut self) -> &mut ZendStr { + // SAFETY: All constructors ensure a valid internal pointer. + unsafe { self.inner.as_mut() } } - /// Borrows the underlying internal pointer of the Zend string. - pub(crate) fn borrow_ptr(&self) -> *mut zend_string { - self.ptr + /// Converts the owned Zend string into the internal pointer, bypassing the [`Drop`] + /// implementation. + /// + /// The caller is responsible for freeing the resulting pointer using the `zend_string_release` + /// function. + pub fn into_inner(self) -> *mut ZendStr { + let this = ManuallyDrop::new(self); + this.inner.as_ptr() } } impl Drop for ZendString { fn drop(&mut self) { - if self.free && !self.ptr.is_null() { - unsafe { ext_php_rs_zend_string_release(self.ptr) }; - } + // SAFETY: All constructors ensure a valid internal pointer. + unsafe { ext_php_rs_zend_string_release(self.inner.as_ptr()) }; } } -impl TryFrom for ZendString { - type Error = Error; +impl Deref for ZendString { + type Target = ZendStr; - fn try_from(value: String) -> Result { - ZendString::new(value.as_str(), false) + fn deref(&self) -> &Self::Target { + self.as_zend_str() } } -impl TryFrom for String { +impl Debug for ZendString { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.as_zend_str().fmt(f) + } +} + +impl Borrow for ZendString { + #[inline] + fn borrow(&self) -> &ZendStr { + self.deref() + } +} + +impl AsRef for ZendString { + #[inline] + fn as_ref(&self) -> &ZendStr { + self + } +} + +impl From<&CStr> for ZendString { + fn from(value: &CStr) -> Self { + Self::from_c_str(value, false) + } +} + +impl From for ZendString { + fn from(value: CString) -> Self { + Self::from_c_str(&value, false) + } +} + +impl TryFrom<&str> for ZendString { type Error = Error; - fn try_from(value: ZendString) -> Result { - >::try_from(&value) + fn try_from(value: &str) -> Result { + Self::new(value, false) } } -impl TryFrom<&ZendString> for String { +impl TryFrom for ZendString { type Error = Error; - fn try_from(s: &ZendString) -> Result { - s.as_str() - .map(|s| s.to_string()) - .ok_or(Error::InvalidPointer) + fn try_from(value: String) -> Result { + Self::new(value.as_str(), false) } } -impl Debug for ZendString { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self.as_str() { - Some(str) => str.fmt(f), - None => Option::<()>::None.fmt(f), - } +impl From for Cow<'_, ZendStr> { + fn from(value: ZendString) -> Self { + Cow::Owned(value) + } +} + +impl From> for ZendString { + fn from(value: Cow<'_, ZendStr>) -> Self { + value.into_owned() } } diff --git a/src/php/types/zval.rs b/src/php/types/zval.rs index 8ba09b37a6..d91acf64aa 100644 --- a/src/php/types/zval.rs +++ b/src/php/types/zval.rs @@ -285,12 +285,20 @@ impl Zval { /// * `val` - The value to set the zval as. /// * `persistent` - Whether the string should persist between requests. pub fn set_string(&mut self, val: &str, persistent: bool) -> Result<()> { - self.change_type(ZvalTypeFlags::StringEx); - let zend_str = ZendString::new(val, persistent)?; - self.value.str_ = zend_str.release(); + self.set_zend_string(ZendString::new(val, persistent)?); Ok(()) } + /// Sets the value of the zval as a Zend string. + /// + /// # Parameters + /// + /// * `val` - String content. + pub fn set_zend_string(&mut self, val: ZendString) { + self.change_type(ZvalTypeFlags::StringEx); + self.value.str_ = val.into_inner(); + } + /// Sets the value of the zval as a binary string, which is represented in Rust as a vector. /// /// # Parameters @@ -307,10 +315,9 @@ impl Zval { /// # Parameters /// /// * `val` - The value to set the zval as. - pub fn set_interned_string(&mut self, val: &str) -> Result<()> { - self.change_type(ZvalTypeFlags::InternedStringEx); - let zend_str = ZendString::new_interned(val)?; - self.value.str_ = zend_str.release(); + /// * `persistent` - Whether the string should persist between requests. + pub fn set_interned_string(&mut self, val: &str, persistent: bool) -> Result<()> { + self.set_zend_string(ZendString::new_interned(val, persistent)?); Ok(()) }