Fixed property visibility #2078
Conversation
We used the same function to change properties of an object:
1. let object->property = expression
2. class A { private object = []; }
And to be able to change object's property we had to use object's scope.
Thus, we have NEVER checked the scope in which we are actually located:
// Global Scope
class Bar {
// Bar's Scope
private prop;
}
class Foo {
// Foo's Scope
public function test (Bar bar) {
// Here we replaced the scope by Bar's one
let bar->prop = 42;
}
}
Fixes #2057
/cc @niden @Jurigag
Codecov Report
@@ Coverage Diff @@
## development #2078 +/- ##
==============================================
Coverage 35.97% 35.98%
- Complexity 8306 8307 +1
==============================================
Files 565 566 +1
Lines 46122 46219 +97
==============================================
+ Hits 16593 16632 +39
- Misses 29529 29587 +58 |
|
@dreamsxin There are 4 failed and 14 passed tests. It seems I need help here. Could you please take a look what wrong with |
|
@dreamsxin Do you have any idea? |
|
@sergeyklay Fatal error: Class 'TestScopePhp' not found |
|
@dreamsxin Try |
|
@sergeyklay /* Use caller's scope */
//zephir_set_scope(ce);
/* Restore original scope */
//zephir_set_scope(scope);Will throw You want to throw an exception with read |
|
Maybe can change Like: PHP_METHOD(Test_Oo_Scopes_AbstractClass, setProperty) {
zephir_method_globals *ZEPHIR_METHOD_GLOBALS_PTR = NULL;
zval *name_param = NULL, *value, value_sub;
zval name;
zval *this_ptr = getThis();
ZVAL_UNDEF(&name);
ZVAL_UNDEF(&value_sub);
ZEPHIR_MM_GROW();
zephir_fetch_params(1, 2, 0, &name_param, &value);
zephir_get_strval(&name, name_param);
zephir_update_property_zval_zval(this_ptr, test_oo_scopes_abstractclass_ce, &name, value);
RETURN_THIS();
} |
|
@dreamsxin Yes, commenting does the trick. But if you run test without scope-related code you will see some failed tests. Do you suggest to modify |
|
If you want the scope of the variable to work, have to modify it. Zephir's scope doesn't have to be exactly the same as PHP's |
|
@sergeyklay Test in 7.4 $tester = new Test\Oo\Scopes\PrivateScopeTester();
$tester->setPropertyObj(new TestScopePhp(), 'privateProperty', 'test');
|
|
@dreamsxin Take a look at 3bbdc7a...b534cf9 As you can see tests for PHP 7.4 failed. |
|
@dreamsxin Removing scope does not help :-/ Any idea? |
|
@sergeyklay I test in local <?php
class UserExample extends Test\Oo\PropertyAccess
{
}
$example = new UserExample();
$example->setPrivateVariable('test');
var_dump($example->getPrivateVariable());
class TestScopeExtending extends Test\Oo\Scopes\AbstractClass
{
private $privateProperty = 'private';
protected $protectedProperty = 'protected';
public $publicProperty = 'public';
/**
* @return string
*/
public function getProtectedProperty(): string
{
return $this->protectedProperty;
}
}
$obj = new TestScopeExtending();
$obj->setProperty('privateProperty2', 'test');
var_dump($obj->getPrivateProperty2());
class TestScopeExtendingMagic extends Test\Oo\Scopes\AbstractClassMagic
{
private $privateProperty2 = 'private';
public function getProtectedProperty(): string
{
return $this->protectedProperty;
}
}
$tester = new Test\Oo\Scopes\PrivateScopeTester();
$object = new TestScopeExtendingMagic();
$actual = $tester->setPropertyObj($object, 'privateProperty', 'test');
var_dump($actual);
var_dump($object->setCount);
$tester = new Test\Oo\Scopes\PrivateScopeTester();
$obj = $tester->setPropertyNew(TestScopeExtendingMagic::class, 'privateProperty', 'test');
var_dump($obj->privateProperty);
var_dump($obj->setCount);See #2079 |
|
@dreamsxin Aha! It seems we don't need to change scope for |
|
@dreamsxin Removing scope-related code from |
This related the way PHP < 7.4 handles object's properties when there is a magic __set method present. Actually we DO NOT change property here (fixed). Only PHP 7.4 throws a Fatal Error. All previous versions just out a Notice and continue execution. /cc @nide @Jurigag @dreamsxin
|
|
|
@sergeyklay PHP < 7.4 Can't be modified, but no exception is thrown class TestScopeExtendingMagic
{
private $privateProperty2 = 'private';
public function getProtectedProperty(): string
{
return $this->protectedProperty;
}
public function getPrivateProperty2()
{
return $this->privateProperty2;
}
}
class PrivateScopeTester
{
public function setPropertyObj($obj, $property, $value)
{
$obj->{$property} = $value;
return $obj->{$property};
}
}
$tester = new PrivateScopeTester();
$object = new TestScopeExtendingMagic();
$tester->setPropertyObj($object, 'privateProperty2', 'CHANGED');
var_dump($object->getPrivateProperty2()); |
|
@dreamsxin Yes, see my comments here f41d57f Question: Can we modify this behavior so that Zephir will throw E_NOTICE for PHP 7.0 - 7.3 ? |
|
Wow great job guys that you fixed this finally. It was really keeping me from updating to 4.0 and suggest it anywhere. |
class PrivateScopeTester
{
public function setPropertyObj($obj, string $property, $value)
{
$obj->{$property} = $value;
}
}
class TestScopePhpMagic
{
public function __set($name, $value)
{
$this->$name = $value;
}
}
class TestScopePhpMagicExtending extends TestScopePhpMagic
{
private $privateProperty2 = 'private2';
}
$obj = new TestScopePhpMagicExtending();
var_dump($obj);
$tester = new PrivateScopeTester();
$tester->setPropertyObj($obj, 'privateProperty2', 'CHANGED');
var_dump($obj);PHP 7.0.33 Property is not changed. But there is no notice/error. |
Hello!
In raising this pull request, I confirm the following:
Small description of change:
Zephir ignored property visibility and has not thrown error when setting private/protected properties in scope that shouldn't intended for it.
Thanks