fix(class): property visibility checks #375

allowed_bindings.rs

@@ -129,6 +129,7 @@ bind! {
     zend_resource,
     zend_string,
     zend_string_init_interned,
+    zend_throw_error,
     zend_throw_exception_ex,
     zend_throw_exception_object,
     zend_type,

crates/macros/src/function.rs

@@ -999,7 +999,8 @@ fn expr_to_php_stub(expr: &Expr) -> String {
     }
 }
 
-/// Returns true if the given type is nullable in PHP (i.e., it's an `Option<T>`).
+/// Returns true if the given type is nullable in PHP (i.e., it's an
+/// `Option<T>`).
 ///
 /// Note: Having a default value does NOT make a type nullable. A parameter with
 /// a default value is optional (can be omitted), but passing `null` explicitly

docsrs_bindings.rs

@@ -1208,6 +1208,13 @@ unsafe extern "C" {
     pub static mut zend_interrupt_function:
         ::std::option::Option<unsafe extern "C" fn(execute_data: *mut zend_execute_data)>;
 }
+unsafe extern "C" {
+    pub fn zend_throw_error(
+        exception_ce: *mut zend_class_entry,
+        format: *const ::std::os::raw::c_char,
+        ...
+    );
+}
 unsafe extern "C" {
     pub static mut zend_standard_class_def: *mut zend_class_entry;
 }

src/builders/ini.rs

@@ -218,7 +218,8 @@ impl AsRef<str> for IniBuilder {
     }
 }
 
-// Ensure the C buffer is properly deinitialized when the builder goes out of scope.
+// Ensure the C buffer is properly deinitialized when the builder goes out of
+// scope.
 impl Drop for IniBuilder {
     fn drop(&mut self) {
         if !self.value.is_null() {

src/types/array/mod.rs

@@ -447,17 +447,21 @@ impl ZendHashTable {
             }
             ArrayKey::String(key) => {
                 unsafe {
+                    // Use raw bytes directly since zend_hash_str_update takes a length.
+                    // This allows keys with embedded null bytes (e.g. PHP property mangling).
                     zend_hash_str_update(
                         self,
-                        CString::new(key.as_str())?.as_ptr(),
+                        key.as_str().as_ptr().cast(),
                         key.len(),
                         &raw mut val,
                     )
                 };
             }
             ArrayKey::Str(key) => {
                 unsafe {
-                    zend_hash_str_update(self, CString::new(key)?.as_ptr(), key.len(), &raw mut val)
+                    // Use raw bytes directly since zend_hash_str_update takes a length.
+                    // This allows keys with embedded null bytes (e.g. PHP property mangling).
+                    zend_hash_str_update(self, key.as_ptr().cast(), key.len(), &raw mut val)
                 };
             }
         }

src/zend/handlers.rs

@@ -1,14 +1,15 @@
-use std::{ffi::c_void, mem::MaybeUninit, os::raw::c_int, ptr};
+use std::{ffi::CString, ffi::c_void, mem::MaybeUninit, os::raw::c_int, ptr};
 
 use crate::{
     class::RegisteredClass,
     exception::PhpResult,
     ffi::{
-        std_object_handlers, zend_is_true, zend_object_handlers, zend_object_std_dtor,
+        ext_php_rs_executor_globals, instanceof_function_slow, std_object_handlers,
+        zend_class_entry, zend_is_true, zend_object_handlers, zend_object_std_dtor,
         zend_std_get_properties, zend_std_has_property, zend_std_read_property,
-        zend_std_write_property,
+        zend_std_write_property, zend_throw_error,
     },
-    flags::ZvalTypeFlags,
+    flags::{PropertyFlags, ZvalTypeFlags},
     types::{ZendClassObject, ZendHashTable, ZendObject, ZendStr, Zval},
 };
 
@@ -108,6 +109,19 @@ impl ZendObjectHandlers {
 
             Ok(match prop {
                 Some(prop_info) => {
+                    // Check visibility before allowing access
+                    let object_ce = unsafe { (*object).ce };
+                    if !unsafe { check_property_access(prop_info.flags, object_ce) } {
+                        let is_private = prop_info.flags.contains(PropertyFlags::Private);
+                        unsafe {
+                            throw_property_access_error(
+                                T::CLASS_NAME,
+                                prop_name.as_str()?,
+                                is_private,
+                            );
+                        }
+                        return Ok(rv);
+                    }
                     prop_info.prop.get(self_, rv_mut)?;
                     rv
                 }
@@ -158,6 +172,19 @@ impl ZendObjectHandlers {
 
             Ok(match prop {
                 Some(prop_info) => {
+                    // Check visibility before allowing access
+                    let object_ce = unsafe { (*object).ce };
+                    if !unsafe { check_property_access(prop_info.flags, object_ce) } {
+                        let is_private = prop_info.flags.contains(PropertyFlags::Private);
+                        unsafe {
+                            throw_property_access_error(
+                                T::CLASS_NAME,
+                                prop_name.as_str()?,
+                                is_private,
+                            );
+                        }
+                        return Ok(value);
+                    }
                     prop_info.prop.set(self_, value_mut)?;
                     value
                 }
@@ -198,7 +225,19 @@ impl ZendObjectHandlers {
                 if val.prop.get(self_, &mut zv).is_err() {
                     continue;
                 }
-                props.insert(name, zv).map_err(|e| {
+
+                // Mangle property name according to visibility for debug output
+                // PHP convention: private = "\0ClassName\0propName", protected =
+                // "\0*\0propName"
+                let mangled_name = if val.flags.contains(PropertyFlags::Private) {
+                    format!("\0{}\0{name}", T::CLASS_NAME)
+                } else if val.flags.contains(PropertyFlags::Protected) {
+                    format!("\0*\0{name}")
+                } else {
+                    name.to_string()
+                };
+
+                props.insert(mangled_name.as_str(), zv).map_err(|e| {
                     format!("Failed to insert value into properties hashtable: {e:?}")
                 })?;
             }
@@ -309,3 +348,94 @@ impl ZendObjectHandlers {
         }
     }
 }
+
+/// Gets the current calling scope from the executor globals.
+///
+/// # Safety
+///
+/// Must only be called during PHP execution when executor globals are valid.
+#[inline]
+unsafe fn get_calling_scope() -> *const zend_class_entry {
+    let eg = unsafe { ext_php_rs_executor_globals().as_ref() };
+    let Some(eg) = eg else {
+        return ptr::null();
+    };
+    let execute_data = eg.current_execute_data;
+
+    if execute_data.is_null() {
+        return ptr::null();
+    }
+
+    let func = unsafe { (*execute_data).func };
+    if func.is_null() {
+        return ptr::null();
+    }
+
+    // Access the common.scope field through the union
+    unsafe { (*func).common.scope }
+}
+
+/// Checks if the calling scope has access to a property with the given flags.
+///
+/// Returns `true` if access is allowed, `false` otherwise.
+///
+/// # Safety
+///
+/// Must only be called during PHP execution when executor globals are valid.
+/// The `object_ce` pointer must be valid.
+#[inline]
+unsafe fn check_property_access(flags: PropertyFlags, object_ce: *const zend_class_entry) -> bool {
+    // Public properties are always accessible
+    if !flags.contains(PropertyFlags::Private) && !flags.contains(PropertyFlags::Protected) {
+        return true;
+    }
+
+    let calling_scope = unsafe { get_calling_scope() };
+
+    if flags.contains(PropertyFlags::Private) {
+        // Private: must be called from the exact same class
+        return calling_scope == object_ce;
+    }
+
+    if flags.contains(PropertyFlags::Protected) {
+        // Protected: must be called from same class or a subclass
+        if calling_scope.is_null() {
+            return false;
+        }
+
+        // Same class check
+        if calling_scope == object_ce {
+            return true;
+        }
+
+        // Check if calling_scope is a subclass of object_ce
+        // or if object_ce is a subclass of calling_scope (for parent access)
+        unsafe {
+            instanceof_function_slow(calling_scope, object_ce)
+                || instanceof_function_slow(object_ce, calling_scope)
+        }
+    } else {
+        true
+    }
+}
+
+/// Throws an error for invalid property access.
+///
+/// # Safety
+///
+/// Must only be called during PHP execution.
+///
+/// # Panics
+///
+/// Panics if the error message cannot be converted to a `CString`.
+unsafe fn throw_property_access_error(class_name: &str, prop_name: &str, is_private: bool) {
+    let visibility = if is_private { "private" } else { "protected" };
+    let message = CString::new(format!(
+        "Cannot access {visibility} property {class_name}::${prop_name}"
+    ))
+    .expect("Failed to create error message");
+
+    unsafe {
+        zend_throw_error(ptr::null_mut(), message.as_ptr());
+    }
+}

tests/src/integration/array/mod.rs

@@ -47,7 +47,8 @@ pub fn test_optional_array_ref(arr: Option<&ZendHashTable>) -> i64 {
     arr.map_or(-1, |ht| i64::try_from(ht.len()).unwrap_or(i64::MAX))
 }
 
-/// Test that `Option<&mut ZendHashTable>` works correctly (anti-regression for issue #515)
+/// Test that `Option<&mut ZendHashTable>` works correctly (anti-regression for
+/// issue #515)
 #[php_function]
 pub fn test_optional_array_mut_ref(arr: Option<&mut ZendHashTable>) -> i64 {
     match arr {

tests/src/integration/class/class.php

@@ -122,3 +122,37 @@
 // Test returning &Self (immutable reference)
 $selfRef = $builder2->getSelf();
 assert($selfRef === $builder2, 'getSelf should return $this');
+
+// Test property visibility (Issue #375)
+$visibilityObj = new TestPropertyVisibility(42, 'private_data', 'protected_data');
+
+// Test public property - should work
+assert($visibilityObj->publicNum === 42, 'Public property read should work');
+$visibilityObj->publicNum = 100;
+assert($visibilityObj->publicNum === 100, 'Public property write should work');
+
+// Test accessing private property through class methods - should work
+assert($visibilityObj->getPrivate() === 'private_data', 'Private property access via method should work');
+$visibilityObj->setPrivate('new_private');
+assert($visibilityObj->getPrivate() === 'new_private', 'Private property set via method should work');
+
+// Test accessing protected property through class methods - should work
+assert($visibilityObj->getProtected() === 'protected_data', 'Protected property access via method should work');
+$visibilityObj->setProtected('new_protected');
+assert($visibilityObj->getProtected() === 'new_protected', 'Protected property set via method should work');
+
+// Test that direct access to private property throws an error
+assert_exception_thrown(fn() => $visibilityObj->privateStr, 'Reading private property should throw');
+assert_exception_thrown(fn() => $visibilityObj->privateStr = 'test', 'Writing private property should throw');
+
+// Test that direct access to protected property throws an error
+assert_exception_thrown(fn() => $visibilityObj->protectedStr, 'Reading protected property should throw');
+assert_exception_thrown(fn() => $visibilityObj->protectedStr = 'test', 'Writing protected property should throw');
+
+// Test var_dump shows mangled names for private/protected properties
+ob_start();
+var_dump($visibilityObj);
+$output = ob_get_clean();
+assert(strpos($output, 'publicNum') !== false, 'var_dump should show public property');
+// Private properties should show as ClassName::propertyName in var_dump
+// Protected properties should show with * prefix

tests/src/integration/class/mod.rs

@@ -277,6 +277,51 @@ impl FluentBuilder {
     }
 }
 
+/// Test class for property visibility (Issue #375)
+#[php_class]
+pub struct TestPropertyVisibility {
+    /// Public property - accessible from anywhere
+    #[php(prop)]
+    pub public_num: i32,
+    /// Private property - only accessible from within the class
+    #[php(prop, flags = ext_php_rs::flags::PropertyFlags::Private)]
+    pub private_str: String,
+    /// Protected property - accessible from class and subclasses
+    #[php(prop, flags = ext_php_rs::flags::PropertyFlags::Protected)]
+    pub protected_str: String,
+}
+
+#[php_impl]
+impl TestPropertyVisibility {
+    pub fn __construct(public_num: i32, private_str: String, protected_str: String) -> Self {
+        Self {
+            public_num,
+            private_str,
+            protected_str,
+        }
+    }
+
+    /// Method to access private property from within the class
+    pub fn get_private(&self) -> String {
+        self.private_str.clone()
+    }
+
+    /// Method to access protected property from within the class
+    pub fn get_protected(&self) -> String {
+        self.protected_str.clone()
+    }
+
+    /// Method to set private property from within the class
+    pub fn set_private(&mut self, value: String) {
+        self.private_str = value;
+    }
+
+    /// Method to set protected property from within the class
+    pub fn set_protected(&mut self, value: String) {
+        self.protected_str = value;
+    }
+}
+
 pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
     builder
         .class::<TestClass>()
@@ -287,6 +332,7 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
         .class::<TestClassProtectedConstruct>()
         .class::<TestStaticProps>()
         .class::<FluentBuilder>()
+        .class::<TestPropertyVisibility>()
         .function(wrap_function!(test_class))
         .function(wrap_function!(throw_exception))
 }

tests/src/integration/mod.rs

@@ -40,7 +40,8 @@ mod test {
             command.arg("--release");
 
             // Build features list dynamically based on compiled features
-            // Note: Using vec_init_then_push pattern here is intentional due to conditional compilation
+            // Note: Using vec_init_then_push pattern here is intentional due to conditional
+            // compilation
             #[allow(clippy::vec_init_then_push)]
             {
                 let mut features = vec![];