]> granicus.if.org Git - php/commitdiff
Fix missing access errors for guarded properties
authorNikita Popov <nikita.ppv@gmail.com>
Thu, 27 Sep 2018 12:58:26 +0000 (14:58 +0200)
committerNikita Popov <nikita.ppv@gmail.com>
Thu, 27 Sep 2018 12:58:26 +0000 (14:58 +0200)
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
Zend/tests/bug48248.phpt
Zend/tests/property_access_errors_for_guarded_properties.phpt [new file with mode: 0644]
Zend/zend_object_handlers.c

index 281d999019f027a4cfe1fd7fab184a39e2ba9c32..3dfa89c055e14ca291a274998723261601de1a99 100644 (file)
@@ -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
index 9cdb7c1a007a446bc75a8a24f78d171c98985587..16fb3273f267b3832cf5f013df8b46c79bcc92bf 100644 (file)
@@ -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 (file)
index 0000000..0a360a9
--- /dev/null
@@ -0,0 +1,55 @@
+--TEST--
+Property access errors should be thrown for overloaded properties protected by recursion guards
+--FILE--
+<?php
+
+function setProp($obj) {
+    $obj->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
index 05c970da8be9e929411703a99d4ced505da154c6..8703cc44dce8d9501cc958c723afe7678e89fb27 100644 (file)
@@ -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. */
                }
        }