php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #41641 Indirect Modification Notice due to modification using __set
Submitted: 2007-06-08 22:50 UTC Modified: 2008-11-10 01:00 UTC
Votes:44
Avg. Score:4.5 ± 0.9
Reproduced:38 of 39 (97.4%)
Same Version:11 (28.9%)
Same OS:8 (21.1%)
From: asnyder at mddev dot com Assigned: helly (profile)
Status: No Feedback Package: Scripting Engine problem
PHP Version: 5.2.3 OS: Linux Fedora Core 4
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2007-06-08 22:50 UTC] asnyder at mddev dot com
Description:
------------
Ok, the bug is as follows. If one gets an array, via the __get, and then continues to modify the subobject properties using a __set. It will call the __set function, but throw the 

Notice: Indirect modification of overloaded property A::$Test has no effect in /var/www/html/Tests/PHPBug.php on line 43

In PHP 5.1.2 this process worked fine, but in 5.2.3 it causes the notice.

Reproduce code:
---------------
class A
{
	private $Test = array("One" => null, 'Two' => null);
	
	function A()
	{
		$this->Test['One'] = new B();
	}
	function __get($val)
	{
		return $this->Test;
	}
}
class B
{
	private $SomeVariable;
	
	function GetSomething(){return $this->SomeVariable;}
	function SetSomething($bool)
	{
		$this->SomeVariable = $bool;
	}
	function __get($val)
	{
		$this->GetSomething();
	}
	function __set($part, $val)
	{
		$this->SetSomething($val);
	}
}
$tmpA = new A();


// Now in PHP 5.1.2 the following code works, in PHP 5.2.3 but throws the notice Indirect modification of overloaded property
$tmpA->Test['One']->Something = true;
//In PHP 5.2.3 one would have to do the following for the notice not to throw. Which is not the expected behavior
//$tmpA->Test['One']->SetSomething(true);

Expected result:
----------------
The SetSomething($val) function should be called, and it IS being called, but with throwing a notice. It should behave like in 5.1.2 and run the SetSomething($val) function without the notice.

One should NOT have to call $tmpA->Test['One']->SetSomething(true) to not raise the notice.

Actual result:
--------------
Calls function and throws notice.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2007-06-11 14:52 UTC] derick@php.net
Thank you for taking the time to write to us, but this is not
a bug. Please double-check the documentation available at
http://www.php.net/manual/ and the instructions on how to report
a bug at http://bugs.php.net/how-to-report.php

It did not properly work in PHP 5.1.x either, now we emit this warning so that you can deal with the broken code.
 [2007-06-11 14:57 UTC] asnyder at mddev dot com
How is this bogus? The code DOES indeed make a change. Why should one have to call SetSomething, instead of the automatic __set triggering and making the modification.

I don't understand your logic. I have many applications that use this logic, and the applicatios worked properly before, with the new notice they don't, unless I explicitly call the function.

What are your thoughts on this?
 [2007-06-25 19:11 UTC] tony2001@php.net
The __get() function returns by value, not by reference, hence the warning.
 [2007-06-25 19:39 UTC] asnyder at mddev dot com
But it DOES have an affect. It did change the SomeVariable value of the actual object regardless of the __get returning by value. In these situations it should NOT trigger the warning. Run the example, and store values in the SomeVariable, you'll see that the actual value DOES get changed. Lets not just assume things without atually reading and running the example. If you were in this situation you would most definately not want it to throw this warning. Realize that much of the Web 2.0 movement using php relise on the __get, and __set methods, and in throwing this warning higly discourages it's use, even when the application of it, such as my example is perfectly legitimate and valid.
 [2007-06-25 19:43 UTC] tony2001@php.net
>But it DOES have an affect.
But the code is STILL wrong, that's what the warning is trying to tell you.
 [2007-06-25 19:49 UTC] asnyder at mddev dot com
Ok, HOW is this code, or code like it "WRONG"? The code has the desired effect. The purpose of the code is to allow the user to call a function via the __set, __get without having to actually call the actual function. For instance, if one would want to have properties. Regardless of what you think the code should be, the code has the effect that the developer wanted, thus the code is not wrong, it's just confusing to you.

What EXACTLY is wrong with this code? What effect is it having that is not the desired effect?
 [2007-07-18 14:26 UTC] jani@php.net
Oops, wrong id, see bug #42030
 [2007-08-17 20:12 UTC] vrana@php.net
Please do not submit the same bug more than once. An existing
bug report already describes this very problem. Even if you feel
that your issue is somewhat different, the resolution is likely
to be the same. 

Thank you for your interest in PHP.

Same as bug #38102.
 [2007-08-22 05:53 UTC] asnyder at mddev dot com
First of all, I don't appreciate your snotty attitude.  Second, if you actuallyread the post, you would realize this is not the same issue. In that post they were actually modifying a value, in my post, I'm simply using the __get, __set as magic methods in order to call a different function. This is VERY different. It's ok for the call to Set and not raise the notice. If you look at the functions there's nothing in danger of being modifed incorrectly. Please read before you post an incorrect response. 

I know its hard from that pedestool your on, but sometimes it's best to come down, and discuss things with other people. I would like to think myself as educated in the ways of PHP, and might consider fixing this myself, and submitting it to the repository. I was hoping to save myself the trouble, and let someone more experienced with the php internals work on it, but if you won't fix it, I will.
 [2007-08-22 14:49 UTC] jani@php.net
Marcus, can you give an educated answer to this..? 
 [2007-09-25 16:22 UTC] brjann at gmail dot com
When 5.2.0 was released, several complaints about this issue were submitted (see http://bugs.php.net/bug.php?id=39449), and it was resolved. But now we're there again. 

Fact is that this seriously breaks wrapping functions that my project relies on, and I still don't understand the logic of modification of overloaded array properties not working is expected behaviour, while modification of non-array properties works.

Will this be fixed? If not, why, and why was it then fixed after bug report 39449? Was it just so that me and others would keep on writing code that is now useless?
 [2007-09-27 20:16 UTC] brjann at gmail dot com
It seems that declaring the getter as "public function &__get(){...}" does the trick. However, it took some googling to find.
 [2007-11-25 20:46 UTC] g42 at gmx dot net
brjann:

well, you're right, it will work if you use "public function &__get(){...}". But that solution will not call __set at all (which is a problem in some cases). 

the following example won't throw a warning and will basically work:

<?php

error_reporting(E_ALL);

class newclass {
  private $properties;

  public function &__get($key) {
    echo "get $key<br>";
    return $this->properties[$key];
  }

  public function __set($key, $val) {
    echo "set $key to $val<br>";
    $this->properties[$key] = $val;
  }
}

$c = new newclass;
$c->array['test'] = 'test';
?>

expected output:
get array
set array to test

actual output:
get array

I understand that it's not possible to catch this modification because it's done via reference, so i won't write a bugreport... Although it's still a bit confusing...
 [2008-11-02 13:11 UTC] jani@php.net
Please try using this CVS snapshot:

  http://snaps.php.net/php5.2-latest.tar.gz
 
For Windows:

  http://windows.php.net/snapshots/


 [2008-11-10 01:00 UTC] php-bugs at lists dot php dot net
No feedback was provided for this bug for over a week, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".
 [2010-01-11 20:37 UTC] cscott at ggot dot org
First, a much simpler test case:

class foo {
	private $bar = array();

	public function __construct() {
		$this->bar[0] = new stdClass;
		$this->bar[0]->five = 5;
	}

	public function __get($n) {
		return $this->bar;
	}
}

$foo = new foo;
$foo->bar[0]->seventeen = 17;
var_dump($foo);
* * *
Of course, this produces:
object(foo)#1 (1) {
  ["bar":"foo":private]=>
  array(1) {
    [0]=>
    object(stdClass)#2 (2) {
      ["five"]=>
      int(5)
      ["seventeen"]=>
      int(17)
    }
  }
}
But also generates the warning.  The warning is certainly useful for 
informing users that they are not actually modifying the array index 
inside the class when using non-objects, but since all objects are 
considered to be passed by reference this warning is indeed quite 
bogus, and breaks formatted responses (e.g. from a web service).

Furthermore, using a by-reference declaration for __get is a horse of 
a different color, as I am passing entire properties by reference, and 
if I'm going to do that I might as well just make them public to begin 
with and circumvent the performance hit incurred with __get.  However, 
there is a reason these properties are private with __get pseudo-
access, so either of these solutions is less than ideal.

Simply put, this notice only occurs when an array index fetched via 
__get is used in conjunction with some type of assignment, so would it 
be so difficult to check, before generating this notice, whether the 
value at that array index is, in fact, a reference?

Fun side node, if you nest objects you can get multiple notices.  
Assuming the class declaration from above:

$foo = new foo;
$foo->bar[0]->baz = new foo; // generates one notice
$foo->bar[0]->baz->bar[0]->eleven = 11; // generates two notices

That code will generate 3 notices altogether, and yet the object 
structure will look *exactly* how one would expect it to look.  
Ironically, one could circumvent all of this by using an object 
instead of an array -- except that objects don't support numerical 
member names, or even access without some massaging, which can even 
end you up back in square one with nothing but wasted time to show for 
it.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 07:01:29 2024 UTC