Skip to content

Don't call __set() on uninitialized typed properties #4854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Don't call __set() on uninitialized typed properties #4854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions Zend/tests/type_declarations/typed_properties_magic_set.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
__set() should not be invoked when setting an uninitialized typed property
--FILE--
<?php

class Test {
public int $foo;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a field that is not typed, so we verify the behavior for that as well? It shouldn't be an issue, but it does indeed prevent a regression


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;

?>
--EXPECT--
int(42)
__set foo = 42
__set foo = 42
2 changes: 1 addition & 1 deletion Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6135,7 +6135,7 @@ void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t flags) /
} else if (!ZEND_TYPE_IS_SET(type)) {
ZVAL_NULL(&value_zv);
} else {
ZVAL_UNDEF(&value_zv);
Z_TYPE_INFO_P(&value_zv) = IS_UNINIT_PROP_EX;
}

zend_declare_typed_property(ce, name, &value_zv, flags, doc_comment, type);
Expand Down
8 changes: 8 additions & 0 deletions Zend/zend_object_handlers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,10 @@ ZEND_API zval *zend_std_write_property(zval *object, zval *member, zval *value,
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_TYPE_INFO_P(variable_ptr) == IS_UNINIT_PROP_EX) {
/* Writes to uninitialized typed properties bypass __set() */
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)) {
Expand Down Expand Up @@ -1113,6 +1117,10 @@ ZEND_API void zend_std_unset_property(zval *object, zval *member, void **cache_s
}
goto exit;
}
if (Z_TYPE_INFO_P(slot) == IS_UNINIT_PROP_EX) {
/* Mark property as fully unset, so that __set() will be called from this point on. */
ZVAL_UNDEF(slot);
}
} else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset))
&& EXPECTED(zobj->properties != NULL)) {
if (UNEXPECTED(GC_REFCOUNT(zobj->properties) > 1)) {
Expand Down
7 changes: 5 additions & 2 deletions Zend/zend_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,9 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
/* zval.u1.v.type_flags */
#define IS_TYPE_REFCOUNTED (1<<0)
#define IS_TYPE_COLLECTABLE (1<<1)
#define IS_TYPE_UNINIT_PROP (1<<2)

#if 1
#if 0
/* This optimized version assumes that we have a single "type_flag" */
/* IS_TYPE_COLLECTABLE may be used only with IS_TYPE_REFCOUNTED */
# define Z_TYPE_INFO_REFCOUNTED(t) (((t) & Z_TYPE_FLAGS_MASK) != 0)
Expand All @@ -576,6 +577,8 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {

#define IS_CONSTANT_AST_EX (IS_CONSTANT_AST | (IS_TYPE_REFCOUNTED << Z_TYPE_FLAGS_SHIFT))

#define IS_UNINIT_PROP_EX (IS_UNDEF | (IS_TYPE_UNINIT_PROP << Z_TYPE_FLAGS_SHIFT))

/* string flags (zval.value->gc.u.flags) */
#define IS_STR_INTERNED GC_IMMUTABLE /* interned string */
#define IS_STR_PERSISTENT GC_PERSISTENT /* allocated using malloc */
Expand Down Expand Up @@ -624,7 +627,7 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
#define Z_CONSTANT(zval) (Z_TYPE(zval) == IS_CONSTANT_AST)
#define Z_CONSTANT_P(zval_p) Z_CONSTANT(*(zval_p))

#if 1
#if 0
/* This optimized version assumes that we have a single "type_flag" */
/* IS_TYPE_COLLECTABLE may be used only with IS_TYPE_REFCOUNTED */
#define Z_REFCOUNTED(zval) (Z_TYPE_FLAGS(zval) != 0)
Expand Down
16 changes: 8 additions & 8 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6448,11 +6448,11 @@ ZEND_VM_C_LABEL(fe_fetch_r_exit):
}
value = &p->val;
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
if (UNEXPECTED(value_type == IS_INDIRECT)) {
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
break;
}
} else {
Expand Down Expand Up @@ -6488,11 +6488,11 @@ ZEND_VM_C_LABEL(fe_fetch_r_exit):

value = &p->val;
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
if (UNEXPECTED(value_type == IS_INDIRECT)) {
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
break;
}
Expand Down Expand Up @@ -6601,11 +6601,11 @@ ZEND_VM_HANDLER(126, ZEND_FE_FETCH_RW, VAR, ANY, JMP_ADDR)
}
value = &p->val;
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
if (UNEXPECTED(value_type == IS_INDIRECT)) {
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
break;
}
} else {
Expand Down Expand Up @@ -6640,11 +6640,11 @@ ZEND_VM_HANDLER(126, ZEND_FE_FETCH_RW, VAR, ANY, JMP_ADDR)

value = &p->val;
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
if (UNEXPECTED(value_type == IS_INDIRECT)) {
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
if ((value_type & Z_TYPE_MASK) != IS_REFERENCE) {
zend_property_info *prop_info =
Expand Down
16 changes: 8 additions & 8 deletions Zend/zend_vm_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21393,11 +21393,11 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_R_SPEC_VAR_HANDLER(ZE
}
value = &p->val;
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
if (UNEXPECTED(value_type == IS_INDIRECT)) {
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
break;
}
} else {
Expand Down Expand Up @@ -21433,11 +21433,11 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_R_SPEC_VAR_HANDLER(ZE

value = &p->val;
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
if (UNEXPECTED(value_type == IS_INDIRECT)) {
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
break;
}
Expand Down Expand Up @@ -21546,11 +21546,11 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_RW_SPEC_VAR_HANDLER(Z
}
value = &p->val;
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
if (UNEXPECTED(value_type == IS_INDIRECT)) {
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
break;
}
} else {
Expand Down Expand Up @@ -21585,11 +21585,11 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_RW_SPEC_VAR_HANDLER(Z

value = &p->val;
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)) {
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)) {
if (UNEXPECTED(value_type == IS_INDIRECT)) {
value = Z_INDIRECT_P(value);
value_type = Z_TYPE_INFO_P(value);
if (EXPECTED(value_type != IS_UNDEF)
if (EXPECTED((value_type & Z_TYPE_MASK) != IS_UNDEF)
&& EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) {
if ((value_type & Z_TYPE_MASK) != IS_REFERENCE) {
zend_property_info *prop_info =
Expand Down