Skip to content

Commit 0641165

Browse files
nicolas-grekasnicolas-grekas
authored
Skip __get() when __isset() materialised the property (phpGH-22181)
After __isset() returns true on ?? or empty(), re-check the property table before calling __get(). When __isset() materialised the property (a pattern used by lazy proxies), its value is returned directly. isset() itself is unchanged. Fixes phpGH-12695
1 parent ab99a29 commit 0641165

6 files changed

Lines changed: 261 additions & 1 deletion

File tree

‎UPGRADING‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ PHP 8.6 UPGRADE NOTES
1919
1. Backward Incompatible Changes
2020
========================================
2121

22+
- Core:
23+
. ??/empty() on a magic property no longer call __get() when __isset()
24+
has materialised the property by writing into the property table.
25+
The freshly-written value is returned directly. isset() is unaffected.
26+
2227
- COM
2328
. It is no longer possible to clone variant objects, this is because
2429
the cloning behaviour was ill defined.

‎Zend/tests/magic_methods/gh12695.phpt‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
--TEST--
2+
GH-12695: ?? on unset property does not call __get() when __isset() materialized the property
3+
--FILE--
4+
<?php
5+
6+
#[AllowDynamicProperties]
7+
class A {
8+
public function __get($n) {
9+
throw new Exception("__get must not be called when __isset materialised the property");
10+
}
11+
public function __isset($n) {
12+
echo " __isset($n)\n";
13+
$this->$n = 123;
14+
return true;
15+
}
16+
}
17+
18+
echo "Dynamic property materialised in __isset, then `??`:\n";
19+
$a = new A;
20+
var_dump($a->foo ?? 'fallback');
21+
22+
echo "\nSame on a declared (unset) property:\n";
23+
class B {
24+
public int $x = 99;
25+
public function __get($n) {
26+
throw new Exception("__get must not be called when __isset materialised the property");
27+
}
28+
public function __isset($n) {
29+
echo " __isset($n)\n";
30+
$this->$n = 7;
31+
return true;
32+
}
33+
}
34+
$b = new B;
35+
unset($b->x);
36+
var_dump($b->x ?? 'fallback');
37+
38+
echo "\nWhen __isset() materialises the property to null, `??` falls back:\n";
39+
#[AllowDynamicProperties]
40+
class D {
41+
public function __get($n) {
42+
throw new Exception("__get must not be called when __isset materialised the property");
43+
}
44+
public function __isset($n) {
45+
echo " __isset($n)\n";
46+
$this->$n = null;
47+
return true;
48+
}
49+
}
50+
$d = new D;
51+
var_dump($d->foo ?? 'fallback');
52+
53+
?>
54+
--EXPECT--
55+
Dynamic property materialised in __isset, then `??`:
56+
__isset(foo)
57+
int(123)
58+
59+
Same on a declared (unset) property:
60+
__isset(x)
61+
int(7)
62+
63+
When __isset() materialises the property to null, `??` falls back:
64+
__isset(foo)
65+
string(8) "fallback"

‎Zend/tests/magic_methods/gh12695_empty.phpt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
--TEST--
2+
GH-12695: empty() on unset property does not call __get() when __isset() materialized the property
3+
--FILE--
4+
<?php
5+
6+
#[AllowDynamicProperties]
7+
class A {
8+
public function __get($n) {
9+
throw new Exception("__get must not be called when __isset materialised the property");
10+
}
11+
public function __isset($n) {
12+
echo " __isset($n)\n";
13+
$this->$n = $GLOBALS['next_value'];
14+
return true;
15+
}
16+
}
17+
18+
echo "empty() when __isset materialised a truthy value: __get is not called, empty=false:\n";
19+
$GLOBALS['next_value'] = 7;
20+
$a = new A;
21+
var_dump(empty($a->foo));
22+
23+
echo "\nempty() when __isset materialised a falsy value: __get is not called, empty=true:\n";
24+
$GLOBALS['next_value'] = 0;
25+
$a = new A;
26+
var_dump(empty($a->bar));
27+
28+
echo "\nempty() with no materialization: __get is still called (legacy path preserved):\n";
29+
class B {
30+
public function __get($n) {
31+
echo " __get($n)\n";
32+
return 'value';
33+
}
34+
public function __isset($n) {
35+
echo " __isset($n)\n";
36+
return true;
37+
}
38+
}
39+
$b = new B;
40+
var_dump(empty($b->any));
41+
42+
?>
43+
--EXPECT--
44+
empty() when __isset materialised a truthy value: __get is not called, empty=false:
45+
__isset(foo)
46+
bool(false)
47+
48+
empty() when __isset materialised a falsy value: __get is not called, empty=true:
49+
__isset(bar)
50+
bool(true)
51+
52+
empty() with no materialization: __get is still called (legacy path preserved):
53+
__isset(any)
54+
__get(any)
55+
bool(false)

‎Zend/tests/magic_methods/gh12695_no_materialization.phpt‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
--TEST--
2+
GH-12695: __get() invocation is based on __isset()'s return value
3+
--FILE--
4+
<?php
5+
6+
/* The re-check after __isset() must not affect the legacy path when
7+
* the property is genuinely magic-only: __get() is still called and
8+
* its return value drives `??`'s null check. */
9+
10+
echo "`??` when __isset=true and __get returns a value: __get is called:\n";
11+
class C {
12+
public function __get($n) {
13+
echo " __get($n)\n";
14+
return 'from-get';
15+
}
16+
public function __isset($n) {
17+
echo " __isset($n)\n";
18+
return true;
19+
}
20+
}
21+
$c = new C;
22+
var_dump($c->any ?? 'fallback');
23+
24+
echo "\n`??` when __isset=true and __get returns null: __get is called and fallback is used:\n";
25+
class D {
26+
public function __get($n) {
27+
echo " __get($n)\n";
28+
return null;
29+
}
30+
public function __isset($n) {
31+
echo " __isset($n)\n";
32+
return true;
33+
}
34+
}
35+
$d = new D;
36+
var_dump($d->any ?? 'fallback');
37+
38+
echo "\n`??` when __isset returns false: __get is not called:\n";
39+
class E {
40+
public function __get($n) {
41+
throw new Exception("__get must not be called when __isset returned false");
42+
}
43+
public function __isset($n) {
44+
echo " __isset($n)\n";
45+
return false;
46+
}
47+
}
48+
$e = new E;
49+
var_dump($e->any ?? 'fallback');
50+
51+
?>
52+
--EXPECT--
53+
`??` when __isset=true and __get returns a value: __get is called:
54+
__isset(any)
55+
__get(any)
56+
string(8) "from-get"
57+
58+
`??` when __isset=true and __get returns null: __get is called and fallback is used:
59+
__isset(any)
60+
__get(any)
61+
string(8) "fallback"
62+
63+
`??` when __isset returns false: __get is not called:
64+
__isset(any)
65+
string(8) "fallback"

‎Zend/tests/magic_methods/gh12695_object_released_in_isset.phpt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
GH-12695: Object freed by __isset() during materialization
3+
--FILE--
4+
<?php
5+
6+
/* The re-check after __isset() copies the materialised value into the
7+
* caller's return-value buffer before releasing the object. This is
8+
* required because __isset() may drop the last external reference to
9+
* the object (here via $obj = null), so the property table is freed
10+
* by OBJ_RELEASE() right after the re-check. */
11+
12+
class C {
13+
public $prop;
14+
public function __isset($name) {
15+
global $obj;
16+
$obj = null;
17+
$this->prop = 'materialised';
18+
return true;
19+
}
20+
}
21+
22+
$obj = new C();
23+
unset($obj->prop);
24+
var_dump($obj->prop ?? 'fb');
25+
26+
?>
27+
--EXPECT--
28+
string(12) "materialised"

‎Zend/zend_object_handlers.c‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,35 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
934934
}
935935

936936
zval_ptr_dtor(&tmp_result);
937+
938+
/* __isset() may have materialised the property by writing into
939+
* the property table. Re-check it before deferring to __get(),
940+
* so the freshly-written value is returned directly without a
941+
* redundant __get() call (GH-12695). The value is copied into
942+
* `rv` because the property table can be freed by the OBJ_RELEASE
943+
* below (e.g. when __isset() drops the last external reference
944+
* to the object). */
945+
if (IS_VALID_PROPERTY_OFFSET(property_offset)) {
946+
retval = OBJ_PROP(zobj, property_offset);
947+
if (Z_TYPE_P(retval) != IS_UNDEF) {
948+
ZVAL_COPY(rv, retval);
949+
retval = rv;
950+
OBJ_RELEASE(zobj);
951+
goto exit;
952+
}
953+
} else if (IS_DYNAMIC_PROPERTY_OFFSET(property_offset)) {
954+
if (zobj->properties != NULL) {
955+
retval = zend_hash_find(zobj->properties, name);
956+
if (retval) {
957+
ZVAL_COPY(rv, retval);
958+
retval = rv;
959+
OBJ_RELEASE(zobj);
960+
goto exit;
961+
}
962+
}
963+
}
964+
retval = &EG(uninitialized_zval);
965+
937966
if (zobj->ce->__get && !((*guard) & IN_GET)) {
938967
goto call_getter;
939968
}
@@ -2498,7 +2527,20 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has
24982527
result = zend_is_true(&rv);
24992528
zval_ptr_dtor(&rv);
25002529
if (has_set_exists == ZEND_PROPERTY_NOT_EMPTY && result) {
2501-
if (EXPECTED(!EG(exception)) && zobj->ce->__get && !((*guard) & IN_GET)) {
2530+
/* GH-12695, see above. */
2531+
zval *prop = NULL;
2532+
if (IS_VALID_PROPERTY_OFFSET(property_offset)) {
2533+
prop = OBJ_PROP(zobj, property_offset);
2534+
if (Z_TYPE_P(prop) == IS_UNDEF) {
2535+
prop = NULL;
2536+
}
2537+
} else if (IS_DYNAMIC_PROPERTY_OFFSET(property_offset)
2538+
&& zobj->properties != NULL) {
2539+
prop = zend_hash_find(zobj->properties, name);
2540+
}
2541+
if (prop) {
2542+
result = i_zend_is_true(prop);
2543+
} else if (EXPECTED(!EG(exception)) && zobj->ce->__get && !((*guard) & IN_GET)) {
25022544
(*guard) |= IN_GET;
25032545
zend_std_call_getter(zobj, name, &rv);
25042546
(*guard) &= ~IN_GET;

0 commit comments

Comments
 (0)