From 01d4365f9c5158077bcc7461a2f3b275edc38274 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Tue, 22 Aug 2023 17:38:30 +0100 Subject: [PATCH 1/9] ext/spl: Fix no Error being thrown when appending ArrayObjey with object backing value This aligns the behaviour of calling it with the append() method --- ext/spl/spl_array.c | 13 ++++----- .../tests/ArrayObject/appending_object.phpt | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 ext/spl/tests/ArrayObject/appending_object.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 4a9a5f59f5c13..44e2a56807d96 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -466,6 +466,12 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec HashTable *ht; spl_hash_key key; + /* Cannot append if backing value is an object */ + if (offset == NULL && spl_array_is_object(intern)) { + zend_throw_error(NULL, "Cannot append properties to objects, use %s::offsetSet() instead", ZSTR_VAL(object->ce->name)); + return; + } + if (check_inherited && intern->fptr_offset_set) { zval tmp; @@ -684,13 +690,6 @@ PHP_METHOD(ArrayObject, offsetSet) void spl_array_iterator_append(zval *object, zval *append_value) /* {{{ */ { - spl_array_object *intern = Z_SPLARRAY_P(object); - - if (spl_array_is_object(intern)) { - zend_throw_error(NULL, "Cannot append properties to objects, use %s::offsetSet() instead", ZSTR_VAL(Z_OBJCE_P(object)->name)); - return; - } - spl_array_write_dimension(Z_OBJ_P(object), NULL, append_value); } /* }}} */ diff --git a/ext/spl/tests/ArrayObject/appending_object.phpt b/ext/spl/tests/ArrayObject/appending_object.phpt new file mode 100644 index 0000000000000..1353eb78bfd32 --- /dev/null +++ b/ext/spl/tests/ArrayObject/appending_object.phpt @@ -0,0 +1,28 @@ +--TEST-- +Cannot append to ArrayObject if backing value is an object +--EXTENSIONS-- +spl +--FILE-- +append('no-key'); + var_dump($c); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +try { + $ao->append('no-key'); + var_dump($c); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +?> +--EXPECT-- +Error: Cannot append properties to objects, use ArrayObject::offsetSet() instead +Error: Cannot append properties to objects, use ArrayObject::offsetSet() instead From 1534af4a05f02b622e794f6cf502b3eb1b5dd6d6 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Tue, 22 Aug 2023 17:48:25 +0100 Subject: [PATCH 2/9] ext/spl: Fix unnecessible value written when using null as key --- ext/spl/spl_array.c | 43 ++++++++++--------- ext/spl/tests/ArrayObject/custom_append.phpt | 42 ++++++++++++++++++ .../tests/ArrayObject/using_null_as_key.phpt | 23 ++++++++++ .../using_null_for_offset_methods.phpt | 25 +++++++++++ ext/spl/tests/bug33136.phpt | 2 - 5 files changed, 113 insertions(+), 22 deletions(-) create mode 100644 ext/spl/tests/ArrayObject/custom_append.phpt create mode 100644 ext/spl/tests/ArrayObject/using_null_as_key.phpt create mode 100644 ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 44e2a56807d96..260efa0eda823 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -465,33 +465,22 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec spl_array_object *intern = spl_array_from_obj(object); HashTable *ht; spl_hash_key key; - - /* Cannot append if backing value is an object */ - if (offset == NULL && spl_array_is_object(intern)) { - zend_throw_error(NULL, "Cannot append properties to objects, use %s::offsetSet() instead", ZSTR_VAL(object->ce->name)); - return; - } - - if (check_inherited && intern->fptr_offset_set) { - zval tmp; - - if (!offset) { - ZVAL_NULL(&tmp); - offset = &tmp; - } - zend_call_method_with_2_params(object, object->ce, &intern->fptr_offset_set, "offsetSet", NULL, offset, value); - return; - } + uint32_t refcount = 0; if (intern->nApplyCount > 0) { zend_throw_error(NULL, "Modification of ArrayObject during sorting is prohibited"); return; } - Z_TRY_ADDREF_P(value); + /* We are appending */ + if (!offset) { + /* Cannot append if backing value is an object */ + if (spl_array_is_object(intern)) { + zend_throw_error(NULL, "Cannot append properties to objects, use %s::offsetSet() instead", ZSTR_VAL(object->ce->name)); + return; + } - uint32_t refcount = 0; - if (!offset || Z_TYPE_P(offset) == IS_NULL) { + Z_TRY_ADDREF_P(value); ht = spl_array_get_hash_table(intern); refcount = spl_array_set_refcount(intern->is_child, ht, 1); zend_hash_next_index_insert(ht, value); @@ -502,6 +491,19 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec return; } + if (check_inherited && intern->fptr_offset_set) { + zval tmp; + + if (!offset) { + ZVAL_NULL(&tmp); + offset = &tmp; + } + zend_call_method_with_2_params(object, object->ce, &intern->fptr_offset_set, "offsetSet", NULL, offset, value); + return; + } + + Z_TRY_ADDREF_P(value); + if (get_hash_key(&key, intern, offset) == FAILURE) { zend_illegal_container_offset(object->ce->name, offset, BP_VAR_W); zval_ptr_dtor(value); @@ -514,6 +516,7 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec zend_hash_update_ind(ht, key.key, value); spl_hash_key_release(&key); } else { + ZEND_ASSERT(!spl_array_is_object(intern)); zend_hash_index_update(ht, key.h, value); } diff --git a/ext/spl/tests/ArrayObject/custom_append.phpt b/ext/spl/tests/ArrayObject/custom_append.phpt new file mode 100644 index 0000000000000..94a8a2e8af8be --- /dev/null +++ b/ext/spl/tests/ArrayObject/custom_append.phpt @@ -0,0 +1,42 @@ +--TEST-- +Overwritting default append method +--EXTENSIONS-- +spl +--FILE-- +data = []; + parent::__construct($v); + } + + function append($value): void + { + $this->data[] = $value; + } +} + +$d = new Dummy(); +$ao = new OverWriteAppend($d); + +$ao->append("value1"); +var_dump($ao); +?> +--EXPECT-- +object(OverWriteAppend)#2 (2) { + ["data":"OverWriteAppend":private]=> + array(1) { + [0]=> + string(6) "value1" + } + ["storage":"ArrayObject":private]=> + object(Dummy)#1 (0) { + } +} diff --git a/ext/spl/tests/ArrayObject/using_null_as_key.phpt b/ext/spl/tests/ArrayObject/using_null_as_key.phpt new file mode 100644 index 0000000000000..ce36a4937c4f2 --- /dev/null +++ b/ext/spl/tests/ArrayObject/using_null_as_key.phpt @@ -0,0 +1,23 @@ +--TEST-- +Using null as key must make the value accessible +--EXTENSIONS-- +spl +--FILE-- +{""}); +var_dump($ao[null]); +?> +--EXPECT-- +object(MyClass)#1 (1) { + [""]=> + string(8) "null key" +} +string(8) "null key" +string(8) "null key" diff --git a/ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt b/ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt new file mode 100644 index 0000000000000..76b0b5f9eba70 --- /dev/null +++ b/ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt @@ -0,0 +1,25 @@ +--TEST-- +Using null as offset must make the value accessible +--EXTENSIONS-- +spl +--FILE-- +offsetSet(null, "null key"); +var_dump($ao->offsetExists(null)); +var_dump($ao->offsetGet(null)); +var_dump($c); +var_dump($c->{""}); +?> +--EXPECT-- +bool(true) +string(8) "null key" +object(MyClass)#1 (1) { + [""]=> + string(8) "null key" +} +string(8) "null key" diff --git a/ext/spl/tests/bug33136.phpt b/ext/spl/tests/bug33136.phpt index 35122909031f2..74aef158e6655 100644 --- a/ext/spl/tests/bug33136.phpt +++ b/ext/spl/tests/bug33136.phpt @@ -49,10 +49,8 @@ var_dump(count($arrayObj)); --EXPECT-- Initiate Obj Assign values -Collection::offsetSet(NULL,foo) Collection::offsetGet(0) string(3) "foo" -Collection::offsetSet(NULL,bar) Collection::offsetGet(0) string(3) "foo" Collection::offsetGet(1) From 1f7c794e6d0ee6e866341ee7be202066885bb9dd Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Tue, 22 Aug 2023 18:01:01 +0100 Subject: [PATCH 3/9] Fix no dynamic prop warning + typed prop --- ext/spl/spl_array.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 260efa0eda823..17aecac33476c 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -513,7 +513,13 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec ht = spl_array_get_hash_table(intern); refcount = spl_array_set_refcount(intern->is_child, ht, 1); if (key.key) { - zend_hash_update_ind(ht, key.key, value); + if (spl_array_is_object(intern)) { + ZEND_ASSERT(Z_TYPE(intern->array) == IS_OBJECT); + zend_object *obj = Z_OBJ(intern->array); + obj->handlers->write_property(obj, key.key, value, NULL); + } else { + zend_hash_update_ind(ht, key.key, value); + } spl_hash_key_release(&key); } else { ZEND_ASSERT(!spl_array_is_object(intern)); From 79211f6eeddc8538c8a06784ec3c617937fae776 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 23 Aug 2023 15:36:36 +0100 Subject: [PATCH 4/9] ext/spl: Fix various bugs relating to property access --- ext/spl/spl_array.c | 139 +++++++++++++++++- .../gh9432-dynamic-props-no-warning.phpt | 38 +++++ .../gh9434-ignore-typed-props.phpt | 25 ++++ .../private_protected_properties.phpt | 63 ++++++++ .../tests/ArrayObject/using_null_as_key.phpt | 3 +- .../using_null_for_offset_methods.phpt | 3 +- .../ArrayObject_overloaded_SplFixedArray.phpt | 3 +- ext/spl/tests/arrayObject_clone_basic3.phpt | 3 +- ext/spl/tests/arrayObject_magicMethods1.phpt | 16 +- ext/spl/tests/arrayObject_magicMethods3.phpt | 4 + ext/spl/tests/arrayObject_magicMethods4.phpt | 4 + ext/spl/tests/arrayObject_magicMethods6.phpt | 4 + 12 files changed, 285 insertions(+), 20 deletions(-) create mode 100644 ext/spl/tests/ArrayObject/gh9432-dynamic-props-no-warning.phpt create mode 100644 ext/spl/tests/ArrayObject/gh9434-ignore-typed-props.phpt create mode 100644 ext/spl/tests/ArrayObject/private_protected_properties.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 17aecac33476c..eff4fc1f554d5 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -460,6 +460,130 @@ static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t re return old_refcount; } /* }}} */ +/* For reasons ArrayObject must not go through the overloaded __set()/__get()/etc. + * magic methods, so to emit a warning for dynamic props (or Error on classes + * that do not support them, such as readonly classes) or an error for incompatible + * typed properties, we need to reimplement the logic of zend_std_write_property() + * somewhat. Thanks SPL. */ +static zval *zend_ignore_set_magic_method_write_property( + HashTable *derived_properties_table, + zend_object *zobj, + zend_string *property_name, + zval *value +) { + ZEND_ASSERT(!Z_ISREF_P(value)); + + zend_class_entry *ce = zobj->ce; + zval *zv; + if (UNEXPECTED(zend_hash_num_elements(&ce->properties_info) == 0) + || UNEXPECTED((zv = zend_hash_find(&ce->properties_info, property_name)) == NULL)) { + /* Dynamic prop handling */ + dynamic_prop: + /* TODO Modifying private/protected props? + if (UNEXPECTED(ZSTR_VAL(member)[0] == '\0') && ZSTR_LEN(member) != 0) { + zend_bad_property_name(); + return NULL; + } + */ + // Dynamic props are not allowed on this class + if (ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES) { + zend_throw_error(NULL, "Cannot create dynamic property %s::$%s", ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); + return NULL; + } + if (UNEXPECTED(!(ce->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES))) { + GC_ADDREF(zobj); + zend_error(E_DEPRECATED, "Creation of dynamic property %s::$%s is deprecated", ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); + if (UNEXPECTED(GC_DELREF(zobj) == 0)) { + zend_objects_store_del(zobj); + if (!EG(exception)) { + /* We cannot continue execution and have to throw an exception */ + zend_throw_error(NULL, "Cannot create dynamic property %s::$%s", ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); + } + return NULL; + } + } + /* Create/Update dynamic prop */ + zend_hash_update_ind(derived_properties_table, property_name, value); + return value; + } + + const zend_property_info *property_info = (zend_property_info*)Z_PTR_P(zv); + uint32_t prop_flags = property_info->flags; + + if (prop_flags & (ZEND_ACC_CHANGED|ZEND_ACC_PROTECTED|ZEND_ACC_PRIVATE)) { + zend_throw_error(NULL, "Cannot access %s property %s::$%s", zend_visibility_string(prop_flags), ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); + return NULL; + } + if (UNEXPECTED(prop_flags & ZEND_ACC_STATIC)) { + zend_error(E_NOTICE, "Accessing static property %s::$%s as non static", ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); + /* Handle like dynamic prop */ + goto dynamic_prop; + } + + uintptr_t property_offset = property_info->offset; + zval *variable_ptr = OBJ_PROP(zobj, property_offset); + + // Property is already set, check readonly violation + if (Z_TYPE_P(variable_ptr) != IS_UNDEF) { + if (UNEXPECTED((prop_flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE))) { + Z_TRY_DELREF_P(value); + zend_readonly_property_modification_error(property_info); + return NULL; + } + } + + /* Remove fact that property can be reinitialized */ + Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE; + Z_TRY_ADDREF_P(value); + /* Check value is of valid type if property has type */ + if (UNEXPECTED(ZEND_TYPE_IS_SET(property_info->type))) { + zval tmp; + ZVAL_COPY_VALUE(&tmp, value); + // Increase refcount to prevent object from being released in __toString() + GC_ADDREF(zobj); + /* Engine never uses strict types */ + bool type_matched = zend_verify_property_type(property_info, &tmp, /* strict_types */ false); + if (UNEXPECTED(GC_DELREF(zobj) == 0)) { + zend_object_released_while_assigning_to_property_error(property_info); + zend_objects_store_del(zobj); + zval_ptr_dtor(&tmp); + variable_ptr = &EG(error_zval); + return NULL; + } + if (UNEXPECTED(!type_matched)) { + zval_ptr_dtor(value); + return NULL; + } + value = &tmp; + Z_PROP_FLAG_P(variable_ptr) = 0; + } + + zend_refcounted *garbage = NULL; + /* Engine never uses strict types */ + variable_ptr = zend_assign_to_variable_ex(variable_ptr, value, IS_TMP_VAR, /* strict_types */ false, &garbage); + if (garbage) { + if (GC_DELREF(garbage) == 0) { + zend_execute_data *execute_data = EG(current_execute_data); + // Assign to result variable before calling the destructor as it may release the object + if (execute_data + && EX(func) + && ZEND_USER_CODE(EX(func)->common.type) + && EX(opline) + && EX(opline)->opcode == ZEND_ASSIGN_OBJ + && EX(opline)->result_type + ) { + ZVAL_COPY_DEREF(EX_VAR(EX(opline)->result.var), variable_ptr); + variable_ptr = NULL; + } + rc_dtor_func(garbage); + } else { + gc_check_possible_root_no_ref(garbage); + } + } + //ZVAL_COPY_VALUE(variable_ptr, value); + return variable_ptr; +} + static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */ { spl_array_object *intern = spl_array_from_obj(object); @@ -492,12 +616,6 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec } if (check_inherited && intern->fptr_offset_set) { - zval tmp; - - if (!offset) { - ZVAL_NULL(&tmp); - offset = &tmp; - } zend_call_method_with_2_params(object, object->ce, &intern->fptr_offset_set, "offsetSet", NULL, offset, value); return; } @@ -513,10 +631,15 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec ht = spl_array_get_hash_table(intern); refcount = spl_array_set_refcount(intern->is_child, ht, 1); if (key.key) { - if (spl_array_is_object(intern)) { + if (spl_array_is_object(intern) && !(intern->ar_flags & SPL_ARRAY_IS_SELF)) { ZEND_ASSERT(Z_TYPE(intern->array) == IS_OBJECT); zend_object *obj = Z_OBJ(intern->array); - obj->handlers->write_property(obj, key.key, value, NULL); + /* For reasons ArrayObject must not go through the overloaded __set()/__get()/etc. + * magic methods, so to emit a warning for dynamic props (or Error on classes + * that do not support them, such as readonly classes) or an error for incompatible + * typed properties, we need to reimplement the logic of zend_std_write_property() + * somewhat. Thanks SPL. */ + zend_ignore_set_magic_method_write_property(ht, obj, key.key, value); } else { zend_hash_update_ind(ht, key.key, value); } diff --git a/ext/spl/tests/ArrayObject/gh9432-dynamic-props-no-warning.phpt b/ext/spl/tests/ArrayObject/gh9432-dynamic-props-no-warning.phpt new file mode 100644 index 0000000000000..e2a96c133c0de --- /dev/null +++ b/ext/spl/tests/ArrayObject/gh9432-dynamic-props-no-warning.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-9432: Dynamic property deprecation warning/readonly error not emitted when modifying through new ArrayObject($targetObj); +--EXTENSIONS-- +spl +--FILE-- +foo1); +$c->foo2 = 2; +var_dump($c); + +readonly class ReadOnly_ {} +$r = new ReadOnly_(); +$z = new ArrayObject($r); +try { + $z['foo'] = 'bar'; +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +var_dump($r); +?> +--EXPECTF-- +Deprecated: Creation of dynamic property MyClass::$foo1 is deprecated in %s on line %d +string(3) "bar" + +Deprecated: Creation of dynamic property MyClass::$foo2 is deprecated in %s on line %d +object(MyClass)#1 (2) { + ["foo1"]=> + string(3) "bar" + ["foo2"]=> + int(2) +} +Error: Cannot create dynamic property ReadOnly_::$foo +object(ReadOnly_)#3 (0) { +} diff --git a/ext/spl/tests/ArrayObject/gh9434-ignore-typed-props.phpt b/ext/spl/tests/ArrayObject/gh9434-ignore-typed-props.phpt new file mode 100644 index 0000000000000..7a74eff28d137 --- /dev/null +++ b/ext/spl/tests/ArrayObject/gh9434-ignore-typed-props.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-9434: new ArrayObject($targetObject) allows setting typed properties to invalid types +--EXTENSIONS-- +spl +--FILE-- +getMessage(), PHP_EOL; +} +var_dump($c); +?> +--EXPECT-- +TypeError: Cannot assign array to property C::$intProp of type int +object(C)#1 (0) { + ["intProp"]=> + uninitialized(int) +} diff --git a/ext/spl/tests/ArrayObject/private_protected_properties.phpt b/ext/spl/tests/ArrayObject/private_protected_properties.phpt new file mode 100644 index 0000000000000..76e7b424f9c06 --- /dev/null +++ b/ext/spl/tests/ArrayObject/private_protected_properties.phpt @@ -0,0 +1,63 @@ +--TEST-- +Cannot append to ArrayObject if backing value is an object +--EXTENSIONS-- +spl +--FILE-- +foo1, $this->foo2); + } +} +$c = new MyClass(); +$ao = new ArrayObject($c); + +try { + $ao['foo1'] = 'bar1'; +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $ao['foo2'] = 'bar2'; +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +var_dump($c); +$c->read(); +var_dump($ao['foo1']); +var_dump($ao['foo2']); +try { + var_dump($c->foo1); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump($c->foo2); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +?> +--EXPECTF-- +Error: Cannot access private property MyClass::$foo1 +Error: Cannot access protected property MyClass::$foo2 +object(MyClass)#1 (2) { + ["foo1":"MyClass":private]=> + NULL + ["foo2":protected]=> + NULL +} +NULL +NULL + +Warning: Undefined array key "foo1" in %s on line %d +NULL + +Warning: Undefined array key "foo2" in %s on line %d +NULL +Error: Cannot access private property MyClass::$foo1 +Error: Cannot access protected property MyClass::$foo2 diff --git a/ext/spl/tests/ArrayObject/using_null_as_key.phpt b/ext/spl/tests/ArrayObject/using_null_as_key.phpt index ce36a4937c4f2..05cc8ce33d101 100644 --- a/ext/spl/tests/ArrayObject/using_null_as_key.phpt +++ b/ext/spl/tests/ArrayObject/using_null_as_key.phpt @@ -14,7 +14,8 @@ var_dump($c); var_dump($c->{""}); var_dump($ao[null]); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: Creation of dynamic property MyClass::$ is deprecated in %s on line %d object(MyClass)#1 (1) { [""]=> string(8) "null key" diff --git a/ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt b/ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt index 76b0b5f9eba70..a7c83b2487947 100644 --- a/ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt +++ b/ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt @@ -15,7 +15,8 @@ var_dump($ao->offsetGet(null)); var_dump($c); var_dump($c->{""}); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: Creation of dynamic property MyClass::$ is deprecated in %s on line %d bool(true) string(8) "null key" object(MyClass)#1 (1) { diff --git a/ext/spl/tests/ArrayObject_overloaded_SplFixedArray.phpt b/ext/spl/tests/ArrayObject_overloaded_SplFixedArray.phpt index 7abbd266e3371..a673bdef32a9a 100644 --- a/ext/spl/tests/ArrayObject_overloaded_SplFixedArray.phpt +++ b/ext/spl/tests/ArrayObject_overloaded_SplFixedArray.phpt @@ -9,7 +9,8 @@ $ao->exchangeArray($fixedArray); $ao[0] = new stdClass(); var_dump($ao); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: Creation of dynamic property SplFixedArray::$0 is deprecated in %s on line %d object(ArrayObject)#1 (1) { ["storage":"ArrayObject":private]=> object(SplFixedArray)#2 (2) { diff --git a/ext/spl/tests/arrayObject_clone_basic3.phpt b/ext/spl/tests/arrayObject_clone_basic3.phpt index 214a2b87b8690..a78429d17c3f5 100644 --- a/ext/spl/tests/arrayObject_clone_basic3.phpt +++ b/ext/spl/tests/arrayObject_clone_basic3.phpt @@ -23,7 +23,8 @@ $clonedOuterArrayObject['new.coAO'] = 'new element added to $clonedOuterArrayObj var_dump($wrappedObject, $innerArrayObject, $outerArrayObject, $clonedOuterArrayObject); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: Creation of dynamic property ArrayObject::$new.oAO is deprecated in %s on line %d object(C)#1 (5) { ["p"]=> string(9) "C::p.orig" diff --git a/ext/spl/tests/arrayObject_magicMethods1.phpt b/ext/spl/tests/arrayObject_magicMethods1.phpt index ce29beb7177fb..1f0b1061d671f 100644 --- a/ext/spl/tests/arrayObject_magicMethods1.phpt +++ b/ext/spl/tests/arrayObject_magicMethods1.phpt @@ -10,20 +10,16 @@ class UsesMagic { private $priv = 'secret'; function __get($name) { - $args = func_get_args(); - echo "In " . __METHOD__ . "(" . implode($args, ',') . ")\n"; + echo "In " . __METHOD__ . "($name)\n"; } function __set($name, $value) { - $args = func_get_args(); - echo "In " . __METHOD__ . "(" . implode($args, ',') . ")\n"; + echo "In " . __METHOD__ . "($name, $value)\n"; } function __isset($name) { - $args = func_get_args(); - echo "In " . __METHOD__ . "(" . implode($args, ',') . ")\n"; + echo "In " . __METHOD__ . "($name)\n"; } function __unset($name) { - $args = func_get_args(); - echo "In " . __METHOD__ . "(" . implode($args, ',') . ")\n"; + echo "In " . __METHOD__ . "($name)\n"; } } @@ -69,6 +65,10 @@ var_dump($ao); ?> --EXPECTF-- --> Write existent, non-existent and dynamic: + +Deprecated: Creation of dynamic property UsesMagic::$dynamic is deprecated in %s on line %d + +Deprecated: Creation of dynamic property UsesMagic::$dynamic is deprecated in %s on line %d Original wrapped object: object(UsesMagic)#1 (5) { ["a"]=> diff --git a/ext/spl/tests/arrayObject_magicMethods3.phpt b/ext/spl/tests/arrayObject_magicMethods3.phpt index 2ff0531e01d43..8217b9e63ccd7 100644 --- a/ext/spl/tests/arrayObject_magicMethods3.phpt +++ b/ext/spl/tests/arrayObject_magicMethods3.phpt @@ -69,6 +69,10 @@ var_dump($ao); ?> --EXPECTF-- --> Write existent, non-existent and dynamic: + +Deprecated: Creation of dynamic property UsesMagic::$dynamic is deprecated in %s on line %d + +Deprecated: Creation of dynamic property UsesMagic::$dynamic is deprecated in %s on line %d Original wrapped object: object(UsesMagic)#1 (5) { ["a"]=> diff --git a/ext/spl/tests/arrayObject_magicMethods4.phpt b/ext/spl/tests/arrayObject_magicMethods4.phpt index e68ef1fc47777..460794778560a 100644 --- a/ext/spl/tests/arrayObject_magicMethods4.phpt +++ b/ext/spl/tests/arrayObject_magicMethods4.phpt @@ -72,6 +72,10 @@ var_dump($ao); ?> --EXPECTF-- --> Write existent, non-existent and dynamic: + +Deprecated: Creation of dynamic property C::$dynamic is deprecated in %s on line %d + +Deprecated: Creation of dynamic property C::$dynamic is deprecated in %s on line %d Original wrapped object: object(C)#1 (5) { ["a"]=> diff --git a/ext/spl/tests/arrayObject_magicMethods6.phpt b/ext/spl/tests/arrayObject_magicMethods6.phpt index 75151902210fb..ef33a40ac5234 100644 --- a/ext/spl/tests/arrayObject_magicMethods6.phpt +++ b/ext/spl/tests/arrayObject_magicMethods6.phpt @@ -72,6 +72,10 @@ var_dump($ao); ?> --EXPECTF-- --> Write existent, non-existent and dynamic: + +Deprecated: Creation of dynamic property C::$dynamic is deprecated in %s on line %d + +Deprecated: Creation of dynamic property C::$dynamic is deprecated in %s on line %d Original wrapped object: object(C)#1 (5) { ["a"]=> From c3204507e5e21b60a4b0165607f3223a49fa9227 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 30 Aug 2023 22:17:57 +0100 Subject: [PATCH 5/9] Use property guard instead of custom write_property logic Although now there is a memory leak? --- ext/spl/spl_array.c | 146 ++---------------- .../tests/ArrayObject/appending_object.phpt | 2 +- ext/spl/tests/arrayObject_clone_basic3.phpt | 18 +-- ext/spl/tests/arrayObject_magicMethods1.phpt | 2 - ext/spl/tests/arrayObject_magicMethods3.phpt | 2 - ext/spl/tests/arrayObject_magicMethods4.phpt | 2 - ext/spl/tests/arrayObject_magicMethods6.phpt | 2 - .../get_object_vars_variation_005.phpt | 1 + 8 files changed, 26 insertions(+), 149 deletions(-) diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index eff4fc1f554d5..a1eb3f2f8beb6 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -460,130 +460,6 @@ static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t re return old_refcount; } /* }}} */ -/* For reasons ArrayObject must not go through the overloaded __set()/__get()/etc. - * magic methods, so to emit a warning for dynamic props (or Error on classes - * that do not support them, such as readonly classes) or an error for incompatible - * typed properties, we need to reimplement the logic of zend_std_write_property() - * somewhat. Thanks SPL. */ -static zval *zend_ignore_set_magic_method_write_property( - HashTable *derived_properties_table, - zend_object *zobj, - zend_string *property_name, - zval *value -) { - ZEND_ASSERT(!Z_ISREF_P(value)); - - zend_class_entry *ce = zobj->ce; - zval *zv; - if (UNEXPECTED(zend_hash_num_elements(&ce->properties_info) == 0) - || UNEXPECTED((zv = zend_hash_find(&ce->properties_info, property_name)) == NULL)) { - /* Dynamic prop handling */ - dynamic_prop: - /* TODO Modifying private/protected props? - if (UNEXPECTED(ZSTR_VAL(member)[0] == '\0') && ZSTR_LEN(member) != 0) { - zend_bad_property_name(); - return NULL; - } - */ - // Dynamic props are not allowed on this class - if (ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES) { - zend_throw_error(NULL, "Cannot create dynamic property %s::$%s", ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); - return NULL; - } - if (UNEXPECTED(!(ce->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES))) { - GC_ADDREF(zobj); - zend_error(E_DEPRECATED, "Creation of dynamic property %s::$%s is deprecated", ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); - if (UNEXPECTED(GC_DELREF(zobj) == 0)) { - zend_objects_store_del(zobj); - if (!EG(exception)) { - /* We cannot continue execution and have to throw an exception */ - zend_throw_error(NULL, "Cannot create dynamic property %s::$%s", ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); - } - return NULL; - } - } - /* Create/Update dynamic prop */ - zend_hash_update_ind(derived_properties_table, property_name, value); - return value; - } - - const zend_property_info *property_info = (zend_property_info*)Z_PTR_P(zv); - uint32_t prop_flags = property_info->flags; - - if (prop_flags & (ZEND_ACC_CHANGED|ZEND_ACC_PROTECTED|ZEND_ACC_PRIVATE)) { - zend_throw_error(NULL, "Cannot access %s property %s::$%s", zend_visibility_string(prop_flags), ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); - return NULL; - } - if (UNEXPECTED(prop_flags & ZEND_ACC_STATIC)) { - zend_error(E_NOTICE, "Accessing static property %s::$%s as non static", ZSTR_VAL(ce->name), ZSTR_VAL(property_name)); - /* Handle like dynamic prop */ - goto dynamic_prop; - } - - uintptr_t property_offset = property_info->offset; - zval *variable_ptr = OBJ_PROP(zobj, property_offset); - - // Property is already set, check readonly violation - if (Z_TYPE_P(variable_ptr) != IS_UNDEF) { - if (UNEXPECTED((prop_flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE))) { - Z_TRY_DELREF_P(value); - zend_readonly_property_modification_error(property_info); - return NULL; - } - } - - /* Remove fact that property can be reinitialized */ - Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE; - Z_TRY_ADDREF_P(value); - /* Check value is of valid type if property has type */ - if (UNEXPECTED(ZEND_TYPE_IS_SET(property_info->type))) { - zval tmp; - ZVAL_COPY_VALUE(&tmp, value); - // Increase refcount to prevent object from being released in __toString() - GC_ADDREF(zobj); - /* Engine never uses strict types */ - bool type_matched = zend_verify_property_type(property_info, &tmp, /* strict_types */ false); - if (UNEXPECTED(GC_DELREF(zobj) == 0)) { - zend_object_released_while_assigning_to_property_error(property_info); - zend_objects_store_del(zobj); - zval_ptr_dtor(&tmp); - variable_ptr = &EG(error_zval); - return NULL; - } - if (UNEXPECTED(!type_matched)) { - zval_ptr_dtor(value); - return NULL; - } - value = &tmp; - Z_PROP_FLAG_P(variable_ptr) = 0; - } - - zend_refcounted *garbage = NULL; - /* Engine never uses strict types */ - variable_ptr = zend_assign_to_variable_ex(variable_ptr, value, IS_TMP_VAR, /* strict_types */ false, &garbage); - if (garbage) { - if (GC_DELREF(garbage) == 0) { - zend_execute_data *execute_data = EG(current_execute_data); - // Assign to result variable before calling the destructor as it may release the object - if (execute_data - && EX(func) - && ZEND_USER_CODE(EX(func)->common.type) - && EX(opline) - && EX(opline)->opcode == ZEND_ASSIGN_OBJ - && EX(opline)->result_type - ) { - ZVAL_COPY_DEREF(EX_VAR(EX(opline)->result.var), variable_ptr); - variable_ptr = NULL; - } - rc_dtor_func(garbage); - } else { - gc_check_possible_root_no_ref(garbage); - } - } - //ZVAL_COPY_VALUE(variable_ptr, value); - return variable_ptr; -} - static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */ { spl_array_object *intern = spl_array_from_obj(object); @@ -620,11 +496,8 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec return; } - Z_TRY_ADDREF_P(value); - if (get_hash_key(&key, intern, offset) == FAILURE) { zend_illegal_container_offset(object->ce->name, offset, BP_VAR_W); - zval_ptr_dtor(value); return; } @@ -637,14 +510,27 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec /* For reasons ArrayObject must not go through the overloaded __set()/__get()/etc. * magic methods, so to emit a warning for dynamic props (or Error on classes * that do not support them, such as readonly classes) or an error for incompatible - * typed properties, we need to reimplement the logic of zend_std_write_property() - * somewhat. Thanks SPL. */ - zend_ignore_set_magic_method_write_property(ht, obj, key.key, value); + * typed properties, we set a property guard so that when calling the write_property + * handler it does not call the magic __set() method. Thanks SPL. */ + if (obj->ce->ce_flags & ZEND_ACC_USE_GUARDS) { + uint32_t *guard = zend_get_property_guard(obj, key.key); + uint32_t backup = *guard; + (*guard) |= ZEND_GUARD_PROPERTY_SET; + obj->handlers->write_property(obj, key.key, value, /* cache_slot */ NULL); + *guard = backup; + } else { + /* No overload method is defined on the object */ + obj->handlers->write_property(obj, key.key, value, /* cache_slot */ NULL); + } } else { + /* Increase refcount for value before adding to the Hashtable */ + Z_TRY_ADDREF_P(value); zend_hash_update_ind(ht, key.key, value); } spl_hash_key_release(&key); } else { + /* Increase refcount for value before adding to the Hashtable */ + Z_TRY_ADDREF_P(value); ZEND_ASSERT(!spl_array_is_object(intern)); zend_hash_index_update(ht, key.h, value); } diff --git a/ext/spl/tests/ArrayObject/appending_object.phpt b/ext/spl/tests/ArrayObject/appending_object.phpt index 1353eb78bfd32..208f70582d0f1 100644 --- a/ext/spl/tests/ArrayObject/appending_object.phpt +++ b/ext/spl/tests/ArrayObject/appending_object.phpt @@ -17,7 +17,7 @@ try { } try { - $ao->append('no-key'); + $ao[] = 'no-key'; var_dump($c); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; diff --git a/ext/spl/tests/arrayObject_clone_basic3.phpt b/ext/spl/tests/arrayObject_clone_basic3.phpt index a78429d17c3f5..e0f30fc58752c 100644 --- a/ext/spl/tests/arrayObject_clone_basic3.phpt +++ b/ext/spl/tests/arrayObject_clone_basic3.phpt @@ -25,7 +25,7 @@ var_dump($wrappedObject, $innerArrayObject, $outerArrayObject, $clonedOuterArray ?> --EXPECTF-- Deprecated: Creation of dynamic property ArrayObject::$new.oAO is deprecated in %s on line %d -object(C)#1 (5) { +object(C)#1 (4) { ["p"]=> string(9) "C::p.orig" ["dynamic1"]=> @@ -34,12 +34,12 @@ object(C)#1 (5) { string(44) "new prop added to $wrappedObject after clone" ["new.iAO"]=> string(35) "new element added $innerArrayObject" +} +object(ArrayObject)#2 (2) { ["new.oAO"]=> string(38) "new element added to $outerArrayObject" -} -object(ArrayObject)#2 (1) { ["storage":"ArrayObject":private]=> - object(C)#1 (5) { + object(C)#1 (4) { ["p"]=> string(9) "C::p.orig" ["dynamic1"]=> @@ -48,15 +48,15 @@ object(ArrayObject)#2 (1) { string(44) "new prop added to $wrappedObject after clone" ["new.iAO"]=> string(35) "new element added $innerArrayObject" - ["new.oAO"]=> - string(38) "new element added to $outerArrayObject" } } object(ArrayObject)#3 (1) { ["storage":"ArrayObject":private]=> - object(ArrayObject)#2 (1) { + object(ArrayObject)#2 (2) { + ["new.oAO"]=> + string(38) "new element added to $outerArrayObject" ["storage":"ArrayObject":private]=> - object(C)#1 (5) { + object(C)#1 (4) { ["p"]=> string(9) "C::p.orig" ["dynamic1"]=> @@ -65,8 +65,6 @@ object(ArrayObject)#3 (1) { string(44) "new prop added to $wrappedObject after clone" ["new.iAO"]=> string(35) "new element added $innerArrayObject" - ["new.oAO"]=> - string(38) "new element added to $outerArrayObject" } } } diff --git a/ext/spl/tests/arrayObject_magicMethods1.phpt b/ext/spl/tests/arrayObject_magicMethods1.phpt index 1f0b1061d671f..a2d7b15dc447f 100644 --- a/ext/spl/tests/arrayObject_magicMethods1.phpt +++ b/ext/spl/tests/arrayObject_magicMethods1.phpt @@ -66,8 +66,6 @@ var_dump($ao); --EXPECTF-- --> Write existent, non-existent and dynamic: -Deprecated: Creation of dynamic property UsesMagic::$dynamic is deprecated in %s on line %d - Deprecated: Creation of dynamic property UsesMagic::$dynamic is deprecated in %s on line %d Original wrapped object: object(UsesMagic)#1 (5) { diff --git a/ext/spl/tests/arrayObject_magicMethods3.phpt b/ext/spl/tests/arrayObject_magicMethods3.phpt index 8217b9e63ccd7..cdb37433cadf8 100644 --- a/ext/spl/tests/arrayObject_magicMethods3.phpt +++ b/ext/spl/tests/arrayObject_magicMethods3.phpt @@ -70,8 +70,6 @@ var_dump($ao); --EXPECTF-- --> Write existent, non-existent and dynamic: -Deprecated: Creation of dynamic property UsesMagic::$dynamic is deprecated in %s on line %d - Deprecated: Creation of dynamic property UsesMagic::$dynamic is deprecated in %s on line %d Original wrapped object: object(UsesMagic)#1 (5) { diff --git a/ext/spl/tests/arrayObject_magicMethods4.phpt b/ext/spl/tests/arrayObject_magicMethods4.phpt index 460794778560a..677fbc799f5b9 100644 --- a/ext/spl/tests/arrayObject_magicMethods4.phpt +++ b/ext/spl/tests/arrayObject_magicMethods4.phpt @@ -73,8 +73,6 @@ var_dump($ao); --EXPECTF-- --> Write existent, non-existent and dynamic: -Deprecated: Creation of dynamic property C::$dynamic is deprecated in %s on line %d - Deprecated: Creation of dynamic property C::$dynamic is deprecated in %s on line %d Original wrapped object: object(C)#1 (5) { diff --git a/ext/spl/tests/arrayObject_magicMethods6.phpt b/ext/spl/tests/arrayObject_magicMethods6.phpt index ef33a40ac5234..a9fddc1d4d289 100644 --- a/ext/spl/tests/arrayObject_magicMethods6.phpt +++ b/ext/spl/tests/arrayObject_magicMethods6.phpt @@ -73,8 +73,6 @@ var_dump($ao); --EXPECTF-- --> Write existent, non-existent and dynamic: -Deprecated: Creation of dynamic property C::$dynamic is deprecated in %s on line %d - Deprecated: Creation of dynamic property C::$dynamic is deprecated in %s on line %d Original wrapped object: object(C)#1 (5) { diff --git a/ext/standard/tests/class_object/get_object_vars_variation_005.phpt b/ext/standard/tests/class_object/get_object_vars_variation_005.phpt index 1ad1bb3933293..f85980fa40a8c 100644 --- a/ext/standard/tests/class_object/get_object_vars_variation_005.phpt +++ b/ext/standard/tests/class_object/get_object_vars_variation_005.phpt @@ -3,6 +3,7 @@ get_object_vars() no-declared/declared discrepancies --FILE-- Date: Sun, 17 Sep 2023 15:41:50 +0100 Subject: [PATCH 6/9] Naming restriction applies (except for integers) --- .../get_object_vars_variation_005.phpt | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/ext/standard/tests/class_object/get_object_vars_variation_005.phpt b/ext/standard/tests/class_object/get_object_vars_variation_005.phpt index f85980fa40a8c..f19ca4ed8a40d 100644 --- a/ext/standard/tests/class_object/get_object_vars_variation_005.phpt +++ b/ext/standard/tests/class_object/get_object_vars_variation_005.phpt @@ -7,40 +7,27 @@ get_object_vars() no-declared/declared discrepancies class Test { public $prop; } - // Using ArrayObject here to get around property name restrictions $obj = new stdClass; $ao = new ArrayObject($obj); -$ao["\0A\0b"] = 42; -$ao["\0*\0b"] = 24; $ao[12] = 6; var_dump(get_object_vars($obj)); $obj = new Test; $ao = new ArrayObject($obj); -$ao["\0A\0b"] = 42; -$ao["\0*\0b"] = 24; $ao[12] = 6; var_dump(get_object_vars($obj)); ?> ---EXPECTF-- -array(3) { - ["%0A%0b"]=> - int(42) - ["%0*%0b"]=> - int(24) +--EXPECT-- +array(1) { [12]=> int(6) } -array(4) { +array(2) { ["prop"]=> NULL - ["%0A%0b"]=> - int(42) - ["%0*%0b"]=> - int(24) [12]=> int(6) } From 20f99d0cb7731d6863472917386891beaa2d6d1d Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 16 Nov 2023 00:43:06 +0000 Subject: [PATCH 7/9] Remove now invalid comment --- .../tests/class_object/get_object_vars_variation_005.phpt | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/standard/tests/class_object/get_object_vars_variation_005.phpt b/ext/standard/tests/class_object/get_object_vars_variation_005.phpt index f19ca4ed8a40d..9b5c5f14c09bf 100644 --- a/ext/standard/tests/class_object/get_object_vars_variation_005.phpt +++ b/ext/standard/tests/class_object/get_object_vars_variation_005.phpt @@ -7,7 +7,6 @@ get_object_vars() no-declared/declared discrepancies class Test { public $prop; } -// Using ArrayObject here to get around property name restrictions $obj = new stdClass; $ao = new ArrayObject($obj); From 7e469922df188fa4616ef40d85235d839698408c Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Fri, 1 Dec 2023 12:42:51 +0000 Subject: [PATCH 8/9] Allow overriding the behaviour of appending operator via append() --- ext/spl/spl_array.c | 36 +++++++++++++--- ext/spl/tests/{ => ArrayObject}/bug33136.phpt | 34 +++++++-------- ext/spl/tests/ArrayObject/bug34548.phpt | 39 ++++++++++++++++++ .../subclassing_behaviour_001.phpt | 41 +++++++++++++++++++ ext/spl/tests/bug34548.phpt | 38 ----------------- 5 files changed, 128 insertions(+), 60 deletions(-) rename ext/spl/tests/{ => ArrayObject}/bug33136.phpt (78%) create mode 100644 ext/spl/tests/ArrayObject/bug34548.phpt create mode 100644 ext/spl/tests/ArrayObject/subclassing_behaviour_001.phpt delete mode 100644 ext/spl/tests/bug34548.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index a1eb3f2f8beb6..d2425c981432c 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -49,10 +49,14 @@ typedef struct _spl_array_object { unsigned char nApplyCount; bool is_child; Bucket *bucket; + /* Overridden ArrayAccess methods */ zend_function *fptr_offset_get; zend_function *fptr_offset_set; zend_function *fptr_offset_has; zend_function *fptr_offset_del; + /* Overridden append() method */ + zend_function *fptr_append; + /* Overridden count() method */ zend_function *fptr_count; zend_class_entry* ce_get_iterator; zend_object std; @@ -192,6 +196,7 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o ZEND_ASSERT(parent); if (inherited) { + /* Find potentially overridden ArrayAccess methods */ intern->fptr_offset_get = zend_hash_str_find_ptr(&class_type->function_table, "offsetget", sizeof("offsetget") - 1); if (intern->fptr_offset_get->common.scope == parent) { intern->fptr_offset_get = NULL; @@ -208,7 +213,12 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o if (intern->fptr_offset_del->common.scope == parent) { intern->fptr_offset_del = NULL; } - /* Find count() method */ + /* Find potentially overridden append() method */ + intern->fptr_append = zend_hash_str_find_ptr(&class_type->function_table, "append", sizeof("append") - 1); + if (intern->fptr_append->common.scope == parent) { + intern->fptr_append = NULL; + } + /* Find potentially overridden count() method */ intern->fptr_count = zend_hash_find_ptr(&class_type->function_table, ZSTR_KNOWN(ZEND_STR_COUNT)); if (intern->fptr_count->common.scope == parent) { intern->fptr_count = NULL; @@ -460,7 +470,7 @@ static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t re return old_refcount; } /* }}} */ -static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */ +static void spl_array_write_dimension_ex(bool check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */ { spl_array_object *intern = spl_array_from_obj(object); HashTable *ht; @@ -474,6 +484,20 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec /* We are appending */ if (!offset) { + /* append() method is overridden, so call it */ + if (check_inherited && intern->fptr_append) { + zend_call_known_function( + intern->fptr_append, + object, + object->ce, + /* retval_ptr */NULL, + /* param_count */ 1, + /* params */ value, + /* named_params */ NULL + ); + return; + } + /* Cannot append if backing value is an object */ if (spl_array_is_object(intern)) { zend_throw_error(NULL, "Cannot append properties to objects, use %s::offsetSet() instead", ZSTR_VAL(object->ce->name)); @@ -491,6 +515,7 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec return; } + /* offsetSet() method is overridden, so call it */ if (check_inherited && intern->fptr_offset_set) { zend_call_method_with_2_params(object, object->ce, &intern->fptr_offset_set, "offsetSet", NULL, offset, value); return; @@ -542,7 +567,7 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec static void spl_array_write_dimension(zend_object *object, zval *offset, zval *value) /* {{{ */ { - spl_array_write_dimension_ex(1, object, offset, value); + spl_array_write_dimension_ex(/* check_inherited */ true, object, offset, value); } /* }}} */ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *object, zval *offset) /* {{{ */ @@ -703,9 +728,10 @@ PHP_METHOD(ArrayObject, offsetSet) if (zend_parse_parameters(ZEND_NUM_ARGS(), "zz", &index, &value) == FAILURE) { RETURN_THROWS(); } - spl_array_write_dimension_ex(0, Z_OBJ_P(ZEND_THIS), index, value); + spl_array_write_dimension_ex(/* check_inherited */ false, Z_OBJ_P(ZEND_THIS), index, value); } /* }}} */ +/* Needed for spl_iterators.c:2938 */ void spl_array_iterator_append(zval *object, zval *append_value) /* {{{ */ { spl_array_write_dimension(Z_OBJ_P(object), NULL, append_value); @@ -719,7 +745,7 @@ PHP_METHOD(ArrayObject, append) if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &value) == FAILURE) { RETURN_THROWS(); } - spl_array_iterator_append(ZEND_THIS, value); + spl_array_write_dimension_ex(/* check_inherited */ false, Z_OBJ_P(ZEND_THIS), /* offset */ NULL, value); } /* }}} */ /* {{{ Unsets the value at the specified $index. */ diff --git a/ext/spl/tests/bug33136.phpt b/ext/spl/tests/ArrayObject/bug33136.phpt similarity index 78% rename from ext/spl/tests/bug33136.phpt rename to ext/spl/tests/ArrayObject/bug33136.phpt index 74aef158e6655..1d5495d295131 100644 --- a/ext/spl/tests/bug33136.phpt +++ b/ext/spl/tests/ArrayObject/bug33136.phpt @@ -9,7 +9,7 @@ class Collection extends ArrayObject function __construct() { - $this->data = array(); + $this->data = []; parent::__construct($this->data); } @@ -26,7 +26,7 @@ class Collection extends ArrayObject } } -echo "\n\nInitiate Obj\n"; +echo "Initiate Obj\n"; $arrayObj = new Collection(); echo "Assign values\n"; @@ -41,7 +41,7 @@ var_dump($arrayObj[1]); $arrayObj["foo"] = "baz"; var_dump($arrayObj["foo"]); -print_r($arrayObj); +var_dump($arrayObj); var_dump(count($arrayObj)); @@ -58,18 +58,18 @@ string(3) "bar" Collection::offsetSet(foo,baz) Collection::offsetGet(foo) string(3) "baz" -Collection Object -( - [data:Collection:private] => Array - ( - ) - - [storage:ArrayObject:private] => Array - ( - [0] => foo - [1] => bar - [foo] => baz - ) - -) +object(Collection)#1 (2) { + ["data":"Collection":private]=> + array(0) { + } + ["storage":"ArrayObject":private]=> + array(3) { + [0]=> + string(3) "foo" + [1]=> + string(3) "bar" + ["foo"]=> + string(3) "baz" + } +} int(3) diff --git a/ext/spl/tests/ArrayObject/bug34548.phpt b/ext/spl/tests/ArrayObject/bug34548.phpt new file mode 100644 index 0000000000000..e0fd2deadf343 --- /dev/null +++ b/ext/spl/tests/ArrayObject/bug34548.phpt @@ -0,0 +1,39 @@ +--TEST-- +Bug #34548 (Method append() in class extended from ArrayObject crashes PHP) +--FILE-- +append($value); + } + } +} + +$data1 = ['one', 'two', 'three']; +$data2 = ['four', 'five']; + +$foo = new Collection($data1); +$foo->add($data2); + +var_dump($foo->getArrayCopy()); + +echo "Done\n"; +?> +--EXPECT-- +array(5) { + [0]=> + string(3) "one" + [1]=> + string(3) "two" + [2]=> + string(5) "three" + [3]=> + string(4) "four" + [4]=> + string(4) "five" +} +Done diff --git a/ext/spl/tests/ArrayObject/subclassing_behaviour_001.phpt b/ext/spl/tests/ArrayObject/subclassing_behaviour_001.phpt new file mode 100644 index 0000000000000..d708c26c9c63f --- /dev/null +++ b/ext/spl/tests/ArrayObject/subclassing_behaviour_001.phpt @@ -0,0 +1,41 @@ +--TEST-- +Subclassing ArrayObject should still call overridden offsetSet method when appending +--EXTENSIONS-- +spl +--FILE-- + +--EXPECT-- +string(12) "A::offsetSet" +NULL +int(1) +string(9) "B::append" +int(1) diff --git a/ext/spl/tests/bug34548.phpt b/ext/spl/tests/bug34548.phpt deleted file mode 100644 index 5f3e00c7f3a19..0000000000000 --- a/ext/spl/tests/bug34548.phpt +++ /dev/null @@ -1,38 +0,0 @@ ---TEST-- -Bug #34548 (Method append() in class extended from ArrayObject crashes PHP) ---FILE-- -append($value); - } - - public function offsetSet($index, $value): void - { - parent::offsetSet($index, $value); - } -} - -$data1=array('one', 'two', 'three'); -$data2=array('four', 'five'); - -$foo=new Collection($data1); -$foo->add($data2); - -print_r($foo->getArrayCopy()); - -echo "Done\n"; -?> ---EXPECT-- -Array -( - [0] => one - [1] => two - [2] => three - [3] => four - [4] => five -) -Done From 127652fbcf996359e2022230bca55a78ed187802 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Fri, 1 Dec 2023 13:00:08 +0000 Subject: [PATCH 9/9] UPGRADING --- UPGRADING | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/UPGRADING b/UPGRADING index a7db0c144e880..5d60f914fb498 100644 --- a/UPGRADING +++ b/UPGRADING @@ -59,6 +59,17 @@ PHP 8.4 UPGRADE NOTES OutOfBoundsException instead of RuntimeException. As OutOfBoundsException is a child class of RuntimeException, code that uses RuntimeException continues to function. + . ArrayObject now follows property write restrictions. + This means, writing a dynamic property now emits a deprecation warning, + it is not possible to assign a value of an incorrect type to a typed + property, nor assigning to a property name which contains null bytes. + ArrayObject continues to bypass any __set() handler and continues to + allow assigning to inaccessible property names (e.g. integer as name). + . Calling ArrayObject::offsetSet() with an offset of type null will now + set an offset instead of appending. + . Instances of classes that extend ArrayObject now call append($value) + instead of offsetSet(null, $value) when using the append operator + e.g. $object[] = $value; - Standard: . round() now validates the value of the $mode parameter and throws a ValueError