Skip to content

fix(array): cast numeric string keys into zend_ulong and allow negative keys #456

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 1, 2025
Merged

fix(array): cast numeric string keys into zend_ulong and allow negative keys #456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6989a08
feat(array): allow negative indices
kakserpom Jul 1, 2025
2a3fd2f
chore: reorder use statements
Xenira Jul 1, 2025
dbadc00
test(array): add tests for array keys
Xenira Jul 1, 2025
28ff7a4
test(array): actually add tests
Xenira Jul 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
248 changes: 219 additions & 29 deletions src/types/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ use std::{
fmt::{Debug, Display},
iter::FromIterator,
ptr,
str::FromStr,
};

use crate::{
boxed::{ZBox, ZBoxable},
convert::{FromZval, IntoZval},
error::{Error, Result},
ffi::zend_ulong,
ffi::{
_zend_new_array, zend_array_count, zend_array_destroy, zend_array_dup, zend_hash_clean,
zend_hash_get_current_data_ex, zend_hash_get_current_key_type_ex,
Expand Down Expand Up @@ -188,9 +190,46 @@ impl ZendHashTable {
/// assert_eq!(ht.get("test").and_then(|zv| zv.str()), Some("hello world"));
/// ```
#[must_use]
pub fn get(&self, key: &'_ str) -> Option<&Zval> {
let str = CString::new(key).ok()?;
unsafe { zend_hash_str_find(self, str.as_ptr(), key.len() as _).as_ref() }
pub fn get<'a, K>(&self, key: K) -> Option<&Zval>
where
K: Into<ArrayKey<'a>>,
{
match key.into() {
ArrayKey::Long(index) => unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_find(self, index as zend_ulong).as_ref()
},
ArrayKey::String(key) => {
if let Ok(index) = i64::from_str(key.as_str()) {
#[allow(clippy::cast_sign_loss)]
unsafe {
zend_hash_index_find(self, index as zend_ulong).as_ref()
}
} else {
unsafe {
zend_hash_str_find(
self,
CString::new(key.as_str()).ok()?.as_ptr(),
key.len() as _,
)
.as_ref()
}
}
}
ArrayKey::Str(key) => {
if let Ok(index) = i64::from_str(key) {
#[allow(clippy::cast_sign_loss)]
unsafe {
zend_hash_index_find(self, index as zend_ulong).as_ref()
}
} else {
unsafe {
zend_hash_str_find(self, CString::new(key).ok()?.as_ptr(), key.len() as _)
.as_ref()
}
}
}
}
}

/// Attempts to retrieve a value from the hash table with a string key.
Expand Down Expand Up @@ -219,9 +258,46 @@ impl ZendHashTable {
// hashtable while only having a reference to it. #461
#[allow(clippy::mut_from_ref)]
#[must_use]
pub fn get_mut(&self, key: &'_ str) -> Option<&mut Zval> {
let str = CString::new(key).ok()?;
unsafe { zend_hash_str_find(self, str.as_ptr(), key.len() as _).as_mut() }
pub fn get_mut<'a, K>(&self, key: K) -> Option<&mut Zval>
where
K: Into<ArrayKey<'a>>,
{
match key.into() {
ArrayKey::Long(index) => unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_find(self, index as zend_ulong).as_mut()
},
ArrayKey::String(key) => {
if let Ok(index) = i64::from_str(key.as_str()) {
#[allow(clippy::cast_sign_loss)]
unsafe {
zend_hash_index_find(self, index as zend_ulong).as_mut()
}
} else {
unsafe {
zend_hash_str_find(
self,
CString::new(key.as_str()).ok()?.as_ptr(),
key.len() as _,
)
.as_mut()
}
}
}
ArrayKey::Str(key) => {
if let Ok(index) = i64::from_str(key) {
#[allow(clippy::cast_sign_loss)]
unsafe {
zend_hash_index_find(self, index as zend_ulong).as_mut()
}
} else {
unsafe {
zend_hash_str_find(self, CString::new(key).ok()?.as_ptr(), key.len() as _)
.as_mut()
}
}
}
}
}

/// Attempts to retrieve a value from the hash table with an index.
Expand All @@ -247,8 +323,11 @@ impl ZendHashTable {
/// assert_eq!(ht.get_index(0).and_then(|zv| zv.long()), Some(100));
/// ```
#[must_use]
pub fn get_index(&self, key: u64) -> Option<&Zval> {
unsafe { zend_hash_index_find(self, key).as_ref() }
pub fn get_index(&self, key: i64) -> Option<&Zval> {
#[allow(clippy::cast_sign_loss)]
unsafe {
zend_hash_index_find(self, key as zend_ulong).as_ref()
}
}

/// Attempts to retrieve a value from the hash table with an index.
Expand Down Expand Up @@ -277,8 +356,11 @@ impl ZendHashTable {
// hashtable while only having a reference to it. #461
#[allow(clippy::mut_from_ref)]
#[must_use]
pub fn get_index_mut(&self, key: u64) -> Option<&mut Zval> {
unsafe { zend_hash_index_find(self, key).as_mut() }
pub fn get_index_mut(&self, key: i64) -> Option<&mut Zval> {
unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_find(self, key as zend_ulong).as_mut()
}
}

/// Attempts to remove a value from the hash table with a string key.
Expand All @@ -305,9 +387,44 @@ impl ZendHashTable {
/// ht.remove("test");
/// assert_eq!(ht.len(), 0);
/// ```
pub fn remove(&mut self, key: &str) -> Option<()> {
let result =
unsafe { zend_hash_str_del(self, CString::new(key).ok()?.as_ptr(), key.len() as _) };
pub fn remove<'a, K>(&mut self, key: K) -> Option<()>
where
K: Into<ArrayKey<'a>>,
{
let result = match key.into() {
ArrayKey::Long(index) => unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_del(self, index as zend_ulong)
},
ArrayKey::String(key) => {
if let Ok(index) = i64::from_str(key.as_str()) {
#[allow(clippy::cast_sign_loss)]
unsafe {
zend_hash_index_del(self, index as zend_ulong)
}
} else {
unsafe {
zend_hash_str_del(
self,
CString::new(key.as_str()).ok()?.as_ptr(),
key.len() as _,
)
}
}
}
ArrayKey::Str(key) => {
if let Ok(index) = i64::from_str(key) {
#[allow(clippy::cast_sign_loss)]
unsafe {
zend_hash_index_del(self, index as zend_ulong)
}
} else {
unsafe {
zend_hash_str_del(self, CString::new(key).ok()?.as_ptr(), key.len() as _)
}
}
}
};

if result < 0 {
None
Expand Down Expand Up @@ -340,8 +457,11 @@ impl ZendHashTable {
/// ht.remove_index(0);
/// assert_eq!(ht.len(), 0);
/// ```
pub fn remove_index(&mut self, key: u64) -> Option<()> {
let result = unsafe { zend_hash_index_del(self, key) };
pub fn remove_index(&mut self, key: i64) -> Option<()> {
let result = unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_del(self, key as zend_ulong)
};

if result < 0 {
None
Expand Down Expand Up @@ -379,12 +499,54 @@ impl ZendHashTable {
/// ht.insert("c", "C");
/// assert_eq!(ht.len(), 3);
/// ```
pub fn insert<V>(&mut self, key: &str, val: V) -> Result<()>
pub fn insert<'a, K, V>(&mut self, key: K, val: V) -> Result<()>
where
K: Into<ArrayKey<'a>>,
V: IntoZval,
{
let mut val = val.into_zval(false)?;
unsafe { zend_hash_str_update(self, CString::new(key)?.as_ptr(), key.len(), &raw mut val) };
match key.into() {
ArrayKey::Long(index) => {
unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_update(self, index as zend_ulong, &raw mut val)
};
}
ArrayKey::String(key) => {
if let Ok(index) = i64::from_str(&key) {
unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_update(self, index as zend_ulong, &raw mut val)
};
} else {
unsafe {
zend_hash_str_update(
self,
CString::new(key.as_str())?.as_ptr(),
key.len(),
&raw mut val,
)
};
}
}
ArrayKey::Str(key) => {
if let Ok(index) = i64::from_str(key) {
unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_update(self, index as zend_ulong, &raw mut val)
};
} else {
unsafe {
zend_hash_str_update(
self,
CString::new(key)?.as_ptr(),
key.len(),
&raw mut val,
)
};
}
}
}
val.release();
Ok(())
}
Expand Down Expand Up @@ -417,12 +579,15 @@ impl ZendHashTable {
/// ht.insert_at_index(0, "C"); // notice overriding index 0
/// assert_eq!(ht.len(), 2);
/// ```
pub fn insert_at_index<V>(&mut self, key: u64, val: V) -> Result<()>
pub fn insert_at_index<V>(&mut self, key: i64, val: V) -> Result<()>
where
V: IntoZval,
{
let mut val = val.into_zval(false)?;
unsafe { zend_hash_index_update(self, key, &raw mut val) };
unsafe {
#[allow(clippy::cast_sign_loss)]
zend_hash_index_update(self, key as zend_ulong, &raw mut val)
};
val.release();
Ok(())
}
Expand Down Expand Up @@ -560,6 +725,8 @@ impl ZendHashTable {
/// }
/// ArrayKey::String(key) => {
/// }
/// ArrayKey::Str(key) => {
/// }
/// }
/// dbg!(key, val);
/// }
Expand Down Expand Up @@ -612,15 +779,25 @@ pub struct Iter<'a> {
}

/// Represents the key of a PHP array, which can be either a long or a string.
#[derive(Debug, PartialEq)]
pub enum ArrayKey {
#[derive(Debug, Clone, PartialEq)]
pub enum ArrayKey<'a> {
/// A numerical key.
/// In Zend API it's represented by `u64` (`zend_ulong`), so the value needs
/// to be cast to `zend_ulong` before passing into Zend functions.
Long(i64),
/// A string key.
String(String),
/// A string key by reference.
Str(&'a str),
}

impl ArrayKey {
impl From<String> for ArrayKey<'_> {
fn from(value: String) -> Self {
Self::String(value)
}
}

impl ArrayKey<'_> {
/// Check if the key is an integer.
///
/// # Returns
Expand All @@ -630,21 +807,34 @@ impl ArrayKey {
pub fn is_long(&self) -> bool {
match self {
ArrayKey::Long(_) => true,
ArrayKey::String(_) => false,
ArrayKey::String(_) | ArrayKey::Str(_) => false,
}
}
}

impl Display for ArrayKey {
impl Display for ArrayKey<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ArrayKey::Long(key) => write!(f, "{key}"),
ArrayKey::String(key) => write!(f, "{key}"),
ArrayKey::Str(key) => write!(f, "{key}"),
}
}
}

impl<'a> FromZval<'a> for ArrayKey {
impl<'a> From<&'a str> for ArrayKey<'a> {
fn from(key: &'a str) -> ArrayKey<'a> {
ArrayKey::Str(key)
}
}

impl<'a> From<i64> for ArrayKey<'a> {
fn from(index: i64) -> ArrayKey<'a> {
ArrayKey::Long(index)
}
}

impl<'a> FromZval<'a> for ArrayKey<'_> {
const TYPE: DataType = DataType::String;

fn from_zval(zval: &'a Zval) -> Option<Self> {
Expand Down Expand Up @@ -686,7 +876,7 @@ impl<'a> Iter<'a> {
}

impl<'a> IntoIterator for &'a ZendHashTable {
type Item = (ArrayKey, &'a Zval);
type Item = (ArrayKey<'a>, &'a Zval);
type IntoIter = Iter<'a>;

/// Returns an iterator over the key(s) and value contained inside the
Expand All @@ -713,7 +903,7 @@ impl<'a> IntoIterator for &'a ZendHashTable {
}

impl<'a> Iterator for Iter<'a> {
type Item = (ArrayKey, &'a Zval);
type Item = (ArrayKey<'a>, &'a Zval);

fn next(&mut self) -> Option<Self::Item> {
self.next_zval()
Expand Down Expand Up @@ -1052,8 +1242,8 @@ impl FromIterator<Zval> for ZBox<ZendHashTable> {
}
}

impl FromIterator<(u64, Zval)> for ZBox<ZendHashTable> {
fn from_iter<T: IntoIterator<Item = (u64, Zval)>>(iter: T) -> Self {
impl FromIterator<(i64, Zval)> for ZBox<ZendHashTable> {
fn from_iter<T: IntoIterator<Item = (i64, Zval)>>(iter: T) -> Self {
let mut ht = ZendHashTable::new();
for (key, val) in iter {
// Inserting a zval cannot fail, as `push` only returns `Err` if converting
Expand Down
2 changes: 1 addition & 1 deletion src/zend/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl ZendObjectHandlers {
let self_ = &mut *obj;
let struct_props = T::get_metadata().get_properties();

for (name, val) in struct_props {
for (&name, val) in struct_props {
let mut zv = Zval::new();
if val.prop.get(self_, &mut zv).is_err() {
continue;
Expand Down
Loading