From 5a4cb3edde32a6cc4bce6439abea21f3e6b4bbf6 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 27 Sep 2018 14:58:26 +0200 Subject: [PATCH] Fix missing access errors for guarded properties If a property access would normally result in a magic method call, but the property is subject to an active recursion guard, the access should behave as if the magic method does not exist. This commit fixes one instance where this was not the case -- we should have been generating a property access error, but instead the operation simply did not do anything. --- Zend/tests/bug38461.phpt | 8 ++- Zend/tests/bug48248.phpt | 9 +-- ..._access_errors_for_guarded_properties.phpt | 55 +++++++++++++++++++ Zend/zend_object_handlers.c | 27 +++++---- 4 files changed, 82 insertions(+), 17 deletions(-) create mode 100644 Zend/tests/property_access_errors_for_guarded_properties.phpt diff --git a/Zend/tests/bug38461.phpt b/Zend/tests/bug38461.phpt index 281d999019..3dfa89c055 100644 --- a/Zend/tests/bug38461.phpt +++ b/Zend/tests/bug38461.phpt @@ -21,5 +21,9 @@ $op->x = 'test'; echo "Done\n"; ?> ---EXPECT-- -Done +--EXPECTF-- +Fatal error: Uncaught Error: Cannot access private property ExtOperation::$x in %s:%d +Stack trace: +#0 %s(%d): Operation->__set('x', 'test') +#1 {main} + thrown in %s on line %d diff --git a/Zend/tests/bug48248.phpt b/Zend/tests/bug48248.phpt index 9cdb7c1a00..16fb3273f2 100644 --- a/Zend/tests/bug48248.phpt +++ b/Zend/tests/bug48248.phpt @@ -21,7 +21,8 @@ var_dump($b->test); ?> --EXPECTF-- -Notice: Undefined property: B::$test in %s on line %d - -Notice: Only variable references should be returned by reference in %s on line %d -NULL +Fatal error: Uncaught Error: Cannot access private property B::$test in %s:%d +Stack trace: +#0 %s(%d): A->__get('test') +#1 {main} + thrown in %s on line %d diff --git a/Zend/tests/property_access_errors_for_guarded_properties.phpt b/Zend/tests/property_access_errors_for_guarded_properties.phpt new file mode 100644 index 0000000000..0a360a9447 --- /dev/null +++ b/Zend/tests/property_access_errors_for_guarded_properties.phpt @@ -0,0 +1,55 @@ +--TEST-- +Property access errors should be thrown for overloaded properties protected by recursion guards +--FILE-- +prop = 42; +} + +function getProp($obj) { + var_dump($obj->prop); +} + +function unsetProp($obj) { + unset($obj->prop); +} + +class Test { + private $prop; + + public function __get($k) { + getProp($this); + } + + public function __set($k, $v) { + setProp($this); + } + + public function __unset($k) { + unsetProp($this); + } +} + +$test = new Test; +try { + $test->prop = "bar"; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + var_dump($test->prop); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + unset($test->prop); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECT-- +Cannot access private property Test::$prop +Cannot access private property Test::$prop +Cannot access private property Test::$prop diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 05c970da8b..8703cc44dc 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -741,8 +741,10 @@ call_getter: } OBJ_RELEASE(zobj); goto exit; - } else if (UNEXPECTED(ZSTR_VAL(name)[0] == '\0') && ZSTR_LEN(name) != 0) { - zend_bad_property_name(); + } else if (UNEXPECTED(IS_WRONG_PROPERTY_OFFSET(property_offset))) { + /* Trigger the correct error */ + zend_get_property_offset(zobj->ce, name, 0, NULL); + ZEND_ASSERT(EG(exception)); retval = &EG(uninitialized_zval); goto exit; } @@ -808,12 +810,13 @@ found: } else if (EXPECTED(!IS_WRONG_PROPERTY_OFFSET(property_offset))) { goto write_std_property; } else { - if (UNEXPECTED(ZSTR_VAL(name)[0] == '\0') && ZSTR_LEN(name) != 0) { - zend_bad_property_name(); - goto exit; - } + /* Trigger the correct error */ + zend_get_property_offset(zobj->ce, name, 0, NULL); + ZEND_ASSERT(EG(exception)); + goto exit; } - } else if (EXPECTED(!IS_WRONG_PROPERTY_OFFSET(property_offset))) { + } else { + ZEND_ASSERT(!IS_WRONG_PROPERTY_OFFSET(property_offset)); write_std_property: if (Z_REFCOUNTED_P(value)) { if (Z_ISREF_P(value)) { @@ -1053,11 +1056,13 @@ ZEND_API void zend_std_unset_property(zval *object, zval *member, void **cache_s (*guard) |= IN_UNSET; /* prevent circular unsetting */ zend_std_call_unsetter(zobj, name); (*guard) &= ~IN_UNSET; + } else if (UNEXPECTED(IS_WRONG_PROPERTY_OFFSET(property_offset))) { + /* Trigger the correct error */ + zend_get_property_offset(zobj->ce, name, 0, NULL); + ZEND_ASSERT(EG(exception)); + goto exit; } else { - if (UNEXPECTED(ZSTR_VAL(name)[0] == '\0') && ZSTR_LEN(name) != 0) { - zend_bad_property_name(); - goto exit; - } + /* Nothing to do: The property already does not exist. */ } } -- 2.40.0