Skip to content
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.

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

'Indirect modification of overloaded property' exception in session bag, probably other similar places too? #1895

Closed
iby opened this issue Jan 24, 2014 · 8 comments

Comments

@iby
Copy link
Contributor

iby commented Jan 24, 2014

$namespace = new \Phalcon\Session\Bag('test');
$namespace->{'prop1'}['prop2'] = 'my value';

I guess the solution would be to add "&" in front of your __get function to pass it as reference public function &__get ($propertyName).

@ghost
Copy link

ghost commented Jan 24, 2014

__get() is an alias for get() and get() does not return references because it accepts a 'default value' parameter.

I have committed a fix that makes __get() an independent method, however, for your code to work you need to

 $namespace = new \Phalcon\Session\Bag('test');
+$namespace->prop1 = array();
 $namespace->{'prop1'}['prop2'] = 'my value';

@iby
Copy link
Contributor Author

iby commented Jan 24, 2014

Thanks! Yes, the original code took that into account, that wasn't the issue. So, does this mean that now we can use it without setting variable first (like below) or still need to set it before doing the above?

class A 
{ 
    protected $variables = [];

    public function __set($name, $value) 
    { 
        $this->variables[$name] = $value; 
    } 

    public function &__get($name) 
    { 
        return $this->variables[$name]; 
    } 

    public function __isset($name) 
    { 
        return isset($this->variables[$name]); 
    } 
} 

$a = new A();
$a->{'b'}['c']['d'] = 'yo';

echo var_dump($a->{'b'});

@ghost
Copy link

ghost commented Jan 24, 2014

Hmm, I think this can work without prior initialization, just need to return null by reference. Need to test a few things…

@iby
Copy link
Contributor Author

iby commented Jan 24, 2014

You can't return null by reference, in php at least. Implementing __isset should do the trick, though? That's what did it in the example above.

@ghost
Copy link

ghost commented Jan 27, 2014

This has been implemented, could you please check?

@iby
Copy link
Contributor Author

iby commented Jan 27, 2014

@sjinks, added a new unit test and created a pull request. The issue remains:

PHPUnit_Framework_Error_Notice : Indirect modification of overloaded property Phalcon\Session\Bag::$a has no effect
#0 /Users/ianbytchek/Development/cphalcon/php-tests/tests/Phalcon/Session/BagTest.php(52): PHPUnit_Util_ErrorHandler::handleError(8, 'Indirect modifi...', '/Users/ianbytch...', 52, Array)
#1 [internal function]: BagTest->testGetSet()
#2 /usr/local/Cellar/php55/5.5.8/lib/php/PHPUnit/Framework/TestCase.php(983): ReflectionMethod->invokeArgs(Object(BagTest), Array)
#3 /usr/local/Cellar/php55/5.5.8/lib/php/PHPUnit/Framework/TestCase.php(838): PHPUnit_Framework_TestCase->runTest()
#4 /usr/local/Cellar/php55/5.5.8/lib/php/PHPUnit/Framework/TestResult.php(648): PHPUnit_Framework_TestCase->runBare()
#5 /usr/local/Cellar/php55/5.5.8/lib/php/PHPUnit/Framework/TestCase.php(783): PHPUnit_Framework_TestResult->run(Object(BagTest))
#6 /usr/local/Cellar/php55/5.5.8/lib/php/PHPUnit/Framework/TestSuite.php(775): PHPUnit_Framework_TestCase->run(Object(PHPUnit_Framework_TestResult))
#7 /usr/local/Cellar/php55/5.5.8/lib/php/PHPUnit/Framework/TestSuite.php(745): PHPUnit_Framework_TestSuite->runTest(Object(BagTest), Object(PHPUnit_Framework_TestResult))
#8 /usr/local/Cellar/php55/5.5.8/lib/php/PHPUnit/TextUI/TestRunner.php(349): PHPUnit_Framework_TestSuite->run(Object(PHPUnit_Framework_TestResult), false, Array, Array, false)
#9 /usr/local/Cellar/php55/5.5.8/lib/php/PHPUnit/TextUI/Command.php(176): PHPUnit_TextUI_TestRunner->doRun(Object(PHPUnit_Framework_TestSuite), Array)
#10 /private/var/folders/w3/zytpgjm11557jppm2zbg8xmh0000gq/T/ide-phpunit.php(268): PHPUnit_TextUI_Command->run(Array, true)
#11 /private/var/folders/w3/zytpgjm11557jppm2zbg8xmh0000gq/T/ide-phpunit.php(506): IDE_Base_PHPUnit_TextUI_Command::main()
#12 {main}

@ghost
Copy link

ghost commented Jan 27, 2014

Did you build Phalcon from build/ or from ext/? If from build/, please run

php build/gen-build.php

first and then rebuild.

@iby
Copy link
Contributor Author

iby commented Jan 27, 2014

That did the job, works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants