From 762ec32d3ae1f42515abcb481fd1b924d9a1b126 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 29 May 2026 15:08:20 +0200 Subject: [PATCH] Fix GH-12695: skip __get() when __isset() materialised the property After __isset() returns true under ?? 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. Closes GH-12695. --- UPGRADING | 5 ++ Zend/tests/magic_methods/gh12695.phpt | 65 +++++++++++++++++++ Zend/tests/magic_methods/gh12695_empty.phpt | 55 ++++++++++++++++ .../gh12695_no_materialization.phpt | 65 +++++++++++++++++++ .../gh12695_object_released_in_isset.phpt | 28 ++++++++ Zend/zend_object_handlers.c | 44 ++++++++++++- 6 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/magic_methods/gh12695.phpt create mode 100644 Zend/tests/magic_methods/gh12695_empty.phpt create mode 100644 Zend/tests/magic_methods/gh12695_no_materialization.phpt create mode 100644 Zend/tests/magic_methods/gh12695_object_released_in_isset.phpt diff --git a/UPGRADING b/UPGRADING index e1976941ddf1..077a7faf6562 100644 --- a/UPGRADING +++ b/UPGRADING @@ -19,6 +19,11 @@ PHP 8.6 UPGRADE NOTES 1. Backward Incompatible Changes ======================================== +- Core: + . ??/empty() on a magic property no longer call __get() when __isset() + has materialised the property by writing into the property table. + The freshly-written value is returned directly. isset() is unaffected. + - COM . It is no longer possible to clone variant objects, this is because the cloning behaviour was ill defined. diff --git a/Zend/tests/magic_methods/gh12695.phpt b/Zend/tests/magic_methods/gh12695.phpt new file mode 100644 index 000000000000..957f474fb6a1 --- /dev/null +++ b/Zend/tests/magic_methods/gh12695.phpt @@ -0,0 +1,65 @@ +--TEST-- +GH-12695: ?? on unset property does not call __get() when __isset() materialized the property +--FILE-- +$n = 123; + return true; + } +} + +echo "Dynamic property materialised in __isset, then `??`:\n"; +$a = new A; +var_dump($a->foo ?? 'fallback'); + +echo "\nSame on a declared (unset) property:\n"; +class B { + public int $x = 99; + public function __get($n) { + throw new Exception("__get must not be called when __isset materialised the property"); + } + public function __isset($n) { + echo " __isset($n)\n"; + $this->$n = 7; + return true; + } +} +$b = new B; +unset($b->x); +var_dump($b->x ?? 'fallback'); + +echo "\nWhen __isset() materialises the property to null, `??` falls back:\n"; +#[AllowDynamicProperties] +class D { + public function __get($n) { + throw new Exception("__get must not be called when __isset materialised the property"); + } + public function __isset($n) { + echo " __isset($n)\n"; + $this->$n = null; + return true; + } +} +$d = new D; +var_dump($d->foo ?? 'fallback'); + +?> +--EXPECT-- +Dynamic property materialised in __isset, then `??`: + __isset(foo) +int(123) + +Same on a declared (unset) property: + __isset(x) +int(7) + +When __isset() materialises the property to null, `??` falls back: + __isset(foo) +string(8) "fallback" diff --git a/Zend/tests/magic_methods/gh12695_empty.phpt b/Zend/tests/magic_methods/gh12695_empty.phpt new file mode 100644 index 000000000000..99806820cc6f --- /dev/null +++ b/Zend/tests/magic_methods/gh12695_empty.phpt @@ -0,0 +1,55 @@ +--TEST-- +GH-12695: empty() on unset property does not call __get() when __isset() materialized the property +--FILE-- +$n = $GLOBALS['next_value']; + return true; + } +} + +echo "empty() when __isset materialised a truthy value: __get is not called, empty=false:\n"; +$GLOBALS['next_value'] = 7; +$a = new A; +var_dump(empty($a->foo)); + +echo "\nempty() when __isset materialised a falsy value: __get is not called, empty=true:\n"; +$GLOBALS['next_value'] = 0; +$a = new A; +var_dump(empty($a->bar)); + +echo "\nempty() with no materialization: __get is still called (legacy path preserved):\n"; +class B { + public function __get($n) { + echo " __get($n)\n"; + return 'value'; + } + public function __isset($n) { + echo " __isset($n)\n"; + return true; + } +} +$b = new B; +var_dump(empty($b->any)); + +?> +--EXPECT-- +empty() when __isset materialised a truthy value: __get is not called, empty=false: + __isset(foo) +bool(false) + +empty() when __isset materialised a falsy value: __get is not called, empty=true: + __isset(bar) +bool(true) + +empty() with no materialization: __get is still called (legacy path preserved): + __isset(any) + __get(any) +bool(false) diff --git a/Zend/tests/magic_methods/gh12695_no_materialization.phpt b/Zend/tests/magic_methods/gh12695_no_materialization.phpt new file mode 100644 index 000000000000..700b605e2984 --- /dev/null +++ b/Zend/tests/magic_methods/gh12695_no_materialization.phpt @@ -0,0 +1,65 @@ +--TEST-- +GH-12695: __get() invocation is based on __isset()'s return value +--FILE-- +any ?? 'fallback'); + +echo "\n`??` when __isset=true and __get returns null: __get is called and fallback is used:\n"; +class D { + public function __get($n) { + echo " __get($n)\n"; + return null; + } + public function __isset($n) { + echo " __isset($n)\n"; + return true; + } +} +$d = new D; +var_dump($d->any ?? 'fallback'); + +echo "\n`??` when __isset returns false: __get is not called:\n"; +class E { + public function __get($n) { + throw new Exception("__get must not be called when __isset returned false"); + } + public function __isset($n) { + echo " __isset($n)\n"; + return false; + } +} +$e = new E; +var_dump($e->any ?? 'fallback'); + +?> +--EXPECT-- +`??` when __isset=true and __get returns a value: __get is called: + __isset(any) + __get(any) +string(8) "from-get" + +`??` when __isset=true and __get returns null: __get is called and fallback is used: + __isset(any) + __get(any) +string(8) "fallback" + +`??` when __isset returns false: __get is not called: + __isset(any) +string(8) "fallback" diff --git a/Zend/tests/magic_methods/gh12695_object_released_in_isset.phpt b/Zend/tests/magic_methods/gh12695_object_released_in_isset.phpt new file mode 100644 index 000000000000..911ffde637a9 --- /dev/null +++ b/Zend/tests/magic_methods/gh12695_object_released_in_isset.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-12695: Object freed by __isset() during materialization +--FILE-- +prop = 'materialised'; + return true; + } +} + +$obj = new C(); +unset($obj->prop); +var_dump($obj->prop ?? 'fb'); + +?> +--EXPECT-- +string(12) "materialised" diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 7ccd9f26190a..313113d7dc28 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -934,6 +934,35 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int } zval_ptr_dtor(&tmp_result); + + /* __isset() may have materialised the property by writing into + * the property table. Re-check it before deferring to __get(), + * so the freshly-written value is returned directly without a + * redundant __get() call (GH-12695). The value is copied into + * `rv` because the property table can be freed by the OBJ_RELEASE + * below (e.g. when __isset() drops the last external reference + * to the object). */ + if (IS_VALID_PROPERTY_OFFSET(property_offset)) { + retval = OBJ_PROP(zobj, property_offset); + if (Z_TYPE_P(retval) != IS_UNDEF) { + ZVAL_COPY(rv, retval); + retval = rv; + OBJ_RELEASE(zobj); + goto exit; + } + } else if (IS_DYNAMIC_PROPERTY_OFFSET(property_offset)) { + if (zobj->properties != NULL) { + retval = zend_hash_find(zobj->properties, name); + if (retval) { + ZVAL_COPY(rv, retval); + retval = rv; + OBJ_RELEASE(zobj); + goto exit; + } + } + } + retval = &EG(uninitialized_zval); + if (zobj->ce->__get && !((*guard) & IN_GET)) { goto call_getter; } @@ -2498,7 +2527,20 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has result = zend_is_true(&rv); zval_ptr_dtor(&rv); if (has_set_exists == ZEND_PROPERTY_NOT_EMPTY && result) { - if (EXPECTED(!EG(exception)) && zobj->ce->__get && !((*guard) & IN_GET)) { + /* GH-12695, see above. */ + zval *prop = NULL; + if (IS_VALID_PROPERTY_OFFSET(property_offset)) { + prop = OBJ_PROP(zobj, property_offset); + if (Z_TYPE_P(prop) == IS_UNDEF) { + prop = NULL; + } + } else if (IS_DYNAMIC_PROPERTY_OFFSET(property_offset) + && zobj->properties != NULL) { + prop = zend_hash_find(zobj->properties, name); + } + if (prop) { + result = i_zend_is_true(prop); + } else if (EXPECTED(!EG(exception)) && zobj->ce->__get && !((*guard) & IN_GET)) { (*guard) |= IN_GET; zend_std_call_getter(zobj, name, &rv); (*guard) &= ~IN_GET;