diff --git a/Zend/tests/readonly_props/readonly_clone_error1.phpt b/Zend/tests/readonly_props/readonly_clone_error1.phpt new file mode 100644 index 0000000000000..98829d175765d --- /dev/null +++ b/Zend/tests/readonly_props/readonly_clone_error1.phpt @@ -0,0 +1,36 @@ +--TEST-- +Readonly property cannot be reset twice during cloning +--FILE-- +bar = 2; + var_dump($this); + $this->bar = 3; + } +} + +$foo = new Foo(1); + +try { + clone $foo; +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} + +echo "done"; + +?> +--EXPECT-- +object(Foo)#2 (1) { + ["bar"]=> + int(2) +} +Cannot modify readonly property Foo::$bar +done diff --git a/Zend/tests/readonly_props/readonly_clone_error2.phpt b/Zend/tests/readonly_props/readonly_clone_error2.phpt new file mode 100644 index 0000000000000..e1e42abad074c --- /dev/null +++ b/Zend/tests/readonly_props/readonly_clone_error2.phpt @@ -0,0 +1,42 @@ +--TEST-- +Readonly property cannot be reset after cloning when there is no custom clone handler +--FILE-- +bar++; + } + + public function wrongCloneNew() + { + $instance = clone $this; + $instance->baz++; + } +} + +$foo = new Foo(1, 1); + +try { + $foo->wrongCloneOld(); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} + +try { + $foo->wrongCloneNew(); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} + +?> +--EXPECT-- +Cannot modify readonly property Foo::$bar +Cannot modify readonly property Foo::$baz diff --git a/Zend/tests/readonly_props/readonly_clone_error3.phpt b/Zend/tests/readonly_props/readonly_clone_error3.phpt new file mode 100644 index 0000000000000..a51ec165c68c3 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_clone_error3.phpt @@ -0,0 +1,44 @@ +--TEST-- +Readonly property cannot be reset after cloning when there is a custom clone handler +--FILE-- +bar++; + } + + public function wrongCloneNew() + { + $instance = clone $this; + $instance->baz++; + } +} + +$foo = new Foo(1, 1); + +try { + $foo->wrongCloneOld(); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} + +try { + $foo->wrongCloneNew(); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} + +?> +--EXPECT-- +Cannot modify readonly property Foo::$bar +Cannot modify readonly property Foo::$baz diff --git a/Zend/tests/readonly_props/readonly_clone_success1.phpt b/Zend/tests/readonly_props/readonly_clone_success1.phpt new file mode 100644 index 0000000000000..72cd9e9622b33 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_clone_success1.phpt @@ -0,0 +1,39 @@ +--TEST-- +Readonly property can be reset once during cloning +--FILE-- +bar++; + } +} + +$foo = new Foo(1); + +var_dump(clone $foo); + +$foo2 = clone $foo; +var_dump($foo2); + +var_dump(clone $foo2); + +?> +--EXPECTF-- +object(Foo)#%d (%d) { + ["bar"]=> + int(2) +} +object(Foo)#%d (%d) { + ["bar"]=> + int(2) +} +object(Foo)#%d (%d) { + ["bar"]=> + int(3) +} diff --git a/Zend/tests/readonly_props/readonly_clone_success2.phpt b/Zend/tests/readonly_props/readonly_clone_success2.phpt new file mode 100644 index 0000000000000..98faba3909518 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_clone_success2.phpt @@ -0,0 +1,46 @@ +--TEST-- +Test that __clone() unset and reassign properties +--FILE-- +bar); + var_dump($this); + $this->bar = new stdClass(); + } +} + +$foo = new Foo(new stdClass()); +var_dump($foo); +$foo2 = clone $foo; + +var_dump($foo); +var_dump($foo2); + +?> +--EXPECTF-- +object(Foo)#1 (%d) { + ["bar"]=> + object(stdClass)#2 (%d) { + } +} +object(Foo)#3 (%d) { + ["bar"]=> + uninitialized(stdClass) +} +object(Foo)#1 (%d) { + ["bar"]=> + object(stdClass)#2 (%d) { + } +} +object(Foo)#3 (%d) { + ["bar"]=> + object(stdClass)#4 (%d) { + } +} diff --git a/Zend/tests/readonly_props/readonly_clone_success3.phpt b/Zend/tests/readonly_props/readonly_clone_success3.phpt new file mode 100644 index 0000000000000..93c3bd8f47f62 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_clone_success3.phpt @@ -0,0 +1,37 @@ +--TEST-- +__clone() can indirectly modify unlocked readonly properties +--FILE-- +bar['bar'] = 'bar'; + } +} + +$foo = new Foo([]); +// First call fills the cache slot +var_dump(clone $foo); +var_dump(clone $foo); + +?> +--EXPECTF-- +object(Foo)#2 (%d) { + ["bar"]=> + array(1) { + ["bar"]=> + string(3) "bar" + } +} +object(Foo)#2 (%d) { + ["bar"]=> + array(1) { + ["bar"]=> + string(3) "bar" + } +} diff --git a/Zend/tests/readonly_props/readonly_clone_success4.phpt b/Zend/tests/readonly_props/readonly_clone_success4.phpt new file mode 100644 index 0000000000000..40b7a29e31265 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_clone_success4.phpt @@ -0,0 +1,39 @@ +--TEST-- +Readonly property can be reset once during cloning even after a type error +--FILE-- +bar = "foo"; + } catch (Error $e) { + echo $e->getMessage() . "\n"; + } + + $this->bar = 2; + } +} + +$foo = new Foo(1); + +var_dump(clone $foo); +var_dump(clone $foo); + +?> +--EXPECTF-- +Cannot assign string to property Foo::$bar of type int +object(Foo)#%d (%d) { + ["bar"]=> + int(2) +} +Cannot assign string to property Foo::$bar of type int +object(Foo)#%d (%d) { + ["bar"]=> + int(2) +} diff --git a/Zend/tests/readonly_props/readonly_coercion_type_error.phpt b/Zend/tests/readonly_props/readonly_coercion_type_error.phpt new file mode 100644 index 0000000000000..d41211d8e9981 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_coercion_type_error.phpt @@ -0,0 +1,23 @@ +--TEST-- +Test failing readonly assignment with coercion +--FILE-- +bar = 'bar'; + try { + $this->bar = 42; + } catch (Error $e) { + echo $e->getMessage(), "\n"; + } + } +} + +new Foo(); + +?> +--EXPECTF-- +Cannot modify readonly property Foo::$bar diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 5247ca1a379af..b7006c15e35cc 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -4224,6 +4224,10 @@ ZEND_API zend_property_info *zend_declare_typed_property(zend_class_entry *ce, z ce->ce_flags |= ZEND_ACC_HAS_TYPE_HINTS; } + if (access_type & ZEND_ACC_READONLY) { + ce->ce_flags |= ZEND_ACC_HAS_READONLY_PROPS; + } + if (ce->type == ZEND_INTERNAL_CLASS) { property_info = pemalloc(sizeof(zend_property_info), 1); } else { diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index 22221f2265c3f..8ff8ab0f509fb 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -241,7 +241,7 @@ typedef struct _zend_oparray_context { /* or IS_CONSTANT_VISITED_MARK | | | */ #define ZEND_CLASS_CONST_IS_CASE (1 << 6) /* | | | X */ /* | | | */ -/* Class Flags (unused: 21,30,31) | | | */ +/* Class Flags (unused: 30,31) | | | */ /* =========== | | | */ /* | | | */ /* Special class types | | | */ @@ -288,6 +288,8 @@ typedef struct _zend_oparray_context { /* | | | */ /* Class is linked apart from variance obligations. | | | */ #define ZEND_ACC_NEARLY_LINKED (1 << 20) /* X | | | */ +/* Class has readonly props | | | */ +#define ZEND_ACC_HAS_READONLY_PROPS (1 << 21) /* X | | | */ /* | | | */ /* stored in opcache (may be partially) | | | */ #define ZEND_ACC_CACHED (1 << 22) /* X | | | */ diff --git a/Zend/zend_enum.c b/Zend/zend_enum.c index 435ebfeaf109c..4abca60fa85b1 100644 --- a/Zend/zend_enum.c +++ b/Zend/zend_enum.c @@ -41,9 +41,17 @@ zend_object *zend_enum_new(zval *result, zend_class_entry *ce, zend_string *case zend_object *zobj = zend_objects_new(ce); ZVAL_OBJ(result, zobj); - ZVAL_STR_COPY(OBJ_PROP_NUM(zobj, 0), case_name); + zval *zname = OBJ_PROP_NUM(zobj, 0); + ZVAL_STR_COPY(zname, case_name); + /* ZVAL_COPY does not set Z_PROP_FLAG, this needs to be cleared to avoid leaving IS_PROP_REINITABLE set */ + Z_PROP_FLAG_P(zname) = 0; + if (backing_value_zv != NULL) { - ZVAL_COPY(OBJ_PROP_NUM(zobj, 1), backing_value_zv); + zval *prop = OBJ_PROP_NUM(zobj, 1); + + ZVAL_COPY(prop, backing_value_zv); + /* ZVAL_COPY does not set Z_PROP_FLAG, this needs to be cleared to avoid leaving IS_PROP_REINITABLE set */ + Z_PROP_FLAG_P(prop) = 0; } return zobj; @@ -179,7 +187,7 @@ void zend_enum_add_interfaces(zend_class_entry *ce) if (ce->enum_backing_type != IS_UNDEF) { ce->interface_names[num_interfaces_before + 1].name = zend_string_copy(zend_ce_backed_enum->name); - ce->interface_names[num_interfaces_before + 1].lc_name = zend_string_init("backedenum", sizeof("backedenum") - 1, 0); + ce->interface_names[num_interfaces_before + 1].lc_name = zend_string_init("backedenum", sizeof("backedenum") - 1, 0); } ce->default_object_handlers = &zend_enum_object_handlers; diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index e15469c539e95..052cd0c3f49e9 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -1009,11 +1009,6 @@ static zend_never_inline zval* zend_assign_to_typed_prop(zend_property_info *inf { zval tmp; - if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) { - zend_readonly_property_modification_error(info); - return &EG(uninitialized_zval); - } - ZVAL_DEREF(value); ZVAL_COPY(&tmp, value); @@ -1022,6 +1017,16 @@ static zend_never_inline zval* zend_assign_to_typed_prop(zend_property_info *inf return &EG(uninitialized_zval); } + if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) { + if (Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE) { + Z_PROP_FLAG_P(property_val) &= ~IS_PROP_REINITABLE; + } else { + zval_ptr_dtor(&tmp); + zend_readonly_property_modification_error(info); + return &EG(uninitialized_zval); + } + } + return zend_assign_to_variable(property_val, &tmp, IS_TMP_VAR, EX_USES_STRICT_TYPES()); } @@ -3161,6 +3166,8 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c ZEND_ASSERT(type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET); if (Z_TYPE_P(ptr) == IS_OBJECT) { ZVAL_COPY(result, ptr); + } else if (Z_PROP_FLAG_P(ptr) & IS_PROP_REINITABLE) { + Z_PROP_FLAG_P(ptr) &= ~IS_PROP_REINITABLE; } else { zend_readonly_property_modification_error(prop_info); ZVAL_ERROR(result); diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index db6bdaff86f63..323b6269ee55e 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -463,6 +463,7 @@ ZEND_API int ZEND_FASTCALL zend_handle_undef_args(zend_execute_data *call); } while (0) #define ZEND_CLASS_HAS_TYPE_HINTS(ce) ((ce->ce_flags & ZEND_ACC_HAS_TYPE_HINTS) == ZEND_ACC_HAS_TYPE_HINTS) +#define ZEND_CLASS_HAS_READONLY_PROPS(ce) ((ce->ce_flags & ZEND_ACC_HAS_READONLY_PROPS) == ZEND_ACC_HAS_READONLY_PROPS) ZEND_API bool zend_verify_property_type(const zend_property_info *info, zval *property, bool strict); ZEND_COLD void zend_verify_property_type_error(const zend_property_info *info, const zval *property); diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 61b1e06e7cb65..028bdebbfb8f7 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1629,7 +1629,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par ce->ce_flags |= ZEND_ACC_EXPLICIT_ABSTRACT_CLASS; } } - ce->ce_flags |= parent_ce->ce_flags & (ZEND_HAS_STATIC_IN_METHODS | ZEND_ACC_HAS_TYPE_HINTS | ZEND_ACC_USE_GUARDS | ZEND_ACC_NOT_SERIALIZABLE | ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES); + ce->ce_flags |= parent_ce->ce_flags & (ZEND_HAS_STATIC_IN_METHODS | ZEND_ACC_HAS_TYPE_HINTS | ZEND_ACC_HAS_READONLY_PROPS | ZEND_ACC_USE_GUARDS | ZEND_ACC_NOT_SERIALIZABLE | ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES); } /* }}} */ diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index a0ac789e95400..5545f7ef7c646 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -615,6 +615,8 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int * to make sure no actual modification is possible. */ ZVAL_COPY(rv, retval); retval = rv; + } else if (Z_PROP_FLAG_P(retval) & IS_PROP_REINITABLE) { + Z_PROP_FLAG_P(retval) &= ~IS_PROP_REINITABLE; } else { zend_readonly_property_modification_error(prop_info); retval = &EG(uninitialized_zval); @@ -633,7 +635,7 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int } } } - if (UNEXPECTED(Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) { + if (UNEXPECTED(Z_PROP_FLAG_P(retval) & IS_PROP_UNINIT)) { /* Skip __get() for uninitialized typed properties */ goto uninit_error; } @@ -810,13 +812,6 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva Z_TRY_ADDREF_P(value); if (UNEXPECTED(prop_info)) { - if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { - Z_TRY_DELREF_P(value); - zend_readonly_property_modification_error(prop_info); - variable_ptr = &EG(error_zval); - goto exit; - } - ZVAL_COPY_VALUE(&tmp, value); // Increase refcount to prevent object from being released in __toString() GC_ADDREF(zobj); @@ -833,6 +828,16 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva variable_ptr = &EG(error_zval); goto exit; } + if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { + if (Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE) { + Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE; + } else { + zval_ptr_dtor(&tmp); + zend_readonly_property_modification_error(prop_info); + variable_ptr = &EG(error_zval); + goto exit; + } + } value = &tmp; } @@ -841,7 +846,7 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva variable_ptr, value, IS_TMP_VAR, property_uses_strict_types()); goto exit; } - if (Z_PROP_FLAG_P(variable_ptr) == IS_PROP_UNINIT) { + if (Z_PROP_FLAG_P(variable_ptr) & IS_PROP_UNINIT) { /* Writes to uninitialized typed properties bypass __set(). */ goto write_std_property; } @@ -1069,7 +1074,7 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam if (UNEXPECTED(Z_TYPE_P(retval) == IS_UNDEF)) { if (EXPECTED(!zobj->ce->__get) || UNEXPECTED((*zend_get_property_guard(zobj, name)) & IN_GET) || - UNEXPECTED(prop_info && Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) { + UNEXPECTED(prop_info && (Z_PROP_FLAG_P(retval) & IS_PROP_UNINIT))) { if (UNEXPECTED(type == BP_VAR_RW || type == BP_VAR_R)) { if (UNEXPECTED(prop_info)) { zend_throw_error(NULL, @@ -1146,8 +1151,12 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void if (Z_TYPE_P(slot) != IS_UNDEF) { if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY))) { - zend_readonly_property_unset_error(prop_info->ce, name); - return; + if (Z_PROP_FLAG_P(slot) & IS_PROP_REINITABLE) { + Z_PROP_FLAG_P(slot) &= ~IS_PROP_REINITABLE; + } else { + zend_readonly_property_unset_error(prop_info->ce, name); + return; + } } if (UNEXPECTED(Z_ISREF_P(slot)) && (ZEND_DEBUG || ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(slot)))) { @@ -1164,7 +1173,7 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void } return; } - if (UNEXPECTED(Z_PROP_FLAG_P(slot) == IS_PROP_UNINIT)) { + if (UNEXPECTED(Z_PROP_FLAG_P(slot) & IS_PROP_UNINIT)) { if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY) && !verify_readonly_initialization_access(prop_info, zobj->ce, name, "unset"))) { return; @@ -1779,7 +1788,7 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has if (Z_TYPE_P(value) != IS_UNDEF) { goto found; } - if (UNEXPECTED(Z_PROP_FLAG_P(value) == IS_PROP_UNINIT)) { + if (UNEXPECTED(Z_PROP_FLAG_P(value) & IS_PROP_UNINIT)) { /* Skip __isset() for uninitialized typed properties */ result = 0; goto exit; diff --git a/Zend/zend_objects.c b/Zend/zend_objects.c index df76fa0bb8dc3..4d40b76854df0 100644 --- a/Zend/zend_objects.c +++ b/Zend/zend_objects.c @@ -192,6 +192,8 @@ ZEND_API zend_object* ZEND_FASTCALL zend_objects_new(zend_class_entry *ce) ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object, zend_object *old_object) { + bool has_clone_method = old_object->ce->clone != NULL; + if (old_object->ce->default_properties_count) { zval *src = old_object->properties_table; zval *dst = new_object->properties_table; @@ -201,6 +203,11 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object, i_zval_ptr_dtor(dst); ZVAL_COPY_VALUE_PROP(dst, src); zval_add_ref(dst); + if (has_clone_method) { + /* Unconditionally add the IS_PROP_REINITABLE flag to avoid a potential cache miss of property_info */ + Z_PROP_FLAG_P(dst) |= IS_PROP_REINITABLE; + } + if (UNEXPECTED(Z_ISREF_P(dst)) && (ZEND_DEBUG || ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(dst)))) { zend_property_info *prop_info = zend_get_property_info_for_slot(new_object, dst); @@ -211,7 +218,7 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object, src++; dst++; } while (src != end); - } else if (old_object->properties && !old_object->ce->clone) { + } else if (old_object->properties && !has_clone_method) { /* fast copy */ if (EXPECTED(old_object->handlers == &std_object_handlers)) { if (EXPECTED(!(GC_FLAGS(old_object->properties) & IS_ARRAY_IMMUTABLE))) { @@ -245,6 +252,10 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object, ZVAL_COPY_VALUE(&new_prop, prop); zval_add_ref(&new_prop); } + if (has_clone_method) { + /* Unconditionally add the IS_PROP_REINITABLE flag to avoid a potential cache miss of property_info */ + Z_PROP_FLAG_P(&new_prop) |= IS_PROP_REINITABLE; + } if (EXPECTED(key)) { _zend_hash_append(new_object->properties, key, &new_prop); } else { @@ -253,9 +264,18 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object, } ZEND_HASH_FOREACH_END(); } - if (old_object->ce->clone) { + if (has_clone_method) { GC_ADDREF(new_object); zend_call_known_instance_method_with_0_params(new_object->ce->clone, new_object, NULL); + + if (ZEND_CLASS_HAS_READONLY_PROPS(new_object->ce)) { + for (uint32_t i = 0; i < new_object->ce->default_properties_count; i++) { + zval* prop = OBJ_PROP_NUM(new_object, i); + /* Unconditionally remove the IS_PROP_REINITABLE flag to avoid a potential cache miss of property_info */ + Z_PROP_FLAG_P(prop) &= ~IS_PROP_REINITABLE; + } + } + OBJ_RELEASE(new_object); } } diff --git a/Zend/zend_types.h b/Zend/zend_types.h index 3d900226e54e5..faed815401948 100644 --- a/Zend/zend_types.h +++ b/Zend/zend_types.h @@ -1273,7 +1273,8 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) { * (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 IS_PROP_UNINIT (1<<0) +#define IS_PROP_REINITABLE (1<<1) /* It has impact only on readonly properties */ #define Z_PROP_FLAG_P(z) Z_EXTRA_P(z) #define ZVAL_COPY_VALUE_PROP(z, v) \ do { *(z) = *(v); } while (0)