Fix bug #78226: Don't call __set() on uninitialized typed properties

Assigning to an uninitialized typed property will no longer trigger
a call to __set(). However, calls to __set() are still triggered if
the property is explicitly unset().

This gives us both the behavior people generally expect, and still
allows ORMs to do lazy initialization by unsetting properties.

For PHP 8, we should fine a way to forbid unsetting of declared
properties entirely, and provide a different way to achieve lazy
initialization.

diff --git a/NEWS b/NEWS
index 5003143e92b21919c452ea16a7a4e047e4bdb8e8..767c3ea25a88d0c724fb011cd00c6e7b959640b1 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 7.4.0RC5
 
+- Core:
+  . Fixed bug #78226 (Unexpected __set behavior with typed properties). (Nikita)
+
 - COM:
   . Fixed bug #78694 (Appending to a variant array causes segfault). (cmb)
 

diff --git a/Zend/tests/type_declarations/typed_properties_magic_set.phpt b/Zend/tests/type_declarations/typed_properties_magic_set.phpt
new file mode 100644 (file)
index 0000000..fd101cf
--- /dev/null
+++ b/Zend/tests/type_declarations/typed_properties_magic_set.phpt
@@ -0,0 +1,54 @@
+--TEST--
+__set() should not be invoked when setting an uninitialized typed property
+--FILE--
+<?php
+
+class Test {
+    public int $foo;
+    public function __set($name, $value) {
+        echo "__set ", $name, " = ", $value, "\n";
+    }
+}
+
+$test = new Test;
+$test->foo = 42;
+var_dump($test->foo);
+
+// __set will be called after unset()
+unset($test->foo);
+$test->foo = 42;
+
+// __set will be called after unset() without prior initialization
+$test = new Test;
+unset($test->foo);
+$test->foo = 42;
+
+class Test2 extends Test {
+}
+
+// Check that inherited properties work correctly
+$test = new Test;
+$test->foo = 42;
+var_dump($test->foo);
+unset($test->foo);
+$test->foo = 42;
+
+// Test that cloning works correctly
+$test = clone $test;
+$test->foo = 42;
+$test = clone new Test;
+$test->foo = 42;
+var_dump($test->foo);
+unset($test->foo);
+$test->foo = 42;
+
+?>
+--EXPECT--
+int(42)
+__set foo = 42
+__set foo = 42
+int(42)
+__set foo = 42
+__set foo = 42
+int(42)
+__set foo = 42

diff --git a/Zend/zend_API.c b/Zend/zend_API.c
index 8fab994297a6a1ceb91cd7f8f096afb98431db86..6d9d10a13f2de331017141a8596ccaa6ba1d4528 100644 (file)
--- a/Zend/zend_API.c
+++ b/Zend/zend_API.c
@@ -1263,13 +1263,13 @@ static zend_always_inline void _object_properties_init(zend_object *object, zend
 
                if (UNEXPECTED(class_type->type == ZEND_INTERNAL_CLASS)) {
                        do {
-                               ZVAL_COPY_OR_DUP(dst, src);
+                               ZVAL_COPY_OR_DUP_PROP(dst, src);
                                src++;
                                dst++;
                        } while (src != end);
                } else {
                        do {
-                               ZVAL_COPY(dst, src);
+                               ZVAL_COPY_PROP(dst, src);
                                src++;
                                dst++;
                        } while (src != end);
@@ -3725,6 +3725,7 @@ ZEND_API int zend_declare_typed_property(zend_class_entry *ce, zend_string *name
                        }
                }
        } else {
+               zval *property_default_ptr;
                if ((property_info_ptr = zend_hash_find_ptr(&ce->properties_info, name)) != NULL &&
                    (property_info_ptr->flags & ZEND_ACC_STATIC) == 0) {
                        property_info->offset = property_info_ptr->offset;
@@ -3745,7 +3746,9 @@ ZEND_API int zend_declare_typed_property(zend_class_entry *ce, zend_string *name
                                ce->properties_info_table[ce->default_properties_count - 1] = property_info;
                        }
                }
-               ZVAL_COPY_VALUE(&ce->default_properties_table[OBJ_PROP_TO_NUM(property_info->offset)], property);
+               property_default_ptr = &ce->default_properties_table[OBJ_PROP_TO_NUM(property_info->offset)];
+               ZVAL_COPY_VALUE(property_default_ptr, property);
+               Z_PROP_FLAG_P(property_default_ptr) = Z_ISUNDEF_P(property) ? IS_PROP_UNINIT : 0;
        }
        if (ce->type & ZEND_INTERNAL_CLASS) {
                switch(Z_TYPE_P(property)) {

diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c
index c7002d313f01d5bb3f0efb0067a480553d080c9a..56d9e6268f62e170806d1c4bf7ebb5b34053fc6c 100644 (file)
--- a/Zend/zend_inheritance.c
+++ b/Zend/zend_inheritance.c
@@ -1167,7 +1167,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
                        do {
                                dst--;
                                src--;
-                               ZVAL_COPY_VALUE(dst, src);
+                               ZVAL_COPY_VALUE_PROP(dst, src);
                        } while (dst != end);
                        pefree(src, ce->type == ZEND_INTERNAL_CLASS);
                        end = ce->default_properties_table;
@@ -1182,7 +1182,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
                        do {
                                dst--;
                                src--;
-                               ZVAL_COPY_OR_DUP(dst, src);
+                               ZVAL_COPY_OR_DUP_PROP(dst, src);
                                if (Z_OPT_TYPE_P(dst) == IS_CONSTANT_AST) {
                                        ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
                                }
@@ -1192,7 +1192,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
                        do {
                                dst--;
                                src--;
-                               ZVAL_COPY(dst, src);
+                               ZVAL_COPY_PROP(dst, src);
                                if (Z_OPT_TYPE_P(dst) == IS_CONSTANT_AST) {
                                        ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
                                }

diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
index 09984390c90246c6209f15bb2e8781602f155dfd..5ac64fffb19d7b5ffbe9826bca1df44f29d6465f 100644 (file)
--- a/Zend/zend_object_handlers.c
+++ b/Zend/zend_object_handlers.c
@@ -836,6 +836,11 @@ found:
                        zend_assign_to_variable(variable_ptr, value, IS_TMP_VAR, EG(current_execute_data) && ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data)));
                        goto exit;
                }
+               if (Z_PROP_FLAG_P(variable_ptr) == IS_PROP_UNINIT) {
+                       /* Writes to uninitializde typed properties bypass __set(). */
+                       Z_PROP_FLAG_P(variable_ptr) = 0;
+                       goto write_std_property;
+               }
        } else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset))) {
                if (EXPECTED(zobj->properties != NULL)) {
                        if (UNEXPECTED(GC_REFCOUNT(zobj->properties) > 1)) {
@@ -1113,6 +1118,8 @@ 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;
        } else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset))
         && EXPECTED(zobj->properties != NULL)) {
                if (UNEXPECTED(GC_REFCOUNT(zobj->properties) > 1)) {

diff --git a/Zend/zend_objects.c b/Zend/zend_objects.c
index eb76887a9d1cbda759f647f04088abd01c127fbc..ac9412a1c6014e318cfa704a63d1f65228a7767c 100644 (file)
--- a/Zend/zend_objects.c
+++ b/Zend/zend_objects.c
@@ -209,7 +209,7 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 
                do {
                        i_zval_ptr_dtor(dst);
-                       ZVAL_COPY_VALUE(dst, src);
+                       ZVAL_COPY_VALUE_PROP(dst, src);
                        zval_add_ref(dst);
                        if (UNEXPECTED(Z_ISREF_P(dst)) &&
                                        (ZEND_DEBUG || ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(dst)))) {

diff --git a/Zend/zend_types.h b/Zend/zend_types.h
index 83877e0d5d4a95fa6cc6c7c718d5c839a4b9b393..171d70a9dfd5af736d59ca9af9060eba94eea1cb 100644 (file)
--- a/Zend/zend_types.h
+++ b/Zend/zend_types.h
@@ -1262,4 +1262,18 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) {
                }                                                                                               \
        } while (0)
 
+/* Properties store a flag distinguishing unset and unintialized properties
+ * (both use IS_UNDEF type) in the Z_EXTRA space. As such we also need to copy
+ * the Z_EXTRA space when copying property default values etc. We define separate
+ * macros for this purpose, so this workaround is easier to remove in the future. */
+#define IS_PROP_UNINIT 1
+#define Z_PROP_FLAG_P(z) Z_EXTRA_P(z)
+#define ZVAL_COPY_VALUE_PROP(z, v) \
+       do { *(z) = *(v); } while (0)
+#define ZVAL_COPY_PROP(z, v) \
+       do { ZVAL_COPY(z, v); Z_PROP_FLAG_P(z) = Z_PROP_FLAG_P(v); } while (0)
+#define ZVAL_COPY_OR_DUP_PROP(z, v) \
+       do { ZVAL_COPY_OR_DUP(z, v); Z_PROP_FLAG_P(z) = Z_PROP_FLAG_P(v); } while (0)
+
+
 #endif /* ZEND_TYPES_H */

diff --git a/ext/opcache/zend_accelerator_util_funcs.c b/ext/opcache/zend_accelerator_util_funcs.c
index 71e98a6bdc87955ea5fa295588fc023bdf031da2..dc7a76b32610f22d55dac83bf040c8e2616e9e93 100644 (file)
--- a/ext/opcache/zend_accelerator_util_funcs.c
+++ b/ext/opcache/zend_accelerator_util_funcs.c
@@ -270,7 +270,7 @@ static void zend_class_copy_ctor(zend_class_entry **pce)
                end = src + ce->default_properties_count;
                ce->default_properties_table = dst;
                for (; src != end; src++, dst++) {
-                       ZVAL_COPY_VALUE(dst, src);
+                       ZVAL_COPY_VALUE_PROP(dst, src);
                }
        }