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.
This commit is contained in:
Nikita Popov 2019-10-24 16:36:25 +02:00
parent 4d8541debb
commit f1848a4b3f
8 changed files with 89 additions and 8 deletions

3
NEWS
View file

@ -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)

View file

@ -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

View file

@ -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)) {

View file

@ -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;
}

View file

@ -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)) {

View file

@ -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)))) {

View file

@ -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 */

View file

@ -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);
}
}