From 84354c62b37a56816a695b18ebd898f9703a9ad2 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 28 Oct 2019 13:34:33 +0100 Subject: [PATCH] Fixed bug #78904: Don't call any magic for uninitialized typed properties We already changed the behavior for __set() in f1848a4. However, it seems that this is also a problem for all the other property magic, see bug #78904. This commit makes the behavior of all the property magic consistent: Magic will not be triggered for uninitialized typed properties, only explicitly unset() ones. This brings behavior more in line how non-typed properties behave and avoids WTF. Closes GH-4974. --- NEWS | 1 + .../typed_properties_040.phpt | 5 +++-- .../typed_properties_072.phpt | 1 + .../typed_properties_073.phpt | 1 + .../typed_properties_074.phpt | 1 + .../typed_properties_magic_set.phpt | 19 +++++++++++++++++++ Zend/zend_object_handlers.c | 17 +++++++++++++++-- 7 files changed, 41 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index dbe15cf1f3..ce5acb513d 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,7 @@ PHP NEWS . Fixed bug #78883 (fgets(STDIN) fails on Windows). (cmb) . Fixed bug #78898 (call_user_func(['parent', ...]) fails while other succeed). (Nikita) + . Fixed bug #78904 (Uninitialized property triggers __get()). (Nikita) - GD: . Fixed bug #78849 (GD build broken with -D SIGNED_COMPARE_SLOW). (cmb) diff --git a/Zend/tests/type_declarations/typed_properties_040.phpt b/Zend/tests/type_declarations/typed_properties_040.phpt index db2b90903c..4da738cb41 100644 --- a/Zend/tests/type_declarations/typed_properties_040.phpt +++ b/Zend/tests/type_declarations/typed_properties_040.phpt @@ -14,12 +14,13 @@ class Foo { $foo = new Foo(); +unset($foo->bar); var_dump($foo->bar); ?> --EXPECTF-- string(3) "bar" -Fatal error: Uncaught TypeError: Typed property Foo::$bar must be int, null used in %s:14 +Fatal error: Uncaught TypeError: Typed property Foo::$bar must be int, null used in %s:%d Stack trace: #0 {main} - thrown in %s on line 14 + thrown in %s on line %d diff --git a/Zend/tests/type_declarations/typed_properties_072.phpt b/Zend/tests/type_declarations/typed_properties_072.phpt index b56c5d632e..13fab7177e 100644 --- a/Zend/tests/type_declarations/typed_properties_072.phpt +++ b/Zend/tests/type_declarations/typed_properties_072.phpt @@ -12,6 +12,7 @@ class Test { } $test = new Test; +unset($test->val); var_dump($test); var_dump($test->val); diff --git a/Zend/tests/type_declarations/typed_properties_073.phpt b/Zend/tests/type_declarations/typed_properties_073.phpt index 7f9b4ff2de..43cb0f507b 100644 --- a/Zend/tests/type_declarations/typed_properties_073.phpt +++ b/Zend/tests/type_declarations/typed_properties_073.phpt @@ -13,6 +13,7 @@ class Test { } $test = new Test; +unset($test->val); var_dump($test); var_dump($val = &$test->val); var_dump($test); diff --git a/Zend/tests/type_declarations/typed_properties_074.phpt b/Zend/tests/type_declarations/typed_properties_074.phpt index 8e6be312c6..dd2fb5f521 100644 --- a/Zend/tests/type_declarations/typed_properties_074.phpt +++ b/Zend/tests/type_declarations/typed_properties_074.phpt @@ -14,6 +14,7 @@ class Test { $test = new Test; $dummyRef = &$test->prop; +unset($test->val); var_dump($test); try { var_dump($test->val); diff --git a/Zend/tests/type_declarations/typed_properties_magic_set.phpt b/Zend/tests/type_declarations/typed_properties_magic_set.phpt index fd101cf24d..70a7d4fe6f 100644 --- a/Zend/tests/type_declarations/typed_properties_magic_set.phpt +++ b/Zend/tests/type_declarations/typed_properties_magic_set.phpt @@ -5,12 +5,29 @@ __set() should not be invoked when setting an uninitialized typed property class Test { public int $foo; + public function __get($name) { + echo "__get ", $name, "\n"; + return null; + } public function __set($name, $value) { echo "__set ", $name, " = ", $value, "\n"; } + public function __isset($name) { + echo "__isset ", $name, "\n"; + return true; + } + public function __unset($name) { + echo "__unset ", $name, "\n"; + } } $test = new Test; +try { + var_dump($test->foo); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +var_dump(isset($test->foo)); $test->foo = 42; var_dump($test->foo); @@ -44,6 +61,8 @@ $test->foo = 42; ?> --EXPECT-- +Typed property Test::$foo must not be accessed before initialization +bool(false) int(42) __set foo = 42 __set foo = 42 diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 3a7b83ed63..610b15c6aa 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -679,6 +679,10 @@ ZEND_API zval *zend_std_read_property(zval *object, zval *member, int type, void if (EXPECTED(Z_TYPE_P(retval) != IS_UNDEF)) { goto exit; } + if (UNEXPECTED(Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) { + /* Skip __get() for uninitialized typed properties */ + goto uninit_error; + } } else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset))) { if (EXPECTED(zobj->properties != NULL)) { if (!IS_UNKNOWN_DYNAMIC_PROPERTY_OFFSET(property_offset)) { @@ -782,6 +786,7 @@ call_getter: } } +uninit_error: if (type != BP_VAR_IS) { if (UNEXPECTED(prop_info)) { zend_throw_error(NULL, "Typed property %s::$%s must not be accessed before initialization", @@ -1126,8 +1131,11 @@ ZEND_API void zend_std_unset_property(zval *object, zval *member, void **cache_s } goto exit; } - /* Reset the IS_PROP_UNINIT flag, if it exists. */ - Z_PROP_FLAG_P(slot) = 0; + if (UNEXPECTED(Z_PROP_FLAG_P(slot) == IS_PROP_UNINIT)) { + /* Reset the IS_PROP_UNINIT flag, if it exists and bypass __unset(). */ + Z_PROP_FLAG_P(slot) = 0; + goto exit; + } } else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset)) && EXPECTED(zobj->properties != NULL)) { if (UNEXPECTED(GC_REFCOUNT(zobj->properties) > 1)) { @@ -1686,6 +1694,11 @@ ZEND_API int zend_std_has_property(zval *object, zval *member, int has_set_exist if (Z_TYPE_P(value) != IS_UNDEF) { goto found; } + if (UNEXPECTED(Z_PROP_FLAG_P(value) == IS_PROP_UNINIT)) { + /* Skip __isset() for uninitialized typed properties */ + result = 0; + goto exit; + } } else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset))) { if (EXPECTED(zobj->properties != NULL)) { if (!IS_UNKNOWN_DYNAMIC_PROPERTY_OFFSET(property_offset)) { -- 2.40.0