php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64497 ReflectionProperty::setValue triggers __set for unset property
Submitted: 2013-03-23 22:45 UTC Modified: 2018-09-29 17:04 UTC
From: me at fixxxer dot me Assigned:
Status: Wont fix Package: Reflection related
PHP Version: 5.4Git-2013-03-23 (snap) OS: any
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: me at fixxxer dot me
New email:
PHP Version: OS:

 

 [2013-03-23 22:45 UTC] me at fixxxer dot me
Description:
------------
I'm not sure it's a bug. And if it is, it's a low priority bug.

But current behavior is illogical: ReflectionProperty represents an existing 
property, and it is logical to assume it never triggers magic methods.

The case is a bit weird. :) We've had a discussion on phpclub.ru forums (in 
Russian, http://phpclub.ru/talk/threads/75432/page-2) about dealing with the 
case when a property has been unset() and there are magic methods present.

When you unset a property, it is visible as undefined ("Undefined property" 
notice), and if there is a __set method, it starts being triggered when setting 
te same property. But property_exists() returns true for this property, so it 
means the property is still there in internal object structures - it is not the 
same as it has never been defined. During the discussion on restoring the 
property without triggering __set, I came up with an idea to use 
ReflectionProperty. But it ended up with a strange result shown in the test 
script.

My point is that ReflectionProperty::setValue should never trigger __set.

Test script:
---------------
class C {

    protected $foo;

    public function test() {
        var_dump('Setting value with ReflectionProperty::setValue...');
        $foo = (new ReflectionClass($this))->getProperty('foo');
        $foo->setAccessible(true);
        $foo->setValue($this, 'foo'); // sets value, ok
        $foo->setAccessible(false);

        var_dump($this->foo);

        var_dump('Unsetting...');
        unset($this->foo);
        var_dump($this->foo);

        var_dump('Setting value with ReflectionProperty::setValue again...');
        $foo = (new ReflectionClass($this))->getProperty('foo');
        $foo->setAccessible(true);
        $foo->setValue($this, 'foo'); // triggers __set() !
        $foo->setAccessible(false);

        var_dump($this->foo);
    }

    public function __set($k, $v) {
        var_dump("__set triggered: [$k] = '$v'");
    }

}

(new C)->test();

Expected result:
----------------
string(50) "Setting value with ReflectionProperty::setValue..."
string(3) "foo"
string(12) "Unsetting..."

Notice: Undefined property: C::$foo in /home/build/tmp/1.php on line 20
NULL
string(56) "Setting value with ReflectionProperty::setValue again..."
string(3) "foo"


Actual result:
--------------
string(50) "Setting value with ReflectionProperty::setValue..."
string(3) "foo"
string(12) "Unsetting..."

Notice: Undefined property: C::$foo in /home/build/tmp/1.php on line 20
NULL
string(56) "Setting value with ReflectionProperty::setValue again..."
string(30) "__set triggered: [foo] = 'foo'"

Notice: Undefined property: C::$foo in /home/build/tmp/1.php on line 28
NULL

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-03-23 23:13 UTC] me at fixxxer dot me
Some more thoughts on this issue.

Existence of unset() property is weird by itself.

I do understand why unset properties aren't completely removed from an object: 
it's required to preserve the visibility scope. But it believe it should be 
completely hidden from "outer world", and should not be exposed to 
property_exists, Reflection or whatever.
 [2017-09-29 07:46 UTC] pierre dot rineau at makina-corpus dot com
> I do understand why unset properties aren't completely removed from an object: 
> it's required to preserve the visibility scope. But it believe it should be 
> completely hidden from "outer world", and should not be exposed to 
> property_exists, Reflection or whatever.

If property is defined as a class property, it should not be unset() but only initialized to null. Removing the property existence from the outside world makes the statically typed class change hence it becomes more similar to python and javascript monkey patching than it is from statically typed and defined language. 

I know that PHP is fundamentally dynamic, but when as a developper I write a statically defined class, it should be modifiable by runtime, else it will trigger undefined or side effect behaviours that cannot exist per language definition.
 [2017-09-29 07:48 UTC] pierre dot rineau at makina-corpus dot com
Sorry: "but when as a developper I write a statically defined class, it should NOT be modifiable by runtime"
 [2018-09-29 17:04 UTC] nikic@php.net
-Status: Open +Status: Wont fix
 [2018-09-29 17:04 UTC] nikic@php.net
The current consensus on the topic of unsetting declared properties is that, while somewhat weird, this is the behavior we want to have. The behavior is used to implement lazy initialization by unsetting a declared property and initializing it in __get.

Reflection getValue() and setValue() methods are (essentially) equivalent to just performing the respective property reads/writes, so they will behave the same way, including potential calls to magic methods and undefined property notices.

In PHP 7.4, as part of the typed properties proposal, there will be a new ReflectionProperty::isInitialized() method which allows to check whether a property is unset.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Dec 27 02:01:29 2024 UTC