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.

Sign up for GitHub

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

Fix lazy proxy calling magic methods twice #18039

Draft
wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
Draft

Fix lazy proxy calling magic methods twice #18039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions Zend/tests/lazy_objects/gh18038-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
GH-18038 001: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class C {
public $_;
public function __set($name, $value) {
var_dump(__METHOD__);
$this->$name = $value * 2;
}
}

$rc = new ReflectionClass(C::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new C;
});

$obj->prop = 1;
var_dump($obj->prop);

?>
--EXPECT--
string(8) "C::__set"
init
int(2)
38 changes: 38 additions & 0 deletions Zend/tests/lazy_objects/gh18038-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
--TEST--
GH-18038 002: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class RealInstance {
public $_;
public function __set($name, $value) {
global $obj;
var_dump(get_class($this)."::".__FUNCTION__);
$obj->$name = $value * 2;
unset($this->$name);
$this->$name = $value * 2;
}
}

#[AllowDynamicProperties]
class Proxy extends RealInstance {
}

$rc = new ReflectionClass(Proxy::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new RealInstance;
});

$real = $rc->initializeLazyObject($obj);
$real->prop = 1;
var_dump($obj->prop);

?>
--EXPECT--
init
string(19) "RealInstance::__set"
string(12) "Proxy::__set"
int(2)
30 changes: 30 additions & 0 deletions Zend/tests/lazy_objects/gh18038-003.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
--TEST--
GH-18038 003: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class C {
public $_;
public function __get($name) {
var_dump(__METHOD__);
return $this->$name;
}
}

$rc = new ReflectionClass(C::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new C;
});

var_dump($obj->prop);

?>
--EXPECTF--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the __get function should get called before instantiating the object? I.e. why does "C::__get" appear before "init"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s how lazy objects are supposed to behave: we interact primarily with the proxy (we execute the proxy’s code), but any access to its state is forwarded to the real instance.

So here we execute the getter, which tries to access a property. This triggers initialization before the access is forwarded.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I see, not what I intuitively expected but it makes sense

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose there's also an implicit assumption here that the initialized properties does not contain dynamic properties. If it does, the initializer would need to be called before __get, to ensure that the property is actually still does not exist after the initializer has been called.

string(8) "C::__get"
init

Warning: Undefined property: C::$prop in %s on line %d
NULL
45 changes: 45 additions & 0 deletions Zend/tests/lazy_objects/gh18038-004.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
--TEST--
GH-18038 004: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class RealInstance {
public $_;
public function __get($name) {
global $obj;
var_dump(get_class($this)."::".__FUNCTION__);
var_dump($obj->$name);
return $this->$name;
}
}

#[AllowDynamicProperties]
class Proxy extends RealInstance {
public function __get($name) {
var_dump(get_class($this)."::".__FUNCTION__);
return $this->$name;
}
}

$rc = new ReflectionClass(Proxy::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new RealInstance;
});

$real = $rc->initializeLazyObject($obj);
var_dump($real->prop);

?>
--EXPECTF--
init
string(19) "RealInstance::__get"
string(12) "Proxy::__get"

Warning: Undefined property: RealInstance::$prop in %s on line %d
NULL

Warning: Undefined property: RealInstance::$prop in %s on line %d
NULL
28 changes: 28 additions & 0 deletions Zend/tests/lazy_objects/gh18038-005.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
GH-18038 005: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class C {
public $_;
public function __isset($name) {
var_dump(__METHOD__);
return isset($this->$name['']);
}
}

$rc = new ReflectionClass(C::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new C;
});

var_dump(isset($obj->prop['']));

?>
--EXPECT--
string(10) "C::__isset"
init
bool(false)
37 changes: 37 additions & 0 deletions Zend/tests/lazy_objects/gh18038-006.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
--TEST--
GH-18038 006: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class C {
public $_;
public function __isset($name) {
var_dump(__METHOD__);
return isset($this->$name['']);
}
public function __get($name) {
var_dump(__METHOD__);
return $this->$name[''];
}
}

$rc = new ReflectionClass(C::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new C;
});

var_dump(isset($obj->prop['']));

?>
--EXPECTF--
string(10) "C::__isset"
string(8) "C::__get"
init

Warning: Undefined property: C::$prop in %s on line %d

Warning: Trying to access array offset on null in %s on line %d
bool(false)
41 changes: 41 additions & 0 deletions Zend/tests/lazy_objects/gh18038-007.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
GH-18038 007: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class RealInstance {
public $_;
public function __isset($name) {
global $obj;
var_dump(get_class($this)."::".__FUNCTION__);
var_dump(isset($obj->$name['']));
return isset($this->$name['']);
}
}

#[AllowDynamicProperties]
class Proxy extends RealInstance {
public function __isset($name) {
var_dump(get_class($this)."::".__FUNCTION__);
return isset($this->$name['']);
}
}

$rc = new ReflectionClass(Proxy::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new RealInstance;
});

$real = $rc->initializeLazyObject($obj);
var_dump(isset($real->prop['']));

?>
--EXPECT--
init
string(21) "RealInstance::__isset"
string(14) "Proxy::__isset"
bool(false)
bool(false)
28 changes: 28 additions & 0 deletions Zend/tests/lazy_objects/gh18038-008.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
GH-18038 008: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class C {
public $_;
public function __isset($name) {
var_dump(__METHOD__);
return isset($this->$name);
}
}

$rc = new ReflectionClass(C::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new C;
});

var_dump(isset($obj->prop));

?>
--EXPECT--
string(10) "C::__isset"
init
bool(false)
41 changes: 41 additions & 0 deletions Zend/tests/lazy_objects/gh18038-009.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
GH-18038 009: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class RealInstance {
public $_;
public function __isset($name) {
global $obj;
var_dump(get_class($this)."::".__FUNCTION__);
var_dump(isset($obj->$name));
return isset($this->$name);
}
}

#[AllowDynamicProperties]
class Proxy extends RealInstance {
public function __isset($name) {
var_dump(get_class($this)."::".__FUNCTION__);
return isset($this->$name);
}
}

$rc = new ReflectionClass(Proxy::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new RealInstance;
});

$real = $rc->initializeLazyObject($obj);
var_dump(isset($real->prop));

?>
--EXPECT--
init
string(21) "RealInstance::__isset"
string(14) "Proxy::__isset"
bool(false)
bool(false)
36 changes: 36 additions & 0 deletions Zend/tests/lazy_objects/gh18038-010.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
--TEST--
GH-18038 010: Lazy proxy calls magic methods twice
--FILE--
<?php

#[AllowDynamicProperties]
class C {
public $_;
public function __unset($name) {
var_dump(__METHOD__);
unset($this->$name);
}
}

$rc = new ReflectionClass(C::class);

$obj = $rc->newLazyProxy(function () {
echo "init\n";
return new C;
});

unset($obj->prop);
var_dump($obj);

?>
--EXPECTF--
string(10) "C::__unset"
init
string(10) "C::__unset"
lazy proxy object(C)#%d (1) {
["instance"]=>
object(C)#%d (1) {
["_"]=>
NULL
}
}
Loading
Loading